From f175acbf7341a8e7ffe4a9d8e751ee72ce9724df Mon Sep 17 00:00:00 2001 From: Lukasz Mierzwa Date: Thu, 6 Jan 2022 15:45:43 +0000 Subject: [PATCH] Include query lines in template check errors --- CHANGELOG.md | 6 ++++++ internal/checks/alerts_template.go | 15 ++++++++++---- internal/checks/alerts_template_test.go | 26 ++++++++++++------------- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33eff330..64962526 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## v0.6.2 + +### Changed + +- `template` check will now include alert query line numbers when reporting issues. + ## v0.6.1 ### Fixed diff --git a/internal/checks/alerts_template.go b/internal/checks/alerts_template.go index d06b9eb0..db780f06 100644 --- a/internal/checks/alerts_template.go +++ b/internal/checks/alerts_template.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "sort" "strings" textTemplate "text/template" "text/template/parse" @@ -124,7 +125,7 @@ func (c TemplateCheck) Check(rule parser.Rule) (problems []Problem) { for _, msg := range checkMetricLabels(msgAggregation, label.Key.Value, label.Value.Value, aggr.Grouping, aggr.Without) { problems = append(problems, Problem{ Fragment: fmt.Sprintf("%s: %s", label.Key.Value, label.Value.Value), - Lines: label.Lines(), + Lines: mergeLines(label.Lines(), rule.AlertingRule.Expr.Lines()), Reporter: TemplateCheckName, Text: msg, Severity: Bug, @@ -139,7 +140,7 @@ func (c TemplateCheck) Check(rule parser.Rule) (problems []Problem) { for _, msg := range checkMetricLabels(msgAbsent, label.Key.Value, label.Value.Value, absentLabels(call), false) { problems = append(problems, Problem{ Fragment: fmt.Sprintf("%s: %s", label.Key.Value, label.Value.Value), - Lines: label.Lines(), + Lines: mergeLines(label.Lines(), rule.AlertingRule.Expr.Lines()), Reporter: TemplateCheckName, Text: msg, Severity: Bug, @@ -165,7 +166,7 @@ func (c TemplateCheck) Check(rule parser.Rule) (problems []Problem) { for _, msg := range checkMetricLabels(msgAggregation, annotation.Key.Value, annotation.Value.Value, aggr.Grouping, aggr.Without) { problems = append(problems, Problem{ Fragment: fmt.Sprintf("%s: %s", annotation.Key.Value, annotation.Value.Value), - Lines: annotation.Lines(), + Lines: mergeLines(annotation.Lines(), rule.AlertingRule.Expr.Lines()), Reporter: TemplateCheckName, Text: msg, Severity: Bug, @@ -180,7 +181,7 @@ func (c TemplateCheck) Check(rule parser.Rule) (problems []Problem) { for _, msg := range checkMetricLabels(msgAbsent, annotation.Key.Value, annotation.Value.Value, absentLabels(call), false) { problems = append(problems, Problem{ Fragment: fmt.Sprintf("%s: %s", annotation.Key.Value, annotation.Value.Value), - Lines: annotation.Lines(), + Lines: mergeLines(annotation.Lines(), rule.AlertingRule.Expr.Lines()), Reporter: TemplateCheckName, Text: msg, Severity: Bug, @@ -360,3 +361,9 @@ func absentLabels(node *parser.PromQLNode) []string { return names } + +func mergeLines(a, b []int) (l []int) { + l = append(a, b...) + sort.Ints(l) + return +} diff --git a/internal/checks/alerts_template_test.go b/internal/checks/alerts_template_test.go index 9d90a92a..aef37dd0 100644 --- a/internal/checks/alerts_template_test.go +++ b/internal/checks/alerts_template_test.go @@ -233,7 +233,7 @@ func TestTemplateCheck(t *testing.T) { problems: []checks.Problem{ { Fragment: `summary: {{ $labels.job }}`, - Lines: []int{4}, + Lines: []int{2, 4}, Reporter: "alerts/template", Text: `template is using "job" label but the query removes it`, Severity: checks.Bug, @@ -247,7 +247,7 @@ func TestTemplateCheck(t *testing.T) { problems: []checks.Problem{ { Fragment: `summary: {{ .Labels.job }}`, - Lines: []int{4}, + Lines: []int{2, 4}, Reporter: "alerts/template", Text: `template is using "job" label but the query removes it`, Severity: checks.Bug, @@ -261,7 +261,7 @@ func TestTemplateCheck(t *testing.T) { problems: []checks.Problem{ { Fragment: `summary: {{ $labels.job }}`, - Lines: []int{4}, + Lines: []int{2, 4}, Reporter: "alerts/template", Text: `template is using "job" label but the query removes it`, Severity: checks.Bug, @@ -275,7 +275,7 @@ func TestTemplateCheck(t *testing.T) { problems: []checks.Problem{ { Fragment: `summary: {{ .Labels.job }}`, - Lines: []int{4}, + Lines: []int{2, 4}, Reporter: "alerts/template", Text: `template is using "job" label but the query removes it`, Severity: checks.Bug, @@ -289,7 +289,7 @@ func TestTemplateCheck(t *testing.T) { problems: []checks.Problem{ { Fragment: `summary: {{ .Labels.job }}`, - Lines: []int{4}, + Lines: []int{2, 4}, Reporter: "alerts/template", Text: `template is using "job" label but the query removes it`, Severity: checks.Bug, @@ -303,7 +303,7 @@ func TestTemplateCheck(t *testing.T) { problems: []checks.Problem{ { Fragment: `summary: {{ .Labels.job }}`, - Lines: []int{4}, + Lines: []int{2, 4}, Reporter: "alerts/template", Text: `template is using "job" label but the query removes it`, Severity: checks.Bug, @@ -323,7 +323,7 @@ func TestTemplateCheck(t *testing.T) { problems: []checks.Problem{ { Fragment: `help: {{ $labels.ixtance }}`, - Lines: []int{6}, + Lines: []int{3, 6}, Reporter: "alerts/template", Text: `template is using "ixtance" label but the query removes it`, Severity: checks.Bug, @@ -355,28 +355,28 @@ func TestTemplateCheck(t *testing.T) { problems: []checks.Problem{ { Fragment: "instance: {{ $labels.instance }}", - Lines: []int{5}, + Lines: []int{3, 5}, Reporter: "alerts/template", Text: `template is using "instance" label but absent() is not passing it`, Severity: checks.Bug, }, { Fragment: `summary: {{ $labels.instance }} on {{ .Labels.foo }} is missing`, - Lines: []int{7}, + Lines: []int{3, 7}, Reporter: "alerts/template", Text: `template is using "instance" label but absent() is not passing it`, Severity: checks.Bug, }, { Fragment: `summary: {{ $labels.instance }} on {{ .Labels.foo }} is missing`, - Lines: []int{7}, + Lines: []int{3, 7}, Reporter: "alerts/template", Text: `template is using "foo" label but absent() is not passing it`, Severity: checks.Bug, }, { Fragment: "help: {{ $labels.xxx }}", - Lines: []int{8}, + Lines: []int{3, 8}, Reporter: "alerts/template", Text: `template is using "xxx" label but absent() is not passing it`, Severity: checks.Bug, @@ -405,7 +405,7 @@ func TestTemplateCheck(t *testing.T) { problems: []checks.Problem{ { Fragment: `summary: {{ $labels.instance }} on {{ .Labels.job }} is missing`, - Lines: []int{5}, + Lines: []int{3, 5}, Reporter: "alerts/template", Text: `template is using "instance" label but the query removes it`, Severity: checks.Bug, @@ -424,7 +424,7 @@ func TestTemplateCheck(t *testing.T) { problems: []checks.Problem{ { Fragment: `summary: {{ .Labels.job }} is missing`, - Lines: []int{5}, + Lines: []int{3, 5}, Reporter: "alerts/template", Text: `template is using "job" label but absent() is not passing it`, Severity: checks.Bug,