From c9ef1694771108840f2f1882bfce69ee988b7c48 Mon Sep 17 00:00:00 2001 From: Lukasz Mierzwa Date: Mon, 25 Jul 2022 18:27:42 +0100 Subject: [PATCH] Add more details to bitbucket CI reports --- cmd/pint/scan.go | 21 ++++-- docs/changelog.md | 6 ++ go.mod | 2 +- internal/checks/alerts_annotation.go | 4 ++ internal/checks/alerts_comparison.go | 4 ++ internal/checks/alerts_count.go | 4 ++ internal/checks/alerts_for.go | 4 ++ internal/checks/alerts_template.go | 4 ++ internal/checks/base.go | 5 ++ internal/checks/promql_aggregation.go | 4 ++ internal/checks/promql_fragile.go | 4 ++ internal/checks/promql_range_query.go | 4 ++ internal/checks/promql_rate.go | 4 ++ internal/checks/promql_regexp.go | 4 ++ internal/checks/promql_series.go | 4 ++ internal/checks/promql_syntax.go | 4 ++ internal/checks/promql_vector_matching.go | 4 ++ internal/checks/query_cost.go | 4 ++ internal/checks/rule_label.go | 4 ++ internal/checks/rule_reject.go | 4 ++ internal/reporter/bitbucket.go | 78 ++++++++++++++++------- internal/reporter/bitbucket_test.go | 56 +++++++++++++--- internal/reporter/reporter.go | 8 ++- 23 files changed, 202 insertions(+), 38 deletions(-) diff --git a/cmd/pint/scan.go b/cmd/pint/scan.go index 5e5d9f19..5d01f5ac 100644 --- a/cmd/pint/scan.go +++ b/cmd/pint/scan.go @@ -10,6 +10,7 @@ import ( "github.com/prometheus/prometheus/model/rulefmt" "github.com/rs/zerolog/log" + "go.uber.org/atomic" "github.com/cloudflare/pint/internal/checks" "github.com/cloudflare/pint/internal/config" @@ -82,9 +83,11 @@ func checkRules(ctx context.Context, workers int, cfg config.Config, entries []d wg.Wait() }() + var onlineChecksCount, offlineChecksCount atomic.Int64 go func() { for _, entry := range entries { - if entry.PathError == nil && entry.Rule.Error.Err == nil { + switch { + case entry.PathError == nil && entry.Rule.Error.Err == nil: if entry.Rule.RecordingRule != nil { rulesParsedTotal.WithLabelValues(config.RecordingRuleType).Inc() log.Debug(). @@ -105,9 +108,15 @@ func checkRules(ctx context.Context, workers int, cfg config.Config, entries []d checkList := cfg.GetChecksForRule(ctx, entry.Path, entry.Rule) for _, check := range checkList { check := check + if check.Meta().IsOnline { + onlineChecksCount.Inc() + } else { + offlineChecksCount.Inc() + } jobs <- scanJob{entry: entry, allEntries: entries, check: check} + checkIterationChecksDone.Inc() } - } else { + default: if entry.Rule.Error.Err != nil { log.Debug(). Str("path", entry.Path). @@ -115,10 +124,8 @@ func checkRules(ctx context.Context, workers int, cfg config.Config, entries []d Msg("Found invalid rule") rulesParsedTotal.WithLabelValues(config.InvalidRuleType).Inc() } - jobs <- scanJob{entry: entry, allEntries: entries, check: nil} } - checkIterationChecksDone.Inc() } defer close(jobs) }() @@ -126,10 +133,14 @@ func checkRules(ctx context.Context, workers int, cfg config.Config, entries []d for result := range results { summary.Reports = append(summary.Reports, result) } + summary.Duration = time.Since(start) + summary.Entries = len(entries) + summary.OnlineChecks = onlineChecksCount.Load() + summary.OfflineChecks = offlineChecksCount.Load() lastRunTime.SetToCurrentTime() - return + return summary } type scanJob struct { diff --git a/docs/changelog.md b/docs/changelog.md index f1a3bc23..4b8395c5 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,5 +1,11 @@ # Changelog +## v0.27.0 + +### Changed + +- Add more details to BitBucket CI reports. + ## v0.26.0 ### Fixed diff --git a/go.mod b/go.mod index d11c9ca3..d543fd42 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( github.com/rs/zerolog v1.27.0 github.com/stretchr/testify v1.8.0 github.com/urfave/cli/v2 v2.11.1 + go.uber.org/atomic v1.9.0 go.uber.org/ratelimit v0.2.0 golang.org/x/oauth2 v0.0.0-20220628200809-02e64fa58f26 gopkg.in/yaml.v3 v3.0.1 @@ -68,7 +69,6 @@ require ( github.com/zclconf/go-cty v1.10.0 // indirect go.opentelemetry.io/otel v1.7.0 // indirect go.opentelemetry.io/otel/trace v1.7.0 // indirect - go.uber.org/atomic v1.9.0 // indirect go.uber.org/goleak v1.1.12 // indirect golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e // indirect golang.org/x/net v0.0.0-20220624214902-1bab6f366d9e // indirect diff --git a/internal/checks/alerts_annotation.go b/internal/checks/alerts_annotation.go index 8ffafd23..ba91cc4d 100644 --- a/internal/checks/alerts_annotation.go +++ b/internal/checks/alerts_annotation.go @@ -23,6 +23,10 @@ type AnnotationCheck struct { severity Severity } +func (c AnnotationCheck) Meta() CheckMeta { + return CheckMeta{IsOnline: false} +} + func (c AnnotationCheck) String() string { if c.valueRe != nil { return fmt.Sprintf("%s(%s=~%s:%v)", AnnotationCheckName, c.key, c.valueRe.anchored, c.isReguired) diff --git a/internal/checks/alerts_comparison.go b/internal/checks/alerts_comparison.go index 23f6e346..eec7aae4 100644 --- a/internal/checks/alerts_comparison.go +++ b/internal/checks/alerts_comparison.go @@ -19,6 +19,10 @@ func NewComparisonCheck() ComparisonCheck { type ComparisonCheck struct{} +func (c ComparisonCheck) Meta() CheckMeta { + return CheckMeta{IsOnline: false} +} + func (c ComparisonCheck) String() string { return ComparisonCheckName } diff --git a/internal/checks/alerts_count.go b/internal/checks/alerts_count.go index e0066681..95bd5902 100644 --- a/internal/checks/alerts_count.go +++ b/internal/checks/alerts_count.go @@ -32,6 +32,10 @@ type AlertsCheck struct { resolve time.Duration } +func (c AlertsCheck) Meta() CheckMeta { + return CheckMeta{IsOnline: true} +} + func (c AlertsCheck) String() string { return fmt.Sprintf("%s(%s)", AlertsCheckName, c.prom.Name()) } diff --git a/internal/checks/alerts_for.go b/internal/checks/alerts_for.go index ad8a31d1..b245e324 100644 --- a/internal/checks/alerts_for.go +++ b/internal/checks/alerts_for.go @@ -20,6 +20,10 @@ func NewAlertsForCheck() AlertsForChecksFor { type AlertsForChecksFor struct{} +func (c AlertsForChecksFor) Meta() CheckMeta { + return CheckMeta{IsOnline: false} +} + func (c AlertsForChecksFor) String() string { return AlertForCheckName } diff --git a/internal/checks/alerts_template.go b/internal/checks/alerts_template.go index 43f23b00..a963636c 100644 --- a/internal/checks/alerts_template.go +++ b/internal/checks/alerts_template.go @@ -75,6 +75,10 @@ func NewTemplateCheck() TemplateCheck { type TemplateCheck struct{} +func (c TemplateCheck) Meta() CheckMeta { + return CheckMeta{IsOnline: false} +} + func (c TemplateCheck) String() string { return TemplateCheckName } diff --git a/internal/checks/base.go b/internal/checks/base.go index 5b0968f5..7069465c 100644 --- a/internal/checks/base.go +++ b/internal/checks/base.go @@ -99,9 +99,14 @@ func (p Problem) LineRange() (int, int) { return p.Lines[0], p.Lines[len(p.Lines)-1] } +type CheckMeta struct { + IsOnline bool +} + type RuleChecker interface { String() string Reporter() string + Meta() CheckMeta Check(ctx context.Context, rule parser.Rule, entries []discovery.Entry) []Problem } diff --git a/internal/checks/promql_aggregation.go b/internal/checks/promql_aggregation.go index b69db1b1..99ad940e 100644 --- a/internal/checks/promql_aggregation.go +++ b/internal/checks/promql_aggregation.go @@ -27,6 +27,10 @@ type AggregationCheck struct { severity Severity } +func (c AggregationCheck) Meta() CheckMeta { + return CheckMeta{IsOnline: false} +} + func (c AggregationCheck) String() string { return fmt.Sprintf("%s(%s:%v)", AggregationCheckName, c.label, c.keep) } diff --git a/internal/checks/promql_fragile.go b/internal/checks/promql_fragile.go index 7d09c4ed..4b0c6bec 100644 --- a/internal/checks/promql_fragile.go +++ b/internal/checks/promql_fragile.go @@ -20,6 +20,10 @@ func NewFragileCheck() FragileCheck { type FragileCheck struct{} +func (c FragileCheck) Meta() CheckMeta { + return CheckMeta{IsOnline: false} +} + func (c FragileCheck) String() string { return FragileCheckName } diff --git a/internal/checks/promql_range_query.go b/internal/checks/promql_range_query.go index 6b404c65..3d58ce32 100644 --- a/internal/checks/promql_range_query.go +++ b/internal/checks/promql_range_query.go @@ -25,6 +25,10 @@ type RangeQueryCheck struct { prom *promapi.FailoverGroup } +func (c RangeQueryCheck) Meta() CheckMeta { + return CheckMeta{IsOnline: true} +} + func (c RangeQueryCheck) String() string { return fmt.Sprintf("%s(%s)", RangeQueryCheckName, c.prom.Name()) } diff --git a/internal/checks/promql_rate.go b/internal/checks/promql_rate.go index 4289e70d..bdb2e237 100644 --- a/internal/checks/promql_rate.go +++ b/internal/checks/promql_rate.go @@ -26,6 +26,10 @@ type RateCheck struct { prom *promapi.FailoverGroup } +func (c RateCheck) Meta() CheckMeta { + return CheckMeta{IsOnline: true} +} + func (c RateCheck) String() string { return fmt.Sprintf("%s(%s)", RateCheckName, c.prom.Name()) } diff --git a/internal/checks/promql_regexp.go b/internal/checks/promql_regexp.go index aec1e5cf..caa347a1 100644 --- a/internal/checks/promql_regexp.go +++ b/internal/checks/promql_regexp.go @@ -21,6 +21,10 @@ func NewRegexpCheck() RegexpCheck { type RegexpCheck struct{} +func (c RegexpCheck) Meta() CheckMeta { + return CheckMeta{IsOnline: false} +} + func (c RegexpCheck) String() string { return RegexpCheckName } diff --git a/internal/checks/promql_series.go b/internal/checks/promql_series.go index 6f5b7ee0..91ead63e 100644 --- a/internal/checks/promql_series.go +++ b/internal/checks/promql_series.go @@ -43,6 +43,10 @@ func NewSeriesCheck(prom *promapi.FailoverGroup) SeriesCheck { return SeriesCheck{prom: prom} } +func (c SeriesCheck) Meta() CheckMeta { + return CheckMeta{IsOnline: true} +} + type SeriesCheck struct { prom *promapi.FailoverGroup } diff --git a/internal/checks/promql_syntax.go b/internal/checks/promql_syntax.go index f79e601b..58bed88c 100644 --- a/internal/checks/promql_syntax.go +++ b/internal/checks/promql_syntax.go @@ -18,6 +18,10 @@ func NewSyntaxCheck() SyntaxCheck { type SyntaxCheck struct{} +func (c SyntaxCheck) Meta() CheckMeta { + return CheckMeta{IsOnline: false} +} + func (c SyntaxCheck) String() string { return SyntaxCheckName } diff --git a/internal/checks/promql_vector_matching.go b/internal/checks/promql_vector_matching.go index fa43126b..0043caac 100644 --- a/internal/checks/promql_vector_matching.go +++ b/internal/checks/promql_vector_matching.go @@ -29,6 +29,10 @@ type VectorMatchingCheck struct { prom *promapi.FailoverGroup } +func (c VectorMatchingCheck) Meta() CheckMeta { + return CheckMeta{IsOnline: true} +} + func (c VectorMatchingCheck) String() string { return fmt.Sprintf("%s(%s)", VectorMatchingCheckName, c.prom.Name()) } diff --git a/internal/checks/query_cost.go b/internal/checks/query_cost.go index 3f1e98b0..3293589b 100644 --- a/internal/checks/query_cost.go +++ b/internal/checks/query_cost.go @@ -30,6 +30,10 @@ type CostCheck struct { severity Severity } +func (c CostCheck) Meta() CheckMeta { + return CheckMeta{IsOnline: true} +} + func (c CostCheck) String() string { if c.maxSeries > 0 { return fmt.Sprintf("%s(%s:%d)", CostCheckName, c.prom.Name(), c.maxSeries) diff --git a/internal/checks/rule_label.go b/internal/checks/rule_label.go index f1d2ea8f..5fc473ad 100644 --- a/internal/checks/rule_label.go +++ b/internal/checks/rule_label.go @@ -23,6 +23,10 @@ type LabelCheck struct { severity Severity } +func (c LabelCheck) Meta() CheckMeta { + return CheckMeta{IsOnline: false} +} + func (c LabelCheck) String() string { return fmt.Sprintf("%s(%s:%v)", LabelCheckName, c.key, c.isReguired) } diff --git a/internal/checks/rule_reject.go b/internal/checks/rule_reject.go index 9d56ee73..5a2c0e24 100644 --- a/internal/checks/rule_reject.go +++ b/internal/checks/rule_reject.go @@ -25,6 +25,10 @@ type Reject struct { severity Severity } +func (c Reject) Meta() CheckMeta { + return CheckMeta{IsOnline: false} +} + func (c Reject) String() string { r := []string{} if c.keyRe != nil { diff --git a/internal/reporter/bitbucket.go b/internal/reporter/bitbucket.go index 1c95a5ea..84cfa9d0 100644 --- a/internal/reporter/bitbucket.go +++ b/internal/reporter/bitbucket.go @@ -15,9 +15,37 @@ import ( "github.com/rs/zerolog/log" ) +const ( + BitBucketDescription = "pint is a Prometheus rule linter/validator.\n" + + "It will inspect all Prometheus recording and alerting rules for problems that could prevent these from working correctly.\n" + + "Checks can be either offline (static checks using only rule definition) or online (validate rule against live Prometheus server)." +) + type BitBucketReport struct { - Title string `json:"title"` - Result string `json:"result"` + Reporter string `json:"reporter"` + Title string `json:"title"` + Result string `json:"result"` + Details string `json:"details"` + Link string `json:"link"` + Data []BitBucketReportData `json:"data"` +} + +type DataType string + +const ( + BooleanType DataType = "BOOLEAN" + DateType DataType = "DATA" + DurationType DataType = "DURATION" + LinkType DataType = "LINK" + NumberType DataType = "NUMBER" + PercentageType DataType = "PERCENTAGE" + TextType DataType = "TEXT" +) + +type BitBucketReportData struct { + Title string `json:"title"` + Type DataType `json:"type"` + Value any `json:"value"` } type BitBucketAnnotation struct { @@ -82,7 +110,7 @@ func (r BitBucketReporter) Submit(summary Summary) (err error) { } } - if err = r.postReport(headCommit, isPassing, annotations); err != nil { + if err = r.postReport(headCommit, isPassing, annotations, summary); err != nil { return err } @@ -163,21 +191,6 @@ func (r BitBucketReporter) bitBucketRequest(method, url string, body []byte) err return nil } -func (r BitBucketReporter) createReport(commit string, isPassing bool) error { - result := "PASS" - if !isPassing { - result = "FAIL" - } - payload, _ := json.Marshal(BitBucketReport{ - Title: fmt.Sprintf("Pint - Prometheus rules linter (version: %s)", r.version), - Result: result, - }) - - url := fmt.Sprintf("%s/rest/insights/1.0/projects/%s/repos/%s/commits/%s/reports/pint", - r.uri, r.project, r.repo, commit) - return r.bitBucketRequest(http.MethodPut, url, payload) -} - func (r BitBucketReporter) createAnnotations(commit string, annotations []BitBucketAnnotation) error { payload, _ := json.Marshal(BitBucketAnnotations{Annotations: annotations}) url := fmt.Sprintf("%s/rest/insights/1.0/projects/%s/repos/%s/commits/%s/reports/pint/annotations", @@ -191,16 +204,35 @@ func (r BitBucketReporter) deleteAnnotations(commit string) error { return r.bitBucketRequest(http.MethodDelete, url, nil) } -func (r BitBucketReporter) postReport(commit string, isPassing bool, annotations []BitBucketAnnotation) error { - err := r.createReport(commit, isPassing) - if err != nil { +func (r BitBucketReporter) postReport(commit string, isPassing bool, annotations []BitBucketAnnotation, summary Summary) error { + result := "PASS" + if !isPassing { + result = "FAIL" + } + payload, _ := json.Marshal(BitBucketReport{ + Title: fmt.Sprintf("pint %s", r.version), + Result: result, + Reporter: "Prometheus rule linter", + Details: BitBucketDescription, + Link: "https://cloudflare.github.io/pint/", + Data: []BitBucketReportData{ + {Title: "Number of rules checked", Type: NumberType, Value: summary.Entries}, + {Title: "Number of problems found", Type: NumberType, Value: len(annotations)}, + {Title: "Number of offline checks", Type: NumberType, Value: summary.OfflineChecks}, + {Title: "Number of online checks", Type: NumberType, Value: summary.OnlineChecks}, + {Title: "Checks duration", Type: DurationType, Value: summary.Duration.Milliseconds()}, + }, + }) + + url := fmt.Sprintf("%s/rest/insights/1.0/projects/%s/repos/%s/commits/%s/reports/pint", + r.uri, r.project, r.repo, commit) + if err := r.bitBucketRequest(http.MethodPut, url, payload); err != nil { return fmt.Errorf("failed to create BitBucket report: %w", err) } // Try to delete annotations when that happens so we don't end up with stale data if we run // pint twice, first with problems found, and second without any. - err = r.deleteAnnotations(commit) - if err != nil { + if err := r.deleteAnnotations(commit); err != nil { return err } diff --git a/internal/reporter/bitbucket_test.go b/internal/reporter/bitbucket_test.go index 005dcb87..0085671a 100644 --- a/internal/reporter/bitbucket_test.go +++ b/internal/reporter/bitbucket_test.go @@ -266,8 +266,18 @@ func TestBitBucketReporter(t *testing.T) { }, }, report: reporter.BitBucketReport{ - Title: "Pint - Prometheus rules linter (version: v0.0.0)", - Result: "FAIL", + Reporter: "Prometheus rule linter", + Title: "pint v0.0.0", + Details: reporter.BitBucketDescription, + Link: "https://cloudflare.github.io/pint/", + Result: "FAIL", + Data: []reporter.BitBucketReportData{ + {Title: "Number of rules checked", Type: reporter.NumberType, Value: float64(0)}, + {Title: "Number of problems found", Type: reporter.NumberType, Value: float64(3)}, + {Title: "Number of offline checks", Type: reporter.NumberType, Value: float64(0)}, + {Title: "Number of online checks", Type: reporter.NumberType, Value: float64(0)}, + {Title: "Checks duration", Type: reporter.DurationType, Value: float64(0)}, + }, }, annotations: reporter.BitBucketAnnotations{ Annotations: []reporter.BitBucketAnnotation{ @@ -337,8 +347,18 @@ func TestBitBucketReporter(t *testing.T) { }, }, report: reporter.BitBucketReport{ - Title: "Pint - Prometheus rules linter (version: v0.0.0)", - Result: "FAIL", + Reporter: "Prometheus rule linter", + Title: "pint v0.0.0", + Details: reporter.BitBucketDescription, + Link: "https://cloudflare.github.io/pint/", + Result: "FAIL", + Data: []reporter.BitBucketReportData{ + {Title: "Number of rules checked", Type: reporter.NumberType, Value: float64(0)}, + {Title: "Number of problems found", Type: reporter.NumberType, Value: float64(1)}, + {Title: "Number of offline checks", Type: reporter.NumberType, Value: float64(0)}, + {Title: "Number of online checks", Type: reporter.NumberType, Value: float64(0)}, + {Title: "Checks duration", Type: reporter.DurationType, Value: float64(0)}, + }, }, annotations: reporter.BitBucketAnnotations{ Annotations: []reporter.BitBucketAnnotation{ @@ -373,8 +393,18 @@ func TestBitBucketReporter(t *testing.T) { }, summary: reporter.Summary{}, report: reporter.BitBucketReport{ - Title: "Pint - Prometheus rules linter (version: v0.0.0)", - Result: "PASS", + Reporter: "Prometheus rule linter", + Title: "pint v0.0.0", + Details: reporter.BitBucketDescription, + Link: "https://cloudflare.github.io/pint/", + Result: "PASS", + Data: []reporter.BitBucketReportData{ + {Title: "Number of rules checked", Type: reporter.NumberType, Value: float64(0)}, + {Title: "Number of problems found", Type: reporter.NumberType, Value: float64(0)}, + {Title: "Number of offline checks", Type: reporter.NumberType, Value: float64(0)}, + {Title: "Number of online checks", Type: reporter.NumberType, Value: float64(0)}, + {Title: "Checks duration", Type: reporter.DurationType, Value: float64(0)}, + }, }, errorHandler: func(err error) error { if err != nil { @@ -455,8 +485,18 @@ func TestBitBucketReporter(t *testing.T) { }, }, report: reporter.BitBucketReport{ - Title: "Pint - Prometheus rules linter (version: v0.0.0)", - Result: "PASS", + Reporter: "Prometheus rule linter", + Title: "pint v0.0.0", + Details: reporter.BitBucketDescription, + Link: "https://cloudflare.github.io/pint/", + Result: "PASS", + Data: []reporter.BitBucketReportData{ + {Title: "Number of rules checked", Type: reporter.NumberType, Value: float64(0)}, + {Title: "Number of problems found", Type: reporter.NumberType, Value: float64(0)}, + {Title: "Number of offline checks", Type: reporter.NumberType, Value: float64(0)}, + {Title: "Number of online checks", Type: reporter.NumberType, Value: float64(0)}, + {Title: "Checks duration", Type: reporter.DurationType, Value: float64(0)}, + }, }, annotations: reporter.BitBucketAnnotations{ Annotations: []reporter.BitBucketAnnotation{ diff --git a/internal/reporter/reporter.go b/internal/reporter/reporter.go index 82974612..f57d9457 100644 --- a/internal/reporter/reporter.go +++ b/internal/reporter/reporter.go @@ -1,6 +1,8 @@ package reporter import ( + "time" + "github.com/cloudflare/pint/internal/checks" "github.com/cloudflare/pint/internal/git" "github.com/cloudflare/pint/internal/parser" @@ -15,7 +17,11 @@ type Report struct { } type Summary struct { - Reports []Report + OfflineChecks int64 + OnlineChecks int64 + Duration time.Duration + Entries int + Reports []Report } func (s Summary) HasFatalProblems() bool {