Skip to content

Commit

Permalink
Remove keys from yaml nodes
Browse files Browse the repository at this point in the history
  • Loading branch information
prymitive committed Dec 18, 2023
1 parent 8d34955 commit 9250403
Show file tree
Hide file tree
Showing 25 changed files with 218 additions and 606 deletions.
4 changes: 2 additions & 2 deletions cmd/pint/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,15 @@ func checkRules(ctx context.Context, workers int, isOffline bool, gen *config.Pr
rulesParsedTotal.WithLabelValues(config.RecordingRuleType).Inc()
slog.Debug("Found recording rule",
slog.String("path", entry.SourcePath),
slog.String("record", entry.Rule.RecordingRule.Record.Value.Value),
slog.String("record", entry.Rule.RecordingRule.Record.Value),
slog.String("lines", entry.Rule.Lines.String()),
)
}
if entry.Rule.AlertingRule != nil {
rulesParsedTotal.WithLabelValues(config.AlertingRuleType).Inc()
slog.Debug("Found alerting rule",
slog.String("path", entry.SourcePath),
slog.String("alert", entry.Rule.AlertingRule.Alert.Value.Value),
slog.String("alert", entry.Rule.AlertingRule.Alert.Value),
slog.String("lines", entry.Rule.Lines.String()),
)
}
Expand Down
14 changes: 1 addition & 13 deletions cmd/pint/tests/0003_lint_workdir.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,6 @@ rules/0003.yaml:28-31 Warning: `instance` label should be removed when aggregati
30 | +
31 | sum(sum) without(job)

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)
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)
28 | expr: |
29 | sum(sum) without(job)
Expand All @@ -72,7 +60,7 @@ rules/0003.yaml:40 Warning: `job` label is required and should be preserved when
rules/0003.yaml: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)
61 | summary: 'error rate: {{ $value }}'

level=INFO msg="Problems found" Fatal=1 Bug=2 Warning=12 Information=1
level=INFO msg="Problems found" Fatal=1 Bug=2 Warning=10 Information=1
level=ERROR msg="Fatal error" err="found 2 problem(s) with severity Bug or higher"
-- rules/0001.yml --
- record: colo_job:fl_cf_html_bytes_in:rate10m
Expand Down
14 changes: 1 addition & 13 deletions cmd/pint/tests/0024_color_output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,6 @@ rules/0003.yaml:28-31 Warning: `instance` label should be removed when aggregati
30 | +
31 | sum(sum) without(job)

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)
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)
28 | expr: |
29 | sum(sum) without(job)
Expand All @@ -63,7 +51,7 @@ rules/0003.yaml:40 Warning: `instance` label should be removed when aggregating
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)

level=INFO msg="Problems found" Fatal=1 Warning=12
level=INFO msg="Problems found" Fatal=1 Warning=10
level=ERROR msg="Fatal error" err="found 1 problem(s) with severity Bug or higher"
-- rules/0001.yml --
- record: colo_job:fl_cf_html_bytes_in:rate10m
Expand Down
4 changes: 2 additions & 2 deletions cmd/pint/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,11 +313,11 @@ func (c *problemCollector) Collect(ch chan<- prometheus.Metric) {
name := "unknown"
if report.Rule.AlertingRule != nil {
kind = "alerting"
name = report.Rule.AlertingRule.Alert.Value.Value
name = report.Rule.AlertingRule.Alert.Value
}
if report.Rule.RecordingRule != nil {
kind = "recording"
name = report.Rule.RecordingRule.Record.Value.Value
name = report.Rule.RecordingRule.Record.Value
}
metric := prometheus.MustNewConstMetric(
c.problem,
Expand Down
15 changes: 3 additions & 12 deletions internal/checks/alerts_comparison.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,7 @@ func (c ComparisonCheck) Check(_ context.Context, _ string, rule parser.Rule, _
if n := utils.HasOuterBinaryExpr(expr); n != nil && n.Op == promParser.LOR {
if (hasComparision(n.LHS) == nil || hasComparision(n.RHS) == nil) && !isAbsent(n.LHS) && !isAbsent(n.RHS) {
problems = append(problems, Problem{
Lines: parser.LineRange{
First: rule.AlertingRule.Expr.Key.Lines.First,
Last: rule.AlertingRule.Expr.Value.Lines.Last,
},
Lines: rule.AlertingRule.Expr.Value.Lines,
Reporter: c.Reporter(),
Text: "Alert query uses `or` operator with one side of the query that will always return a result, this alert will always fire.",
Details: ComparisonCheckDetails,
Expand All @@ -74,10 +71,7 @@ func (c ComparisonCheck) Check(_ context.Context, _ string, rule parser.Rule, _
if n := hasComparision(expr.Node); n != nil {
if n.ReturnBool && hasComparision(n.LHS) == nil && hasComparision(n.RHS) == nil {
problems = append(problems, Problem{
Lines: parser.LineRange{
First: rule.AlertingRule.Expr.Key.Lines.First,
Last: rule.AlertingRule.Expr.Value.Lines.Last,
},
Lines: rule.AlertingRule.Expr.Value.Lines,
Reporter: c.Reporter(),
Text: "Alert query uses `bool` modifier for comparison, this means it will always return a result and the alert will always fire.",
Details: ComparisonCheckDetails,
Expand All @@ -92,10 +86,7 @@ func (c ComparisonCheck) Check(_ context.Context, _ string, rule parser.Rule, _
}

problems = append(problems, Problem{
Lines: parser.LineRange{
First: rule.AlertingRule.Expr.Key.Lines.First,
Last: rule.AlertingRule.Expr.Value.Lines.Last,
},
Lines: rule.AlertingRule.Expr.Value.Lines,
Reporter: c.Reporter(),
Text: "Alert query doesn't have any condition, it will always fire if the metric exists.",
Details: ComparisonCheckDetails,
Expand Down
14 changes: 4 additions & 10 deletions internal/checks/alerts_count.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,7 @@ func (c AlertsCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []
if err != nil {
text, severity := textAndSeverityFromError(err, c.Reporter(), c.prom.Name(), Bug)
problems = append(problems, Problem{
Lines: parser.LineRange{
First: rule.AlertingRule.Expr.Key.Lines.First,
Last: rule.AlertingRule.Expr.Value.Lines.Last,
},
Lines: rule.AlertingRule.Expr.Value.Lines,
Reporter: c.Reporter(),
Text: text,
Severity: severity,
Expand All @@ -97,11 +94,11 @@ func (c AlertsCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []

var forDur model.Duration
if rule.AlertingRule.For != nil {
forDur, _ = model.ParseDuration(rule.AlertingRule.For.Value.Value)
forDur, _ = model.ParseDuration(rule.AlertingRule.For.Value)
}
var keepFiringForDur model.Duration
if rule.AlertingRule.KeepFiringFor != nil {
keepFiringForDur, _ = model.ParseDuration(rule.AlertingRule.KeepFiringFor.Value.Value)
keepFiringForDur, _ = model.ParseDuration(rule.AlertingRule.KeepFiringFor.Value)
}

var alerts int
Expand All @@ -118,10 +115,7 @@ func (c AlertsCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []

delta := qr.Series.Until.Sub(qr.Series.From).Round(time.Minute)
problems = append(problems, Problem{
Lines: parser.LineRange{
First: rule.AlertingRule.Expr.Key.Lines.First,
Last: rule.AlertingRule.Expr.Value.Lines.Last,
},
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=1&g0.range_input=%s).`,
Expand Down
4 changes: 2 additions & 2 deletions internal/checks/alerts_for.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ func (c AlertsForChecksFor) Check(_ context.Context, _ string, rule parser.Rule,
}

if rule.AlertingRule.For != nil {
problems = append(problems, c.checkField(rule.AlertingRule.For.Key.Value, rule.AlertingRule.For.Value.Value, rule.AlertingRule.For.Value.Lines)...)
problems = append(problems, c.checkField("for", rule.AlertingRule.For.Value, rule.AlertingRule.For.Lines)...)
}
if rule.AlertingRule.KeepFiringFor != nil {
problems = append(problems, c.checkField(rule.AlertingRule.KeepFiringFor.Key.Value, rule.AlertingRule.KeepFiringFor.Value.Value, rule.AlertingRule.KeepFiringFor.Value.Lines)...)
problems = append(problems, c.checkField("keep_firing_for", rule.AlertingRule.KeepFiringFor.Value, rule.AlertingRule.KeepFiringFor.Lines)...)
}

return problems
Expand Down
9 changes: 3 additions & 6 deletions internal/checks/promql_aggregation.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ func (c AggregationCheck) Check(_ context.Context, _ string, rule parser.Rule, _
}

if c.nameRegex != nil {
if rule.RecordingRule != nil && !c.nameRegex.MustExpand(rule).MatchString(rule.RecordingRule.Record.Value.Value) {
if rule.RecordingRule != nil && !c.nameRegex.MustExpand(rule).MatchString(rule.RecordingRule.Record.Value) {
return nil
}
if rule.AlertingRule != nil && !c.nameRegex.MustExpand(rule).MatchString(rule.AlertingRule.Alert.Value.Value) {
if rule.AlertingRule != nil && !c.nameRegex.MustExpand(rule).MatchString(rule.AlertingRule.Alert.Value) {
return nil
}
}
Expand All @@ -75,10 +75,7 @@ func (c AggregationCheck) Check(_ context.Context, _ string, rule parser.Rule, _

for _, problem := range c.checkNode(expr.Query) {
problems = append(problems, Problem{
Lines: parser.LineRange{
First: expr.Key.Lines.First,
Last: expr.Value.Lines.Last,
},
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Text: problem.text,
Severity: c.severity,
Expand Down
5 changes: 1 addition & 4 deletions internal/checks/promql_fragile.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,7 @@ func (c FragileCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []d

for _, problem := range c.checkNode(expr.Query) {
problems = append(problems, Problem{
Lines: parser.LineRange{
First: expr.Key.Lines.First,
Last: expr.Value.Lines.Last,
},
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Text: problem.text,
Severity: Warning,
Expand Down
15 changes: 3 additions & 12 deletions internal/checks/promql_range_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,7 @@ func (c RangeQueryCheck) Check(ctx context.Context, _ string, rule parser.Rule,
if err != nil {
text, severity := textAndSeverityFromError(err, c.Reporter(), c.prom.Name(), Warning)
problems = append(problems, Problem{
Lines: parser.LineRange{
First: expr.Key.Lines.First,
Last: expr.Value.Lines.Last,
},
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Text: text,
Severity: severity,
Expand All @@ -74,10 +71,7 @@ func (c RangeQueryCheck) Check(ctx context.Context, _ string, rule parser.Rule,
r, err := model.ParseDuration(v)
if err != nil {
problems = append(problems, Problem{
Lines: parser.LineRange{
First: expr.Key.Lines.First,
Last: expr.Value.Lines.Last,
},
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Text: fmt.Sprintf("Cannot parse --storage.tsdb.retention.time=%q flag value: %s", v, err),
Severity: Warning,
Expand All @@ -89,10 +83,7 @@ func (c RangeQueryCheck) Check(ctx context.Context, _ string, rule parser.Rule,

for _, problem := range c.checkNode(ctx, expr.Query, retention, flags.URI) {
problems = append(problems, Problem{
Lines: parser.LineRange{
First: expr.Key.Lines.First,
Last: expr.Value.Lines.Last,
},
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Text: problem.text,
Severity: problem.severity,
Expand Down
12 changes: 3 additions & 9 deletions internal/checks/promql_rate.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,7 @@ func (c RateCheck) Check(ctx context.Context, _ string, rule parser.Rule, entrie
if err != nil {
text, severity := textAndSeverityFromError(err, c.Reporter(), c.prom.Name(), Bug)
problems = append(problems, Problem{
Lines: parser.LineRange{
First: expr.Key.Lines.First,
Last: expr.Value.Lines.Last,
},
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Text: text,
Severity: severity,
Expand All @@ -85,10 +82,7 @@ func (c RateCheck) Check(ctx context.Context, _ string, rule parser.Rule, entrie
done := &completedList{}
for _, problem := range c.checkNode(ctx, expr.Query, entries, cfg, done) {
problems = append(problems, Problem{
Lines: parser.LineRange{
First: expr.Key.Lines.First,
Last: expr.Value.Lines.Last,
},
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Text: problem.text,
Details: problem.details,
Expand Down Expand Up @@ -153,7 +147,7 @@ func (c RateCheck) checkNode(ctx context.Context, node *parser.PromQLNode, entri
if e.Rule.Error.Err != nil {
continue
}
if e.Rule.RecordingRule != nil && e.Rule.RecordingRule.Expr.SyntaxError == nil && e.Rule.RecordingRule.Record.Value.Value == s.Name {
if e.Rule.RecordingRule != nil && e.Rule.RecordingRule.Expr.SyntaxError == nil && e.Rule.RecordingRule.Record.Value == s.Name {
for _, sm := range utils.HasOuterSum(e.Rule.RecordingRule.Expr.Query) {
if sv, ok := sm.Expr.(*promParser.VectorSelector); ok {
metadata, err := c.prom.Metadata(ctx, sv.Name)
Expand Down
10 changes: 2 additions & 8 deletions internal/checks/promql_regexp.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,7 @@ func (c RegexpCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []di

}
problems = append(problems, Problem{
Lines: parser.LineRange{
First: expr.Key.Lines.First,
Last: expr.Value.Lines.Last,
},
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Text: text,
Details: RegexpCheckDetails,
Expand All @@ -136,10 +133,7 @@ func (c RegexpCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []di
}
if beginText > 1 || endText > 1 {
problems = append(problems, Problem{
Lines: parser.LineRange{
First: expr.Key.Lines.First,
Last: expr.Value.Lines.Last,
},
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Text: fmt.Sprintf("Prometheus regexp matchers are automatically fully anchored so match for `%s` will result in `%s%s\"^%s$\"`, remove regexp anchors `^` and/or `$`.",
lm, lm.Name, lm.Type, lm.Value,
Expand Down
Loading

0 comments on commit 9250403

Please sign in to comment.