Merge pull request #92 from cloudflare/OBS-984

Fix series check with __name__ selectors

CHANGELOG.md

@@ -1,12 +1,22 @@
 # Changelog
 
+## v0.4.3
+
+### Fixed
+
+- Fixed `series` check handling of queries with `{__name__="foo"}` selectors.
+
 ## v0.4.2
 
-- Improve `template` check handling of `absent` calls on aggregated metrics, like
+### Fixed
+
+- Fixed `template` check handling of `absent` calls on aggregated metrics, like
   `absent(sum(nonexistent{job="myjob"}))`.
 
 ## v0.4.1
 
+### Added
+
 - `template` check will now warn if any template is referencing a label that is not passed to
   `absent()`.
   Example:

internal/checks/alerts_count_test.go

@@ -25,7 +25,9 @@ func TestAlertsCheck(t *testing.T) {
 			t.Fatal(err)
 		}
 		query := r.Form.Get("query")
-		if query != `up{job="foo"} == 0` {
+		if query != `up{job="foo"} == 0` &&
+			query != `{__name__="up", job="foo"} == 0` &&
+			query != `{__name__=~"(up|foo)", job="foo"} == 0` {
 			t.Fatalf("Prometheus got invalid query: %s", query)
 		}
 
@@ -146,6 +148,40 @@ func TestAlertsCheck(t *testing.T) {
 				},
 			},
 		},
+		{
+			description: "{__name__=}",
+			content: `
+- alert: foo
+  expr: '{__name__="up", job="foo"} == 0'
+`,
+			checker: checks.NewAlertsCheck("prom", srv.URL+"/alerts/", time.Second*5, time.Hour*24, time.Minute*6, time.Minute*10),
+			problems: []checks.Problem{
+				{
+					Fragment: `{__name__="up", job="foo"} == 0`,
+					Lines:    []int{3},
+					Reporter: "alerts/count",
+					Text:     "query using prom would trigger 3 alert(s) in the last 1d",
+					Severity: checks.Information,
+				},
+			},
+		},
+		{
+			description: "{__name__=~}",
+			content: `
+- alert: foo
+  expr: '{__name__=~"(up|foo)", job="foo"} == 0'
+`,
+			checker: checks.NewAlertsCheck("prom", srv.URL+"/alerts/", time.Second*5, time.Hour*24, time.Minute*6, time.Minute*10),
+			problems: []checks.Problem{
+				{
+					Fragment: `{__name__=~"(up|foo)", job="foo"} == 0`,
+					Lines:    []int{3},
+					Reporter: "alerts/count",
+					Text:     "query using prom would trigger 3 alert(s) in the last 1d",
+					Severity: checks.Information,
+				},
+			},
+		},
 	}
 
 	runTests(t, testCases)

internal/checks/promql_rate_test.go

@@ -109,6 +109,56 @@ func TestRateCheck(t *testing.T) {
 				},
 			},
 		},
+		{
+			description: "irate{__name__} > 3x scrape_interval",
+			content: `
+- record: foo
+  expr: irate({__name__="foo"}[5m])
+`,
+			checker: checks.NewRateCheck("prom", srv.URL+"/1m/", time.Second),
+		},
+		{
+			description: "irate{__name__=~} > 3x scrape_interval",
+			content: `
+- record: foo
+  expr: irate({__name__=~"(foo|bar)_total"}[5m])
+`,
+			checker: checks.NewRateCheck("prom", srv.URL+"/1m/", time.Second),
+		},
+		{
+			description: "irate{__name__} < 3x scrape_interval",
+			content: `
+- record: foo
+  expr: irate({__name__="foo"}[2m])
+`,
+			checker: checks.NewRateCheck("prom", srv.URL+"/1m/", time.Second),
+			problems: []checks.Problem{
+				{
+					Fragment: `irate({__name__="foo"}[2m])`,
+					Lines:    []int{3},
+					Reporter: "promql/rate",
+					Text:     "duration for irate() is recommended to be at least 3 x scrape_interval, prom is using 1m scrape_interval",
+					Severity: checks.Warning,
+				},
+			},
+		},
+		{
+			description: "irate{__name__=~} < 3x scrape_interval",
+			content: `
+- record: foo
+  expr: irate({__name__=~"(foo|bar)_total"}[2m])
+`,
+			checker: checks.NewRateCheck("prom", srv.URL+"/1m/", time.Second),
+			problems: []checks.Problem{
+				{
+					Fragment: `irate({__name__=~"(foo|bar)_total"}[2m])`,
+					Lines:    []int{3},
+					Reporter: "promql/rate",
+					Text:     "duration for irate() is recommended to be at least 3 x scrape_interval, prom is using 1m scrape_interval",
+					Severity: checks.Warning,
+				},
+			},
+		},
 		{
 			description: "irate == 3x scrape interval",
 			content:     "- record: foo\n  expr: irate(foo[3m])\n",

internal/checks/promql_vectormatching_test.go

@@ -20,7 +20,11 @@ func TestVectorMatchingCheck(t *testing.T) {
 		query := r.Form.Get("query")
 
 		switch query {
-		case "count(foo / ignoring(notfound) foo)", "count(foo_with_notfound / ignoring(notfound) foo_with_notfound)", "count(foo_with_notfound / ignoring(notfound) foo)", "count(foo / ignoring(notfound) foo_with_notfound)":
+		case "count(foo / ignoring(notfound) foo)",
+			"count(foo_with_notfound / ignoring(notfound) foo_with_notfound)",
+			`count({__name__="foo_with_notfound"} / ignoring(notfound) foo_with_notfound)`,
+			"count(foo_with_notfound / ignoring(notfound) foo)",
+			"count(foo / ignoring(notfound) foo_with_notfound)":
 			w.WriteHeader(200)
 			w.Header().Set("Content-Type", "application/json")
 			_, _ = w.Write([]byte(`{
@@ -298,6 +302,14 @@ func TestVectorMatchingCheck(t *testing.T) {
 				},
 			},
 		},
+		{
+			description: "one to one matching with ignoring() - both present - {__name__=}",
+			content: `
+- record: foo
+  expr: '{__name__="foo_with_notfound"} / ignoring(notfound) foo_with_notfound'
+`,
+			checker: checks.NewVectorMatchingCheck("prom", srv.URL, time.Second*1, checks.Warning),
+		},
 	}
 	runTests(t, testCases)
 }

internal/checks/query_cost_test.go

@@ -25,7 +25,7 @@ func TestCostCheck(t *testing.T) {
 			t.Fatal(err)
 		}
 		query := r.Form.Get("query")
-		if query != "count(sum(foo))" {
+		if query != "count(sum(foo))" && query != `count(sum({__name__="foo"}))` {
 			t.Fatalf("Prometheus got invalid query: %s", query)
 		}
 
@@ -207,6 +207,23 @@ func TestCostCheck(t *testing.T) {
 				},
 			},
 		},
+		{
+			description: "7 results",
+			content: `
+- record: foo
+  expr: 'sum({__name__="foo"})'
+`,
+			checker: checks.NewCostCheck("prom", srv.URL+"/7/", time.Second*5, 101, 0, checks.Bug),
+			problems: []checks.Problem{
+				{
+					Fragment: `sum({__name__="foo"})`,
+					Lines:    []int{3},
+					Reporter: "query/cost",
+					Text:     `RE:query using prom completed in 0\...s returning 7 result\(s\) with 707B estimated memory usage`,
+					Severity: checks.Information,
+				},
+			},
+		},
 	}
 
 	cmpText := cmp.Comparer(func(x, y string) bool {

internal/checks/query_series.go

@@ -71,12 +71,15 @@ func (c SeriesCheck) countSeries(expr parser.PromQLExpr, selector promParser.Vec
 
 	if series == 0 {
 		if len(selector.LabelMatchers) > 1 {
-			// retry selector without labels
+			// retry selector with only __name__ label
 			s := promParser.VectorSelector{
-				Name: selector.Name,
-				LabelMatchers: []*labels.Matcher{
-					{Name: labels.MetricName, Value: selector.Name},
-				},
+				Name:          selector.Name,
+				LabelMatchers: []*labels.Matcher{},
+			}
+			for _, lm := range selector.LabelMatchers {
+				if lm.Name == labels.MetricName {
+					s.LabelMatchers = append(s.LabelMatchers, lm)
+				}
 			}
 			p := c.countSeries(expr, s)
 			// if we have zero series without any label selector then the whole

internal/checks/query_series_test.go

@@ -22,7 +22,7 @@ func TestSeriesCheck(t *testing.T) {
 		query := r.Form.Get("query")
 
 		switch query {
-		case "count(notfound)":
+		case "count(notfound)", `count({__name__="notfound",job="bar"})`:
 			w.WriteHeader(200)
 			w.Header().Set("Content-Type", "application/json")
 			_, _ = w.Write([]byte(`{
@@ -32,7 +32,7 @@ func TestSeriesCheck(t *testing.T) {
 					"result":[]
 				}
 			}`))
-		case "count(found_1)":
+		case "count(found_1)", `count({__name__="notfound"})`:
 			w.WriteHeader(200)
 			w.Header().Set("Content-Type", "application/json")
 			_, _ = w.Write([]byte(`{
@@ -224,6 +224,23 @@ func TestSeriesCheck(t *testing.T) {
 				},
 			},
 		},
+		{
+			description: "series missing, {__name__=}",
+			content: `
+- record: foo
+  expr: '{__name__="notfound", job="bar"}'
+`,
+			checker: checks.NewSeriesCheck("prom", srv.URL, time.Second*5, checks.Warning),
+			problems: []checks.Problem{
+				{
+					Fragment: `{__name__="notfound",job="bar"}`,
+					Lines:    []int{3},
+					Reporter: "query/series",
+					Text:     `query using prom completed without any results for {__name__="notfound",job="bar"}`,
+					Severity: checks.Warning,
+				},
+			},
+		},
 	}
 	runTests(t, testCases)
 }

internal/promapi/range.go

@@ -86,9 +86,6 @@ func RangeQuery(uri string, timeout time.Duration, expr string, start, end time.
 	case model.ValMatrix:
 		samples := result.(model.Matrix)
 		qr.Samples = samples
-
-	case model.ValString:
-		fmt.Println("ValString")
 	default:
 		log.Error().Err(err).Str("uri", uri).Str("query", expr).Msgf("Range query returned unknown result type: %v", result)
 		return nil, fmt.Errorf("unknown result type: %v", result)