Add comment option to all checks

docs/changelog.md

@@ -1,5 +1,30 @@
 # Changelog
 
+## v0.56.0
+
+### Added
+
+- Check using custom rules now accept an optional `comment` option for adding
+  a text comment to all reported problems.
+  Checks supporting `comment` option:
+  - [alerts/annotation](checks/alerts/annotation.md)
+  - [alerts/count](checks/alerts/count.md)
+  - [promql/aggregate](checks/promql/aggregate.md)
+  - [query/cost](checks/query/cost.md)
+  - [rule/for](checks/rule/for.md)
+  - [rule/label](checks/rule/label.md)
+  - [rule/link](checks/rule/link.md)
+  - [rule/reject](checks/rule/reject.md)
+  Example:
+
+  ```js
+  for {
+    comment  = "All alert rules must have `for: 5m` (or more) to avoid flaky alerts."
+    severity = "bug"
+    min      = "5m"
+  }
+  ```
+
 ## v0.55.0
 
 ### Added

docs/checks/alerts/annotation.md

@@ -14,6 +14,7 @@ Syntax:
 
 ```js
 annotation "$pattern" {
+  comment  = "..."
   severity = "bug|warning|info"
   token    = "(.*)"
   value    = "(.*)"
@@ -25,6 +26,7 @@ annotation "$pattern" {
 - `$pattern` - regexp pattern to match annotation name on, this can be templated
   to reference checked rule fields, see [Configuration](../../configuration.md)
   for details.
+- `comment` - set a custom comment that will be added to reported problems.
 - `severity` - set custom severity for reported issues, defaults to a warning.
 - `token` - optional regexp to tokenize annotation value before validating it.
   By default the whole annotation value is validated against `value` regexp or
@@ -33,7 +35,7 @@ annotation "$pattern" {
   to a regexp that captures a single sub-string.
 - `value` - optional value regexp to enforce, if not set only pint will only
   check if the annotation exists.
-- `values` - optional list of allowed values - this is alternative to using
+- `values` - optional list of allowed values - this is alternative to using.
   `value` regexp. Set this to the list of all possible valid annotation values.
 - `required` - if `true` pint will require every rule to have this annotation set,
   if `false` it will only check values where annotation is set.
@@ -63,6 +65,7 @@ rule {
 
   annotation "dashboard" {
     severity = "bug"
+    comment  = "You must add a link do a Grafana dashboard showing impact of this alert"
     value    = "https://grafana\.example\.com/.+"
   }
 }

docs/checks/alerts/count.md

@@ -21,6 +21,7 @@ alerts {
   step       = "1m"
   resolve    = "5m"
   minCount   = 0
+  comment    = "..."
   severity   = "bug|warning|info"
 }
 ```
@@ -35,6 +36,7 @@ alerts {
 - `minCount` - minimal number of alerts for this check to report it. Default to `0`.
   Set this to a no-zero value if you want this check to report only if the estimated
   number of alerts is high enough.
+- `comment` - set a custom comment that will be added to reported problems.
 - `severity` - set custom severity for reported issues, defaults to `info`.
   This can be only set when `minCount` is set to a non-zero value.
 
@@ -76,6 +78,7 @@ rule {
     step       = "1m"
     resolve    = "5m"
     minCount   = 50
+    comment    = "You cannot add an rule that would immediately fire 50+ alerts, fix the problem first"
     severity   = "bug"
   }
 }

docs/checks/promql/aggregate.md

@@ -16,18 +16,20 @@ Syntax:
 
 ```js
 aggregate "$pattern" {
+  comment  = "..."
   severity = "bug|warning|info"
-  keep = [ "...", ... ]
-  strip = [ "...", ... ]
+  keep     = [ "...", ... ]
+  strip    = [ "...", ... ]
 }
 ```
 
 - `$pattern` - regexp pattern to match rule name on, this can be templated
   to reference checked rule fields, see [Configuration](../../configuration.md)
-  for details
-- `severity` - set custom severity for reported issues, defaults to a warning
-- `keep` - list of label names that must be preserved
-- `strip` - list of label names that must be stripped
+  for details.
+- `comment` - set a custom comment that will be added to reported problems.
+- `severity` - set custom severity for reported issues, defaults to a warning.
+- `keep` - list of label names that must be preserved.
+- `strip` - list of label names that must be stripped.
 
 ## How to enable it
 
@@ -46,7 +48,8 @@ rule {
     kind = "recording"
   }
   aggregate ".+" {
-    keep = ["job"]
+    keep    = ["job"]
+    comment = "`job` label must be kept on all aggregated metrics so we can link it with a scrape job"
   }
 }
 ```

docs/checks/query/cost.md

@@ -62,6 +62,7 @@ Syntax:
 
 ```js
 cost {
+  comment               = "..."
   severity              = "bug|warning|info"
   maxSeries             = 5000
   maxPeakSamples        = 10000
@@ -70,6 +71,7 @@ cost {
 }
 ```
 
+- `comment` - set a custom comment that will be added to reported problems.
 - `severity` - set custom severity for reported issues, defaults to a warning.
   This is only used when query result series exceed `maxSeries` value (if set).
   If `maxSeries` is not set or when results count is below it pint will still
@@ -122,6 +124,7 @@ rule {
     maxPeakSamples        = 300000
     maxEvaluationDuration = "30s"
     severity              = "bug"
+    comment               = "This query is too expensive to run" 
   }
 }
 ```

docs/checks/rule/for.md

@@ -23,13 +23,21 @@ blocks, depending on which alerting rule field you want to enforce.
 Syntax:
 
 ```js
-for|keep_firing_for {
+for {
+  comment  = "comment"
+  severity = "bug|warning|info"
+  min      = "5m"
+  max      = "10m"
+}
+keep_firing_for {
+  comment  = "comment"
   severity = "bug|warning|info"
   min      = "5m"
   max      = "10m"
 }
 ```
 
+- `comment` - set a custom comment that will be added to reported problems.
 - `severity` - set custom severity for reported issues, defaults to a bug.
 - `min` - minimum required `for` value for matching alerting rules.
   If not set minimum `for` duration won't be enforced.
@@ -42,9 +50,9 @@ Enforce that all alerts have `for` fields of `5m` or more:
 
 ```js
 for {
+  comment  = "All alert rules must have for:5m (or more) to avoid flaky alerts"
   severity = "bug"
   min      = "5m"
-  max      = "10m"
 }
 ```
 

docs/checks/rule/label.md

@@ -17,6 +17,7 @@ Syntax:
 
 ```js
 label "$pattern" {
+  comment  = "..."
   severity = "bug|warning|info"
   token    = "(.*)"
   value    = "(.*)"
@@ -28,6 +29,7 @@ label "$pattern" {
 - `$pattern` - regexp pattern to match label name on, this can be templated
   to reference checked rule fields, see [Configuration](../../configuration.md)
   for details.
+- `comment` - set a custom comment that will be added to reported problems.
 - `severity` - set custom severity for reported issues, defaults to a warning.
 - `token` - optional regexp to tokenize label value before validating it.
   By default the whole label value is validated against `value` regexp or
@@ -58,6 +60,7 @@ rule {
   }
 
   label "severity" {
+    comment  = "You must set a `severity` label on all alert rules"
     value    = "(warning|critical)"
     required = true
   }

docs/checks/rule/link.md

@@ -16,6 +16,7 @@ Syntax:
 
 ```js
 link "$pattern" {
+  comment  = "..."
   severity = "bug|warning|info"
   uri      = "..."
   timeout  = "1m"
@@ -28,6 +29,7 @@ link "$pattern" {
   rule fields, see [Configuration](../../configuration.md) for details.
 - `uri` - optional URI rewrite rule, this will be used as the URI for all
   requests if specified, regexp capture groups can be referenced here.
+- `comment` - set a custom comment that will be added to reported problems.
 - `severity` - set custom severity for reported issues, defaults to a bug.
 - `timeout` - timeout to be used for all request, defaults to 1 minute.
 - `headers` - a list of HTTP headers to set on all requests.
@@ -61,7 +63,9 @@ Only validate specific hostname.
 
 ```js
 rule {
-  link "https?://runbooks.example.com/.+" {}
+  link "https?://runbooks.example.com/.+" {
+    comment = "Please link every alert to a run-book"
+  }
 }
 ```
 

docs/checks/rule/reject.md

@@ -15,6 +15,7 @@ Syntax:
 
 ```js
 reject "$pattern" {
+  comment           = "..."
   severity          = "bug|warning|info"
   label_keys        = true|false
   label_values      = true|false
@@ -25,7 +26,8 @@ reject "$pattern" {
 
 - `$pattern` - regexp pattern to reject, this can be templated
   to reference checked rule fields, see [Configuration](../../configuration.md)
-  for details
+  for details.
+- `comment` - set a custom comment that will be added to reported problems.
 - `severity` - set custom severity for reported issues, defaults to a bug.
 - `label_keys` - if true label keys for recording and alerting rules will
   be checked.
@@ -52,7 +54,8 @@ rule {
   }
 
   reject "https?://.+" {
-    label_keys = true
+    comment      = "Don't use URIs as alert labels, pass them as annotations instead"
+    label_keys   = true
     label_values = true
   }
 }

internal/checks/alerts_annotation.go

@@ -16,13 +16,14 @@ const (
 	AnnotationCheckName = "alerts/annotation"
 )
 
-func NewAnnotationCheck(keyRe, tokenRe, valueRe *TemplatedRegexp, values []string, isReguired bool, severity Severity) AnnotationCheck {
+func NewAnnotationCheck(keyRe, tokenRe, valueRe *TemplatedRegexp, values []string, isReguired bool, comment string, severity Severity) AnnotationCheck {
 	return AnnotationCheck{
 		keyRe:      keyRe,
 		tokenRe:    tokenRe,
 		valueRe:    valueRe,
 		values:     values,
 		isReguired: isReguired,
+		comment:    comment,
 		severity:   severity,
 	}
 }
@@ -31,9 +32,10 @@ type AnnotationCheck struct {
 	keyRe      *TemplatedRegexp
 	tokenRe    *TemplatedRegexp
 	valueRe    *TemplatedRegexp
+	comment    string
 	values     []string
-	isReguired bool
 	severity   Severity
+	isReguired bool
 }
 
 func (c AnnotationCheck) Meta() CheckMeta {
@@ -70,6 +72,7 @@ func (c AnnotationCheck) Check(_ context.Context, _ string, rule parser.Rule, _
 				Lines:    rule.Lines,
 				Reporter: c.Reporter(),
 				Text:     fmt.Sprintf("`%s` annotation is required.", c.keyRe.original),
+				Details:  maybeComment(c.comment),
 				Severity: c.severity,
 			})
 		}
@@ -89,6 +92,7 @@ func (c AnnotationCheck) Check(_ context.Context, _ string, rule parser.Rule, _
 			Lines:    rule.AlertingRule.Annotations.Lines,
 			Reporter: c.Reporter(),
 			Text:     fmt.Sprintf("`%s` annotation is required.", c.keyRe.original),
+			Details:  maybeComment(c.comment),
 			Severity: c.severity,
 		})
 		return problems
@@ -113,6 +117,7 @@ func (c AnnotationCheck) checkValue(rule parser.Rule, value string, lines parser
 			Lines:    lines,
 			Reporter: c.Reporter(),
 			Text:     fmt.Sprintf("`%s` annotation value `%s` must match `%s`.", c.keyRe.original, value, c.valueRe.anchored),
+			Details:  maybeComment(c.comment),
 			Severity: c.severity,
 		})
 	}
@@ -131,6 +136,11 @@ func (c AnnotationCheck) checkValue(rule parser.Rule, value string, lines parser
 					break
 				}
 			}
+			if c.comment != "" {
+				details.WriteRune('\n')
+				details.WriteString("Rule comment: ")
+				details.WriteString(c.comment)
+			}
 			problems = append(problems, Problem{
 				Lines:    lines,
 				Reporter: c.Reporter(),
@@ -142,3 +152,10 @@ func (c AnnotationCheck) checkValue(rule parser.Rule, value string, lines parser
 	}
 	return problems
 }
+
+func maybeComment(c string) string {
+	if c != "" {
+		return fmt.Sprintf("Rule comment: %s", c)
+	}
+	return ""
+}