Merge pull request #1173 from cloudflare/alerts/template

Improve alerts/template messages
cloudflare · Oct 28, 2024 · fbb89a2 · fbb89a2
2 parents cfb942f + 13c2f1c
commit fbb89a2
Show file tree

Hide file tree

Showing 4 changed files with 277 additions and 68 deletions.
diff --git a/internal/checks/alerts_template.go b/internal/checks/alerts_template.go
@@ -33,13 +33,9 @@ Since there are no matching time series there are also no labels. If some time s
 This means that the only labels you can get back from absent call are the ones you pass to it.
 If you're hoping to get instance specific labels this way and alert when some target is down then that won't work, use the ` + "`up`" + ` metric instead.`
 	TemplateCheckOnDetails = `Using [vector matching](https://prometheus.io/docs/prometheus/latest/querying/operators/#vector-matching) operations will impact which labels are available on the results of your query.
-When using ` + "`on()`" + `make sure that all labels you're trying to use in this templare match what the query can return.`
+When using ` + "`on()`" + ` make sure that all labels you're trying to use in this templare match what the query can return.
+For queries using ` + "ignoring()`" + ` any label included there will be stripped from the results.`
 	TemplateCheckLabelsDetails = `This query doesn't seem to be using any time series and so cannot have any labels.`
-
-	msgAggregation = "Template is using `%s` label but the query removes it."
-	msgAbsent      = "Template is using `%s` label but `absent()` is not passing it."
-	msgOn          = "Template is using `%s` label but the query uses `on(...)` without it being set there, this label will be missing from the query result."
-	msgVector      = "Template is using `%s` label but the query doesn't produce any labels."
 )
 
 var (
@@ -73,10 +69,10 @@ var (
 		"humanizeDuration":   dummyFuncMap,
 		"humanizePercentage": dummyFuncMap,
 		"humanizeTimestamp":  dummyFuncMap,
+		"toTime":             dummyFuncMap,
 		"pathPrefix":         dummyFuncMap,
 		"externalURL":        dummyFuncMap,
 		"parseDuration":      dummyFuncMap,
-		"toTime":             dummyFuncMap,
 	}
 )
 
@@ -456,11 +452,11 @@ func checkQueryLabels(labelName, labelValue string, src utils.Source) (problems
 				}
 				for _, s := range append(src.Alternatives, src) {
 					if s.FixedLabels && !slices.Contains(s.IncludedLabels, v[1]) {
-						problems = append(problems, textForProblem(v[1], s, Bug))
+						problems = append(problems, textForProblem(v[1], "", s, Bug))
 						goto NEXT
 					}
 					if slices.Contains(s.ExcludedLabels, v[1]) {
-						problems = append(problems, textForProblem(v[1], s, Bug))
+						problems = append(problems, textForProblem(v[1], v[1], s, Bug))
 						goto NEXT
 					}
 				}
@@ -473,19 +469,23 @@ func checkQueryLabels(labelName, labelValue string, src utils.Source) (problems
 	return problems
 }
 
-func textForProblem(label string, src utils.Source, severity Severity) exprProblem {
-	// FIXME add query fragment to the details
-
+func textForProblem(label, reasonLabel string, src utils.Source, severity Severity) exprProblem {
 	switch {
 	case src.Operation == "absent":
 		return exprProblem{
-			text:     fmt.Sprintf(msgAbsent, label),
+			text:     fmt.Sprintf("Template is using `%s` label but `%s` is not passing it.", label, src.Call.String()),
 			details:  TemplateCheckAbsentDetails,
 			severity: severity,
 		}
-	case src.Returns == promParser.ValueTypeScalar, src.Returns == promParser.ValueTypeString, src.Operation == "vector":
+	case src.Operation == "vector":
+		return exprProblem{
+			text:     fmt.Sprintf("Template is using `%s` label but `%s` doesn't produce any labels.", label, src.Call.String()),
+			details:  TemplateCheckLabelsDetails,
+			severity: severity,
+		}
+	case src.Returns == promParser.ValueTypeScalar, src.Returns == promParser.ValueTypeString:
 		return exprProblem{
-			text:     fmt.Sprintf(msgVector, label),
+			text:     fmt.Sprintf("Template is using `%s` label but the query doesn't produce any labels.", label),
 			details:  TemplateCheckLabelsDetails,
 			severity: severity,
 		}
@@ -496,13 +496,14 @@ func textForProblem(label string, src utils.Source, severity Severity) exprProbl
 		promParser.CardManyToOne.String(),
 	}, src.Operation):
 		return exprProblem{
-			text:     fmt.Sprintf(msgOn, label),
+			text: fmt.Sprintf("Template is using `%s` label but the query results won't have this label. %s",
+				label, src.ExcludeReason[reasonLabel]),
 			details:  TemplateCheckOnDetails,
 			severity: severity,
 		}
 	default:
 		return exprProblem{
-			text:     fmt.Sprintf(msgAggregation, label),
+			text:     fmt.Sprintf("Template is using `%s` label but the query removes it.", label),
 			details:  TemplateCheckAggregationDetails,
 			severity: severity,
 		}

diff --git a/internal/checks/alerts_template_test.go b/internal/checks/alerts_template_test.go
@@ -496,7 +496,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  5,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `instance` label but `absent()` is not passing it.",
+						Text:     "Template is using `instance` label but `absent(foo{job=\"bar\"})` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},
@@ -506,7 +506,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  7,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `instance` label but `absent()` is not passing it.",
+						Text:     "Template is using `instance` label but `absent(foo{job=\"bar\"})` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},
@@ -516,7 +516,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  7,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `foo` label but `absent()` is not passing it.",
+						Text:     "Template is using `foo` label but `absent(foo{job=\"bar\"})` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},
@@ -526,7 +526,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  8,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `xxx` label but `absent()` is not passing it.",
+						Text:     "Template is using `xxx` label but `absent(foo{job=\"bar\"})` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},
@@ -551,7 +551,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  5,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `instance` label but `absent()` is not passing it.",
+						Text:     "Template is using `instance` label but `absent(sum by (job, instance) (foo))` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},
@@ -561,7 +561,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  5,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `job` label but `absent()` is not passing it.",
+						Text:     "Template is using `job` label but `absent(sum by (job, instance) (foo))` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},
@@ -586,7 +586,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  5,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `instance` label but `absent()` is not passing it.",
+						Text:     "Template is using `instance` label but `absent(sum by (job) (foo))` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},
@@ -596,7 +596,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  5,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `job` label but `absent()` is not passing it.",
+						Text:     "Template is using `job` label but `absent(sum by (job) (foo))` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},
@@ -621,7 +621,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  5,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `job` label but `absent()` is not passing it.",
+						Text:     "Template is using `job` label but `absent({job=~\".+\"})` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},
@@ -646,7 +646,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  5,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `job` label but `absent()` is not passing it.",
+						Text:     "Template is using `job` label but `absent(bar)` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},
@@ -683,7 +683,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  5,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `cluster` label but `absent()` is not passing it.",
+						Text:     "Template is using `cluster` label but `absent(foo{job=\"xxx\"})` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},
@@ -693,7 +693,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  5,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `env` label but `absent()` is not passing it.",
+						Text:     "Template is using `env` label but `absent(foo{job=\"xxx\"})` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},
@@ -730,7 +730,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  5,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `cluster` label but `absent()` is not passing it.",
+						Text:     "Template is using `cluster` label but `absent(foo{job=\"xxx\"})` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},
@@ -740,7 +740,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  5,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `env` label but `absent()` is not passing it.",
+						Text:     "Template is using `env` label but `absent(foo{job=\"xxx\"})` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},
@@ -1216,7 +1216,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  4,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `instance` label but the query doesn't produce any labels.",
+						Text:     "Template is using `instance` label but `vector(1)` doesn't produce any labels.",
 						Details:  checks.TemplateCheckLabelsDetails,
 						Severity: checks.Bug,
 					},
@@ -1236,7 +1236,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  4,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `instance` label but the query doesn't produce any labels.",
+						Text:     "Template is using `instance` label but `vector(1)` doesn't produce any labels.",
 						Details:  checks.TemplateCheckLabelsDetails,
 						Severity: checks.Bug,
 					},
@@ -1292,7 +1292,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  6,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `job_name` label but the query uses `on(...)` without it being set there, this label will be missing from the query result.",
+						Text:     "Template is using `job_name` label but the query results won't have this label. Query is using one-to-one vector matching with `on(instance, app_name)`, only labels included inside `on(...)` will be present on the results.",
 						Details:  checks.TemplateCheckOnDetails,
 						Severity: checks.Bug,
 					},
@@ -1302,7 +1302,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  4,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `app_type` label but the query uses `on(...)` without it being set there, this label will be missing from the query result.",
+						Text:     "Template is using `app_type` label but the query results won't have this label. Query is using one-to-one vector matching with `on(instance, app_name)`, only labels included inside `on(...)` will be present on the results.",
 						Details:  checks.TemplateCheckOnDetails,
 						Severity: checks.Bug,
 					},
@@ -1390,6 +1390,31 @@ func TestTemplateCheck(t *testing.T) {
 			prometheus: noProm,
 			problems:   noProblems,
 		},
+		{
+			description: "bar * ignoring(job) foo",
+			content: `
+- alert: Foo
+  expr: bar * ignoring(job) foo
+  annotations:
+    summary: '{{ .Labels.job }} in cluster {{$labels.cluster}}/{{ $labels.env }} is missing'
+`,
+			checker:    newTemplateCheck,
+			prometheus: noProm,
+			problems: func(_ string) []checks.Problem {
+				return []checks.Problem{
+					{
+						Lines: parser.LineRange{
+							First: 5,
+							Last:  5,
+						},
+						Reporter: checks.TemplateCheckName,
+						Text:     "Template is using `job` label but the query results won't have this label. Query is using one-to-one vector matching with `ignoring(job)`, all labels included inside `ignoring(...)` will be removed on the results.",
+						Details:  checks.TemplateCheckOnDetails,
+						Severity: checks.Bug,
+					},
+				}
+			},
+		},
 	}
 	runTests(t, testCases)
 }