diff --git a/docs/changelog.md b/docs/changelog.md index 843491cf..84504f30 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -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 diff --git a/internal/checks/promql_counter.go b/internal/checks/promql_counter.go index 3876838d..409e8ac1 100644 --- a/internal/checks/promql_counter.go +++ b/internal/checks/promql_counter.go @@ -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{}{} } diff --git a/internal/checks/promql_counter_test.go b/internal/checks/promql_counter_test.go index 548d08f1..4613ce30 100644 --- a/internal/checks/promql_counter_test.go +++ b/internal/checks/promql_counter_test.go @@ -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) }