Skip to content

Commit

Permalink
Stop checking rule if we got an entry
Browse files Browse the repository at this point in the history
  • Loading branch information
prymitive committed Mar 25, 2022
1 parent 4e6b17e commit d7e7137
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 8 deletions.
7 changes: 7 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Changelog

## v0.15.5

### Fixed

- `promql/series` check was reporting both `Warning` and `Bug` problems for the
same metric when it was using newly added recording rule.

## v0.15.4

### Fixed
Expand Down
9 changes: 8 additions & 1 deletion internal/checks/promql_series.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,11 @@ func (c SeriesCheck) Check(ctx context.Context, rule parser.Rule, entries []disc
}

// if it's not there recurse checks onto it
problems = append(problems, c.Check(ctx, rrEntry.Rule, entries)...)
for _, p := range c.Check(ctx, rrEntry.Rule, entries) {
p.Lines = expr.Lines()
problems = append(problems, p)
}
continue
}

problems = append(problems, Problem{
Expand Down Expand Up @@ -529,5 +533,8 @@ func wrapLabelReplaces(q string, lms []*labels.Matcher) string {
onLabels = append(onLabels, lm.Name)
}
}
if len(onLabels) == 0 {
return fmt.Sprintf(`count(%s)`, q)
}
return fmt.Sprintf(`count(%s AND on(%s) %s)`, q, strings.Join(onLabels, ","), out)
}
107 changes: 100 additions & 7 deletions internal/checks/promql_series_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,13 +362,6 @@ func TestSeriesCheck(t *testing.T) {
Text: noMetricText("prom", uri, "foo", "1w"),
Severity: checks.Bug,
},
{
Fragment: "foo:bar",
Lines: []int{2},
Reporter: checks.SeriesCheckName,
Text: noMetricText("prom", uri, "foo:bar", "1w"),
Severity: checks.Bug,
},
}
},
mocks: []*prometheusMock{
Expand Down Expand Up @@ -1362,6 +1355,106 @@ func TestSeriesCheck(t *testing.T) {
},
},
},
{
description: "alert rule using 2 recording rules",
content: "- alert: foo\n expr: sum(foo:count) / sum(foo:sum) > 120\n",
checker: newSeriesCheck,
entries: mustParseContent("- record: foo:count\n expr: count(foo)\n- record: foo:sum\n expr: sum(foo)\n"),
problems: func(uri string) []checks.Problem {
return []checks.Problem{
{
Fragment: "foo:count",
Lines: []int{2},
Reporter: checks.SeriesCheckName,
Text: noMetricRRText("prom", uri, "foo:count", "1w"),
Severity: checks.Information,
},
{
Fragment: `foo`,
Lines: []int{2},
Reporter: checks.SeriesCheckName,
Text: noSeriesText("prom", uri, "foo", "1w"),
Severity: checks.Bug,
},
{
Fragment: "foo:sum",
Lines: []int{2},
Reporter: checks.SeriesCheckName,
Text: noMetricRRText("prom", uri, "foo:sum", "1w"),
Severity: checks.Information,
},
{
Fragment: `foo`,
Lines: []int{2},
Reporter: checks.SeriesCheckName,
Text: noSeriesText("prom", uri, "foo", "1w"),
Severity: checks.Bug,
},
}
},
mocks: []*prometheusMock{
{
conds: []requestCondition{
requireQueryPath,
formCond{key: "query", value: `count(foo:count)`},
},
resp: respondWithEmptyVector(),
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: `count(foo:count)`},
},
resp: respondWithEmptyMatrix(),
},

{
conds: []requestCondition{
requireQueryPath,
formCond{key: "query", value: `count(foo:sum)`},
},
resp: respondWithEmptyVector(),
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: `count(foo:sum)`},
},
resp: respondWithEmptyMatrix(),
},

{
conds: []requestCondition{
requireQueryPath,
formCond{key: "query", value: `count(sum(foo))`},
},
resp: respondWithEmptyVector(),
},

{
conds: []requestCondition{
requireQueryPath,
formCond{key: "query", value: `count(count(foo))`},
},
resp: respondWithEmptyVector(),
},

{
conds: []requestCondition{
requireQueryPath,
formCond{key: "query", value: `count(foo)`},
},
resp: respondWithEmptyVector(),
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: `count(foo)`},
},
resp: respondWithEmptyMatrix(),
},
},
},
}
runTests(t, testCases)
}

0 comments on commit d7e7137

Please sign in to comment.