Skip misconfigured vts entries (#227)

* Extra filtering for VTS zones Misconfigured ingress resources have the ability to create invalid VTS entries. These are long-lived and are not rectified againt the running nginx config. Because of this, it is possible that the metrics exported by feed can flip between the valid and invalid VTS data causing erroneous metrics to be exported. This adds an extra filter to ensure that metrics are not generated for misconfigured ingress resources. * Update Changelog
sky-uk · Jun 17, 2020 · 0e2cd58 · 0e2cd58
1 parent 8777ed9
commit 0e2cd58
Show file tree

Hide file tree

Showing 3 changed files with 213 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,6 @@
+# v3.1.7
+* [BUGFIX] Do not generate Prometheus metrics for invalid nginx-vts entries
+
 # v3.1.6
 * [BUGFIX] Reduce the number of DescribeTargetGroups to avoid reaching the API limits
 * [BUGFIX] Do not set the nlb updater as initialised if it failed to describe the target groups

diff --git a/nginx/nginx_metrics.go b/nginx/nginx_metrics.go
@@ -142,8 +142,8 @@ func updateIngressMetrics(metrics VTSMetrics) {
 			}
 
 			strs := strings.Split(zone, "::")
-			if len(strs) != 2 {
-				log.Warnf("filter name not formatted as expected for %s, got %s without a '::' split", host, zone)
+			if len(strs) != 2 || strs[1] == "" {
+				log.Warnf("filter name not formatted as expected for %s, got %s without a valid '::' split", host, zone)
 				continue
 			}
 			path := strs[0]

diff --git a/nginx/nginx_test.go b/nginx/nginx_test.go
@@ -2047,6 +2047,15 @@ func TestUpdatesMetricsFromNginxStatusPage(t *testing.T) {
 	assertEndpointRequestCounters(t,
 		"kube-system.10.254.201.199.80", "10.254.201.199:80",
 		2910.0, 1570.0, 1.0, 10.0, 9.0, 2.0, 3.0)
+
+	// Assert that hosts with both valid and invalid entries for the same path generate metrics for the correct, valid VTS entry
+	assertIngressRequestCounters(t,
+		"ingress-with-valid-duplicate-path.sandbox.cosmic.sky", "/path/",
+		5000.0, 2000.0, 0.0, 5.0, 0.0, 0.0, 0.0)
+	// Assert that invalid paths do not generate metrics, even if the VTS data shows hits (e.g. 3xx's)
+	assertIngressRequestCounters(t,
+		"ingress-with-invalid-path.sandbox.cosmic.sky", "/bad/",
+		0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0)
 }
 
 func assertIngressRequestCounters(t *testing.T, host, path string, in, out, ones, twos, threes, fours, fives float64) {
@@ -2267,6 +2276,84 @@ var statusResponseBody = []byte(`{
         "scarce": 0
       }
     },
+    "ingress-with-valid-duplicate-path.sandbox.cosmic.sky": {
+      "requestCounter": 5,
+      "inBytes": 5000,
+      "outBytes": 2000,
+      "responses": {
+        "1xx": 0,
+        "2xx": 5,
+        "3xx": 0,
+        "4xx": 0,
+        "5xx": 0,
+        "miss": 0,
+        "bypass": 0,
+        "expired": 0,
+        "stale": 0,
+        "updating": 0,
+        "revalidated": 0,
+        "hit": 0,
+        "scarce": 0
+      },
+      "overCounts": {
+        "maxIntegerSize": 18446744073709551615,
+        "requestCounter": 0,
+        "inBytes": 0,
+        "outBytes": 0,
+        "1xx": 0,
+        "2xx": 0,
+        "3xx": 0,
+        "4xx": 0,
+        "5xx": 0,
+        "miss": 0,
+        "bypass": 0,
+        "expired": 0,
+        "stale": 0,
+        "updating": 0,
+        "revalidated": 0,
+        "hit": 0,
+        "scarce": 0
+      }
+    },
+    "ingress-with-invalid-path.sandbox.cosmic.sky": {
+      "requestCounter": 10,
+      "inBytes": 10,
+      "outBytes": 5,
+      "responses": {
+        "1xx": 0,
+        "2xx": 0,
+        "3xx": 10,
+        "4xx": 0,
+        "5xx": 0,
+        "miss": 0,
+        "bypass": 0,
+        "expired": 0,
+        "stale": 0,
+        "updating": 0,
+        "revalidated": 0,
+        "hit": 0,
+        "scarce": 0
+      },
+      "overCounts": {
+        "maxIntegerSize": 18446744073709551615,
+        "requestCounter": 0,
+        "inBytes": 0,
+        "outBytes": 0,
+        "1xx": 0,
+        "2xx": 0,
+        "3xx": 0,
+        "4xx": 0,
+        "5xx": 0,
+        "miss": 0,
+        "bypass": 0,
+        "expired": 0,
+        "stale": 0,
+        "updating": 0,
+        "revalidated": 0,
+        "hit": 0,
+        "scarce": 0
+      }
+    },
     "*": {
       "requestCounter": 10,
       "inBytes": 2910,
@@ -2389,6 +2476,127 @@ var statusResponseBody = []byte(`{
           "scarce": 0
         }
       }
+    },
+    "ingress-with-valid-duplicate-path.sandbox.cosmic.sky": {
+      "/path/::some-app.10.254.204.100.8080": {
+        "requestCounter": 10,
+        "inBytes": 5000,
+        "outBytes": 2000,
+        "responses": {
+          "1xx": 0,
+          "2xx": 5,
+          "3xx": 0,
+          "4xx": 0,
+          "5xx": 0,
+          "miss": 0,
+          "bypass": 0,
+          "expired": 0,
+          "stale": 0,
+          "updating": 0,
+          "revalidated": 0,
+          "hit": 0,
+          "scarce": 0
+        },
+        "overCounts": {
+          "maxIntegerSize": 18446744073709551615,
+          "requestCounter": 0,
+          "inBytes": 0,
+          "outBytes": 0,
+          "1xx": 0,
+          "2xx": 0,
+          "3xx": 0,
+          "4xx": 0,
+          "5xx": 0,
+          "miss": 0,
+          "bypass": 0,
+          "expired": 0,
+          "stale": 0,
+          "updating": 0,
+          "revalidated": 0,
+          "hit": 0,
+          "scarce": 0
+        }
+      },
+      "/path/::": {
+        "requestCounter": 50,
+        "inBytes": 50,
+        "outBytes": 50,
+        "responses": {
+          "1xx": 50,
+          "2xx": 50,
+          "3xx": 50,
+          "4xx": 50,
+          "5xx": 50,
+          "miss": 50,
+          "bypass": 0,
+          "expired": 0,
+          "stale": 0,
+          "updating": 0,
+          "revalidated": 0,
+          "hit": 0,
+          "scarce": 0
+        },
+        "overCounts": {
+          "maxIntegerSize": 18446744073709551615,
+          "requestCounter": 0,
+          "inBytes": 0,
+          "outBytes": 0,
+          "1xx": 0,
+          "2xx": 0,
+          "3xx": 0,
+          "4xx": 0,
+          "5xx": 0,
+          "miss": 0,
+          "bypass": 0,
+          "expired": 0,
+          "stale": 0,
+          "updating": 0,
+          "revalidated": 0,
+          "hit": 0,
+          "scarce": 0
+        }
+      }      
+    },
+    "ingress-with-invalid-path.sandbox.cosmic.sky": {
+      "/bad::": {
+        "requestCounter": 10,
+        "inBytes": 10,
+        "outBytes": 5,
+        "responses": {
+          "1xx": 0,
+          "2xx": 10,
+          "3xx": 0,
+          "4xx": 0,
+          "5xx": 0,
+          "miss": 0,
+          "bypass": 0,
+          "expired": 0,
+          "stale": 0,
+          "updating": 0,
+          "revalidated": 0,
+          "hit": 0,
+          "scarce": 0
+        },
+        "overCounts": {
+          "maxIntegerSize": 18446744073709551615,
+          "requestCounter": 0,
+          "inBytes": 0,
+          "outBytes": 0,
+          "1xx": 0,
+          "2xx": 0,
+          "3xx": 0,
+          "4xx": 0,
+          "5xx": 0,
+          "miss": 0,
+          "bypass": 0,
+          "expired": 0,
+          "stale": 0,
+          "updating": 0,
+          "revalidated": 0,
+          "hit": 0,
+          "scarce": 0
+        }
+      }
     }
   },
   "upstreamZones": {