Skip to content

Commit

Permalink
Improve alerts/template logic
Browse files Browse the repository at this point in the history
  • Loading branch information
prymitive committed Nov 26, 2021
1 parent 454cb49 commit 3b6c7e8
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 2 deletions.
20 changes: 20 additions & 0 deletions internal/checks/alerts_template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,26 @@ func TestTemplateCheck(t *testing.T) {
},
},
},
{
description: "annotation label missing from metrics (group_left)",
content: `
- alert: Foo Is Down
expr: count(build_info) by (instance, version) != ignoring(package) group_left(foo) count(package_installed) by (instance, version, package)
annotations:
summary: '{{ $labels.instance }} on {{ .Labels.foo }} in down'
help: '{{ $labels.ixtance }}'
`,
checker: checks.NewTemplateCheck(checks.Bug),
problems: []checks.Problem{
{
Fragment: `help: {{ $labels.ixtance }}`,
Lines: []int{6},
Reporter: "alerts/template",
Text: `template is using "ixtance" label but the query doesn't preseve it`,
Severity: checks.Bug,
},
},
},
}
runTests(t, testCases)
}
38 changes: 36 additions & 2 deletions internal/parser/utils/aggregation.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,43 @@ func HasOuterAggregation(node *parser.PromQLNode) (aggs []*promParser.AggregateE

NEXT:
if n, ok := node.Node.(*promParser.BinaryExpr); ok {
if n.VectorMatching != nil {
switch n.VectorMatching.Card {
case promParser.CardOneToOne:
case promParser.CardOneToMany:
for i, child := range node.Children {
if i == len(node.Children)-1 {
a := HasOuterAggregation(child)
if len(a) > 0 && !a[0].Without {
a[0].Grouping = append(a[0].Grouping, n.VectorMatching.Include...)
}
return a
}
}
case promParser.CardManyToOne:
a := HasOuterAggregation(node.Children[0])
if len(a) > 0 && !a[0].Without {
a[0].Grouping = append(a[0].Grouping, n.VectorMatching.Include...)
}
return a
case promParser.CardManyToMany:
default:
log.Warn().Str("matching", n.VectorMatching.Card.String()).Msg("Unsupported VectorMatching operation")
}
}

if n.Op.IsComparisonOperator() {
for _, child := range node.Children {
return HasOuterAggregation(child)
for i, child := range node.Children {
if n.VectorMatching != nil {
a := HasOuterAggregation(child)
if len(a) > 0 && !a[0].Without {
a[0].Grouping = append(a[0].Grouping, n.VectorMatching.Include...)
}
return a
}
if i == 0 {
return HasOuterAggregation(child)
}
}
} else {
switch n.Op {
Expand Down
12 changes: 12 additions & 0 deletions internal/parser/utils/aggregation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,18 @@ func TestHasOuterAggregation(t *testing.T) {
expr: "(foo unless on(instance, version, package) bar) and on(instance) (sum(enabled) by(instance) > 0)",
output: []string{},
},
{
expr: "count(build_info) by (instance, version) != ignoring(bar) group_left(package) count(foo) by (instance, version, package)",
output: []string{"count by(instance, version, package) (build_info)"},
},
{
expr: "sum(foo) without() != on() group_left(instance) sum(vector(0))",
output: []string{"sum without() (foo)"},
},
{
expr: "sum(foo) != on() group_right(instance) sum(vector(0))",
output: []string{"sum by(instance) (vector(0))"},
},
}

for _, tc := range testCases {
Expand Down

0 comments on commit 3b6c7e8

Please sign in to comment.