Merge pull request #2859 from aledbf/fix-metrics

Fix inconsistent metric labels

internal/ingress/metric/collectors/controller.go

@@ -17,6 +17,7 @@ limitations under the License.
 package collectors
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/golang/glog"
@@ -27,7 +28,7 @@ import (
 )
 
 var (
-	operation    = []string{"namespace", "class"}
+	operation    = []string{"controller_namespace", "controller_class", "controller_pod"}
 	sslLabelHost = []string{"namespace", "class", "host"}
 )
 
@@ -43,6 +44,7 @@ type Controller struct {
 	reloadOperationErrors *prometheus.CounterVec
 	sslExpireTime         *prometheus.GaugeVec
 
+	constLabels prometheus.Labels
 	labels      prometheus.Labels
 }
 
@@ -56,6 +58,8 @@ func NewController(pod, namespace, class string) *Controller {
 	}
 
 	cm := &Controller{
+		constLabels: constLabels,
+
 		labels: prometheus.Labels{
 			"namespace": namespace,
 			"class":     class,
@@ -115,12 +119,12 @@ func NewController(pod, namespace, class string) *Controller {
 
 // IncReloadCount increment the reload counter
 func (cm *Controller) IncReloadCount() {
-	cm.reloadOperation.With(cm.labels).Inc()
+	cm.reloadOperation.With(cm.constLabels).Inc()
 }
 
 // IncReloadErrorCount increment the reload error counter
 func (cm *Controller) IncReloadErrorCount() {
-	cm.reloadOperationErrors.With(cm.labels).Inc()
+	cm.reloadOperationErrors.With(cm.constLabels).Inc()
 }
 
 // ConfigSuccess set a boolean flag according to the output of the controller configuration reload
@@ -186,7 +190,7 @@ func (cm *Controller) RemoveMetrics(hosts []string, registry prometheus.Gatherer
 
 	for _, mf := range mfs {
 		metricName := mf.GetName()
-		if "ssl_expire_time_seconds" != metricName {
+		if fmt.Sprintf("%v_ssl_expire_time_seconds", PrometheusNamespace) != metricName {
 			continue
 		}
 

internal/ingress/metric/collectors/controller_test.go

@@ -54,7 +54,7 @@ func TestControllerCounters(t *testing.T) {
 			},
 			want: metadata + `
 				nginx_ingress_controller_config_last_reload_successful{controller_class="nginx",controller_namespace="default",controller_pod="pod"} 1
-				nginx_ingress_controller_success{class="nginx",namespace="default"} 1
+				nginx_ingress_controller_success{controller_class="nginx",controller_namespace="default",controller_pod="pod"} 1
 			`,
 			metrics: []string{"nginx_ingress_controller_config_last_reload_successful", "nginx_ingress_controller_success"},
 		},
@@ -66,7 +66,7 @@ func TestControllerCounters(t *testing.T) {
 			want: `
 				# HELP nginx_ingress_controller_errors Cumulative number of Ingress controller errors during reload operations
 				# TYPE nginx_ingress_controller_errors counter
-				nginx_ingress_controller_errors{class="nginx",namespace="default"} 1
+				nginx_ingress_controller_errors{controller_class="nginx",controller_namespace="default",controller_pod="pod"} 1
 			`,
 			metrics: []string{"nginx_ingress_controller_errors"},
 		},
@@ -120,3 +120,39 @@ func TestControllerCounters(t *testing.T) {
 		})
 	}
 }
+
+func TestRemoveMetrics(t *testing.T) {
+	cm := NewController("pod", "default", "nginx")
+	reg := prometheus.NewPedanticRegistry()
+	if err := reg.Register(cm); err != nil {
+		t.Errorf("registering collector failed: %s", err)
+	}
+
+	t1, _ := time.Parse(
+		time.RFC3339,
+		"2012-11-01T22:08:41+00:00")
+
+	servers := []*ingress.Server{
+		{
+			Hostname: "demo",
+			SSLCert: ingress.SSLCert{
+				ExpireTime: t1,
+			},
+		},
+		{
+			Hostname: "invalid",
+			SSLCert: ingress.SSLCert{
+				ExpireTime: time.Unix(0, 0),
+			},
+		},
+	}
+	cm.SetSSLExpireTime(servers)
+
+	cm.RemoveMetrics([]string{"demo"}, reg)
+
+	if err := GatherAndCompare(cm, "", []string{"nginx_ingress_controller_ssl_expire_time_seconds"}, reg); err != nil {
+		t.Errorf("unexpected collecting result:\n%s", err)
+	}
+
+	reg.Unregister(cm)
+}