Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 5 additions & 0 deletions internal/checks/query_cost.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 53 additions & 0 deletions internal/checks/query_cost_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
Expand Down
5 changes: 5 additions & 0 deletions internal/checks/query_cost_test.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down
Loading