Skip to content

Commit

Permalink
Pass full path info to checks
Browse files Browse the repository at this point in the history
  • Loading branch information
prymitive committed Apr 2, 2024
1 parent 7545108 commit d72401d
Show file tree
Hide file tree
Showing 48 changed files with 978 additions and 533 deletions.
12 changes: 8 additions & 4 deletions cmd/pint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,10 @@ func verifyOwners(entries []discovery.Entry, allowedOwners []*regexp.Regexp) (re
}
if entry.Owner == "" {
reports = append(reports, reporter.Report{
ReportedPath: entry.ReportedPath,
SourcePath: entry.SourcePath,
Path: discovery.Path{
Name: entry.Path.Name,
SymlinkTarget: entry.Path.SymlinkTarget,
},
ModifiedLines: entry.ModifiedLines,
Rule: entry.Rule,
Problem: checks.Problem{
Expand All @@ -163,8 +165,10 @@ func verifyOwners(entries []discovery.Entry, allowedOwners []*regexp.Regexp) (re
}
}
reports = append(reports, reporter.Report{
ReportedPath: entry.ReportedPath,
SourcePath: entry.SourcePath,
Path: discovery.Path{
Name: entry.Path.Name,
SymlinkTarget: entry.Path.SymlinkTarget,
},
ModifiedLines: entry.ModifiedLines,
Rule: entry.Rule,
Problem: checks.Problem{
Expand Down
40 changes: 25 additions & 15 deletions cmd/pint/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,15 @@ func checkRules(ctx context.Context, workers int, isOffline bool, gen *config.Pr
if entry.Rule.RecordingRule != nil {
rulesParsedTotal.WithLabelValues(config.RecordingRuleType).Inc()
slog.Debug("Found recording rule",
slog.String("path", entry.SourcePath),
slog.String("path", entry.Path.Name),
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("path", entry.Path.Name),
slog.String("alert", entry.Rule.AlertingRule.Alert.Value),
slog.String("lines", entry.Rule.Lines.String()),
)
Expand All @@ -146,7 +146,7 @@ func checkRules(ctx context.Context, workers int, isOffline bool, gen *config.Pr
default:
if entry.Rule.Error.Err != nil {
slog.Debug("Found invalid rule",
slog.String("path", entry.SourcePath),
slog.String("path", entry.Path.Name),
slog.String("lines", entry.Rule.Lines.String()),
)
rulesParsedTotal.WithLabelValues(config.InvalidRuleType).Inc()
Expand Down Expand Up @@ -189,8 +189,10 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte
switch {
case errors.As(job.entry.PathError, &ignoreErr):
results <- reporter.Report{
ReportedPath: job.entry.ReportedPath,
SourcePath: job.entry.SourcePath,
Path: discovery.Path{
Name: job.entry.Path.Name,
SymlinkTarget: job.entry.Path.SymlinkTarget,
},
ModifiedLines: job.entry.ModifiedLines,
Problem: checks.Problem{
Lines: parser.LineRange{
Expand All @@ -205,8 +207,10 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte
}
case errors.As(job.entry.PathError, &commentErr):
results <- reporter.Report{
ReportedPath: job.entry.ReportedPath,
SourcePath: job.entry.SourcePath,
Path: discovery.Path{
Name: job.entry.Path.Name,
SymlinkTarget: job.entry.Path.SymlinkTarget,
},
ModifiedLines: job.entry.ModifiedLines,
Problem: checks.Problem{
Lines: parser.LineRange{
Expand All @@ -222,8 +226,10 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte
case job.entry.PathError != nil:
line, e := tryDecodingYamlError(job.entry.PathError)
results <- reporter.Report{
ReportedPath: job.entry.ReportedPath,
SourcePath: job.entry.SourcePath,
Path: discovery.Path{
Name: job.entry.Path.Name,
SymlinkTarget: job.entry.Path.SymlinkTarget,
},
ModifiedLines: job.entry.ModifiedLines,
Problem: checks.Problem{
Lines: parser.LineRange{
Expand All @@ -242,8 +248,10 @@ If this file is a template that will be rendered into valid YAML then you can in
}
case job.entry.Rule.Error.Err != nil:
results <- reporter.Report{
ReportedPath: job.entry.ReportedPath,
SourcePath: job.entry.SourcePath,
Path: discovery.Path{
Name: job.entry.Path.Name,
SymlinkTarget: job.entry.Path.SymlinkTarget,
},
ModifiedLines: job.entry.ModifiedLines,
Rule: job.entry.Rule,
Problem: checks.Problem{
Expand All @@ -263,19 +271,21 @@ This usually means that it's missing some required fields.`,
if job.entry.State == discovery.Unknown {
slog.Warn(
"Bug: unknown rule state",
slog.String("path", job.entry.ReportedPath),
slog.String("path", job.entry.Path.String()),
slog.Int("line", job.entry.Rule.Lines.First),
slog.String("name", job.entry.Rule.Name()),
)
}

start := time.Now()
problems := job.check.Check(ctx, job.entry.ReportedPath, job.entry.Rule, job.allEntries)
problems := job.check.Check(ctx, job.entry.Path, job.entry.Rule, job.allEntries)
checkDuration.WithLabelValues(job.check.Reporter()).Observe(time.Since(start).Seconds())
for _, problem := range problems {
results <- reporter.Report{
ReportedPath: job.entry.ReportedPath,
SourcePath: job.entry.SourcePath,
Path: discovery.Path{
Name: job.entry.Path.Name,
SymlinkTarget: job.entry.Path.SymlinkTarget,
},
ModifiedLines: job.entry.ModifiedLines,
Rule: job.entry.Rule,
Problem: problem,
Expand Down
4 changes: 2 additions & 2 deletions cmd/pint/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ func (c *problemCollector) scan(ctx context.Context, workers int, isOffline bool
fileOwners := map[string]string{}
for _, entry := range entries {
if entry.Owner != "" {
fileOwners[entry.ReportedPath] = entry.Owner
fileOwners[entry.Path.SymlinkTarget] = entry.Owner
}
}
c.fileOwners = fileOwners
Expand Down Expand Up @@ -377,7 +377,7 @@ func (c *problemCollector) Collect(ch chan<- prometheus.Metric) {
c.problem,
prometheus.GaugeValue,
1,
report.SourcePath,
report.Path.Name,
kind,
name,
strings.ToLower(report.Problem.Severity.String()),
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/alerts_annotation.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (c AnnotationCheck) Reporter() string {
return AnnotationCheckName
}

func (c AnnotationCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c AnnotationCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
if rule.AlertingRule == nil {
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/alerts_comparison.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (c ComparisonCheck) Reporter() string {
return ComparisonCheckName
}

func (c ComparisonCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c ComparisonCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
if rule.AlertingRule == nil {
return problems
}
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/alerts_count.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (c AlertsCheck) Reporter() string {
return AlertsCheckName
}

func (c AlertsCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c AlertsCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
if rule.AlertingRule == nil {
return problems
}
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/alerts_external_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (c AlertsExternalLabelsCheck) Reporter() string {
return AlertsExternalLabelsCheckName
}

func (c AlertsExternalLabelsCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c AlertsExternalLabelsCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
if rule.AlertingRule == nil {
return problems
}
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/alerts_for.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (c AlertsForChecksFor) Reporter() string {
return AlertForCheckName
}

func (c AlertsForChecksFor) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c AlertsForChecksFor) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
if rule.AlertingRule == nil {
return problems
}
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/alerts_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (c TemplateCheck) Reporter() string {
return TemplateCheckName
}

func (c TemplateCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c TemplateCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
if rule.AlertingRule == nil {
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ type RuleChecker interface {
String() string
Reporter() string
Meta() CheckMeta
Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) []Problem
Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) []Problem
}

type exprProblem struct {
Expand Down
10 changes: 6 additions & 4 deletions internal/checks/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func runTests(t *testing.T, testCases []checkTest) {
if tc.ctx != nil {
ctx = tc.ctx()
}
problems := tc.checker(prom).Check(ctx, entry.SourcePath, entry.Rule, tc.entries)
problems := tc.checker(prom).Check(ctx, entry.Path, entry.Rule, tc.entries)
require.Equal(t, tc.problems(uri), problems)
}

Expand All @@ -166,7 +166,7 @@ func runTests(t *testing.T, testCases []checkTest) {
require.NoError(t, err, "cannot parse rule content")
t.Run(tc.description+" (bogus rules)", func(_ *testing.T) {
for _, entry := range entries {
_ = tc.checker(newSimpleProm("prom")).Check(context.Background(), entry.SourcePath, entry.Rule, tc.entries)
_ = tc.checker(newSimpleProm("prom")).Check(context.Background(), entry.Path, entry.Rule, tc.entries)
}
})
}
Expand All @@ -181,8 +181,10 @@ func parseContent(content string) (entries []discovery.Entry, err error) {

for _, rule := range rules {
entries = append(entries, discovery.Entry{
SourcePath: "fake.yml",
ReportedPath: "fake.yml",
Path: discovery.Path{
Name: "fake.yml",
SymlinkTarget: "fake.yml",
},
ModifiedLines: rule.Lines.Expand(),
Rule: rule,
})
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/labels_conflict.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (c LabelsConflictCheck) Reporter() string {
return LabelsConflictCheckName
}

func (c LabelsConflictCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c LabelsConflictCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
var labels *parser.YamlMap
if rule.AlertingRule != nil && rule.AlertingRule.Expr.SyntaxError == nil && rule.AlertingRule.Labels != nil {
labels = rule.AlertingRule.Labels
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/promql_aggregation.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (c AggregationCheck) Reporter() string {
return AggregationCheckName
}

func (c AggregationCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c AggregationCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
expr := rule.Expr()
if expr.SyntaxError != nil {
return nil
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/promql_counter.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (c CounterCheck) Reporter() string {
return CounterCheckName
}

func (c CounterCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c CounterCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
expr := rule.Expr()

if expr.SyntaxError != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/promql_fragile.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (c FragileCheck) Reporter() string {
return FragileCheckName
}

func (c FragileCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c FragileCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
expr := rule.Expr()
if expr.SyntaxError != nil {
return nil
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/promql_range_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (c RangeQueryCheck) Reporter() string {
return RangeQueryCheckName
}

func (c RangeQueryCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c RangeQueryCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
expr := rule.Expr()

if expr.SyntaxError != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/promql_rate.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (c RateCheck) Reporter() string {
return RateCheckName
}

func (c RateCheck) Check(ctx context.Context, _ string, rule parser.Rule, entries []discovery.Entry) (problems []Problem) {
func (c RateCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, entries []discovery.Entry) (problems []Problem) {
expr := rule.Expr()

if expr.SyntaxError != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/promql_regexp.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (c RegexpCheck) Reporter() string {
return RegexpCheckName
}

func (c RegexpCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c RegexpCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
expr := rule.Expr()
if expr.SyntaxError != nil {
return nil
Expand Down
6 changes: 3 additions & 3 deletions internal/checks/promql_series.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (c SeriesCheck) Reporter() string {
return SeriesCheckName
}

func (c SeriesCheck) Check(ctx context.Context, _ string, rule parser.Rule, entries []discovery.Entry) (problems []Problem) {
func (c SeriesCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, entries []discovery.Entry) (problems []Problem) {
var settings *PromqlSeriesSettings
if s := ctx.Value(SettingsKey(c.Reporter())); s != nil {
settings = s.(*PromqlSeriesSettings)
Expand Down Expand Up @@ -163,7 +163,7 @@ func (c SeriesCheck) Check(ctx context.Context, _ string, rule parser.Rule, entr
slog.Debug(
"Metric is provided by alerting rule",
slog.String("selector", (&selector).String()),
slog.String("path", arEntry.SourcePath),
slog.String("path", arEntry.Path.Name),
)
} else {
problems = append(problems, Problem{
Expand Down Expand Up @@ -262,7 +262,7 @@ func (c SeriesCheck) Check(ctx context.Context, _ string, rule parser.Rule, entr
}
if rrEntry != nil {
// Validate recording rule instead
slog.Debug("Metric is provided by recording rule", slog.String("selector", (&bareSelector).String()), slog.String("path", rrEntry.SourcePath))
slog.Debug("Metric is provided by recording rule", slog.String("selector", (&bareSelector).String()), slog.String("path", rrEntry.Path.Name))
problems = append(problems, Problem{
Lines: expr.Value.Lines,
Reporter: c.Reporter(),
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/promql_syntax.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (c SyntaxCheck) Reporter() string {
return SyntaxCheckName
}

func (c SyntaxCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c SyntaxCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
expr := rule.Expr()
if expr.SyntaxError != nil {
problems = append(problems, Problem{
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/promql_vector_matching.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (c VectorMatchingCheck) Reporter() string {
return VectorMatchingCheckName
}

func (c VectorMatchingCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c VectorMatchingCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
expr := rule.Expr()
if expr.SyntaxError != nil {
return nil
Expand Down
2 changes: 1 addition & 1 deletion internal/checks/query_cost.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (c CostCheck) Reporter() string {
return CostCheckName
}

func (c CostCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
func (c CostCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) {
expr := rule.Expr()

if expr.SyntaxError != nil {
Expand Down
6 changes: 3 additions & 3 deletions internal/checks/rule_dependency.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (c RuleDependencyCheck) Reporter() string {
return RuleDependencyCheckName
}

func (c RuleDependencyCheck) Check(_ context.Context, _ string, rule parser.Rule, entries []discovery.Entry) (problems []Problem) {
func (c RuleDependencyCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, entries []discovery.Entry) (problems []Problem) {
var broken []*brokenDependency
var dep *brokenDependency
for _, entry := range entries {
Expand Down Expand Up @@ -130,7 +130,7 @@ func (c RuleDependencyCheck) usesVector(entry discovery.Entry, name string) *bro
return &brokenDependency{
kind: "recording",
metric: name,
path: entry.ReportedPath,
path: entry.Path.SymlinkTarget,
line: expr.Value.Lines.First,
name: entry.Rule.Name(),
}
Expand All @@ -155,7 +155,7 @@ func (c RuleDependencyCheck) usesAlert(entry discovery.Entry, name string) *brok
return &brokenDependency{
kind: "alerting",
metric: vs.String(),
path: entry.ReportedPath,
path: entry.Path.SymlinkTarget,
line: expr.Value.Lines.First,
name: entry.Rule.Name(),
}
Expand Down
Loading

0 comments on commit d72401d

Please sign in to comment.