Skip to content

Commit

Permalink
Merge pull request cloudflare#81 from cloudflare/exclude
Browse files Browse the repository at this point in the history
Add ignore block to match expessions
  • Loading branch information
prymitive authored Nov 26, 2021
2 parents 41d2178 + 33c33e0 commit 18ea562
Show file tree
Hide file tree
Showing 10 changed files with 373 additions and 31 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# Changelog

## v0.3.0

### Added

- `rule` block can now specify `ignore` conditions that have the same syntax as `match`
but will disable `rule` for matching alerting and recording rules #48.
- `match` and `ignore` blocks can now filter alerting and recording rules by name.
`record` will be used as name for recording rules and `alert` for alerting rules.

## v0.2.0

### Added
Expand Down
30 changes: 30 additions & 0 deletions cmd/pint/tests/0018_match_alerting.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
pint.ok -l debug lint rules
! stdout .
cmp stderr stderr.txt

-- stderr.txt --
level=info msg="Loading configuration file" path=.pint.hcl
level=info msg="File parsed" path=rules/0001.yml rules=2
level=debug msg="Found recording rule" lines=1-2 path=rules/0001.yml record=colo:recording
level=debug msg="Configured checks for rule" enabled=["promql/syntax"] path=rules/0001.yml rule=colo:recording
level=debug msg="Found alerting rule" alert=colo:alerting lines=4-5 path=rules/0001.yml
level=debug msg="Configured checks for rule" enabled=["promql/syntax","promql/without(job:true)","promql/by(job:true)"] path=rules/0001.yml rule=colo:alerting
rules/0001.yml:5: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/without)
expr: sum(bar) without(job)

-- rules/0001.yml --
- record: "colo:recording"
expr: sum(foo) without(job)

- alert: "colo:alerting"
expr: sum(bar) without(job)

-- .pint.hcl --
rule {
match {
kind = "alerting"
}
aggregate ".+" {
keep = [ "job" ]
}
}
30 changes: 30 additions & 0 deletions cmd/pint/tests/0019_match_recording.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
pint.ok -l debug lint rules
! stdout .
cmp stderr stderr.txt

-- stderr.txt --
level=info msg="Loading configuration file" path=.pint.hcl
level=info msg="File parsed" path=rules/0001.yml rules=2
level=debug msg="Found recording rule" lines=1-2 path=rules/0001.yml record=colo:recording
level=debug msg="Configured checks for rule" enabled=["promql/syntax","promql/without(job:true)","promql/by(job:true)"] path=rules/0001.yml rule=colo:recording
level=debug msg="Found alerting rule" alert=colo:alerting lines=4-5 path=rules/0001.yml
level=debug msg="Configured checks for rule" enabled=["promql/syntax"] path=rules/0001.yml rule=colo:alerting
rules/0001.yml:2: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/without)
expr: sum(foo) without(job)

-- rules/0001.yml --
- record: "colo:recording"
expr: sum(foo) without(job)

- alert: "colo:alerting"
expr: sum(bar) without(job)

-- .pint.hcl --
rule {
match {
kind = "recording"
}
aggregate ".+" {
keep = [ "job" ]
}
}
30 changes: 30 additions & 0 deletions cmd/pint/tests/0020_ignore_kind.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
pint.ok -l debug lint rules
! stdout .
cmp stderr stderr.txt

-- stderr.txt --
level=info msg="Loading configuration file" path=.pint.hcl
level=info msg="File parsed" path=rules/0001.yml rules=2
level=debug msg="Found recording rule" lines=1-2 path=rules/0001.yml record=colo:recording
level=debug msg="Configured checks for rule" enabled=["promql/syntax","promql/without(job:true)","promql/by(job:true)"] path=rules/0001.yml rule=colo:recording
level=debug msg="Found alerting rule" alert=colo:alerting lines=4-5 path=rules/0001.yml
level=debug msg="Configured checks for rule" enabled=["promql/syntax"] path=rules/0001.yml rule=colo:alerting
rules/0001.yml:2: job label is required and should be preserved when aggregating "^.+$" rules, remove job from without() (promql/without)
expr: sum(foo) without(job)

-- rules/0001.yml --
- record: "colo:recording"
expr: sum(foo) without(job)

- alert: "colo:alerting"
expr: sum(bar) without(job)

-- .pint.hcl --
rule {
ignore {
kind = "alerting"
}
aggregate ".+" {
keep = [ "job" ]
}
}
21 changes: 21 additions & 0 deletions cmd/pint/tests/0021_ignore_all.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
pint.error -l debug lint rules
! stdout .
cmp stderr stderr.txt

-- stderr.txt --
level=info msg="Loading configuration file" path=.pint.hcl
level=fatal msg="Fatal error" error="failed to load config file \".pint.hcl\": ignore block must have at least one condition"
-- rules/0001.yml --
- record: "colo:recording"
expr: sum(foo) without(job)

- alert: "colo:alerting"
expr: sum(bar) without(job)

-- .pint.hcl --
rule {
ignore {}
aggregate ".+" {
keep = [ "job" ]
}
}
18 changes: 17 additions & 1 deletion docs/CONFIGURATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,19 @@ Syntax:
```JS
rule {
match {
path = "..."
path = "(.+)"
name = "(.+)"
kind = "alerting|recording"
annotation "(.*)" {
value = "(.*)"
}
label "(.*)" {
value = "(.*)"
}
}
ignore {
path = "(.+)"
name = "(.+)"
kind = "alerting|recording"
annotation "(.*)" {
value = "(.*)"
Expand All @@ -144,12 +156,16 @@ rule {
```

- `match:path` - only files matching this pattern will be checked by this rule
- `match:name` - only rules with names (`record` for recording rules and `alert` for alerting
rules) matching this pattern will be checked rule
- `match:kind` - optional rule type filter, only rule of this type will be checked
- `match:annotation` - optional annotation filter, only alert rules with at least one
annotation matching this pattern will be checked by this rule.
- `match:label` - optional annotation filter, only rules with at least one label
matching this pattern will be checked by this rule. For recording rules only static
labels set on the recording rule are considered.
- `ignore` - works exactly like `match` but does the opposite - any alerting or recording rule
matching all conditions defined on `ignore` will not be checked by this `rule` block.

Example:

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ require (
github.com/prometheus/prometheus v1.8.2-0.20211105201321-411021ada9ab
github.com/rogpeppe/go-internal v1.8.0
github.com/rs/zerolog v1.26.0
github.com/stretchr/testify v1.7.0
github.com/urfave/cli/v2 v2.3.0
golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b
Expand Down Expand Up @@ -54,7 +55,6 @@ require (
github.com/prometheus/procfs v0.7.3 // indirect
github.com/russross/blackfriday/v2 v2.1.0 // indirect
github.com/sergi/go-diff v1.2.0 // indirect
github.com/stretchr/testify v1.7.0 // indirect
github.com/uber/jaeger-client-go v2.29.1+incompatible // indirect
github.com/uber/jaeger-lib v2.4.1+incompatible // indirect
github.com/zclconf/go-cty v1.10.0 // indirect
Expand Down
8 changes: 6 additions & 2 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ func (cfg Config) GetChecksForRule(path string, r parser.Rule) []checks.RuleChec
continue
}
enabled = append(enabled, c)

}
}

Expand Down Expand Up @@ -151,7 +150,12 @@ func Load(path string, failOnMissing bool) (cfg Config, err error) {

for _, rule := range cfg.Rules {
if rule.Match != nil {
if err = rule.Match.validate(); err != nil {
if err = rule.Match.validate(true); err != nil {
return cfg, err
}
}
if rule.Ignore != nil {
if err = rule.Ignore.validate(false); err != nil {
return cfg, err
}
}
Expand Down
78 changes: 51 additions & 27 deletions internal/config/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,21 @@ func (ml MatchLabel) isMatching(rule parser.Rule) bool {

type Match struct {
Path string `hcl:"path,optional"`
Name string `hcl:"name,optional"`
Kind string `hcl:"kind,optional"`
Label *MatchLabel `hcl:"label,block"`
Annotation *MatchLabel `hcl:"annotation,block"`
}

func (m Match) validate() error {
func (m Match) validate(allowEmpty bool) error {
if _, err := regexp.Compile(m.Path); err != nil {
return err
}

if _, err := regexp.Compile(m.Name); err != nil {
return err
}

switch m.Kind {
case "":
// not set
Expand All @@ -96,11 +101,52 @@ func (m Match) validate() error {
}
}

if !allowEmpty && m.Path == "" && m.Name == "" && m.Kind == "" && m.Label == nil && m.Annotation == nil {
return fmt.Errorf("ignore block must have at least one condition")
}

return nil
}

func (m Match) IsMatch(path string, r parser.Rule) bool {
if m.Kind != "" {
if r.AlertingRule != nil && m.Kind != alertingRuleType {
return false
}
if r.RecordingRule != nil && m.Kind != recordingRuleType {
return false
}
}

if m.Path != "" {
re := strictRegex(m.Path)
if !re.MatchString(path) {
return false
}
}

if m.Name != "" {
re := strictRegex(m.Name)
if r.AlertingRule != nil && !re.MatchString(r.AlertingRule.Alert.Value.Value) {
return false
}
if r.RecordingRule != nil && !re.MatchString(r.RecordingRule.Record.Value.Value) {
return false
}
}

if m.Label != nil {
if !m.Label.isMatching(r) {
return false
}
}

return true
}

type Rule struct {
Match *Match `hcl:"match,block"`
Ignore *Match `hcl:"ignore,block"`
Aggregate []AggregateSettings `hcl:"aggregate,block"`
Rate *RateSettings `hcl:"rate,block"`
Annotation []AnnotationSettings `hcl:"annotation,block"`
Expand All @@ -117,34 +163,12 @@ type Rule struct {
func (rule Rule) resolveChecks(path string, r parser.Rule, enabledChecks, disabledChecks []string, proms []PrometheusConfig) []checks.RuleChecker {
enabled := []checks.RuleChecker{}

if rule.Match != nil && rule.Match.Kind != "" {
var isAllowed bool
recordingEnabled := rule.Match.Kind == recordingRuleType
alertingEnabled := rule.Match.Kind == alertingRuleType
if r.AlertingRule != nil && alertingEnabled {
isAllowed = true
}
if r.RecordingRule != nil && recordingEnabled {
isAllowed = true
}
if !isAllowed {
return enabled
}
if rule.Ignore != nil && rule.Ignore.IsMatch(path, r) {
return enabled
}

if rule.Match != nil {
if rule.Match.Path != "" {
re := strictRegex(rule.Match.Path)
if !re.MatchString(path) {
return enabled
}
}

if rule.Match.Label != nil {
if !rule.Match.Label.isMatching(r) {
return enabled
}
}
if rule.Match != nil && !rule.Match.IsMatch(path, r) {
return enabled
}

if len(rule.Aggregate) > 0 {
Expand Down
Loading

0 comments on commit 18ea562

Please sign in to comment.