diff --git a/docs/changelog.md b/docs/changelog.md index 78de0fa2..4ffc449c 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -6,6 +6,7 @@ - Fixed a crash in `promql/series` check when Prometheus instance becomes unavailable - #682. +- Fixed false positive reports in `alerts/template` check - #681. ## v0.44.1 diff --git a/internal/checks/alerts_template_test.go b/internal/checks/alerts_template_test.go index a539caeb..69ca9507 100644 --- a/internal/checks/alerts_template_test.go +++ b/internal/checks/alerts_template_test.go @@ -388,7 +388,7 @@ func TestTemplateCheck(t *testing.T) { }, { description: "annotation label missing from metrics (1+)", - content: "- alert: Foo Is Down\n expr: 1 + sum(foo) by(job) + sum(foo) by(notjob)\n annotations:\n summary: '{{ .Labels.job }}'\n", + content: "- alert: Foo Is Down\n expr: 1 + sum(foo) by(notjob)\n annotations:\n summary: '{{ .Labels.job }}'\n", checker: newTemplateCheck, prometheus: noProm, problems: func(uri string) []checks.Problem { @@ -989,6 +989,29 @@ func TestTemplateCheck(t *testing.T) { sum(foo:count) by(job) > 20 labels: notify: "{{ $labels.notify }}" +`, + checker: newTemplateCheck, + prometheus: noProm, + problems: noProblems, + }, + { + description: "abs / scalar", + content: ` +- alert: ScyllaNonBalancedcqlTraffic + expr: > + abs(rate(scylla_cql_updates{conditional="no"}[1m]) - scalar(avg(rate(scylla_cql_updates{conditional="no"}[1m])))) + / + scalar(stddev(rate(scylla_cql_updates{conditional="no"}[1m])) + 100) > 2 + for: 10s + labels: + advisor: balanced + dashboard: cql + severity: moderate + status: "1" + team: team_devops + annotations: + description: CQL queries are not balanced among shards {{ $labels.instance }} shard {{ $labels.shard }} + summary: CQL queries are not balanced `, checker: newTemplateCheck, prometheus: noProm, diff --git a/internal/parser/utils/aggregation.go b/internal/parser/utils/aggregation.go index 9bcf4759..9e8d8059 100644 --- a/internal/parser/utils/aggregation.go +++ b/internal/parser/utils/aggregation.go @@ -78,7 +78,14 @@ NEXT: aggs = append(aggs, HasOuterAggregation(child)...) } return aggs - case promParser.DIV, promParser.LUNLESS, promParser.LAND: + case promParser.LUNLESS, promParser.LAND: + for _, child := range node.Children { + return HasOuterAggregation(child) + } + case promParser.DIV, promParser.SUB, promParser.ADD: + if _, ok := n.LHS.(*promParser.NumberLiteral); ok { + goto CHILDREN + } for _, child := range node.Children { return HasOuterAggregation(child) } @@ -86,6 +93,7 @@ NEXT: } } +CHILDREN: for _, child := range node.Children { aggs = append(aggs, HasOuterAggregation(child)...) } diff --git a/internal/parser/utils/aggregation_test.go b/internal/parser/utils/aggregation_test.go index e1fe509d..065adf2c 100644 --- a/internal/parser/utils/aggregation_test.go +++ b/internal/parser/utils/aggregation_test.go @@ -45,7 +45,7 @@ func TestHasOuterAggregation(t *testing.T) { }, { expr: "sum(foo) + sum(bar)", - output: []string{"sum(foo)", "sum(bar)"}, + output: []string{"sum(foo)"}, }, { expr: "foo / on(bbb) sum(bar)", @@ -64,7 +64,7 @@ func TestHasOuterAggregation(t *testing.T) { }, { expr: "1 + sum(foo) by(job) + sum(foo) by(notjob)", - output: []string{"sum by (job) (foo)", "sum by (notjob) (foo)"}, + output: []string{"sum by (job) (foo)"}, }, { expr: "sum(foo) by (job) > count(bar)", @@ -128,6 +128,15 @@ func TestHasOuterAggregation(t *testing.T) { expr: "(quantile(0.9, foo))", output: []string{"quantile(0.9, foo)"}, }, + { + expr: `abs(rate(scylla_cql_updates{conditional="no"}[1m]) - scalar(avg(rate(scylla_cql_updates{conditional="no"}[1m]))))`, + }, + { + expr: `abs(rate(scylla_cql_updates{conditional="no"}[1m]) + scalar(avg(rate(scylla_cql_updates{conditional="no"}[1m]))))`, + }, + { + expr: `abs(rate(scylla_cql_updates{conditional="no"}[1m]) / scalar(avg(rate(scylla_cql_updates{conditional="no"}[1m]))))`, + }, } for _, tc := range testCases {