Skip to content

Commit

Permalink
Add more details to problem reports
Browse files Browse the repository at this point in the history
  • Loading branch information
prymitive committed Nov 8, 2023
1 parent a44b16a commit 05b4c99
Show file tree
Hide file tree
Showing 130 changed files with 1,859 additions and 1,585 deletions.
1 change: 1 addition & 0 deletions .github/workflows/compile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ jobs:
if: startsWith(github.ref, 'refs/tags/')
uses: goreleaser/goreleaser-action@v5
with:
version: v1.21.2
args: release --clean
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
4 changes: 2 additions & 2 deletions cmd/pint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func verifyOwners(entries []discovery.Entry, allowedOwners []*regexp.Regexp) (re
Problem: checks.Problem{
Lines: entry.Rule.Lines(),
Reporter: discovery.RuleOwnerComment,
Text: fmt.Sprintf(`%s comments are required in all files, please add a "# pint %s $owner" somewhere in this file and/or "# pint %s $owner" on top of each rule`,
Text: fmt.Sprintf("`%s` comments are required in all files, please add a `# pint %s $owner` somewhere in this file and/or `# pint %s $owner` on top of each rule.",
discovery.RuleOwnerComment, discovery.FileOwnerComment, discovery.RuleOwnerComment),
Severity: checks.Bug,
},
Expand All @@ -168,7 +168,7 @@ func verifyOwners(entries []discovery.Entry, allowedOwners []*regexp.Regexp) (re
Problem: checks.Problem{
Lines: entry.Rule.Lines(),
Reporter: discovery.RuleOwnerComment,
Text: fmt.Sprintf("this rule is set as owned by %q but %q doesn't match any of the allowed owner values", entry.Owner, entry.Owner),
Text: fmt.Sprintf("This rule is set as owned by `%s` but `%s` doesn't match any of the allowed owner values.", entry.Owner, entry.Owner),
Severity: checks.Bug,
},
})
Expand Down
13 changes: 10 additions & 3 deletions cmd/pint/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"context"
"errors"
"fmt"
"log/slog"
"regexp"
"strconv"
Expand Down Expand Up @@ -183,7 +184,7 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte
Problem: checks.Problem{
Lines: job.entry.ModifiedLines,
Reporter: ignoreFileReporter,
Text: "This file was excluded from pint checks",
Text: "This file was excluded from pint checks.",
Severity: checks.Information,
},
Owner: job.entry.Owner,
Expand All @@ -197,7 +198,11 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte
Problem: checks.Problem{
Lines: []int{line},
Reporter: yamlParseReporter,
Text: e,
Text: fmt.Sprintf("YAML parser returned an error when parsing this file: `%s`.", e),
Details: `pint cannot read this file because YAML parser returned an error.
This usually means that you have an indention error or the file doesn't have the YAML structure required by Prometheus for [recording](https://prometheus.io/docs/prometheus/latest/configuration/recording_rules/) and [alerting](https://prometheus.io/docs/prometheus/latest/configuration/alerting_rules/) rules.
If this file is a template that will be rendered into valid YAML then you can instruct pint to ignore some lines using comments, see [pint docs](https://cloudflare.github.io/pint/ignoring.html).
`,
Severity: checks.Fatal,
},
Owner: job.entry.Owner,
Expand All @@ -212,7 +217,9 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte
Fragment: job.entry.Rule.Error.Fragment,
Lines: []int{job.entry.Rule.Error.Line},
Reporter: yamlParseReporter,
Text: job.entry.Rule.Error.Err.Error(),
Text: fmt.Sprintf("This rule is not a valid Prometheus rule: `%s`.", job.entry.Rule.Error.Err.Error()),
Details: `This Prometheus rule is not valid.
This usually means that it's missing some required fields.`,
Severity: checks.Fatal,
},
Owner: job.entry.Owner,
Expand Down
2 changes: 1 addition & 1 deletion cmd/pint/tests/0001_match_path.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ cmp stderr stderr.txt
-- stderr.txt --
level=INFO msg="Loading configuration file" path=.pint.hcl
level=INFO msg="Finding all rules to check" paths=["rules"]
rules/0002.yml:2 Bug: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/aggregate)
rules/0002.yml:2 Bug: `job` label is required and should be preserved when aggregating `^.+$` rules, remove job from `without()`. (promql/aggregate)
2 | expr: sum(foo) without(job)

level=INFO msg="Problems found" Bug=1
Expand Down
28 changes: 14 additions & 14 deletions cmd/pint/tests/0003_lint_workdir.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,58 +6,58 @@ cmp stderr stderr.txt
-- stderr.txt --
level=INFO msg="Loading configuration file" path=.pint.hcl
level=INFO msg="Finding all rules to check" paths=["rules"]
rules/0001.yml:2 Warning: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/aggregate)
rules/0001.yml:2 Warning: `job` label is required and should be preserved when aggregating `^.+$` rules, remove job from `without()`. (promql/aggregate)
2 | expr: sum(rate(fl_cf_html_bytes_in[10m])) WITHOUT (colo_id, instance, node_type, region, node_status, job, colo_name)

rules/0001.yml:6 Warning: instance label should be removed when aggregating "^colo(?:_.+)?:.+$" rules, use without(instance, ...) (promql/aggregate)
rules/0001.yml:6 Warning: `instance` label should be removed when aggregating `^colo(?:_.+)?:.+$` rules, use `without(instance, ...)`. (promql/aggregate)
6 | expr: sum(irate(foo[3m])) WITHOUT (colo_id)

rules/0002.yaml:2 Bug: unnecessary regexp match on static string job=~"foo", use job="foo" instead (promql/regexp)
rules/0002.yaml:2 Bug: Unnecessary regexp match on static string `job=~"foo"`, use `job="foo"` instead. (promql/regexp)
2 | expr: up{job=~"foo"} == 0

rules/0002.yaml:5 Bug: unnecessary regexp match on static string job!~"foo", use job!="foo" instead (promql/regexp)
rules/0002.yaml:5 Bug: Unnecessary regexp match on static string `job!~"foo"`, use `job!="foo"` instead. (promql/regexp)
5 | expr: up{job!~"foo"} == 0

rules/0003.yaml:11 Warning: instance label should be removed when aggregating "^colo(?:_.+)?:.+$" rules, use without(instance, ...) (promql/aggregate)
rules/0003.yaml:11 Warning: `instance` label should be removed when aggregating `^colo(?:_.+)?:.+$` rules, use `without(instance, ...)`. (promql/aggregate)
11 | expr: sum(foo) without(job)

rules/0003.yaml:11 Warning: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/aggregate)
rules/0003.yaml:11 Warning: `job` label is required and should be preserved when aggregating `^.+$` rules, remove job from `without()`. (promql/aggregate)
11 | expr: sum(foo) without(job)

rules/0003.yaml:14 Fatal: syntax error: unexpected right parenthesis ')' (promql/syntax)
rules/0003.yaml:14 Fatal: Syntax error: unexpected right parenthesis ')'. (promql/syntax)
14 | expr: sum(foo) by ())

rules/0003.yaml:22-25 Warning: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/aggregate)
rules/0003.yaml:22-25 Warning: `job` label is required and should be preserved when aggregating `^.+$` rules, remove job from `without()`. (promql/aggregate)
22 | expr: |
23 | sum(
24 | multiline
25 | ) without(job, instance)

rules/0003.yaml:28-31 Warning: instance label should be removed when aggregating "^colo(?:_.+)?:.+$" rules, use without(instance, ...) (promql/aggregate)
rules/0003.yaml:28-31 Warning: `instance` label should be removed when aggregating `^colo(?:_.+)?:.+$` rules, use `without(instance, ...)`. (promql/aggregate)
28 | expr: |
29 | sum(sum) without(job)
30 | +
31 | sum(sum) without(job)

rules/0003.yaml:28-31 Warning: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/aggregate)
rules/0003.yaml:28-31 Warning: `job` label is required and should be preserved when aggregating `^.+$` rules, remove job from `without()`. (promql/aggregate)
28 | expr: |
29 | sum(sum) without(job)
30 | +
31 | sum(sum) without(job)

rules/0003.yaml:34-37 Warning: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/aggregate)
rules/0003.yaml:34-37 Warning: `job` label is required and should be preserved when aggregating `^.+$` rules, remove job from `without()`. (promql/aggregate)
34 | expr: >-
35 | sum(
36 | multiline2
37 | ) without(job, instance)

rules/0003.yaml:40 Warning: instance label should be removed when aggregating "^colo(?:_.+)?:.+$" rules, remove instance from by() (promql/aggregate)
rules/0003.yaml:40 Warning: `instance` label should be removed when aggregating `^colo(?:_.+)?:.+$` rules, remove instance from `by()`. (promql/aggregate)
40 | expr: sum(byinstance) by(instance)

rules/0003.yaml:40 Warning: job label is required and should be preserved when aggregating "^.+$" rules, use by(job, ...) (promql/aggregate)
rules/0003.yaml:40 Warning: `job` label is required and should be preserved when aggregating `^.+$` rules, use `by(job, ...)`. (promql/aggregate)
40 | expr: sum(byinstance) by(instance)

rules/0003.yaml:58-61 Information: using the value of rate(errors[5m]) inside this annotation might be hard to read, consider using one of humanize template functions to make it more human friendly (alerts/template)
rules/0003.yaml:58-61 Information: Using the value of `rate(errors[5m])` inside this annotation might be hard to read, consider using one of humanize template functions to make it more human friendly. (alerts/template)
58 | expr: sum(rate(errors[5m])) > 0.5
..
61 | summary: 'error rate: {{ $value }}'
Expand Down
4 changes: 2 additions & 2 deletions cmd/pint/tests/0004_fail_invalid_yaml.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ cmp stderr stderr.txt
level=INFO msg="Loading configuration file" path=.pint.hcl
level=INFO msg="Finding all rules to check" paths=["rules"]
level=ERROR msg="Failed to parse file content" err="yaml: line 4: did not find expected key" path=rules/bad.yaml lines=1-7
rules/bad.yaml:4 Fatal: did not find expected key (yaml/parse)
rules/bad.yaml:4 Fatal: YAML parser returned an error when parsing this file: `did not find expected key`. (yaml/parse)
4 |

rules/ok.yml:5 Fatal: syntax error: unclosed left bracket (promql/syntax)
rules/ok.yml:5 Fatal: Syntax error: unclosed left bracket. (promql/syntax)
5 | expr: sum(foo[5m)

level=INFO msg="Problems found" Fatal=2
Expand Down
2 changes: 1 addition & 1 deletion cmd/pint/tests/0006_rr_labels.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ cmp stderr stderr.txt
-- stderr.txt --
level=INFO msg="Loading configuration file" path=.pint.hcl
level=INFO msg="Finding all rules to check" paths=["rules"]
rules/0001.yml:8 Fatal: incomplete rule, no alert or record key (yaml/parse)
rules/0001.yml:8 Fatal: This rule is not a valid Prometheus rule: `incomplete rule, no alert or record key`. (yaml/parse)
8 | - expr: sum(foo)

level=INFO msg="Problems found" Fatal=1
Expand Down
26 changes: 13 additions & 13 deletions cmd/pint/tests/0007_alerts.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,47 +5,47 @@ cmp stderr stderr.txt
-- stderr.txt --
level=INFO msg="Loading configuration file" path=.pint.hcl
level=INFO msg="Finding all rules to check" paths=["rules"]
rules/0001.yml:1-2 Bug: url annotation is required (alerts/annotation)
rules/0001.yml:1-2 Bug: `url` annotation is required. (alerts/annotation)
1 | - alert: Always
2 | expr: up

rules/0001.yml:1-2 Warning: severity label is required (rule/label)
rules/0001.yml:1-2 Warning: `severity` label is required. (rule/label)
1 | - alert: Always
2 | expr: up

rules/0001.yml:2 Warning: alert query doesn't have any condition, it will always fire if the metric exists (alerts/comparison)
rules/0001.yml:2 Warning: Alert query doesn't have any condition, it will always fire if the metric exists. (alerts/comparison)
2 | expr: up

rules/0001.yml:9-10 Bug: url annotation is required (alerts/annotation)
rules/0001.yml:9-10 Bug: `url` annotation is required. (alerts/annotation)
9 | - alert: ServiceIsDown
10 | expr: up == 0

rules/0001.yml:9-10 Warning: severity label is required (rule/label)
rules/0001.yml:9-10 Warning: `severity` label is required. (rule/label)
9 | - alert: ServiceIsDown
10 | expr: up == 0

rules/0001.yml:14 Warning: severity label value must match "^critical|warning|info$" (rule/label)
rules/0001.yml:14 Warning: `severity` label value must match `^critical|warning|info$`. (rule/label)
14 | severity: bad

rules/0001.yml:16 Bug: url annotation value must match "^https://wiki.example.com/page/(.+).html$" (alerts/annotation)
rules/0001.yml:16 Bug: `url` annotation value must match "^https://wiki.example.com/page/(.+).html$". (alerts/annotation)
16 | url: bad

rules/0002.yml:5 Fatal: template parse error: undefined variable "$label" (alerts/template)
rules/0002.yml:5 Fatal: Template failed to parse with this error: `undefined variable "$label"`. (alerts/template)
5 | summary: 'Instance {{ $label.instance }} down'

rules/0002.yml:6 Fatal: template parse error: undefined variable "$valuexx" (alerts/template)
rules/0002.yml:6 Fatal: Template failed to parse with this error: `undefined variable "$valuexx"`. (alerts/template)
6 | func: '{{ $valuexx | xxx }}'

rules/0002.yml:9 Fatal: template parse error: undefined variable "$label" (alerts/template)
rules/0002.yml:9 Fatal: Template failed to parse with this error: `undefined variable "$label"`. (alerts/template)
9 | summary: 'Instance {{ $label.instance }} down'

rules/0002.yml:10 Fatal: template parse error: function "xxx" not defined (alerts/template)
rules/0002.yml:10 Fatal: Template failed to parse with this error: `function "xxx" not defined`. (alerts/template)
10 | func: '{{ $value | xxx }}'

rules/0002.yml:11 Bug: using $value in labels will generate a new alert on every value change, move it to annotations (alerts/template)
rules/0002.yml:11 Bug: Using `$value` in labels will generate a new alert on every value change, move it to annotations. (alerts/template)
11 | bar: 'Some {{$value}} value'

rules/0002.yml:12 Bug: using .Value in labels will generate a new alert on every value change, move it to annotations (alerts/template)
rules/0002.yml:12 Bug: Using `.Value` in labels will generate a new alert on every value change, move it to annotations. (alerts/template)
12 | val: '{{ .Value|humanizeDuration }}'

level=INFO msg="Problems found" Fatal=4 Bug=5 Warning=4
Expand Down
4 changes: 2 additions & 2 deletions cmd/pint/tests/0008_recording_rule_prometheus.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ cmp stderr stderr.txt
-- stderr.txt --
level=INFO msg="Loading configuration file" path=.pint.hcl
level=INFO msg="Finding all rules to check" paths=["rules"]
rules/0001.yml:5 Bug: instance label should be removed when aggregating "^colo(?:_.+)?:.+$" rules, remove instance from by() (promql/aggregate)
rules/0001.yml:5 Bug: `instance` label should be removed when aggregating `^colo(?:_.+)?:.+$` rules, remove instance from `by()`. (promql/aggregate)
5 | expr: sum by (instance) (http_inprogress_requests)

rules/0001.yml:5 Warning: job label is required and should be preserved when aggregating "^.+$" rules, use by(job, ...) (promql/aggregate)
rules/0001.yml:5 Warning: `job` label is required and should be preserved when aggregating `^.+$` rules, use `by(job, ...)`. (promql/aggregate)
5 | expr: sum by (instance) (http_inprogress_requests)

level=INFO msg="Problems found" Bug=1 Warning=1
Expand Down
6 changes: 3 additions & 3 deletions cmd/pint/tests/0009_alerting_rule_prometheus.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ cmp stderr stderr.txt
-- stderr.txt --
level=INFO msg="Loading configuration file" path=.pint.hcl
level=INFO msg="Finding all rules to check" paths=["rules"]
rules/0001.yml:11-13 Bug: link annotation is required (alerts/annotation)
rules/0001.yml:11-13 Bug: `link` annotation is required. (alerts/annotation)
11 | annotations:
12 | summary: "Instance {{ $labels.instance }} down"
13 | description: "{{ $labels.instance }} of job {{ $labels.job }} has been down for more than 5 minutes."

rules/0001.yml:17 Warning: job label is required and should be preserved when aggregating "^.+$" rules, use by(job, ...) (promql/aggregate)
rules/0001.yml:17 Warning: `job` label is required and should be preserved when aggregating `^.+$` rules, use `by(job, ...)`. (promql/aggregate)
17 | expr: sum by (instance) (http_inprogress_requests) > 0

rules/0001.yml:19-21 Bug: link annotation is required (alerts/annotation)
rules/0001.yml:19-21 Bug: `link` annotation is required. (alerts/annotation)
19 | annotations:
20 | summary: "High request latency on {{ $labels.instance }}"
21 | description: "{{ $labels.instance }} has a median request latency above 1s (current value: {{ $value }}s)"
Expand Down
2 changes: 1 addition & 1 deletion cmd/pint/tests/0010_syntax_check.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ cmp stderr stderr.txt
level=INFO msg="Loading configuration file" path=.pint.hcl
level=INFO msg="Finding all rules to check" paths=["rules"]
level=ERROR msg="Failed to parse file content" err="yaml: line 6: did not find expected '-' indicator" path=rules/1.yaml lines=1-12
rules/1.yaml:6 Fatal: did not find expected '-' indicator (yaml/parse)
rules/1.yaml:6 Fatal: YAML parser returned an error when parsing this file: `did not find expected '-' indicator`. (yaml/parse)
6 |

level=INFO msg="Problems found" Fatal=1
Expand Down
10 changes: 5 additions & 5 deletions cmd/pint/tests/0011_ignore_rules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@ cmp stderr stderr.txt
-- stderr.txt --
level=INFO msg="Loading configuration file" path=.pint.hcl
level=INFO msg="Finding all rules to check" paths=["rules"]
rules/1.yaml:5 Fatal: syntax error: unexpected right parenthesis ')' (promql/syntax)
rules/1.yaml:5 Fatal: Syntax error: unexpected right parenthesis ')'. (promql/syntax)
5 | expr: sum(errors_total) by )

rules/1.yaml:16 Warning: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/aggregate)
rules/1.yaml:16 Warning: `job` label is required and should be preserved when aggregating `^.+$` rules, remove job from `without()`. (promql/aggregate)
16 | expr: sum(errors_total) without(job)

rules/1.yaml:22 Fatal: syntax error: unexpected right parenthesis ')' (promql/syntax)
rules/1.yaml:22 Fatal: Syntax error: unexpected right parenthesis ')'. (promql/syntax)
22 | expr: sum(errors_total) by )

rules/1.yaml:33 Warning: alert query doesn't have any condition, it will always fire if the metric exists (alerts/comparison)
rules/1.yaml:33 Warning: Alert query doesn't have any condition, it will always fire if the metric exists. (alerts/comparison)
33 | expr: sum(errors_total) without(job)

rules/1.yaml:33 Warning: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/aggregate)
rules/1.yaml:33 Warning: `job` label is required and should be preserved when aggregating `^.+$` rules, remove job from `without()`. (promql/aggregate)
33 | expr: sum(errors_total) without(job)

level=INFO msg="Problems found" Fatal=2 Warning=3
Expand Down
2 changes: 1 addition & 1 deletion cmd/pint/tests/0012_issue_20.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ level=INFO msg="Loading configuration file" path=.pint.hcl
level=INFO msg="Finding all rules to check" paths=["rules"]
level=WARN msg="Tried to read more lines than present in the source file, this is likely due to '
' usage in some rules, see https://github.com/cloudflare/pint/issues/20 for details" path=rules/1.yaml
rules/1.yaml:9-13 Warning: runbook_url annotation is required (alerts/annotation)
rules/1.yaml:9-13 Warning: `runbook_url` annotation is required. (alerts/annotation)
9 | annotations:
10 | summary: "HAProxy server healthcheck failure (instance {{ $labels.instance }})"
11 | description: "Some server healthcheck are failing on {{ $labels.server }}\n VALUE = {{ $value }}\n LABELS: {{ $labels }}"
Expand Down
Loading

0 comments on commit 05b4c99

Please sign in to comment.