Skip to content

Commit

Permalink
More strict type checks for promql/counter
Browse files Browse the repository at this point in the history
  • Loading branch information
prymitive committed Mar 21, 2024
1 parent 65b6380 commit 8474620
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 12 deletions.
8 changes: 8 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Changelog

## v0.56.1

### Fixed

- [promql/counter](checks/promql/counter.md) check will now consider a metric
to be a counter only if all metadata entries for it use `TYPE counter`.
Previously it would check for at least one metadata entry with `TYPE counter`.

## v0.56.0

### Added
Expand Down
30 changes: 18 additions & 12 deletions internal/checks/promql_counter.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,22 +105,28 @@ LOOP:
Text: text,
Severity: severity,
})
continue
continue LOOP
}
if len(metadata.Metadata) == 0 {
// No metadata so we don't know what type it uses.
continue LOOP
}
for _, m := range metadata.Metadata {
if m.Type == v1.MetricTypeCounter {
problems = append(problems, Problem{
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Text: fmt.Sprintf("`%s` is a counter according to metrics metadata from %s, you can't use its value directly.",
selector.Name,
promText(c.prom.Name(), metadata.URI),
),
Details: CounterCheckDetails,
Severity: Bug,
})
if m.Type != v1.MetricTypeCounter {
// There's metadata with non-counter type, so it's not always a counter.
continue LOOP
}
}
problems = append(problems, Problem{
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Text: fmt.Sprintf("`%s` is a counter according to metrics metadata from %s, you can't use its value directly.",
selector.Name,
promText(c.prom.Name(), metadata.URI),
),
Details: CounterCheckDetails,
Severity: Bug,
})

done[selector.Name] = struct{}{}
}
Expand Down
18 changes: 18 additions & 0 deletions internal/checks/promql_counter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,24 @@ func TestCounterCheck(t *testing.T) {
},
},
},
{
description: "counter > 1 / mixed metadata",
content: `
- alert: my alert
expr: http_requests_total{cluster="prod"} > 1
`,
checker: newCounterCheck,
prometheus: newSimpleProm,
problems: noProblems,
mocks: []*prometheusMock{
{
conds: []requestCondition{requireMetadataPath},
resp: metadataResponse{metadata: map[string][]v1.Metadata{
"http_requests_total": {{Type: "counter"}, {Type: "gauge"}, {Type: "counter"}},
}},
},
},
},
}
runTests(t, testCases)
}

0 comments on commit 8474620

Please sign in to comment.