Merge pull request #1752 from cloudflare/query

Fix query/cost suggestions

Author: prymitive

docs/changelog.md

@@ -7,6 +7,7 @@
 - [promql/vector_matching](checks/promql/vector_matching.md) check no longer reports problems that are also reported by [promql/impossible](checks/promql/impossible.md).
 - Fixed problem description in [promql/series](checks/promql/series.md) check for
   disable comments that have no effect.
+- Fixed invalid suggestions from the [query/cost](checks/query/cost.md) check.
 
 ## v0.79.0
 

internal/checks/query_cost.go

@@ -478,6 +478,11 @@ func (c CostCheck) isSuggestionFor(src, potential source.Source, join *source.Jo
 					goto NEXT
 				}
 			}
+			for _, name := range src.TransformedLabels(source.PossibleLabel, source.GuaranteedLabel) {
+				if !potential.CanHaveLabel(name) {
+					goto NEXT
+				}
+			}
 			var extra string
 			if len(lms) > 0 {
 				var buf strings.Builder

internal/checks/query_cost_test.go

@@ -1705,6 +1705,59 @@ func TestCostCheck(t *testing.T) {
 				},
 			},
 		},
+		{
+			// Query uses by(instance) aggregation but the recording rule uses without(instance),
+			// suggesting this rule would produce different label sets and is not a valid substitution.
+			description: "suggest recording rule / by vs without mismatch",
+			content:     "- alert: foo\n  expr: sum by(instance) (rate(foo_total[5m])) > 10\n",
+			checker: func(prom *promapi.FailoverGroup) checks.RuleChecker {
+				return checks.NewCostCheck(prom, 100, 100, 0, 0, "check comment", checks.Warning)
+			},
+			prometheus: newSimpleProm,
+			entries: mustParseContent(`
+- record: colo:foo
+  expr: sum(rate(foo_total[5m])) without(instance)
+`),
+			mocks: []*prometheusMock{
+				{
+					conds: []requestCondition{
+						requireQueryPath,
+						formCond{key: "query", value: "count(\nsum by(instance) (rate(foo_total[5m])) > 10\n)"},
+					},
+					resp: vectorResponse{
+						samples: []*model.Sample{
+							generateSample(map[string]string{}),
+							generateSample(map[string]string{}),
+							generateSample(map[string]string{}),
+							generateSample(map[string]string{}),
+							generateSample(map[string]string{}),
+							generateSample(map[string]string{}),
+							generateSample(map[string]string{}),
+						},
+						stats: promapi.QueryStats{
+							Samples: promapi.QuerySamples{
+								TotalQueryableSamples: 99,
+								PeakSamples:           19,
+							},
+							Timings: promapi.QueryTimings{
+								EvalTotalTime: 60.3,
+							},
+						},
+					},
+				},
+				{
+					conds: []requestCondition{
+						requireQueryPath,
+						formCond{key: "query", value: checks.BytesPerSampleQuery},
+					},
+					resp: vectorResponse{
+						samples: []*model.Sample{
+							generateSampleWithValue(map[string]string{}, 2048),
+						},
+					},
+				},
+			},
+		},
 		{
 			description: "comments at the end",
 			content: `

internal/checks/query_cost_test.snap

@@ -397,6 +397,11 @@
 
 ---
 
+[TestCostCheck/suggest_recording_rule_/_by_vs_without_mismatch - 1]
+[]
+
+---
+
 [TestCostCheck/suggest_recording_rule_/_complex - 1]
 - description: suggest recording rule / complex
   content: |