diff --git a/docs/changelog.md b/docs/changelog.md index caaf2f4f..98f1c974 100644 --- a/docs/changelog.md +++ b/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 diff --git a/docs/checks/alerts/annotation.md b/docs/checks/alerts/annotation.md index 576e812e..8b05eb4b 100644 --- a/docs/checks/alerts/annotation.md +++ b/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/.+" } } diff --git a/docs/checks/alerts/count.md b/docs/checks/alerts/count.md index 7b8febad..fd7053eb 100644 --- a/docs/checks/alerts/count.md +++ b/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" } } diff --git a/docs/checks/promql/aggregate.md b/docs/checks/promql/aggregate.md index 370eb058..8f38d431 100644 --- a/docs/checks/promql/aggregate.md +++ b/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" } } ``` diff --git a/docs/checks/query/cost.md b/docs/checks/query/cost.md index de922160..884daf0c 100644 --- a/docs/checks/query/cost.md +++ b/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" } } ``` diff --git a/docs/checks/rule/for.md b/docs/checks/rule/for.md index 5f90b124..8bd37605 100644 --- a/docs/checks/rule/for.md +++ b/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" } ``` diff --git a/docs/checks/rule/label.md b/docs/checks/rule/label.md index d9532bcd..a6de9f94 100644 --- a/docs/checks/rule/label.md +++ b/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 } diff --git a/docs/checks/rule/link.md b/docs/checks/rule/link.md index 800e2cae..61c02a75 100644 --- a/docs/checks/rule/link.md +++ b/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" + } } ``` diff --git a/docs/checks/rule/reject.md b/docs/checks/rule/reject.md index 4952615d..d2df2e1b 100644 --- a/docs/checks/rule/reject.md +++ b/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 } } diff --git a/internal/checks/alerts_annotation.go b/internal/checks/alerts_annotation.go index 3005badd..21b79b02 100644 --- a/internal/checks/alerts_annotation.go +++ b/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 "" +} diff --git a/internal/checks/alerts_annotation_test.go b/internal/checks/alerts_annotation_test.go index 50dbde10..9daf4c9e 100644 --- a/internal/checks/alerts_annotation_test.go +++ b/internal/checks/alerts_annotation_test.go @@ -14,7 +14,7 @@ func TestAnnotationCheck(t *testing.T) { description: "ignores recording rules", content: "- record: foo\n expr: sum(foo) without(\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, checks.Warning) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -23,7 +23,7 @@ func TestAnnotationCheck(t *testing.T) { description: "doesn't ignore rules with syntax errors", content: "- alert: foo\n expr: sum(foo) without(\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, checks.Warning) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -44,7 +44,7 @@ func TestAnnotationCheck(t *testing.T) { description: "no annotations / required", content: "- alert: foo\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, checks.Warning) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -65,7 +65,7 @@ func TestAnnotationCheck(t *testing.T) { description: "no annotations / not required", content: "- alert: foo\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, checks.Warning) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -74,7 +74,7 @@ func TestAnnotationCheck(t *testing.T) { description: "missing annotation / required", content: "- alert: foo\n expr: sum(foo)\n annotations:\n foo: bar\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, checks.Warning) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -95,7 +95,7 @@ func TestAnnotationCheck(t *testing.T) { description: "missing annotation / not required", content: "- alert: foo\n expr: sum(foo)\n annotations:\n foo: bar\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, checks.Warning) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -104,7 +104,7 @@ func TestAnnotationCheck(t *testing.T) { description: "wrong annotation value / required", content: "- alert: foo\n expr: sum(foo)\n annotations:\n severity: bar\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, checks.Warning) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -125,7 +125,7 @@ func TestAnnotationCheck(t *testing.T) { description: "wrong annotation value / not required", content: "- alert: foo\n expr: sum(foo)\n annotations:\n severity: bar\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, checks.Warning) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -146,7 +146,7 @@ func TestAnnotationCheck(t *testing.T) { description: "valid annotation / required", content: "- alert: foo\n expr: sum(foo)\n annotations:\n severity: info\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info|debug"), nil, true, checks.Warning) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info|debug"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -155,7 +155,7 @@ func TestAnnotationCheck(t *testing.T) { description: "valid annotation / not required", content: "- alert: foo\n expr: sum(foo)\n annotations:\n severity: info\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info|debug"), nil, false, checks.Warning) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info|debug"), nil, false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -164,7 +164,7 @@ func TestAnnotationCheck(t *testing.T) { description: "templated annotation value / passing", content: "- alert: foo\n expr: sum(foo)\n for: 5m\n annotations:\n for: 5m\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("for"), nil, checks.MustTemplatedRegexp("{{ $for }}"), nil, true, checks.Bug) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("for"), nil, checks.MustTemplatedRegexp("{{ $for }}"), nil, true, "", checks.Bug) }, prometheus: noProm, problems: noProblems, @@ -173,7 +173,7 @@ func TestAnnotationCheck(t *testing.T) { description: "templated annotation value / passing", content: "- alert: foo\n expr: sum(foo)\n for: 5m\n annotations:\n for: 4m\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("for"), nil, checks.MustTemplatedRegexp("{{ $for }}"), nil, true, checks.Bug) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("for"), nil, checks.MustTemplatedRegexp("{{ $for }}"), nil, true, "", checks.Bug) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -194,7 +194,7 @@ func TestAnnotationCheck(t *testing.T) { description: "valid annotation key regex / required", content: "- alert: foo\n expr: sum(foo)\n annotations:\n annotation_1: info\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("annotation_.*"), nil, checks.MustTemplatedRegexp("critical|info|debug"), nil, true, checks.Warning) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("annotation_.*"), nil, checks.MustTemplatedRegexp("critical|info|debug"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -203,7 +203,7 @@ func TestAnnotationCheck(t *testing.T) { description: "valid annotation key regex / not required", content: "- alert: foo\n expr: sum(foo)\n annotations:\n annotation_1: info\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("annotation_.*"), nil, checks.MustTemplatedRegexp("critical|info|debug"), nil, false, checks.Warning) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("annotation_.*"), nil, checks.MustTemplatedRegexp("critical|info|debug"), nil, false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -212,7 +212,7 @@ func TestAnnotationCheck(t *testing.T) { description: "wrong annotation key regex value / required", content: "- alert: foo\n expr: sum(foo)\n annotations:\n annotation_1: bar\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("annotation_.*"), nil, checks.MustTemplatedRegexp("critical"), nil, true, checks.Warning) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("annotation_.*"), nil, checks.MustTemplatedRegexp("critical"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -233,7 +233,7 @@ func TestAnnotationCheck(t *testing.T) { description: "wrong annotation key regex value / not required", content: "- alert: foo\n expr: sum(foo)\n annotations:\n annotation_1: bar\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("annotation_.*"), nil, checks.MustTemplatedRegexp("critical"), nil, false, checks.Warning) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("annotation_.*"), nil, checks.MustTemplatedRegexp("critical"), nil, false, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -254,7 +254,7 @@ func TestAnnotationCheck(t *testing.T) { description: "invalid value / token / valueRe", content: "- alert: foo\n expr: rate(foo[1m])\n annotations:\n components: api db\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("components"), checks.MustRawTemplatedRegexp("\\w+"), checks.MustTemplatedRegexp("api|memcached"), nil, false, checks.Bug) + return checks.NewAnnotationCheck(checks.MustTemplatedRegexp("components"), checks.MustRawTemplatedRegexp("\\w+"), checks.MustTemplatedRegexp("api|memcached"), nil, false, "rule comment", checks.Bug) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -266,6 +266,7 @@ func TestAnnotationCheck(t *testing.T) { }, Reporter: checks.AnnotationCheckName, Text: "`components` annotation value `db` must match `^api|memcached$`.", + Details: "Rule comment: rule comment", Severity: checks.Bug, }, } @@ -281,6 +282,7 @@ func TestAnnotationCheck(t *testing.T) { nil, []string{"api", "memcached", "storage", "prometheus", "kvm", "mysql", "memsql", "haproxy", "postgresql"}, false, + "rule comment", checks.Bug, ) }, @@ -294,7 +296,7 @@ func TestAnnotationCheck(t *testing.T) { }, Reporter: checks.AnnotationCheckName, Text: "`components` annotation value `db` is not one of valid values.", - Details: "List of allowed values:\n\n- `api`\n- `memcached`\n- `storage`\n- `prometheus`\n- `kvm`\n- `mysql`\n\nAnd 3 other value(s).", + Details: "List of allowed values:\n\n- `api`\n- `memcached`\n- `storage`\n- `prometheus`\n- `kvm`\n- `mysql`\n\nAnd 3 other value(s).\nRule comment: rule comment", Severity: checks.Bug, }, } diff --git a/internal/checks/alerts_count.go b/internal/checks/alerts_count.go index d566f0dd..a109deda 100644 --- a/internal/checks/alerts_count.go +++ b/internal/checks/alerts_count.go @@ -19,19 +19,21 @@ const ( AlertsCheckName = "alerts/count" ) -func NewAlertsCheck(prom *promapi.FailoverGroup, lookBack, step, resolve time.Duration, minCount int, severity Severity) AlertsCheck { +func NewAlertsCheck(prom *promapi.FailoverGroup, lookBack, step, resolve time.Duration, minCount int, comment string, severity Severity) AlertsCheck { return AlertsCheck{ prom: prom, lookBack: lookBack, step: step, resolve: resolve, minCount: minCount, + comment: comment, severity: severity, } } type AlertsCheck struct { prom *promapi.FailoverGroup + comment string lookBack time.Duration step time.Duration resolve time.Duration @@ -77,6 +79,7 @@ func (c AlertsCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ [] Lines: rule.AlertingRule.Expr.Value.Lines, Reporter: c.Reporter(), Text: text, + Details: maybeComment(c.comment), Severity: severity, }) return problems @@ -114,13 +117,18 @@ func (c AlertsCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ [] } delta := qr.Series.Until.Sub(qr.Series.From).Round(time.Minute) + details := fmt.Sprintf(`To get a preview of the alerts that would fire please [click here](%s/graph?g0.expr=%s&g0.tab=0&g0.range_input=%s).`, + qr.PublicURI, url.QueryEscape(rule.AlertingRule.Expr.Value.Value), output.HumanizeDuration(delta), + ) + if c.comment != "" { + details = fmt.Sprintf("%s\n%s", details, maybeComment(c.comment)) + } + problems = append(problems, Problem{ Lines: rule.AlertingRule.Expr.Value.Lines, Reporter: c.Reporter(), Text: fmt.Sprintf("%s would trigger %d alert(s) in the last %s.", promText(c.prom.Name(), qr.URI), alerts, output.HumanizeDuration(delta)), - Details: fmt.Sprintf(`To get a preview of the alerts that would fire please [click here](%s/graph?g0.expr=%s&g0.tab=0&g0.range_input=%s).`, - qr.PublicURI, url.QueryEscape(rule.AlertingRule.Expr.Value.Value), output.HumanizeDuration(delta), - ), + Details: details, Severity: c.severity, }) return problems diff --git a/internal/checks/alerts_count_test.go b/internal/checks/alerts_count_test.go index 6342616f..e9617793 100644 --- a/internal/checks/alerts_count_test.go +++ b/internal/checks/alerts_count_test.go @@ -14,18 +14,22 @@ import ( ) func newAlertsCheck(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAlertsCheck(prom, time.Hour*24, time.Minute, time.Minute*5, 0, checks.Information) + return checks.NewAlertsCheck(prom, time.Hour*24, time.Minute, time.Minute*5, 0, "", checks.Information) } func alertsText(name, uri string, count int, since string) string { return fmt.Sprintf("`%s` Prometheus server at %s would trigger %d alert(s) in the last %s.", name, uri, count, since) } -func alertsDetails(uri, query, since string) string { - return fmt.Sprintf( +func alertsDetails(uri, query, since, comment string) string { + details := fmt.Sprintf( `To get a preview of the alerts that would fire please [click here](%s/graph?g0.expr=%s&g0.tab=0&g0.range_input=%s).`, uri, url.QueryEscape(query), since, ) + if comment != "" { + details = fmt.Sprintf("%s\nRule comment: %s", details, comment) + } + return details } func TestAlertsCountCheck(t *testing.T) { @@ -109,7 +113,7 @@ func TestAlertsCountCheck(t *testing.T) { }, Reporter: "alerts/count", Text: alertsText("prom", uri, 0, "1d"), - Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d"), + Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d", ""), Severity: checks.Information, }, } @@ -138,7 +142,7 @@ func TestAlertsCountCheck(t *testing.T) { }, Reporter: "alerts/count", Text: alertsText("prom", uri, 7, "1d"), - Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d"), + Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d", ""), Severity: checks.Information, }, } @@ -226,7 +230,7 @@ func TestAlertsCountCheck(t *testing.T) { }, Reporter: "alerts/count", Text: alertsText("prom", uri, 2, "1d"), - Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d"), + Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d", ""), Severity: checks.Information, }, } @@ -291,7 +295,7 @@ func TestAlertsCountCheck(t *testing.T) { description: "minCount=2", content: "- alert: Foo Is Down\n for: 10m\n expr: up{job=\"foo\"} == 0\n", checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAlertsCheck(prom, time.Hour*24, time.Minute, time.Minute*5, 2, checks.Information) + return checks.NewAlertsCheck(prom, time.Hour*24, time.Minute, time.Minute*5, 2, "rule comment", checks.Information) }, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -303,7 +307,7 @@ func TestAlertsCountCheck(t *testing.T) { }, Reporter: "alerts/count", Text: alertsText("prom", uri, 2, "1d"), - Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d"), + Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d", "rule comment"), Severity: checks.Information, }, } @@ -368,7 +372,7 @@ func TestAlertsCountCheck(t *testing.T) { description: "minCount=2 severity=bug", content: "- alert: Foo Is Down\n for: 10m\n expr: up{job=\"foo\"} == 0\n", checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAlertsCheck(prom, time.Hour*24, time.Minute, time.Minute*5, 2, checks.Bug) + return checks.NewAlertsCheck(prom, time.Hour*24, time.Minute, time.Minute*5, 2, "", checks.Bug) }, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -380,7 +384,7 @@ func TestAlertsCountCheck(t *testing.T) { }, Reporter: "alerts/count", Text: alertsText("prom", uri, 2, "1d"), - Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d"), + Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d", ""), Severity: checks.Bug, }, } @@ -445,7 +449,7 @@ func TestAlertsCountCheck(t *testing.T) { description: "minCount=10", content: "- alert: Foo Is Down\n for: 10m\n expr: up{job=\"foo\"} == 0\n", checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAlertsCheck(prom, time.Hour*24, time.Minute, time.Minute*5, 10, checks.Information) + return checks.NewAlertsCheck(prom, time.Hour*24, time.Minute, time.Minute*5, 10, "", checks.Information) }, prometheus: newSimpleProm, problems: noProblems, @@ -522,7 +526,7 @@ func TestAlertsCountCheck(t *testing.T) { }, Reporter: "alerts/count", Text: alertsText("prom", uri, 3, "1d"), - Details: alertsDetails(uri, `{__name__="up", job="foo"} == 0`, "1d"), + Details: alertsDetails(uri, `{__name__="up", job="foo"} == 0`, "1d", ""), Severity: checks.Information, }, } @@ -582,7 +586,7 @@ func TestAlertsCountCheck(t *testing.T) { }, Reporter: "alerts/count", Text: alertsText("prom", uri, 3, "1d"), - Details: alertsDetails(uri, `{__name__=~"(up|foo)", job="foo"} == 0`, "1d"), + Details: alertsDetails(uri, `{__name__=~"(up|foo)", job="foo"} == 0`, "1d", ""), Severity: checks.Information, }, @@ -640,7 +644,7 @@ func TestAlertsCountCheck(t *testing.T) { }, Reporter: "alerts/count", Text: alertsText("prom", uri, 3, "1d"), - Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d"), + Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d", ""), Severity: checks.Information, }, @@ -698,7 +702,7 @@ func TestAlertsCountCheck(t *testing.T) { }, Reporter: "alerts/count", Text: alertsText("prom", uri, 2, "1d"), - Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d"), + Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d", ""), Severity: checks.Information, }, } @@ -773,7 +777,7 @@ func TestAlertsCountCheck(t *testing.T) { }, Reporter: "alerts/count", Text: alertsText("prom", uri, 1, "1d"), - Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d"), + Details: alertsDetails(uri, `up{job="foo"} == 0`, "1d", ""), Severity: checks.Information, }, } diff --git a/internal/checks/promql_aggregation.go b/internal/checks/promql_aggregation.go index 8009fac4..567d97af 100644 --- a/internal/checks/promql_aggregation.go +++ b/internal/checks/promql_aggregation.go @@ -15,15 +15,22 @@ const ( AggregationCheckName = "promql/aggregate" ) -func NewAggregationCheck(nameRegex *TemplatedRegexp, label string, keep bool, severity Severity) AggregationCheck { - return AggregationCheck{nameRegex: nameRegex, label: label, keep: keep, severity: severity} +func NewAggregationCheck(nameRegex *TemplatedRegexp, label string, keep bool, comment string, severity Severity) AggregationCheck { + return AggregationCheck{ + nameRegex: nameRegex, + label: label, + keep: keep, + comment: comment, + severity: severity, + } } type AggregationCheck struct { nameRegex *TemplatedRegexp label string - keep bool + comment string severity Severity + keep bool } func (c AggregationCheck) Meta() CheckMeta { @@ -78,6 +85,7 @@ func (c AggregationCheck) Check(_ context.Context, _ string, rule parser.Rule, _ Lines: expr.Value.Lines, Reporter: c.Reporter(), Text: problem.text, + Details: maybeComment(c.comment), Severity: c.severity, }) } diff --git a/internal/checks/promql_aggregation_test.go b/internal/checks/promql_aggregation_test.go index c9039195..72dafdd9 100644 --- a/internal/checks/promql_aggregation_test.go +++ b/internal/checks/promql_aggregation_test.go @@ -14,7 +14,7 @@ func TestAggregationCheck(t *testing.T) { description: "ignores rules with syntax errors", content: "- record: foo\n expr: sum(foo) without(\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -23,7 +23,7 @@ func TestAggregationCheck(t *testing.T) { description: "name must match / recording", content: "- record: foo\n expr: sum(foo) without(job)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp("bar"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp("bar"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -32,7 +32,7 @@ func TestAggregationCheck(t *testing.T) { description: "name must match /alerting", content: "- alert: foo\n expr: sum(foo) without(job)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp("bar"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp("bar"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -41,7 +41,7 @@ func TestAggregationCheck(t *testing.T) { description: "uses label from labels map / recording", content: "- record: foo\n expr: sum(foo) without(job)\n labels:\n job: foo\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -50,7 +50,7 @@ func TestAggregationCheck(t *testing.T) { description: "uses label from labels map / alerting", content: "- alert: foo\n expr: sum(foo) without(job)\n labels:\n job: foo\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -59,7 +59,7 @@ func TestAggregationCheck(t *testing.T) { description: "must keep job label / warning", content: "- record: foo\n expr: sum(foo) without(instance, job)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -80,7 +80,7 @@ func TestAggregationCheck(t *testing.T) { description: "must keep job label / bug", content: "- record: foo\n expr: sum(foo) without(instance, job)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Bug) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "some text", checks.Bug) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -92,6 +92,7 @@ func TestAggregationCheck(t *testing.T) { }, Reporter: checks.AggregationCheckName, Text: "`job` label is required and should be preserved when aggregating `^.+$` rules, remove job from `without()`.", + Details: "Rule comment: some text", Severity: checks.Bug, }, } @@ -101,7 +102,7 @@ func TestAggregationCheck(t *testing.T) { description: "must strip job label", content: "- record: foo\n expr: sum(foo) without(instance)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", false, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", false, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -122,7 +123,7 @@ func TestAggregationCheck(t *testing.T) { description: "must strip job label / being stripped", content: "- record: foo\n expr: sum(foo) without(instance,job)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", false, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -131,7 +132,7 @@ func TestAggregationCheck(t *testing.T) { description: "must strip job label / empty without", content: "- record: foo\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", false, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -140,7 +141,7 @@ func TestAggregationCheck(t *testing.T) { description: "nested rule must keep job label", content: "- record: foo\n expr: sum(sum(foo) without(job)) by(job)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -161,7 +162,7 @@ func TestAggregationCheck(t *testing.T) { description: "passing most outer aggregation should stop further strip checks", content: "- record: foo\n expr: sum(sum(foo) without(foo)) without(instance)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "instance", false, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "instance", false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -170,7 +171,7 @@ func TestAggregationCheck(t *testing.T) { description: "passing most outer aggregation should stop further checks", content: "- record: foo\n expr: sum(sum(foo) without(foo)) without(bar)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "instance", false, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "instance", false, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -200,7 +201,7 @@ func TestAggregationCheck(t *testing.T) { description: "passing most outer aggregation should continue further keep checks", content: "- record: foo\n expr: sum(sum(foo) without(job)) without(instance)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -221,7 +222,7 @@ func TestAggregationCheck(t *testing.T) { description: "Right hand side of AND is ignored", content: "- record: foo\n expr: foo AND on(instance) max(bar) without()\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -230,7 +231,7 @@ func TestAggregationCheck(t *testing.T) { description: "Left hand side of AND is checked", content: "- record: foo\n expr: max (foo) without(job) AND on(instance) bar\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -251,7 +252,7 @@ func TestAggregationCheck(t *testing.T) { description: "Right hand side of group_left() is ignored", content: "- record: foo\n expr: sum without(id) (foo) / on(type) group_left() sum without(job) (bar)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -260,7 +261,7 @@ func TestAggregationCheck(t *testing.T) { description: "Left hand side of group_left() is checked", content: "- record: foo\n expr: sum without(job) (foo) / on(type) group_left() sum without(job) (bar)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -281,7 +282,7 @@ func TestAggregationCheck(t *testing.T) { description: "Left hand side of group_right() is ignored", content: "- record: foo\n expr: sum without(job) (foo) / on(type) group_right() sum without(id) (bar)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -290,7 +291,7 @@ func TestAggregationCheck(t *testing.T) { description: "Right hand side of group_right() is checked", content: "- record: foo\n expr: sum without(job) (foo) / on(type) group_right() sum without(job) (bar)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -311,7 +312,7 @@ func TestAggregationCheck(t *testing.T) { description: "nested count", content: "- record: foo\n expr: count(count(bar) without ())\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "instance", false, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "instance", false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -321,7 +322,7 @@ func TestAggregationCheck(t *testing.T) { description: "ignores rules with syntax errors", content: "- record: foo\n expr: sum(foo) without(\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -330,7 +331,7 @@ func TestAggregationCheck(t *testing.T) { description: "name must match", content: "- record: foo\n expr: sum(foo) without(job)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp("bar"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp("bar"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -339,7 +340,7 @@ func TestAggregationCheck(t *testing.T) { description: "uses label from labels map", content: "- record: foo\n expr: sum(foo) by(instance)\n labels:\n job: foo\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -348,7 +349,7 @@ func TestAggregationCheck(t *testing.T) { description: "must keep job label / warning", content: "- record: foo\n expr: sum(foo) by(instance)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -369,7 +370,7 @@ func TestAggregationCheck(t *testing.T) { description: "must keep job label / bug", content: "- record: foo\n expr: sum(foo) by(instance)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Bug) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Bug) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -390,7 +391,7 @@ func TestAggregationCheck(t *testing.T) { description: "must strip job label", content: "- record: foo\n expr: sum(foo) by(job)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", false, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", false, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -411,7 +412,7 @@ func TestAggregationCheck(t *testing.T) { description: "must strip job label / being stripped", content: "- record: foo\n expr: sum(foo) by(instance)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", false, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -420,7 +421,7 @@ func TestAggregationCheck(t *testing.T) { description: "nested rule must keep job label", content: "- record: foo\n expr: sum(sum(foo) by(instance)) by(job)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -441,7 +442,7 @@ func TestAggregationCheck(t *testing.T) { description: "Right hand side of AND is ignored", content: "- record: foo\n expr: foo AND on(instance) max(bar)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -450,7 +451,7 @@ func TestAggregationCheck(t *testing.T) { description: "Left hand side of AND is checked", content: "- record: foo\n expr: max (foo) by(instance) AND on(instance) bar\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -471,7 +472,7 @@ func TestAggregationCheck(t *testing.T) { description: "Right hand side of group_left() is ignored", content: "- record: foo\n expr: sum by(job) (foo) / on(type) group_left() sum by(type) (bar)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -480,7 +481,7 @@ func TestAggregationCheck(t *testing.T) { description: "Left hand side of group_left() is checked", content: "- record: foo\n expr: sum by(type) (foo) / on(type) group_left() sum by(job) (bar)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -501,7 +502,7 @@ func TestAggregationCheck(t *testing.T) { description: "Left hand side of group_right() is ignored", content: "- record: foo\n expr: sum by(type) (foo) / on(type) group_right() sum by(job) (bar)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -510,7 +511,7 @@ func TestAggregationCheck(t *testing.T) { description: "Right hand side of group_right() is checked", content: "- record: foo\n expr: sum by(job) (foo) / on(type) group_right() sum by(type) (bar)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -531,7 +532,7 @@ func TestAggregationCheck(t *testing.T) { description: "nested count", content: "- record: foo\n expr: count(count(bar) by (instance))\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "instance", false, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "instance", false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -540,7 +541,7 @@ func TestAggregationCheck(t *testing.T) { description: "nested count AND nested count", content: "- record: foo\n expr: count(count(bar) by (instance)) AND count(count(bar) by (instance))\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "instance", false, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "instance", false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -549,7 +550,7 @@ func TestAggregationCheck(t *testing.T) { description: "nested by(without())", content: "- record: foo\n expr: sum(sum(foo) by(instance)) without(job)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -579,7 +580,7 @@ func TestAggregationCheck(t *testing.T) { description: "nested by(without())", content: "- record: foo\n expr: sum(sum(foo) by(instance,job)) without(job)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -600,7 +601,7 @@ func TestAggregationCheck(t *testing.T) { description: "nested by(without())", content: "- record: foo\n expr: sum(sum(foo) by(instance)) without(instance)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -621,7 +622,7 @@ func TestAggregationCheck(t *testing.T) { description: "nested by(without())", content: "- record: foo\n expr: sum(sum(foo) by(instance)) without(job)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "instance", false, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "instance", false, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -651,7 +652,7 @@ func TestAggregationCheck(t *testing.T) { description: "must keep job label / sum()", content: "- record: foo\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -672,7 +673,7 @@ func TestAggregationCheck(t *testing.T) { description: "must keep job label / sum() by()", content: "- record: foo\n expr: sum(foo) by()\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -693,7 +694,7 @@ func TestAggregationCheck(t *testing.T) { description: "must strip job label / sum() without()", content: "- record: foo\n expr: sum(foo) without()\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", false, checks.Warning) + return checks.NewAggregationCheck(checks.MustTemplatedRegexp(".+"), "job", false, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { diff --git a/internal/checks/query_cost.go b/internal/checks/query_cost.go index 20bdee00..3766deaa 100644 --- a/internal/checks/query_cost.go +++ b/internal/checks/query_cost.go @@ -16,19 +16,21 @@ const ( BytesPerSampleQuery = "avg(avg_over_time(go_memstats_alloc_bytes[2h]) / avg_over_time(prometheus_tsdb_head_series[2h]))" ) -func NewCostCheck(prom *promapi.FailoverGroup, maxSeries, maxTotalSamples, maxPeakSamples int, maxEvaluationDuration time.Duration, severity Severity) CostCheck { +func NewCostCheck(prom *promapi.FailoverGroup, maxSeries, maxTotalSamples, maxPeakSamples int, maxEvaluationDuration time.Duration, comment string, severity Severity) CostCheck { return CostCheck{ prom: prom, maxSeries: maxSeries, maxTotalSamples: maxTotalSamples, maxPeakSamples: maxPeakSamples, maxEvaluationDuration: maxEvaluationDuration, + comment: comment, severity: severity, } } type CostCheck struct { prom *promapi.FailoverGroup + comment string maxSeries int maxTotalSamples int maxPeakSamples int @@ -74,6 +76,7 @@ func (c CostCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []di Lines: expr.Value.Lines, Reporter: c.Reporter(), Text: text, + Details: maybeComment(c.comment), Severity: severity, }) return problems @@ -108,6 +111,7 @@ func (c CostCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []di Lines: expr.Value.Lines, Reporter: c.Reporter(), Text: fmt.Sprintf("%s returned %d result(s)%s%s", promText(c.prom.Name(), qr.URI), series, estimate, above), + Details: maybeComment(c.comment), Severity: severity, }) @@ -116,6 +120,7 @@ func (c CostCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []di Lines: expr.Value.Lines, Reporter: c.Reporter(), Text: fmt.Sprintf("%s queried %d samples in total when executing this query, which is more than the configured limit of %d.", promText(c.prom.Name(), qr.URI), qr.Stats.Samples.TotalQueryableSamples, c.maxTotalSamples), + Details: maybeComment(c.comment), Severity: c.severity, }) } @@ -125,6 +130,7 @@ func (c CostCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []di Lines: expr.Value.Lines, Reporter: c.Reporter(), Text: fmt.Sprintf("%s queried %d peak samples when executing this query, which is more than the configured limit of %d.", promText(c.prom.Name(), qr.URI), qr.Stats.Samples.PeakSamples, c.maxPeakSamples), + Details: maybeComment(c.comment), Severity: c.severity, }) } @@ -135,6 +141,7 @@ func (c CostCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []di Lines: expr.Value.Lines, Reporter: c.Reporter(), Text: fmt.Sprintf("%s took %s when executing this query, which is more than the configured limit of %s.", promText(c.prom.Name(), qr.URI), output.HumanizeDuration(evalDur), output.HumanizeDuration(c.maxEvaluationDuration)), + Details: maybeComment(c.comment), Severity: c.severity, }) } diff --git a/internal/checks/query_cost_test.go b/internal/checks/query_cost_test.go index e6b2a9c4..b1e71371 100644 --- a/internal/checks/query_cost_test.go +++ b/internal/checks/query_cost_test.go @@ -44,7 +44,7 @@ func TestCostCheck(t *testing.T) { description: "ignores rules with syntax errors", content: "- record: foo\n expr: sum(foo) without(\n", checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 0, 0, 0, 0, checks.Bug) + return checks.NewCostCheck(prom, 0, 0, 0, 0, "", checks.Bug) }, prometheus: newSimpleProm, problems: noProblems, @@ -53,7 +53,7 @@ func TestCostCheck(t *testing.T) { description: "empty response", content: content, checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 0, 0, 0, 0, checks.Bug) + return checks.NewCostCheck(prom, 0, 0, 0, 0, "", checks.Bug) }, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -83,7 +83,7 @@ func TestCostCheck(t *testing.T) { description: "response timeout", content: content, checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 0, 0, 0, 0, checks.Bug) + return checks.NewCostCheck(prom, 0, 0, 0, 0, "", checks.Bug) }, prometheus: func(uri string) *promapi.FailoverGroup { return simpleProm("prom", uri, time.Millisecond*50, true) @@ -115,7 +115,7 @@ func TestCostCheck(t *testing.T) { description: "bad request", content: content, checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 0, 0, 0, 0, checks.Bug) + return checks.NewCostCheck(prom, 0, 0, 0, 0, "", checks.Bug) }, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -145,7 +145,7 @@ func TestCostCheck(t *testing.T) { description: "connection refused", content: content, checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 0, 0, 0, 0, checks.Bug) + return checks.NewCostCheck(prom, 0, 0, 0, 0, "", checks.Bug) }, prometheus: func(_ string) *promapi.FailoverGroup { return simpleProm("prom", "http://127.0.0.1:1111", time.Second*5, false) @@ -168,7 +168,7 @@ func TestCostCheck(t *testing.T) { description: "1 result", content: content, checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 0, 0, 0, 0, checks.Bug) + return checks.NewCostCheck(prom, 0, 0, 0, 0, "", checks.Bug) }, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -209,7 +209,7 @@ func TestCostCheck(t *testing.T) { description: "7 results", content: content, checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 0, 0, 0, 0, checks.Bug) + return checks.NewCostCheck(prom, 0, 0, 0, 0, "", checks.Bug) }, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -260,7 +260,7 @@ func TestCostCheck(t *testing.T) { description: "7 result with MB", content: content, checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 0, 0, 0, 0, checks.Bug) + return checks.NewCostCheck(prom, 0, 0, 0, 0, "", checks.Bug) }, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -311,7 +311,7 @@ func TestCostCheck(t *testing.T) { description: "7 results with 1 series max (1KB bps)", content: content, checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 1, 0, 0, 0, checks.Bug) + return checks.NewCostCheck(prom, 1, 0, 0, 0, "", checks.Bug) }, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -362,7 +362,7 @@ func TestCostCheck(t *testing.T) { description: "6 results with 5 series max", content: content, checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 5, 0, 0, 0, checks.Bug) + return checks.NewCostCheck(prom, 5, 0, 0, 0, "", checks.Bug) }, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -408,7 +408,7 @@ func TestCostCheck(t *testing.T) { description: "7 results with 5 series max / infi", content: content, checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 5, 0, 0, 0, checks.Information) + return checks.NewCostCheck(prom, 5, 0, 0, 0, "rule comment", checks.Information) }, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -420,6 +420,7 @@ func TestCostCheck(t *testing.T) { }, Reporter: "query/cost", Text: costText("prom", uri, 7) + maxSeriesText(5) + ".", + Details: "Rule comment: rule comment", Severity: checks.Information, }, } @@ -458,7 +459,7 @@ func TestCostCheck(t *testing.T) { expr: 'sum({__name__="foo"})' `, checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 0, 0, 0, 0, checks.Bug) + return checks.NewCostCheck(prom, 0, 0, 0, 0, "", checks.Bug) }, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -509,7 +510,7 @@ func TestCostCheck(t *testing.T) { description: "1s eval, 5s limit", content: content, checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 0, 0, 0, time.Second*5, checks.Bug) + return checks.NewCostCheck(prom, 0, 0, 0, time.Second*5, "", checks.Bug) }, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -557,7 +558,7 @@ func TestCostCheck(t *testing.T) { description: "stats", content: content, checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 0, 100, 10, time.Second*5, checks.Bug) + return checks.NewCostCheck(prom, 0, 100, 10, time.Second*5, "", checks.Bug) }, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -636,7 +637,7 @@ func TestCostCheck(t *testing.T) { description: "stats - info", content: content, checker: func(prom *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewCostCheck(prom, 0, 100, 10, time.Second*5, checks.Information) + return checks.NewCostCheck(prom, 0, 100, 10, time.Second*5, "some text", checks.Information) }, prometheus: newSimpleProm, problems: func(uri string) []checks.Problem { @@ -648,6 +649,7 @@ func TestCostCheck(t *testing.T) { }, Reporter: "query/cost", Text: costText("prom", uri, 1) + memUsageText("4.0KiB") + ".", + Details: "Rule comment: some text", Severity: checks.Information, }, { @@ -657,6 +659,7 @@ func TestCostCheck(t *testing.T) { }, Reporter: "query/cost", Text: totalSamplesText("prom", uri, 200, 100), + Details: "Rule comment: some text", Severity: checks.Information, }, { @@ -666,6 +669,7 @@ func TestCostCheck(t *testing.T) { }, Reporter: "query/cost", Text: peakSamplesText("prom", uri, 20, 10), + Details: "Rule comment: some text", Severity: checks.Information, }, { @@ -675,6 +679,7 @@ func TestCostCheck(t *testing.T) { }, Reporter: "query/cost", Text: evalDurText("prom", uri, "5s100ms", "5s"), + Details: "Rule comment: some text", Severity: checks.Information, }, } diff --git a/internal/checks/rule_for.go b/internal/checks/rule_for.go index 90ca2803..e0c5fd77 100644 --- a/internal/checks/rule_for.go +++ b/internal/checks/rule_for.go @@ -23,17 +23,19 @@ const ( RuleForKeepFiringFor RuleForKey = "keep_firing_for" ) -func NewRuleForCheck(key RuleForKey, minFor, maxFor time.Duration, severity Severity) RuleForCheck { +func NewRuleForCheck(key RuleForKey, minFor, maxFor time.Duration, comment string, severity Severity) RuleForCheck { return RuleForCheck{ key: key, minFor: minFor, maxFor: maxFor, + comment: comment, severity: severity, } } type RuleForCheck struct { key RuleForKey + comment string severity Severity minFor time.Duration maxFor time.Duration @@ -83,6 +85,7 @@ func (c RuleForCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []d Lines: lines, Reporter: c.Reporter(), Text: fmt.Sprintf("This alert rule must have a `%s` field with a minimum duration of %s.", c.key, output.HumanizeDuration(c.minFor)), + Details: maybeComment(c.comment), Severity: c.severity, }) } @@ -92,6 +95,7 @@ func (c RuleForCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []d Lines: lines, Reporter: c.Reporter(), Text: fmt.Sprintf("This alert rule must have a `%s` field with a maximum duration of %s.", c.key, output.HumanizeDuration(c.maxFor)), + Details: maybeComment(c.comment), Severity: c.severity, }) } diff --git a/internal/checks/rule_for_test.go b/internal/checks/rule_for_test.go index 9847554b..c4722143 100644 --- a/internal/checks/rule_for_test.go +++ b/internal/checks/rule_for_test.go @@ -24,7 +24,7 @@ func TestRuleForCheck(t *testing.T) { description: "recording rule", content: "- record: foo\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleForCheck(checks.RuleForFor, 0, 0, checks.Bug) + return checks.NewRuleForCheck(checks.RuleForFor, 0, 0, "", checks.Bug) }, prometheus: noProm, problems: noProblems, @@ -33,7 +33,7 @@ func TestRuleForCheck(t *testing.T) { description: "alerting rule, no for, 0-0", content: "- alert: foo\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleForCheck(checks.RuleForFor, 0, 0, checks.Bug) + return checks.NewRuleForCheck(checks.RuleForFor, 0, 0, "", checks.Bug) }, prometheus: noProm, problems: noProblems, @@ -42,7 +42,7 @@ func TestRuleForCheck(t *testing.T) { description: "alerting rule, for:1m, 0-0", content: "- alert: foo\n for: 1m\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleForCheck(checks.RuleForFor, 0, 0, checks.Bug) + return checks.NewRuleForCheck(checks.RuleForFor, 0, 0, "", checks.Bug) }, prometheus: noProm, problems: noProblems, @@ -51,7 +51,7 @@ func TestRuleForCheck(t *testing.T) { description: "alerting rule, for:1m, 1s-0", content: "- alert: foo\n for: 1m\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleForCheck(checks.RuleForFor, time.Second, 0, checks.Bug) + return checks.NewRuleForCheck(checks.RuleForFor, time.Second, 0, "", checks.Bug) }, prometheus: noProm, problems: noProblems, @@ -60,7 +60,7 @@ func TestRuleForCheck(t *testing.T) { description: "alerting rule, for:1m, 1s-2m", content: "- alert: foo\n for: 1m\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleForCheck(checks.RuleForFor, time.Second, time.Minute*2, checks.Bug) + return checks.NewRuleForCheck(checks.RuleForFor, time.Second, time.Minute*2, "", checks.Bug) }, prometheus: noProm, problems: noProblems, @@ -69,7 +69,7 @@ func TestRuleForCheck(t *testing.T) { description: "alerting rule, for:4m, 5m-10m", content: "- alert: foo\n for: 4m\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleForCheck(checks.RuleForFor, time.Minute*5, time.Minute*10, checks.Warning) + return checks.NewRuleForCheck(checks.RuleForFor, time.Minute*5, time.Minute*10, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -90,7 +90,7 @@ func TestRuleForCheck(t *testing.T) { description: "alerting rule, for:5m, 1s-2m", content: "- alert: foo\n for: 5m\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleForCheck(checks.RuleForFor, time.Second, time.Minute*2, checks.Warning) + return checks.NewRuleForCheck(checks.RuleForFor, time.Second, time.Minute*2, "some text", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -102,6 +102,7 @@ func TestRuleForCheck(t *testing.T) { }, Reporter: "rule/for", Text: forMax("for", "2m"), + Details: "Rule comment: some text", Severity: checks.Warning, }, } @@ -111,7 +112,7 @@ func TestRuleForCheck(t *testing.T) { description: "alerting rule, for:1d, 5m-0", content: "- alert: foo\n for: 1d\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleForCheck(checks.RuleForFor, time.Minute*5, 0, checks.Warning) + return checks.NewRuleForCheck(checks.RuleForFor, time.Minute*5, 0, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -120,7 +121,7 @@ func TestRuleForCheck(t *testing.T) { description: "alerting rule, for:14m, 5m-10m, keep_firing_for enforced", content: "- alert: foo\n for: 14m\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleForCheck(checks.RuleForKeepFiringFor, 0, time.Minute*10, checks.Warning) + return checks.NewRuleForCheck(checks.RuleForKeepFiringFor, 0, time.Minute*10, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -129,7 +130,7 @@ func TestRuleForCheck(t *testing.T) { description: "alerting rule, keep_firing_for:4m, 5m-10m", content: "- alert: foo\n keep_firing_for: 4m\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleForCheck(checks.RuleForKeepFiringFor, time.Minute*5, time.Minute*10, checks.Warning) + return checks.NewRuleForCheck(checks.RuleForKeepFiringFor, time.Minute*5, time.Minute*10, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { diff --git a/internal/checks/rule_label.go b/internal/checks/rule_label.go index 8ed2179a..d6e1fb7b 100644 --- a/internal/checks/rule_label.go +++ b/internal/checks/rule_label.go @@ -15,13 +15,14 @@ const ( LabelCheckName = "rule/label" ) -func NewLabelCheck(keyRe, tokenRe, valueRe *TemplatedRegexp, values []string, isReguired bool, severity Severity) LabelCheck { +func NewLabelCheck(keyRe, tokenRe, valueRe *TemplatedRegexp, values []string, isReguired bool, comment string, severity Severity) LabelCheck { return LabelCheck{ keyRe: keyRe, tokenRe: tokenRe, valueRe: valueRe, values: values, isReguired: isReguired, + comment: comment, severity: severity, } } @@ -30,9 +31,10 @@ type LabelCheck struct { keyRe *TemplatedRegexp tokenRe *TemplatedRegexp valueRe *TemplatedRegexp + comment string values []string - isReguired bool severity Severity + isReguired bool } func (c LabelCheck) Meta() CheckMeta { @@ -77,6 +79,7 @@ func (c LabelCheck) checkRecordingRule(rule parser.Rule) (problems []Problem) { Lines: rule.Lines, Reporter: c.Reporter(), Text: fmt.Sprintf("`%s` label is required.", c.keyRe.original), + Details: maybeComment(c.comment), Severity: c.severity, }) } @@ -90,6 +93,7 @@ func (c LabelCheck) checkRecordingRule(rule parser.Rule) (problems []Problem) { Lines: rule.RecordingRule.Labels.Lines, Reporter: c.Reporter(), Text: fmt.Sprintf("`%s` label is required.", c.keyRe.original), + Details: maybeComment(c.comment), Severity: c.severity, }) } @@ -114,6 +118,7 @@ func (c LabelCheck) checkAlertingRule(rule parser.Rule) (problems []Problem) { Lines: rule.Lines, Reporter: c.Reporter(), Text: fmt.Sprintf("`%s` label is required.", c.keyRe.original), + Details: maybeComment(c.comment), Severity: c.severity, }) } @@ -133,6 +138,7 @@ func (c LabelCheck) checkAlertingRule(rule parser.Rule) (problems []Problem) { Lines: rule.AlertingRule.Labels.Lines, Reporter: c.Reporter(), Text: fmt.Sprintf("`%s` label is required.", c.keyRe.original), + Details: maybeComment(c.comment), Severity: c.severity, }) return problems @@ -157,6 +163,7 @@ func (c LabelCheck) checkValue(rule parser.Rule, value string, lines parser.Line Lines: lines, Reporter: c.Reporter(), Text: fmt.Sprintf("`%s` label value `%s` must match `%s`.", c.keyRe.original, value, c.valueRe.anchored), + Details: maybeComment(c.comment), Severity: c.severity, }) } @@ -175,6 +182,11 @@ func (c LabelCheck) checkValue(rule parser.Rule, value string, lines parser.Line break } } + if c.comment != "" { + details.WriteRune('\n') + details.WriteString("Rule comment: ") + details.WriteString(c.comment) + } problems = append(problems, Problem{ Lines: lines, Reporter: c.Reporter(), diff --git a/internal/checks/rule_label_test.go b/internal/checks/rule_label_test.go index 19d85110..3588ebb1 100644 --- a/internal/checks/rule_label_test.go +++ b/internal/checks/rule_label_test.go @@ -14,7 +14,7 @@ func TestLabelCheck(t *testing.T) { description: "doesn't ignore rules with syntax errors", content: "- record: foo\n expr: sum(foo) without(\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, "some text", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -26,6 +26,7 @@ func TestLabelCheck(t *testing.T) { }, Reporter: checks.LabelCheckName, Text: "`severity` label is required.", + Details: "Rule comment: some text", Severity: checks.Warning, }, } @@ -35,7 +36,7 @@ func TestLabelCheck(t *testing.T) { description: "no labels in recording rule / required", content: "- record: foo\n expr: rate(foo[1m])\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -56,7 +57,7 @@ func TestLabelCheck(t *testing.T) { description: "no labels in recording rule / not required", content: "- record: foo\n expr: rate(foo[1m])\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -71,6 +72,7 @@ func TestLabelCheck(t *testing.T) { checks.MustTemplatedRegexp("critical"), nil, true, + "", checks.Warning, ) }, @@ -93,7 +95,7 @@ func TestLabelCheck(t *testing.T) { description: "missing label in recording rule / not required", content: "- record: foo\n expr: rate(foo[1m])\n labels:\n foo: bar\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -102,7 +104,7 @@ func TestLabelCheck(t *testing.T) { description: "invalid value in recording rule / required", content: "- record: foo\n expr: rate(foo[1m])\n labels:\n severity: warning\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -123,7 +125,7 @@ func TestLabelCheck(t *testing.T) { description: "invalid value in recording rule / not required", content: "- record: foo\n expr: rate(foo[1m])\n labels:\n severity: warning\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -150,6 +152,7 @@ func TestLabelCheck(t *testing.T) { checks.MustTemplatedRegexp("(1|2|3)"), nil, true, + "some text", checks.Warning, ) }, @@ -163,6 +166,7 @@ func TestLabelCheck(t *testing.T) { }, Reporter: checks.LabelCheckName, Text: "`priority` label value `2a` must match `^(1|2|3)$`.", + Details: "Rule comment: some text", Severity: checks.Warning, }, } @@ -178,6 +182,7 @@ func TestLabelCheck(t *testing.T) { checks.MustTemplatedRegexp("(1|2|3)"), nil, false, + "", checks.Warning, ) }, @@ -200,7 +205,7 @@ func TestLabelCheck(t *testing.T) { description: "no labels in alerting rule / required", content: "- alert: foo\n expr: rate(foo[1m])\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -221,7 +226,7 @@ func TestLabelCheck(t *testing.T) { description: "no labels in alerting rule / not required", content: "- alert: foo\n expr: rate(foo[1m])\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -230,7 +235,7 @@ func TestLabelCheck(t *testing.T) { description: "missing label in alerting rule / required", content: "- alert: foo\n expr: rate(foo[1m])\n labels:\n foo: bar\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -251,7 +256,7 @@ func TestLabelCheck(t *testing.T) { description: "missing label in alerting rule / not required", content: "- alert: foo\n expr: rate(foo[1m])\n labels:\n foo: bar\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical"), nil, false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -260,7 +265,7 @@ func TestLabelCheck(t *testing.T) { description: "invalid value in alerting rule / required", content: "- alert: foo\n expr: rate(foo[1m])\n labels:\n severity: warning\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info"), nil, true, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -281,7 +286,7 @@ func TestLabelCheck(t *testing.T) { description: "invalid value in alerting rule / not required", content: "- alert: foo\n expr: rate(foo[1m])\n labels:\n severity: warning\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info"), nil, false, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info"), nil, false, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -302,7 +307,7 @@ func TestLabelCheck(t *testing.T) { description: "valid recording rule / required", content: "- record: foo\n expr: rate(foo[1m])\n labels:\n severity: critical\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info"), nil, true, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -311,7 +316,7 @@ func TestLabelCheck(t *testing.T) { description: "valid recording rule / not required", content: "- record: foo\n expr: rate(foo[1m])\n labels:\n severity: critical\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info"), nil, false, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info"), nil, false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -320,7 +325,7 @@ func TestLabelCheck(t *testing.T) { description: "valid alerting rule / required", content: "- alert: foo\n expr: rate(foo[1m])\n labels:\n severity: info\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info"), nil, true, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -329,7 +334,7 @@ func TestLabelCheck(t *testing.T) { description: "valid alerting rule / not required", content: "- alert: foo\n expr: rate(foo[1m])\n labels:\n severity: info\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info"), nil, false, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("severity"), nil, checks.MustTemplatedRegexp("critical|info"), nil, false, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -338,7 +343,7 @@ func TestLabelCheck(t *testing.T) { description: "templated label value / passing", content: "- alert: foo\n expr: sum(foo)\n for: 5m\n labels:\n for: 'must wait 5m to fire'\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("for"), nil, checks.MustTemplatedRegexp("must wait {{$for}} to fire"), nil, true, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("for"), nil, checks.MustTemplatedRegexp("must wait {{$for}} to fire"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: noProblems, @@ -347,7 +352,7 @@ func TestLabelCheck(t *testing.T) { description: "templated label value / not passing", content: "- alert: foo\n expr: sum(foo)\n for: 4m\n labels:\n for: 'must wait 5m to fire'\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("for"), nil, checks.MustTemplatedRegexp("must wait {{$for}} to fire"), nil, true, checks.Warning) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("for"), nil, checks.MustTemplatedRegexp("must wait {{$for}} to fire"), nil, true, "", checks.Warning) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -368,7 +373,7 @@ func TestLabelCheck(t *testing.T) { description: "invalid value in alerting rule / token / valueRe", content: "- alert: foo\n expr: rate(foo[1m])\n labels:\n components: api db\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("components"), checks.MustRawTemplatedRegexp("\\w+"), checks.MustTemplatedRegexp("api|memcached"), nil, false, checks.Bug) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("components"), checks.MustRawTemplatedRegexp("\\w+"), checks.MustTemplatedRegexp("api|memcached"), nil, false, "", checks.Bug) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -395,6 +400,7 @@ func TestLabelCheck(t *testing.T) { nil, []string{"api", "memcached", "storage", "prometheus", "kvm", "mysql", "memsql", "haproxy", "postgresql"}, false, + "", checks.Bug, ) }, @@ -418,7 +424,7 @@ func TestLabelCheck(t *testing.T) { description: "invalid value in recording rule / token / valueRe", content: "- record: foo\n expr: rate(foo[1m])\n labels:\n components: api db\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewLabelCheck(checks.MustTemplatedRegexp("components"), checks.MustRawTemplatedRegexp("\\w+"), checks.MustTemplatedRegexp("api|memcached"), nil, false, checks.Bug) + return checks.NewLabelCheck(checks.MustTemplatedRegexp("components"), checks.MustRawTemplatedRegexp("\\w+"), checks.MustTemplatedRegexp("api|memcached"), nil, false, "", checks.Bug) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -445,6 +451,7 @@ func TestLabelCheck(t *testing.T) { nil, []string{"api", "memcached", "storage", "prometheus", "kvm", "mysql", "memsql", "haproxy"}, false, + "some text", checks.Warning, ) }, @@ -458,7 +465,7 @@ func TestLabelCheck(t *testing.T) { }, Reporter: checks.LabelCheckName, Text: "`components` label value `db` is not one of valid values.", - Details: "List of allowed values:\n\n- `api`\n- `memcached`\n- `storage`\n- `prometheus`\n- `kvm`\n- `mysql`\n- `memsql`\n- `haproxy`\n", + Details: "List of allowed values:\n\n- `api`\n- `memcached`\n- `storage`\n- `prometheus`\n- `kvm`\n- `mysql`\n- `memsql`\n- `haproxy`\n\nRule comment: some text", Severity: checks.Warning, }, } diff --git a/internal/checks/rule_link.go b/internal/checks/rule_link.go index 54540492..00bb97f5 100644 --- a/internal/checks/rule_link.go +++ b/internal/checks/rule_link.go @@ -20,13 +20,14 @@ const ( RuleLinkCheckName = "rule/link" ) -func NewRuleLinkCheck(re *TemplatedRegexp, uriRewrite string, timeout time.Duration, headers map[string]string, s Severity) RuleLinkCheck { +func NewRuleLinkCheck(re *TemplatedRegexp, uriRewrite string, timeout time.Duration, headers map[string]string, comment string, s Severity) RuleLinkCheck { return RuleLinkCheck{ scheme: []string{"http", "https"}, re: re, uriRewrite: uriRewrite, timeout: timeout, headers: headers, + comment: comment, severity: s, } } @@ -35,6 +36,7 @@ type RuleLinkCheck struct { re *TemplatedRegexp headers map[string]string uriRewrite string + comment string scheme []string timeout time.Duration severity Severity @@ -113,6 +115,7 @@ func (c RuleLinkCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ }, Reporter: c.Reporter(), Text: fmt.Sprintf("GET request for %s returned an error: %s.", uri, err), + Details: maybeComment(c.comment), Severity: c.severity, }) slog.Debug("Link request returned an error", slog.String("uri", uri), slog.Any("err", err)) @@ -129,6 +132,7 @@ func (c RuleLinkCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ }, Reporter: c.Reporter(), Text: fmt.Sprintf("GET request for %s returned invalid status code: `%s`.", uri, resp.Status), + Details: maybeComment(c.comment), Severity: c.severity, }) slog.Debug("Link request returned invalid status code", slog.String("uri", uri), slog.String("status", resp.Status)) diff --git a/internal/checks/rule_link_test.go b/internal/checks/rule_link_test.go index 8b4591ec..f5538eed 100644 --- a/internal/checks/rule_link_test.go +++ b/internal/checks/rule_link_test.go @@ -52,7 +52,7 @@ func TestRuleLinkCheck(t *testing.T) { description: "ignores recording rules", content: "- record: foo\n expr: sum(foo)\n", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleLinkCheck(nil, "", time.Second, nil, checks.Bug) + return checks.NewRuleLinkCheck(nil, "", time.Second, nil, "", checks.Bug) }, prometheus: noProm, problems: noProblems, @@ -61,7 +61,7 @@ func TestRuleLinkCheck(t *testing.T) { description: "ignores unparsable link annotations", content: "- alert: foo\n expr: sum(foo)\n annotations:\n link: \"%gh&%ij\"", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleLinkCheck(nil, "", time.Second, nil, checks.Bug) + return checks.NewRuleLinkCheck(nil, "", time.Second, nil, "", checks.Bug) }, prometheus: noProm, problems: noProblems, @@ -70,7 +70,7 @@ func TestRuleLinkCheck(t *testing.T) { description: "ignores non link annotations", content: "- alert: foo\n expr: sum(foo)\n annotations:\n link: not a link", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleLinkCheck(nil, "", time.Second, nil, checks.Bug) + return checks.NewRuleLinkCheck(nil, "", time.Second, nil, "", checks.Bug) }, prometheus: noProm, problems: noProblems, @@ -79,7 +79,7 @@ func TestRuleLinkCheck(t *testing.T) { description: "ignores links without regexp match", content: "- alert: foo\n expr: sum(foo)\n annotations:\n link: http://foo.example.com", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleLinkCheck(checks.MustTemplatedRegexp("ftp://xxx.com"), "", time.Second, nil, checks.Bug) + return checks.NewRuleLinkCheck(checks.MustTemplatedRegexp("ftp://xxx.com"), "", time.Second, nil, "", checks.Bug) }, prometheus: noProm, problems: noProblems, @@ -88,7 +88,7 @@ func TestRuleLinkCheck(t *testing.T) { description: "link with no host", content: "- alert: foo\n expr: sum(foo)\n annotations:\n link: http://", checker: func(_ *promapi.FailoverGroup) checks.RuleChecker { - return checks.NewRuleLinkCheck(checks.MustTemplatedRegexp(".*"), "", time.Second, nil, checks.Bug) + return checks.NewRuleLinkCheck(checks.MustTemplatedRegexp(".*"), "", time.Second, nil, "some text", checks.Bug) }, prometheus: noProm, problems: func(_ string) []checks.Problem { @@ -100,6 +100,7 @@ func TestRuleLinkCheck(t *testing.T) { }, Reporter: "rule/link", Text: `GET request for http: returned an error: Get "http:": http: no Host in request URL.`, + Details: "Rule comment: some text", Severity: checks.Bug, }, } @@ -114,6 +115,7 @@ func TestRuleLinkCheck(t *testing.T) { "", time.Second, map[string]string{"X-Host": "200.example.com"}, + "", checks.Bug, ) }, @@ -129,6 +131,7 @@ func TestRuleLinkCheck(t *testing.T) { "", time.Second, map[string]string{"X-Host": "400.example.com"}, + "some text", checks.Bug, ) }, @@ -142,6 +145,7 @@ func TestRuleLinkCheck(t *testing.T) { }, Reporter: "rule/link", Text: fmt.Sprintf("GET request for %s/dashboard returned invalid status code: `400 Bad Request`.", srv.URL), + Details: "Rule comment: some text", Severity: checks.Bug, }, } @@ -156,6 +160,7 @@ func TestRuleLinkCheck(t *testing.T) { "", time.Second, map[string]string{"X-Host": "400.example.com"}, + "", checks.Warning, ) }, @@ -192,6 +197,7 @@ func TestRuleLinkCheck(t *testing.T) { "", time.Second, map[string]string{"X-Host": "headers.example.com", "X-Auth": "mykey"}, + "", checks.Bug, ) }, @@ -207,6 +213,7 @@ func TestRuleLinkCheck(t *testing.T) { fmt.Sprintf(srv.URL+"/rewrite"), time.Second, map[string]string{"X-Host": "rewrite.example.com"}, + "", checks.Information, ) }, @@ -222,6 +229,7 @@ func TestRuleLinkCheck(t *testing.T) { "", time.Second, map[string]string{}, + "", checks.Bug, ) }, diff --git a/internal/config/__snapshots__/config_test.snap b/internal/config/__snapshots__/config_test.snap index 530699d3..949e6c6b 100755 --- a/internal/config/__snapshots__/config_test.snap +++ b/internal/config/__snapshots__/config_test.snap @@ -680,6 +680,7 @@ }, { "name": ".+", + "comment": "this is rule comment", "severity": "bug", "strip": [ "instance", @@ -729,6 +730,7 @@ "rules": [ { "cost": { + "comment": "this is rule comment", "severity": "warning", "maxSeries": 10000 } @@ -801,6 +803,7 @@ "label": [ { "key": "team", + "comment": "this is rule comment", "severity": "warning" } ] @@ -810,6 +813,7 @@ { "key": "summary", "value": "foo.+", + "comment": "this is rule comment", "severity": "bug", "required": true } @@ -874,10 +878,12 @@ }, { "key": ".* +.*", + "comment": "this is rule comment", "label_keys": true, "annotation_keys": true }, { + "comment": "this is rule comment", "severity": "bug", "annotation_values": true } @@ -1273,6 +1279,7 @@ "annotation": [ { "key": "summary", + "comment": "this is rule comment", "required": true } ] @@ -1585,6 +1592,7 @@ "annotation": [ { "key": "summary", + "comment": "this is rule comment", "required": true } ] @@ -1689,6 +1697,7 @@ "headers": { "X-Auth": "xxx" }, + "comment": "this is rule comment", "severity": "bug" } ] @@ -1805,6 +1814,7 @@ "range": "1d", "step": "1m", "resolve": "5m", + "comment": "this is rule comment", "severity": "bug", "minCount": 100 } @@ -2100,6 +2110,7 @@ "rules": [ { "cost": { + "comment": "this is rule comment", "severity": "info" } }, @@ -2565,6 +2576,7 @@ "key": "priority", "token": "\\w+", "value": "(1|2|3|4|5)", + "comment": "this is rule comment", "severity": "bug", "required": true } diff --git a/internal/config/aggregate.go b/internal/config/aggregate.go index 027a91c4..6a584fc4 100644 --- a/internal/config/aggregate.go +++ b/internal/config/aggregate.go @@ -8,6 +8,7 @@ import ( type AggregateSettings struct { Name string `hcl:",label" json:"name"` + Comment string `hcl:"comment,optional" json:"comment,omitempty"` Severity string `hcl:"severity,optional" json:"severity,omitempty"` Keep []string `hcl:"keep,optional" json:"keep,omitempty"` Strip []string `hcl:"strip,optional" json:"strip,omitempty"` diff --git a/internal/config/alerts.go b/internal/config/alerts.go index ef325186..78d1eee4 100644 --- a/internal/config/alerts.go +++ b/internal/config/alerts.go @@ -10,6 +10,7 @@ type AlertsSettings struct { Range string `hcl:"range" json:"range"` Step string `hcl:"step" json:"step"` Resolve string `hcl:"resolve" json:"resolve"` + Comment string `hcl:"comment,optional" json:"comment,omitempty"` Severity string `hcl:"severity,optional" json:"severity,omitempty"` MinCount int `hcl:"minCount,optional" json:"minCount,omitempty"` } diff --git a/internal/config/annotation.go b/internal/config/annotation.go index a99e7a8f..be4f95c2 100644 --- a/internal/config/annotation.go +++ b/internal/config/annotation.go @@ -10,6 +10,7 @@ type AnnotationSettings struct { Key string `hcl:",label" json:"key"` Token string `hcl:"token,optional" json:"token,omitempty"` Value string `hcl:"value,optional" json:"value,omitempty"` + Comment string `hcl:"comment,optional" json:"comment,omitempty"` Severity string `hcl:"severity,optional" json:"severity,omitempty"` Values []string `hcl:"values,optional" json:"values,omitempty"` Required bool `hcl:"required,optional" json:"required,omitempty"` diff --git a/internal/config/config_test.go b/internal/config/config_test.go index e630fa6a..046e20e7 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -451,6 +451,7 @@ rule { keep = ["job"] } aggregate ".+" { + comment = "this is rule comment" severity = "bug" strip = ["instance", "rack"] } @@ -480,6 +481,7 @@ rule { rule { cost { maxSeries = 10000 + comment = "this is rule comment" severity = "warning" } } @@ -566,6 +568,7 @@ rule { rule { label "team" { severity = "warning" + comment = "this is rule comment" required = false } annotation "summary" { @@ -577,6 +580,7 @@ rule { annotation "summary" { value = "foo.+" severity = "bug" + comment = "this is rule comment" required = true } } @@ -621,6 +625,7 @@ prometheus "prom2" { } rule { cost { + comment = "this is rule comment" severity = "info" } } @@ -677,10 +682,12 @@ rule { label_values = true } reject ".* +.*" { + comment = "this is rule comment" annotation_keys = true label_keys = true } reject "" { + comment = "this is rule comment" annotation_values = true severity = "bug" } @@ -900,6 +907,7 @@ rule { } } label "priority" { + comment = "this is rule comment" severity = "bug" token = "\\w+" value = "(1|2|3|4|5)" @@ -1155,6 +1163,7 @@ rule { } annotation "summary" { required = true + comment = "this is rule comment" } } `, @@ -1313,6 +1322,7 @@ rule { } annotation "summary" { required = true + comment = "this is rule comment" } } `, @@ -1367,6 +1377,7 @@ rule { headers = { X-Auth = "xxx" } + comment = "this is rule comment" severity = "bug" } } @@ -1675,6 +1686,7 @@ rule { step = "1m" resolve = "5m" minCount = 100 + comment = "this is rule comment" severity = "bug" } } diff --git a/internal/config/cost.go b/internal/config/cost.go index 71266c7b..a2dca4f8 100644 --- a/internal/config/cost.go +++ b/internal/config/cost.go @@ -8,6 +8,7 @@ import ( type CostSettings struct { MaxEvaluationDuration string `hcl:"maxEvaluationDuration,optional" json:"maxEvaluationDuration,omitempty"` + Comment string `hcl:"comment,optional" json:"comment,omitempty"` Severity string `hcl:"severity,optional" json:"severity,omitempty"` MaxSeries int `hcl:"maxSeries,optional" json:"maxSeries,omitempty"` MaxPeakSamples int `hcl:"maxPeakSamples,optional" json:"maxPeakSamples,omitempty"` diff --git a/internal/config/for.go b/internal/config/for.go index c829492d..74a66b96 100644 --- a/internal/config/for.go +++ b/internal/config/for.go @@ -10,6 +10,7 @@ import ( type ForSettings struct { Min string `hcl:"min,optional" json:"min,omitempty"` Max string `hcl:"max,optional" json:"max,omitempty"` + Comment string `hcl:"comment,optional" json:"comment,omitempty"` Severity string `hcl:"severity,optional" json:"severity,omitempty"` } diff --git a/internal/config/reject.go b/internal/config/reject.go index f8851658..b9e307e8 100644 --- a/internal/config/reject.go +++ b/internal/config/reject.go @@ -6,6 +6,7 @@ import ( type RejectSettings struct { Regex string `hcl:",label" json:"key,omitempty"` + Comment string `hcl:"comment,optional" json:"comment,omitempty"` Severity string `hcl:"severity,optional" json:"severity,omitempty"` LabelKeys bool `hcl:"label_keys,optional" json:"label_keys,omitempty"` LabelValues bool `hcl:"label_values,optional" json:"label_values,omitempty"` diff --git a/internal/config/rule.go b/internal/config/rule.go index 178d9ea4..9eba89d2 100644 --- a/internal/config/rule.go +++ b/internal/config/rule.go @@ -129,13 +129,13 @@ func (rule Rule) resolveChecks(ctx context.Context, path string, r parser.Rule, for _, label := range aggr.Keep { enabled = append(enabled, checkMeta{ name: checks.AggregationCheckName, - check: checks.NewAggregationCheck(nameRegex, label, true, severity), + check: checks.NewAggregationCheck(nameRegex, label, true, aggr.Comment, severity), }) } for _, label := range aggr.Strip { enabled = append(enabled, checkMeta{ name: checks.AggregationCheckName, - check: checks.NewAggregationCheck(nameRegex, label, false, severity), + check: checks.NewAggregationCheck(nameRegex, label, false, aggr.Comment, severity), }) } } @@ -147,7 +147,7 @@ func (rule Rule) resolveChecks(ctx context.Context, path string, r parser.Rule, for _, prom := range prometheusServers { enabled = append(enabled, checkMeta{ name: checks.CostCheckName, - check: checks.NewCostCheck(prom, rule.Cost.MaxSeries, rule.Cost.MaxTotalSamples, rule.Cost.MaxPeakSamples, evalDur, severity), + check: checks.NewCostCheck(prom, rule.Cost.MaxSeries, rule.Cost.MaxTotalSamples, rule.Cost.MaxPeakSamples, evalDur, rule.Cost.Comment, severity), tags: prom.Tags(), }) } @@ -165,7 +165,7 @@ func (rule Rule) resolveChecks(ctx context.Context, path string, r parser.Rule, severity := ann.getSeverity(checks.Warning) enabled = append(enabled, checkMeta{ name: checks.AnnotationCheckName, - check: checks.NewAnnotationCheck(checks.MustTemplatedRegexp(ann.Key), tokenRegex, valueRegex, ann.Values, ann.Required, severity), + check: checks.NewAnnotationCheck(checks.MustTemplatedRegexp(ann.Key), tokenRegex, valueRegex, ann.Values, ann.Required, ann.Comment, severity), }) } } @@ -182,7 +182,7 @@ func (rule Rule) resolveChecks(ctx context.Context, path string, r parser.Rule, severity := lab.getSeverity(checks.Warning) enabled = append(enabled, checkMeta{ name: checks.LabelCheckName, - check: checks.NewLabelCheck(checks.MustTemplatedRegexp(lab.Key), tokenRegex, valueRegex, lab.Values, lab.Required, severity), + check: checks.NewLabelCheck(checks.MustTemplatedRegexp(lab.Key), tokenRegex, valueRegex, lab.Values, lab.Required, lab.Comment, severity), }) } } @@ -204,7 +204,7 @@ func (rule Rule) resolveChecks(ctx context.Context, path string, r parser.Rule, for _, prom := range prometheusServers { enabled = append(enabled, checkMeta{ name: checks.AlertsCheckName, - check: checks.NewAlertsCheck(prom, qRange, qStep, qResolve, rule.Alerts.MinCount, severity), + check: checks.NewAlertsCheck(prom, qRange, qStep, qResolve, rule.Alerts.MinCount, rule.Alerts.Comment, severity), tags: prom.Tags(), }) } @@ -252,7 +252,7 @@ func (rule Rule) resolveChecks(ctx context.Context, path string, r parser.Rule, } enabled = append(enabled, checkMeta{ name: checks.RuleLinkCheckName, - check: checks.NewRuleLinkCheck(re, link.URI, timeout, link.Headers, severity), + check: checks.NewRuleLinkCheck(re, link.URI, timeout, link.Headers, link.Comment, severity), }) } @@ -260,7 +260,7 @@ func (rule Rule) resolveChecks(ctx context.Context, path string, r parser.Rule, severity, minFor, maxFor := rule.For.resolve() enabled = append(enabled, checkMeta{ name: checks.RuleForCheckName, - check: checks.NewRuleForCheck(checks.RuleForFor, minFor, maxFor, severity), + check: checks.NewRuleForCheck(checks.RuleForFor, minFor, maxFor, rule.For.Comment, severity), }) } @@ -268,7 +268,7 @@ func (rule Rule) resolveChecks(ctx context.Context, path string, r parser.Rule, severity, minFor, maxFor := rule.KeepFiringFor.resolve() enabled = append(enabled, checkMeta{ name: checks.RuleForCheckName, - check: checks.NewRuleForCheck(checks.RuleForKeepFiringFor, minFor, maxFor, severity), + check: checks.NewRuleForCheck(checks.RuleForKeepFiringFor, minFor, maxFor, rule.KeepFiringFor.Comment, severity), }) } diff --git a/internal/config/rule_link.go b/internal/config/rule_link.go index 2f953a29..e612d5a3 100644 --- a/internal/config/rule_link.go +++ b/internal/config/rule_link.go @@ -9,6 +9,7 @@ type RuleLinkSettings struct { URI string `hcl:"uri,optional" json:"uri,omitempty"` Timeout string `hcl:"timeout,optional" json:"timeout,omitempty"` Headers map[string]string `hcl:"headers,optional" json:"headers,omitempty"` + Comment string `hcl:"comment,optional" json:"comment,omitempty"` Severity string `hcl:"severity,optional" json:"severity,omitempty"` }