Include query lines in template check errors

cloudflare · Jan 6, 2022 · f175acb · f175acb
1 parent a043fd8
commit f175acb
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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,