Skip to content

Commit

Permalink
Merge pull request #210 from cloudflare/owner
Browse files Browse the repository at this point in the history
Allow setting rule owner via comments
  • Loading branch information
prymitive authored Apr 1, 2022
2 parents 6f4cf77 + 4ce0bf5 commit 40b7cf8
Show file tree
Hide file tree
Showing 18 changed files with 288 additions and 32 deletions.
42 changes: 42 additions & 0 deletions cmd/pint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,20 @@ import (
"github.com/urfave/cli/v2"
)

var requireOwnerFlag = "require-owner"

var lintCmd = &cli.Command{
Name: "lint",
Usage: "Lint specified files",
Action: actionLint,
Flags: []cli.Flag{
&cli.BoolFlag{
Name: requireOwnerFlag,
Aliases: []string{"r"},
Value: false,
Usage: "Require all rules to have an owner set via comment",
},
},
}

func actionLint(c *cli.Context) error {
Expand All @@ -40,6 +50,10 @@ func actionLint(c *cli.Context) error {
ctx := context.WithValue(context.Background(), config.CommandKey, config.LintCommand)
summary := checkRules(ctx, meta.workers, meta.cfg, entries)

if c.Bool(requireOwnerFlag) {
summary.Reports = append(summary.Reports, verifyOwners(entries)...)
}

r := reporter.NewConsoleReporter(os.Stderr)
err = r.Submit(summary)
if err != nil {
Expand All @@ -63,3 +77,31 @@ func actionLint(c *cli.Context) error {

return nil
}

func verifyOwners(entries []discovery.Entry) (reports []reporter.Report) {
done := map[string]struct{}{}
for _, entry := range entries {
if _, ok := done[entry.Path]; ok {
continue
}

if entry.Owner == "" {
reports = append(reports, reporter.Report{
Path: entry.Path,
ModifiedLines: []int{1},
Rule: entry.Rule,
Problem: checks.Problem{
Lines: []int{1},
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`,
discovery.RuleOwnerComment, discovery.FileOwnerComment, discovery.RuleOwnerComment),
Severity: checks.Bug,
},
})
}

done[entry.Path] = struct{}{}
}

return
}
2 changes: 2 additions & 0 deletions cmd/pint/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte
Text: job.entry.Rule.Error.Err.Error(),
Severity: checks.Fatal,
},
Owner: job.entry.Owner,
}
continue
} else {
Expand All @@ -153,6 +154,7 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte
ModifiedLines: job.entry.ModifiedLines,
Rule: job.entry.Rule,
Problem: problem,
Owner: job.entry.Owner,
}
}
}
Expand Down
21 changes: 17 additions & 4 deletions cmd/pint/tests/0042_watch_metrics.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,26 @@ sleep 3
curl -s http://127.0.0.1:6042/metrics | grep -vE 'process_|go_info' | perl -pe "s/^([a-zA-Z].+)[ ]([0-9\.\-\+eE]+)$/\1/g" > curl.txt
cat pint.pid | xargs kill

-- rules/1.yml --
-- rules/unknown.yml --
- record: broken
expr: foo / count())

- record: aggregate
expr: sum(foo) without(job)

- alert: comparison
expr: foo
-- rules/bob.yml --
# pint file/owner bob

- alert: ok
expr: foo > 0

- alert: broken
expr: foo / count())

-- rules/alice.yml --
- alert: broken
expr: foo / count())
# pint rule/owner alice

-- .pint.hcl --
rule {
Expand Down Expand Up @@ -277,7 +288,9 @@ pint_check_iterations_total
pint_last_run_time_seconds
# HELP pint_problem Prometheus rule problem reported by pint
# TYPE pint_problem gauge
pint_problem{filename="rules/1.yml",kind="recording",name="broken",problem="syntax error: no arguments for aggregate expression provided",reporter="promql/syntax",severity="fatal"}
pint_problem{filename="rules/alice.yml",kind="alerting",name="broken",owner="alice",problem="syntax error: no arguments for aggregate expression provided",reporter="promql/syntax",severity="fatal"}
pint_problem{filename="rules/bob.yml",kind="alerting",name="broken",owner="bob",problem="syntax error: no arguments for aggregate expression provided",reporter="promql/syntax",severity="fatal"}
pint_problem{filename="rules/unknown.yml",kind="recording",name="broken",owner="",problem="syntax error: no arguments for aggregate expression provided",reporter="promql/syntax",severity="fatal"}
# HELP pint_problems Total number of problems reported by pint
# TYPE pint_problems gauge
pint_problems
Expand Down
2 changes: 1 addition & 1 deletion cmd/pint/tests/0048_watch_limit.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,5 @@ rule {
}

-- metrics.txt --
pint_problem{filename="rules/1.yml",kind="recording",name="broken",problem="syntax error: no arguments for aggregate expression provided",reporter="promql/syntax",severity="fatal"} 1
pint_problem{filename="rules/1.yml",kind="recording",name="broken",owner="",problem="syntax error: no arguments for aggregate expression provided",reporter="promql/syntax",severity="fatal"} 1
pint_problems 1
6 changes: 3 additions & 3 deletions cmd/pint/tests/0049_watch_severity_warning.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ rule {
}

-- metrics.txt --
pint_problem{filename="rules/1.yml",kind="alerting",name="comparison",problem="alert query doesn't have any condition, it will always fire if the metric exists",reporter="alerts/comparison",severity="warning"} 1
pint_problem{filename="rules/1.yml",kind="recording",name="aggregate",problem="job label is required and should be preserved when aggregating \"^.+$\" rules, remove job from without()",reporter="promql/aggregate",severity="warning"} 1
pint_problem{filename="rules/1.yml",kind="recording",name="broken",problem="syntax error: no arguments for aggregate expression provided",reporter="promql/syntax",severity="fatal"} 1
pint_problem{filename="rules/1.yml",kind="alerting",name="comparison",owner="",problem="alert query doesn't have any condition, it will always fire if the metric exists",reporter="alerts/comparison",severity="warning"} 1
pint_problem{filename="rules/1.yml",kind="recording",name="aggregate",owner="",problem="job label is required and should be preserved when aggregating \"^.+$\" rules, remove job from without()",reporter="promql/aggregate",severity="warning"} 1
pint_problem{filename="rules/1.yml",kind="recording",name="broken",owner="",problem="syntax error: no arguments for aggregate expression provided",reporter="promql/syntax",severity="fatal"} 1
pint_problems 3
2 changes: 1 addition & 1 deletion cmd/pint/tests/0050_watch_severity_fatal.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,5 @@ rule {
}

-- metrics.txt --
pint_problem{filename="rules/1.yml",kind="recording",name="broken",problem="syntax error: no arguments for aggregate expression provided",reporter="promql/syntax",severity="fatal"} 1
pint_problem{filename="rules/1.yml",kind="recording",name="broken",owner="",problem="syntax error: no arguments for aggregate expression provided",reporter="promql/syntax",severity="fatal"} 1
pint_problems 1
20 changes: 11 additions & 9 deletions cmd/pint/tests/0054_watch_metrics_prometheus.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ cat prometheus.pid | xargs kill
- record: aggregate
expr: sum(foo) without(job)

-- rules/2.yml --
# pint rule/owner bob and alice
- alert: comparison
expr: foo

Expand Down Expand Up @@ -64,15 +66,15 @@ pint_check_iterations_total
pint_last_run_time_seconds
# HELP pint_problem Prometheus rule problem reported by pint
# TYPE pint_problem gauge
pint_problem{filename="rules/1.yml",kind="alerting",name="comparison",problem="cound't run \"promql/rate\" checks due to prometheus \"prom1\" at http://127.0.0.1:7054 connection error: failed to query Prometheus config: server_error: server error: 500",reporter="promql/rate",severity="bug"}
pint_problem{filename="rules/1.yml",kind="alerting",name="comparison",problem="cound't run \"promql/rate\" checks due to prometheus \"prom2\" at http://127.0.0.1:1054 connection error: failed to query Prometheus config: Get \"http://127.0.0.1:1054/api/v1/status/config\": dial tcp 127.0.0.1:1054: connect: connection refused",reporter="promql/rate",severity="bug"}
pint_problem{filename="rules/1.yml",kind="alerting",name="comparison",problem="cound't run \"promql/series\" checks due to prometheus \"prom2\" at http://127.0.0.1:1054 connection error: Post \"http://127.0.0.1:1054/api/v1/query\": dial tcp 127.0.0.1:1054: connect: connection refused",reporter="promql/series",severity="bug"}
pint_problem{filename="rules/1.yml",kind="alerting",name="comparison",problem="prometheus \"prom1\" at http://127.0.0.1:7054 failed with: bad_response: Unmarshal: there are bytes left after unmarshal, error found in #10 byte of ...|y\"\n }Fatal error|..., bigger context ...|:\"bad_data\",\n \"error\":\"bogus query\"\n }Fatal error|...",reporter="promql/series",severity="bug"}
pint_problem{filename="rules/1.yml",kind="recording",name="aggregate",problem="cound't run \"promql/rate\" checks due to prometheus \"prom1\" at http://127.0.0.1:7054 connection error: failed to query Prometheus config: server_error: server error: 500",reporter="promql/rate",severity="bug"}
pint_problem{filename="rules/1.yml",kind="recording",name="aggregate",problem="cound't run \"promql/rate\" checks due to prometheus \"prom2\" at http://127.0.0.1:1054 connection error: failed to query Prometheus config: Get \"http://127.0.0.1:1054/api/v1/status/config\": dial tcp 127.0.0.1:1054: connect: connection refused",reporter="promql/rate",severity="bug"}
pint_problem{filename="rules/1.yml",kind="recording",name="aggregate",problem="cound't run \"promql/series\" checks due to prometheus \"prom2\" at http://127.0.0.1:1054 connection error: Post \"http://127.0.0.1:1054/api/v1/query\": dial tcp 127.0.0.1:1054: connect: connection refused",reporter="promql/series",severity="bug"}
pint_problem{filename="rules/1.yml",kind="recording",name="aggregate",problem="prometheus \"prom1\" at http://127.0.0.1:7054 failed with: bad_response: Unmarshal: there are bytes left after unmarshal, error found in #10 byte of ...|y\"\n }Fatal error|..., bigger context ...|:\"bad_data\",\n \"error\":\"bogus query\"\n }Fatal error|...",reporter="promql/series",severity="bug"}
pint_problem{filename="rules/1.yml",kind="recording",name="broken",problem="syntax error: no arguments for aggregate expression provided",reporter="promql/syntax",severity="fatal"}
pint_problem{filename="rules/1.yml",kind="recording",name="aggregate",owner="",problem="cound't run \"promql/rate\" checks due to prometheus \"prom1\" at http://127.0.0.1:7054 connection error: failed to query Prometheus config: server_error: server error: 500",reporter="promql/rate",severity="bug"}
pint_problem{filename="rules/1.yml",kind="recording",name="aggregate",owner="",problem="cound't run \"promql/rate\" checks due to prometheus \"prom2\" at http://127.0.0.1:1054 connection error: failed to query Prometheus config: Get \"http://127.0.0.1:1054/api/v1/status/config\": dial tcp 127.0.0.1:1054: connect: connection refused",reporter="promql/rate",severity="bug"}
pint_problem{filename="rules/1.yml",kind="recording",name="aggregate",owner="",problem="cound't run \"promql/series\" checks due to prometheus \"prom2\" at http://127.0.0.1:1054 connection error: Post \"http://127.0.0.1:1054/api/v1/query\": dial tcp 127.0.0.1:1054: connect: connection refused",reporter="promql/series",severity="bug"}
pint_problem{filename="rules/1.yml",kind="recording",name="aggregate",owner="",problem="prometheus \"prom1\" at http://127.0.0.1:7054 failed with: bad_response: Unmarshal: there are bytes left after unmarshal, error found in #10 byte of ...|y\"\n }Fatal error|..., bigger context ...|:\"bad_data\",\n \"error\":\"bogus query\"\n }Fatal error|...",reporter="promql/series",severity="bug"}
pint_problem{filename="rules/1.yml",kind="recording",name="broken",owner="",problem="syntax error: no arguments for aggregate expression provided",reporter="promql/syntax",severity="fatal"}
pint_problem{filename="rules/2.yml",kind="alerting",name="comparison",owner="bob and alice",problem="cound't run \"promql/rate\" checks due to prometheus \"prom1\" at http://127.0.0.1:7054 connection error: failed to query Prometheus config: server_error: server error: 500",reporter="promql/rate",severity="bug"}
pint_problem{filename="rules/2.yml",kind="alerting",name="comparison",owner="bob and alice",problem="cound't run \"promql/rate\" checks due to prometheus \"prom2\" at http://127.0.0.1:1054 connection error: failed to query Prometheus config: Get \"http://127.0.0.1:1054/api/v1/status/config\": dial tcp 127.0.0.1:1054: connect: connection refused",reporter="promql/rate",severity="bug"}
pint_problem{filename="rules/2.yml",kind="alerting",name="comparison",owner="bob and alice",problem="cound't run \"promql/series\" checks due to prometheus \"prom2\" at http://127.0.0.1:1054 connection error: Post \"http://127.0.0.1:1054/api/v1/query\": dial tcp 127.0.0.1:1054: connect: connection refused",reporter="promql/series",severity="bug"}
pint_problem{filename="rules/2.yml",kind="alerting",name="comparison",owner="bob and alice",problem="prometheus \"prom1\" at http://127.0.0.1:7054 failed with: bad_response: Unmarshal: there are bytes left after unmarshal, error found in #10 byte of ...|y\"\n }Fatal error|..., bigger context ...|:\"bad_data\",\n \"error\":\"bogus query\"\n }Fatal error|...",reporter="promql/series",severity="bug"}
# HELP pint_problems Total number of problems reported by pint
# TYPE pint_problems gauge
pint_problems
Expand Down
6 changes: 3 additions & 3 deletions cmd/pint/tests/0057_watch_metrics_prometheus_ignore.txt
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ pint_check_iterations_total
pint_last_run_time_seconds
# HELP pint_problem Prometheus rule problem reported by pint
# TYPE pint_problem gauge
pint_problem{filename="rules/1.yml",kind="alerting",name="comparison",problem="prometheus \"prom1\" at http://127.0.0.1:7057 failed with: bad_response: Unmarshal: there are bytes left after unmarshal, error found in #10 byte of ...|y\"\n }Fatal error|..., bigger context ...|:\"bad_data\",\n \"error\":\"bogus query\"\n }Fatal error|...",reporter="promql/series",severity="bug"}
pint_problem{filename="rules/1.yml",kind="recording",name="aggregate",problem="prometheus \"prom1\" at http://127.0.0.1:7057 failed with: bad_response: Unmarshal: there are bytes left after unmarshal, error found in #10 byte of ...|y\"\n }Fatal error|..., bigger context ...|:\"bad_data\",\n \"error\":\"bogus query\"\n }Fatal error|...",reporter="promql/series",severity="bug"}
pint_problem{filename="rules/1.yml",kind="recording",name="broken",problem="syntax error: no arguments for aggregate expression provided",reporter="promql/syntax",severity="fatal"}
pint_problem{filename="rules/1.yml",kind="alerting",name="comparison",owner="",problem="prometheus \"prom1\" at http://127.0.0.1:7057 failed with: bad_response: Unmarshal: there are bytes left after unmarshal, error found in #10 byte of ...|y\"\n }Fatal error|..., bigger context ...|:\"bad_data\",\n \"error\":\"bogus query\"\n }Fatal error|...",reporter="promql/series",severity="bug"}
pint_problem{filename="rules/1.yml",kind="recording",name="aggregate",owner="",problem="prometheus \"prom1\" at http://127.0.0.1:7057 failed with: bad_response: Unmarshal: there are bytes left after unmarshal, error found in #10 byte of ...|y\"\n }Fatal error|..., bigger context ...|:\"bad_data\",\n \"error\":\"bogus query\"\n }Fatal error|...",reporter="promql/series",severity="bug"}
pint_problem{filename="rules/1.yml",kind="recording",name="broken",owner="",problem="syntax error: no arguments for aggregate expression provided",reporter="promql/syntax",severity="fatal"}
# HELP pint_problems Total number of problems reported by pint
# TYPE pint_problems gauge
pint_problems
Expand Down
34 changes: 34 additions & 0 deletions cmd/pint/tests/0066_lint_owner.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
pint.error --no-color lint --require-owner rules
! stdout .
cmp stderr stderr.txt

-- stderr.txt --
level=info msg="File parsed" path=rules/1.yml rules=2
level=info msg="File parsed" path=rules/2.yml rules=1
level=info msg="File parsed" path=rules/3.yml rules=1
rules/1.yml:1: rule/owner comments are required in all files, please add a "# pint file/owner $owner" somewhere in this file and/or "# pint rule/owner $owner" on top of each rule (rule/owner)
- alert: No Owner

rules/3.yml:1: rule/owner comments are required in all files, please add a "# pint file/owner $owner" somewhere in this file and/or "# pint rule/owner $owner" on top of each rule (rule/owner)
- alert: No Owner

level=info msg="Problems found" Bug=2
level=fatal msg="Fatal error" error="problems found"
-- rules/1.yml --
- alert: No Owner
expr: up > 0
# pint rule/owner bob
- alert: Owner Set
expr: up == 0

-- rules/2.yml --
- alert: Owner Set
expr: up{job="foo"} == 0

# pint file/owner bob

-- rules/3.yml --
- alert: No Owner
expr: up{job="foo"} == 0

# pint rule/owner bob
3 changes: 2 additions & 1 deletion cmd/pint/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func newProblemCollector(cfg config.Config, paths []string, minSeverity checks.S
problem: prometheus.NewDesc(
"pint_problem",
"Prometheus rule problem reported by pint",
[]string{"filename", "kind", "name", "severity", "reporter", "problem"},
[]string{"filename", "kind", "name", "severity", "reporter", "problem", "owner"},
prometheus.Labels{},
),
problems: prometheus.NewDesc(
Expand Down Expand Up @@ -286,6 +286,7 @@ func (c *problemCollector) Collect(ch chan<- prometheus.Metric) {
strings.ToLower(report.Problem.Severity.String()),
report.Problem.Reporter,
report.Problem.Text,
report.Owner,
)

var out dto.Metric
Expand Down
11 changes: 11 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
# Changelog

## v0.16.0

### Added

- When running `pint watch` exported metric can include `owner` label for each rule.
This is useful to route alerts based on `pint_problem` metrics to the right team.
To set a rule owner add a `# pint file/owner $owner` comment in a file, to set
an owner for all rules in that file. You can also set an owner per rule, by adding
`# pint rule/owner $owner` comment around given rule.
To enforce ownership comments in all files pass `--require-owner` flag to `pint lint`.

## v0.15.7

### Fixed
Expand Down
23 changes: 21 additions & 2 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ or both:
pint lint path/to/dir file.yml path/file.yml path/dir
```

### Daemon
### Watch mode

Run the daemon:
Run pint as a daemon in watch mode:

```shell
pint watch rules.yml
Expand Down Expand Up @@ -92,6 +92,25 @@ Available metrics:
- `pint_problems` - this metric is the total number of all problems detected by pint,
including those not exported due to the `--max-problems` flag.

`pint problem` metric can include `owner` label for each rule. This is useful
to route alerts based on metrics to the right team.
To set a rule owner add a `# pint file/owner $owner` comment in a file, to set
an owner for all rules in that file. You can also set an owner per rule, by adding
`# pint rule/owner $owner` comment around given rule.

Example:

```yaml
# pint file/owner bob
- alert: ...
expr: ...
# pint rule/owner alice
- alert: ...
expr: ...
```

## Release Notes

See [changelog](changelog.md) for history of changes.
Expand Down
17 changes: 15 additions & 2 deletions internal/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ import (
"github.com/rs/zerolog/log"
)

const (
FileOwnerComment = "file/owner"
RuleOwnerComment = "rule/owner"
)

type RuleFinder interface {
Find() ([]Entry, error)
}
Expand All @@ -17,6 +22,7 @@ type Entry struct {
PathError error
ModifiedLines []int
Rule parser.Rule
Owner string
}

func readFile(path string) (entries []Entry, err error) {
Expand All @@ -33,6 +39,8 @@ func readFile(path string) (entries []Entry, err error) {
return nil, err
}

fileOwner, _ := parser.GetComment(string(content), FileOwnerComment)

rules, err := p.Parse(content)
if err != nil {
log.Error().Str("path", path).Err(err).Msg("Failed to parse file content")
Expand All @@ -44,9 +52,14 @@ func readFile(path string) (entries []Entry, err error) {
}

for _, rule := range rules {
owner, ok := rule.GetComment(RuleOwnerComment)
if !ok {
owner = fileOwner
}
entries = append(entries, Entry{
Path: path,
Rule: rule,
Path: path,
Rule: rule,
Owner: owner,
})
}

Expand Down
14 changes: 8 additions & 6 deletions internal/discovery/glob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestGlobPathFinder(t *testing.T) {
}

p := parser.NewParser()
testRuleBody := "- record: foo\n expr: sum(foo)\n"
testRuleBody := "# pint file/owner bob\n\n- record: foo\n expr: sum(foo)\n"
testRules, err := p.Parse([]byte(testRuleBody))
require.NoError(t, err)

Expand Down Expand Up @@ -60,18 +60,20 @@ func TestGlobPathFinder(t *testing.T) {
finder: discovery.NewGlobFinder("*"),
entries: []discovery.Entry{
{
Path: "bar.yml",
Rule: testRules[0],
Path: "bar.yml",
Rule: testRules[0],
Owner: "bob",
},
},
},
{
files: map[string]string{"foo/bar.yml": testRuleBody},
files: map[string]string{"foo/bar.yml": testRuleBody + "\n\n# pint file/owner alice\n"},
finder: discovery.NewGlobFinder("*"),
entries: []discovery.Entry{
{
Path: "foo/bar.yml",
Rule: testRules[0],
Path: "foo/bar.yml",
Rule: testRules[0],
Owner: "alice",
},
},
},
Expand Down
Loading

0 comments on commit 40b7cf8

Please sign in to comment.