Avoid queries without name matchers

linode-obs · Apr 5, 2023 · b5a54d0 · b5a54d0
1 parent 552e44c
commit b5a54d0
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 5 deletions.
diff --git a/docs/changelog.md b/docs/changelog.md
@@ -7,6 +7,12 @@
 - If a query run by pint fails because it was too expensive too run it will
   now be reported as a warning instead of an error.
 
+### Fixed
+
+- When validating queries using `{__name__=~"...", foo="bar"}` selectors pint could end up
+  running queries matching a single label, like `count({foo="bar"})`, which could return too many
+  results. This version ensures that queries always include name matcher to avoid that.
+
 ## v0.43.0
 
 ### Added

diff --git a/internal/checks/promql_series.go b/internal/checks/promql_series.go
@@ -345,6 +345,7 @@ func (c SeriesCheck) Check(ctx context.Context, _ string, rule parser.Rule, entr
 				Name:          metricName,
 				LabelMatchers: []*labels.Matcher{lm},
 			}
+			addNameSelectorIfNeeded(&labelSelector, selector.LabelMatchers)
 			log.Debug().Str("check", c.Reporter()).Stringer("selector", &labelSelector).Stringer("matcher", lm).Msg("Checking if there are historical series matching filter")
 
 			trsLabel, err := c.prom.RangeQuery(ctx, fmt.Sprintf("count(%s)", labelSelector.String()), params)
@@ -650,3 +651,20 @@ func newest(ranges []promapi.MetricTimeRange) (ts time.Time) {
 	}
 	return ts
 }
+
+func addNameSelectorIfNeeded(selector *promParser.VectorSelector, matchers []*labels.Matcher) {
+	if selector.Name != "" {
+		return
+	}
+	for _, lm := range selector.LabelMatchers {
+		if lm.Name == model.MetricNameLabel {
+			return
+		}
+	}
+
+	for _, lm := range matchers {
+		if lm.Name == model.MetricNameLabel {
+			selector.LabelMatchers = append(selector.LabelMatchers, lm)
+		}
+	}
+}
diff --git a/internal/checks/promql_series_test.go b/internal/checks/promql_series_test.go
@@ -2969,15 +2969,22 @@ func TestSeriesCheck(t *testing.T) {
 			problems: func(uri string) []checks.Problem {
 				return []checks.Problem{
 					{
-						Fragment: `{__name__=~"(foo|bar)_panics_total"}`,
+						Fragment: `{__name__=~"(foo|bar)_panics_total",job="myjob"}`,
 						Lines:    []int{2},
 						Reporter: checks.SeriesCheckName,
-						Text:     noSeriesText("prom", uri, `{__name__=~"(foo|bar)_panics_total"}`, "1w"),
+						Text:     noFilterMatchText("prom", uri, `{__name__=~"(foo|bar)_panics_total"}`, "job", `{job="myjob"}`, "1w"),
 						Severity: checks.Bug,
 					},
 				}
 			},
 			mocks: []*prometheusMock{
+				{
+					conds: []requestCondition{
+						requireRangeQueryPath,
+						formCond{key: "query", value: "count(up)"},
+					},
+					resp: respondWithSingleRangeVector1W(),
+				},
 				{
 					conds: []requestCondition{
 						requireQueryPath,
@@ -2990,14 +2997,30 @@ func TestSeriesCheck(t *testing.T) {
 						requireRangeQueryPath,
 						formCond{key: "query", value: `count({__name__=~"(foo|bar)_panics_total"})`},
 					},
-					resp: respondWithEmptyMatrix(),
+					resp: respondWithSingleRangeVector1W(),
 				},
 				{
 					conds: []requestCondition{
 						requireRangeQueryPath,
-						formCond{key: "query", value: "count(up)"},
+						formCond{key: "query", value: `absent({__name__=~"(foo|bar)_panics_total",job=~".+"})`},
 					},
-					resp: respondWithSingleRangeVector1W(),
+					resp: matrixResponse{
+						samples: []*model.SampleStream{
+							generateSampleStream(
+								map[string]string{},
+								time.Now().Add(time.Minute*-50),
+								time.Now(),
+								time.Minute*5,
+							),
+						},
+					},
+				},
+				{
+					conds: []requestCondition{
+						requireRangeQueryPath,
+						formCond{key: "query", value: `count({__name__=~"(foo|bar)_panics_total",job="myjob"})`},
+					},
+					resp: respondWithEmptyMatrix(),
 				},
 			},
 		},