fix inconsistent-label-cardinality for prometheus metrics: nginx_ingr…

…ess_controller_requests (#8225) * fix inconsistent-label-cardinality for prometheus metrics: nginx_ingress_controller_requests * add host to collectorLabels only if metricsPerHost is true
kubernetes · Feb 13, 2022 · 86964b1 · 86964b1
1 parent 53a232f
commit 86964b1
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 4 deletions.
diff --git a/internal/ingress/metric/collectors/socket.go b/internal/ingress/metric/collectors/socket.go
@@ -217,6 +217,7 @@ func NewSocketCollector(pod, namespace, class string, metricsPerHost bool, bucke
 	}
 
 	sc.metricMapping = map[string]interface{}{
+		prometheus.BuildFQName(PrometheusNamespace, "", "requests"):                 sc.requests,
 		prometheus.BuildFQName(PrometheusNamespace, "", "request_duration_seconds"): sc.requestTime,
 		prometheus.BuildFQName(PrometheusNamespace, "", "request_size"):             sc.requestLength,
 
@@ -258,16 +259,19 @@ func (sc *SocketCollector) handleMessage(msg []byte) {
 			"service":   stats.Service,
 			"canary":    stats.Canary,
 		}
-		if sc.metricsPerHost {
-			requestLabels["host"] = stats.Host
-		}
 
 		collectorLabels := prometheus.Labels{
 			"namespace": stats.Namespace,
 			"ingress":   stats.Ingress,
 			"status":    stats.Status,
 			"service":   stats.Service,
 			"canary":    stats.Canary,
+			"method":    stats.Method,
+			"path":      stats.Path,
+		}
+		if sc.metricsPerHost {
+			requestLabels["host"] = stats.Host
+			collectorLabels["host"] = stats.Host
 		}
 
 		latencyLabels := prometheus.Labels{
@@ -415,6 +419,12 @@ func (sc *SocketCollector) RemoveMetrics(ingresses []string, registry prometheus
 					klog.V(2).InfoS("metric not removed", "name", metricName, "ingress", ingKey, "labels", labels)
 				}
 			}
+
+			if c, ok := metric.(*prometheus.CounterVec); ok {
+				if removed := c.Delete(labels); !removed {
+					klog.V(2).InfoS("metric not removed", "name", metricName, "ingress", ingKey, "labels", labels)
+				}
+			}
 		}
 	}
 }

diff --git a/internal/ingress/metric/collectors/socket_test.go b/internal/ingress/metric/collectors/socket_test.go
@@ -153,7 +153,35 @@ func TestCollector(t *testing.T) {
 			wantAfter: `
 			`,
 		},
-
+		{
+			name: "valid metric object should update requests metrics",
+			data: []string{`[{
+				"host":"testshop.com",
+				"status":"200",
+				"bytesSent":150.0,
+				"method":"GET",
+				"path":"/admin",
+				"requestLength":300.0,
+				"requestTime":60.0,
+				"upstreamName":"test-upstream",
+				"upstreamIP":"1.1.1.1:8080",
+				"upstreamResponseTime":200,
+				"upstreamStatus":"220",
+				"namespace":"test-app-production",
+				"ingress":"web-yml",
+				"service":"test-app",
+				"canary":""
+			}]`},
+			metrics: []string{"nginx_ingress_controller_requests"},
+			wantBefore: `
+				# HELP nginx_ingress_controller_requests The total number of client requests.
+				# TYPE nginx_ingress_controller_requests counter
+				nginx_ingress_controller_requests{canary="",controller_class="ingress",controller_namespace="default",controller_pod="pod",host="testshop.com",ingress="web-yml",method="GET",namespace="test-app-production",path="/admin",service="test-app",status="200"} 1
+			`,
+			removeIngresses: []string{"test-app-production/web-yml"},
+			wantAfter: `
+			`,
+		},
 		{
 			name: "valid metric object with canary information should update prometheus metrics",
 			data: []string{`[{