Skip to content

Commit

Permalink
Refactor config loading
Browse files Browse the repository at this point in the history
  • Loading branch information
@prymitive
prymitive committed Jan 17, 2022
1 parent bc78f80 commit 647f6e6
Show file tree
Hide file tree
Showing 6 changed files with 300 additions and 89 deletions.
3 changes: 3 additions & 0 deletions internal/checks/query_cost.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ type CostCheck struct {
}

func (c CostCheck) String() string {
if c.maxSeries > 0 {
return fmt.Sprintf("%s(%s:%d)", CostCheckName, c.prom.Name(), c.maxSeries)
}
return fmt.Sprintf("%s(%s)", CostCheckName, c.prom.Name())
}

Expand Down
99 changes: 48 additions & 51 deletions internal/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/json"
"fmt"
"os"
"strings"
"time"

"github.com/cloudflare/pint/internal/checks"
Expand Down Expand Up @@ -71,20 +70,23 @@ func (cfg Config) String() string {
func (cfg *Config) GetChecksForRule(path string, r parser.Rule) []checks.RuleChecker {
enabled := []checks.RuleChecker{}

if isEnabled(cfg.Checks.Enabled, cfg.Checks.Disabled, checks.SyntaxCheckName, r, nil) {
enabled = append(enabled, checks.NewSyntaxCheck())
}

if isEnabled(cfg.Checks.Enabled, cfg.Checks.Disabled, checks.AlertForCheckName, r, nil) {
enabled = append(enabled, checks.NewAlertsForCheck())
}

if isEnabled(cfg.Checks.Enabled, cfg.Checks.Disabled, checks.ComparisonCheckName, r, nil) {
enabled = append(enabled, checks.NewComparisonCheck())
}

if isEnabled(cfg.Checks.Enabled, cfg.Checks.Disabled, checks.TemplateCheckName, r, nil) {
enabled = append(enabled, checks.NewTemplateCheck())
allChecks := []checkMeta{
{
name: checks.SyntaxCheckName,
check: checks.NewSyntaxCheck(),
},
{
name: checks.AlertForCheckName,
check: checks.NewAlertsForCheck(),
},
{
name: checks.ComparisonCheckName,
check: checks.NewComparisonCheck(),
},
{
name: checks.TemplateCheckName,
check: checks.NewTemplateCheck(),
},
}

proms := []*promapi.Prometheus{}
Expand All @@ -100,47 +102,41 @@ func (cfg *Config) GetChecksForRule(path string, r parser.Rule) []checks.RuleChe
}
}

for _, prom := range proms {
if isEnabled(cfg.Checks.Enabled, cfg.Checks.Disabled, checks.RateCheckName, r, prom) {
enabled = append(enabled, checks.NewRateCheck(prom))
}
for _, p := range proms {
allChecks = append(allChecks, checkMeta{
name: checks.RateCheckName,
check: checks.NewRateCheck(p),
})
allChecks = append(allChecks, checkMeta{
name: checks.SeriesCheckName,
check: checks.NewSeriesCheck(p),
})
allChecks = append(allChecks, checkMeta{
name: checks.VectorMatchingCheckName,
check: checks.NewVectorMatchingCheck(p),
})
}

for _, prom := range proms {
if isEnabled(cfg.Checks.Enabled, cfg.Checks.Disabled, checks.SeriesCheckName, r, prom) {

enabled = append(enabled, checks.NewSeriesCheck(prom))
}
for _, rule := range cfg.Rules {
allChecks = append(allChecks, rule.resolveChecks(path, r, cfg.Checks.Enabled, cfg.Checks.Disabled, proms)...)
}

for _, prom := range proms {
if isEnabled(cfg.Checks.Enabled, cfg.Checks.Disabled, checks.VectorMatchingCheckName, r, prom) {
enabled = append(enabled, checks.NewVectorMatchingCheck(prom))
for _, cm := range allChecks {
// check if rule was disabled
if !isEnabled(cfg.Checks.Enabled, cfg.Checks.Disabled, r, cm.name, cm.check) {
continue
}
}

for _, rule := range cfg.Rules {
for _, c := range rule.resolveChecks(path, r, cfg.Checks.Enabled, cfg.Checks.Disabled, proms) {
// FIXME redundant ?
if r.HasComment(fmt.Sprintf("disable %s", removeRedundantSpaces(c.String()))) {
log.Debug().
Str("path", path).
Str("check", c.String()).
Msg("Check disabled by comment")
continue
}
// check if rule was already enabled
var v bool
for _, er := range enabled {
if er.String() == c.String() {
v = true
break
}
}
if !v {
enabled = append(enabled, c)
// check if rule was already enabled
var v bool
for _, er := range enabled {
if er.String() == cm.check.String() {
v = true
break
}
}
if !v {
enabled = append(enabled, cm.check)
}
}

el := []string{}
Expand Down Expand Up @@ -272,6 +268,7 @@ func parseDuration(d string) (time.Duration, error) {
return time.Duration(mdur), nil
}

func removeRedundantSpaces(line string) string {
return strings.Join(strings.Fields(line), " ")
type checkMeta struct {
name string
check checks.RuleChecker
}
194 changes: 194 additions & 0 deletions internal/config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,8 @@ rule {
rule {
cost {
bytesPerSample = 4096
maxSeries = 10000
severity = "warning"
}
}
`,
Expand Down Expand Up @@ -377,6 +379,198 @@ rule {
checks.LabelCheckName + "(team:false)",
},
},
{
title: "multiple cost checks",
config: `
prometheus "prom1" {
uri = "http://localhost"
timeout = "1s"
paths = [ "rules.yml" ]
}
prometheus "prom2" {
uri = "http://localhost"
timeout = "1s"
paths = [ "rules.yml" ]
}
rule {
cost {
bytesPerSample = 4096
severity = "info"
}
}
rule {
cost {
bytesPerSample = 4096
maxSeries = 10000
severity = "warning"
}
}
rule {
cost {
bytesPerSample = 4096
maxSeries = 20000
severity = "bug"
}
}
`,
path: "rules.yml",
rule: newRule(t, `
# pint disable query/series
# pint disable promql/rate
# pint disable promql/vector_matching(prom1)
# pint disable promql/vector_matching(prom2)
- record: foo
expr: sum(foo)
`),
checks: []string{
checks.SyntaxCheckName,
checks.AlertForCheckName,
checks.ComparisonCheckName,
checks.TemplateCheckName,
checks.CostCheckName + "(prom1)",
checks.CostCheckName + "(prom2)",
checks.CostCheckName + "(prom1:10000)",
checks.CostCheckName + "(prom2:10000)",
checks.CostCheckName + "(prom1:20000)",
checks.CostCheckName + "(prom2:20000)",
},
},
{
title: "reject rules",
config: `
rule {
reject "http://.+" {
label_keys = true
label_values = true
}
reject ".* +.*" {
annotation_keys = true
label_keys = true
}
reject "" {
annotation_values = true
severity = "bug"
}
}
`,
path: "rules.yml",
rule: newRule(t, "- record: foo\n expr: sum(foo)\n"),
checks: []string{
checks.SyntaxCheckName,
checks.AlertForCheckName,
checks.ComparisonCheckName,
checks.TemplateCheckName,
checks.RejectCheckName + "(key=~'^http://.+$')",
checks.RejectCheckName + "(val=~'^http://.+$')",
checks.RejectCheckName + "(key=~'^.* +.*$')",
checks.RejectCheckName + "(val=~'^$')",
},
},
{
title: "rule with label match / type mismatch",
config: `
rule {
match {
kind = "alerting"
label "cluster" {
value = "prod"
}
}
label "priority" {
severity = "bug"
value = "(1|2|3|4|5)"
required = true
}
}
`,
path: "rules.yml",
rule: newRule(t, "- record: foo\n expr: sum(foo)\n"),
checks: []string{
checks.SyntaxCheckName,
checks.AlertForCheckName,
checks.ComparisonCheckName,
checks.TemplateCheckName,
},
},
{
title: "rule with label match / no label",
config: `
rule {
match {
kind = "alerting"
label "cluster" {
value = "prod"
}
}
label "priority" {
severity = "bug"
value = "(1|2|3|4|5)"
required = true
}
}
`,
path: "rules.yml",
rule: newRule(t, "- alert: foo\n expr: sum(foo)\n"),
checks: []string{
checks.SyntaxCheckName,
checks.AlertForCheckName,
checks.ComparisonCheckName,
checks.TemplateCheckName,
},
},
{
title: "rule with label match / label mismatch",
config: `
rule {
match {
kind = "alerting"
label "cluster" {
value = "prod"
}
}
label "priority" {
severity = "bug"
value = "(1|2|3|4|5)"
required = true
}
}
`,
path: "rules.yml",
rule: newRule(t, "- alert: foo\n expr: sum(foo)\n labels:\n cluster: dev\n"),
checks: []string{
checks.SyntaxCheckName,
checks.AlertForCheckName,
checks.ComparisonCheckName,
checks.TemplateCheckName,
},
},
{
title: "rule with label match / label match",
config: `
rule {
match {
kind = "alerting"
label "cluster" {
value = "prod"
}
}
label "priority" {
severity = "bug"
value = "(1|2|3|4|5)"
required = true
}
}
`,
path: "rules.yml",
rule: newRule(t, "- alert: foo\n expr: sum(foo)\n labels:\n cluster: prod\n"),
checks: []string{
checks.SyntaxCheckName,
checks.AlertForCheckName,
checks.ComparisonCheckName,
checks.TemplateCheckName,
checks.LabelCheckName + "(priority:true)",
},
},
}

dir := t.TempDir()
Expand Down
5 changes: 0 additions & 5 deletions internal/config/reject.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package config

import (
"errors"
"regexp"

"github.com/cloudflare/pint/internal/checks"
Expand All @@ -17,10 +16,6 @@ type RejectSettings struct {
}

func (rs RejectSettings) validate() error {
if rs.Regex == "" {
return errors.New("reject key must be set")
}

if _, err := regexp.Compile(rs.Regex); err != nil {
return err
}
Expand Down
1 change: 0 additions & 1 deletion internal/config/reject_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ func TestRejectSettings(t *testing.T) {
},
{
conf: RejectSettings{},
err: errors.New("reject key must be set"),
},
{
conf: RejectSettings{
Expand Down
Loading

0 comments on commit 647f6e6

Please sign in to comment.