diff --git a/cmd/pint/ci.go b/cmd/pint/ci.go
index 1c7edf24..800ceca0 100644
--- a/cmd/pint/ci.go
+++ b/cmd/pint/ci.go
@@ -317,7 +317,7 @@ func detectGithubActions(gh *config.GitHub) *config.GitHub {
if gh == nil {
isNil = true
- gh = &config.GitHub{Timeout: time.Minute.String()} // nolint:exhaustruct
+ gh = &config.GitHub{Timeout: time.Minute.String()} // nolint: exhaustruct
}
if repo := os.Getenv("GITHUB_REPOSITORY"); repo != "" {
diff --git a/cmd/pint/scan.go b/cmd/pint/scan.go
index 1a655bb9..c3fb9207 100644
--- a/cmd/pint/scan.go
+++ b/cmd/pint/scan.go
@@ -123,6 +123,22 @@ func checkRules(ctx context.Context, workers int, isOffline bool, gen *config.Pr
summary.OnlineChecks = onlineChecksCount.Load()
summary.OfflineChecks = offlineChecksCount.Load()
+ for _, prom := range gen.Servers() {
+ for api, names := range prom.GetDisabledChecks() {
+ summary.MarkCheckDisabled(prom.Name(), api, names)
+ }
+ }
+ for _, pd := range summary.GetPrometheusDetails() {
+ for _, dc := range pd.DisabledChecks {
+ slog.Warn(
+ "Some checks were disabled because configured server doesn't seem to support all Prometheus APIs",
+ slog.String("prometheus", pd.Name),
+ slog.String("api", dc.API),
+ slog.Any("checks", dc.Checks),
+ )
+ }
+ }
+
lastRunTime.SetToCurrentTime()
return summary, nil
diff --git a/cmd/pint/watch.go b/cmd/pint/watch.go
index 73533207..1342cd51 100644
--- a/cmd/pint/watch.go
+++ b/cmd/pint/watch.go
@@ -283,7 +283,7 @@ type problemCollector struct {
}
func newProblemCollector(cfg config.Config, f pathFinderFunc, minSeverity checks.Severity, maxProblems int) *problemCollector {
- return &problemCollector{ // nolint:exhaustruct
+ return &problemCollector{ // nolint: exhaustruct
finder: f,
cfg: cfg,
fileOwners: map[string]string{},
diff --git a/docs/changelog.md b/docs/changelog.md
index 788ee915..c282d433 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -8,8 +8,11 @@
### Changed
-- Report warnings instead of errors if Prometheus server used for checks doesn't support these API
- endpoints:
+- When pint runs online checks against Thanos or other service that implements some, but not all, Prometheus APIs
+ it would reports errors due to some API requests returning `404 Not Found` HTTP errors.
+ Now pint will automatically disable checks that rely on Prometheus API endpoints that are not supported
+ and create a summary comment (when running `pint ci`) that lists disabled checks.
+ This applies to these API endpoints:
- `/api/v1/status/config`
- `/api/v1/status/flags`
diff --git a/internal/checks/alerts_absent.go b/internal/checks/alerts_absent.go
index bb7591a0..39d978c3 100644
--- a/internal/checks/alerts_absent.go
+++ b/internal/checks/alerts_absent.go
@@ -2,6 +2,7 @@ package checks
import (
"context"
+ "errors"
"fmt"
"time"
@@ -72,6 +73,10 @@ func (c AlertsAbsentCheck) Check(ctx context.Context, _ discovery.Path, rule par
cfg, err := c.prom.Config(ctx, 0)
if err != nil {
+ if errors.Is(err, promapi.ErrUnsupported) {
+ c.prom.DisableCheck(promapi.APIPathConfig, c.Reporter())
+ return problems
+ }
text, severity := textAndSeverityFromError(err, c.Reporter(), c.prom.Name(), Warning)
problems = append(problems, Problem{
Lines: rule.AlertingRule.Expr.Value.Lines,
diff --git a/internal/checks/alerts_absent_test.go b/internal/checks/alerts_absent_test.go
index c8f1b1c8..2f172bf7 100644
--- a/internal/checks/alerts_absent_test.go
+++ b/internal/checks/alerts_absent_test.go
@@ -274,19 +274,7 @@ func TestAlertsAbsentCheck(t *testing.T) {
content: "- alert: foo\n expr: absent(foo)\n",
checker: newAlertsAbsentCheck,
prometheus: newSimpleProm,
- problems: func(s string) []checks.Problem {
- return []checks.Problem{
- {
- Lines: parser.LineRange{
- First: 2,
- Last: 2,
- },
- Reporter: checks.AlertsAbsentCheckName,
- Text: checkUnsupported(checks.AlertsAbsentCheckName, "prom", s, promapi.APIPathConfig),
- Severity: checks.Warning,
- },
- }
- },
+ problems: noProblems,
mocks: []*prometheusMock{
{
conds: []requestCondition{requireConfigPath},
diff --git a/internal/checks/alerts_external_labels.go b/internal/checks/alerts_external_labels.go
index 2cfe0201..6bbcfed6 100644
--- a/internal/checks/alerts_external_labels.go
+++ b/internal/checks/alerts_external_labels.go
@@ -2,6 +2,7 @@ package checks
import (
"context"
+ "errors"
"fmt"
"github.com/cloudflare/pint/internal/discovery"
@@ -55,6 +56,10 @@ func (c AlertsExternalLabelsCheck) Check(ctx context.Context, _ discovery.Path,
cfg, err := c.prom.Config(ctx, 0)
if err != nil {
+ if errors.Is(err, promapi.ErrUnsupported) {
+ c.prom.DisableCheck(promapi.APIPathConfig, c.Reporter())
+ return problems
+ }
text, severity := textAndSeverityFromError(err, c.Reporter(), c.prom.Name(), Bug)
problems = append(problems, Problem{
Lines: rule.Lines,
diff --git a/internal/checks/alerts_external_labels_test.go b/internal/checks/alerts_external_labels_test.go
index 39fa48fa..01337a8e 100644
--- a/internal/checks/alerts_external_labels_test.go
+++ b/internal/checks/alerts_external_labels_test.go
@@ -2,6 +2,7 @@ package checks_test
import (
"fmt"
+ "net/http"
"testing"
"time"
@@ -155,6 +156,19 @@ func TestAlertsExternalLabelsCountCheck(t *testing.T) {
},
},
},
+ {
+ description: "config 404",
+ content: content,
+ checker: newAlertsExternalLabelsCheck,
+ prometheus: newSimpleProm,
+ problems: noProblems,
+ mocks: []*prometheusMock{
+ {
+ conds: []requestCondition{requireConfigPath},
+ resp: httpResponse{code: http.StatusNotFound, body: "Not Found"},
+ },
+ },
+ },
}
runTests(t, testCases)
diff --git a/internal/checks/base.go b/internal/checks/base.go
index 5ecd9fee..c01515e3 100644
--- a/internal/checks/base.go
+++ b/internal/checks/base.go
@@ -148,9 +148,6 @@ func textAndSeverityFromError(err error, reporter, prom string, s Severity) (tex
}
switch {
- case promapi.IsUnsupportedError(err):
- text = fmt.Sprintf("Couldn't run `%s` checks on %s because it's %s.", reporter, promDesc, err)
- severity = Warning
case promapi.IsQueryTooExpensive(err):
text = fmt.Sprintf("Couldn't run `%s` checks on %s because some queries are too expensive: `%s`.", reporter, promDesc, err)
severity = Warning
diff --git a/internal/checks/base_test.go b/internal/checks/base_test.go
index c54da4b0..ca5f8b91 100644
--- a/internal/checks/base_test.go
+++ b/internal/checks/base_test.go
@@ -596,7 +596,3 @@ func checkErrorUnableToRun(c, name, uri, err string) string {
func checkErrorTooExpensiveToRun(c, name, uri, err string) string {
return fmt.Sprintf("Couldn't run `%s` checks on `%s` Prometheus server at %s because some queries are too expensive: `%s`.", c, name, uri, err)
}
-
-func checkUnsupported(c, name, uri, path string) string {
- return fmt.Sprintf("Couldn't run `%s` checks on `%s` Prometheus server at %s because it's unsupported: this server doesn't seem to support `%s` API endpoint.", c, name, uri, path)
-}
diff --git a/internal/checks/labels_conflict.go b/internal/checks/labels_conflict.go
index fe78c7e6..e51a7f3e 100644
--- a/internal/checks/labels_conflict.go
+++ b/internal/checks/labels_conflict.go
@@ -2,6 +2,7 @@ package checks
import (
"context"
+ "errors"
"fmt"
"github.com/cloudflare/pint/internal/discovery"
@@ -56,6 +57,10 @@ func (c LabelsConflictCheck) Check(ctx context.Context, _ discovery.Path, rule p
cfg, err := c.prom.Config(ctx, 0)
if err != nil {
+ if errors.Is(err, promapi.ErrUnsupported) {
+ c.prom.DisableCheck(promapi.APIPathConfig, c.Reporter())
+ return problems
+ }
text, severity := textAndSeverityFromError(err, c.Reporter(), c.prom.Name(), Warning)
problems = append(problems, Problem{
Lines: labels.Lines,
diff --git a/internal/checks/labels_conflict_test.go b/internal/checks/labels_conflict_test.go
index 9ab361cb..555a61cb 100644
--- a/internal/checks/labels_conflict_test.go
+++ b/internal/checks/labels_conflict_test.go
@@ -183,19 +183,7 @@ func TestLabelsConflictCheck(t *testing.T) {
content: "- record: foo\n expr: up == 0\n labels:\n foo: bar\n",
checker: newLabelsConflict,
prometheus: newSimpleProm,
- problems: func(uri string) []checks.Problem {
- return []checks.Problem{
- {
- Lines: parser.LineRange{
- First: 3,
- Last: 4,
- },
- Reporter: checks.LabelsConflictCheckName,
- Text: checkUnsupported(checks.LabelsConflictCheckName, "prom", uri, promapi.APIPathConfig),
- Severity: checks.Warning,
- },
- }
- },
+ problems: noProblems,
mocks: []*prometheusMock{
{
conds: []requestCondition{requireConfigPath},
diff --git a/internal/checks/promql_counter.go b/internal/checks/promql_counter.go
index ebc886a9..b60489e1 100644
--- a/internal/checks/promql_counter.go
+++ b/internal/checks/promql_counter.go
@@ -2,6 +2,7 @@ package checks
import (
"context"
+ "errors"
"fmt"
v1 "github.com/prometheus/client_golang/api/prometheus/v1"
@@ -99,6 +100,10 @@ LOOP:
metadata, err := c.prom.Metadata(ctx, selector.Name)
if err != nil {
+ if errors.Is(err, promapi.ErrUnsupported) {
+ c.prom.DisableCheck(promapi.APIPathMetadata, c.Reporter())
+ return problems
+ }
text, severity := textAndSeverityFromError(err, c.Reporter(), c.prom.Name(), Warning)
problems = append(problems, Problem{
Lines: expr.Value.Lines,
diff --git a/internal/checks/promql_range_query.go b/internal/checks/promql_range_query.go
index 8ace14f4..bfa985dc 100644
--- a/internal/checks/promql_range_query.go
+++ b/internal/checks/promql_range_query.go
@@ -2,6 +2,7 @@ package checks
import (
"context"
+ "errors"
"fmt"
"time"
@@ -77,6 +78,10 @@ func (c RangeQueryCheck) Check(ctx context.Context, _ discovery.Path, rule parse
flags, err := c.prom.Flags(ctx)
if err != nil {
+ if errors.Is(err, promapi.ErrUnsupported) {
+ c.prom.DisableCheck(promapi.APIPathFlags, c.Reporter())
+ return problems
+ }
text, severity := textAndSeverityFromError(err, c.Reporter(), c.prom.Name(), Warning)
problems = append(problems, Problem{
Lines: expr.Value.Lines,
diff --git a/internal/checks/promql_range_query_test.go b/internal/checks/promql_range_query_test.go
index 793b9acc..d0bc5661 100644
--- a/internal/checks/promql_range_query_test.go
+++ b/internal/checks/promql_range_query_test.go
@@ -99,19 +99,7 @@ func TestRangeQueryCheck(t *testing.T) {
content: "- record: foo\n expr: rate(foo[30d])\n",
checker: newRangeQueryCheck,
prometheus: newSimpleProm,
- problems: func(uri string) []checks.Problem {
- return []checks.Problem{
- {
- Lines: parser.LineRange{
- First: 2,
- Last: 2,
- },
- Reporter: "promql/range_query",
- Text: checkUnsupported(checks.RangeQueryCheckName, "prom", uri, promapi.APIPathFlags),
- Severity: checks.Warning,
- },
- }
- },
+ problems: noProblems,
mocks: []*prometheusMock{
{
conds: []requestCondition{requireFlagsPath},
diff --git a/internal/checks/promql_rate.go b/internal/checks/promql_rate.go
index 0511c009..2f146acf 100644
--- a/internal/checks/promql_rate.go
+++ b/internal/checks/promql_rate.go
@@ -2,6 +2,7 @@ package checks
import (
"context"
+ "errors"
"fmt"
"slices"
"time"
@@ -69,6 +70,10 @@ func (c RateCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule
cfg, err := c.prom.Config(ctx, 0)
if err != nil {
+ if errors.Is(err, promapi.ErrUnsupported) {
+ c.prom.DisableCheck(promapi.APIPathConfig, c.Reporter())
+ return problems
+ }
text, severity := textAndSeverityFromError(err, c.Reporter(), c.prom.Name(), Bug)
problems = append(problems, Problem{
Lines: expr.Value.Lines,
@@ -119,6 +124,9 @@ func (c RateCheck) checkNode(ctx context.Context, node *parser.PromQLNode, entri
done.values = append(done.values, s.Name)
metadata, err := c.prom.Metadata(ctx, s.Name)
if err != nil {
+ if errors.Is(err, promapi.ErrUnsupported) {
+ continue
+ }
text, severity := textAndSeverityFromError(err, c.Reporter(), c.prom.Name(), Bug)
problems = append(problems, exprProblem{
text: text,
@@ -152,6 +160,9 @@ func (c RateCheck) checkNode(ctx context.Context, node *parser.PromQLNode, entri
for _, vs := range src.Selectors {
metadata, err := c.prom.Metadata(ctx, vs.Name)
if err != nil {
+ if errors.Is(err, promapi.ErrUnsupported) {
+ continue
+ }
text, severity := textAndSeverityFromError(err, c.Reporter(), c.prom.Name(), Bug)
problems = append(problems, exprProblem{
text: text,
diff --git a/internal/checks/promql_rate_test.go b/internal/checks/promql_rate_test.go
index 61d31abd..e9e46cc7 100644
--- a/internal/checks/promql_rate_test.go
+++ b/internal/checks/promql_rate_test.go
@@ -545,15 +545,6 @@ func TestRateCheck(t *testing.T) {
Details: checks.RateCheckDetails,
Severity: checks.Bug,
},
- {
- Lines: parser.LineRange{
- First: 2,
- Last: 2,
- },
- Reporter: "promql/rate",
- Text: checkUnsupported(checks.RateCheckName, "prom", uri, promapi.APIPathMetadata),
- Severity: checks.Warning,
- },
}
},
mocks: []*prometheusMock{
@@ -933,6 +924,85 @@ func TestRateCheck(t *testing.T) {
},
},
},
+ {
+ description: "config 404",
+ content: "- alert: my alert\n expr: rate(my:sum[5m])\n",
+ entries: mustParseContent("- record: my:sum\n expr: sum(foo)\n"),
+ checker: newRateCheck,
+ prometheus: newSimpleProm,
+ problems: noProblems,
+ mocks: []*prometheusMock{
+ {
+ conds: []requestCondition{requireConfigPath},
+ resp: httpResponse{code: http.StatusNotFound, body: "Not Found"},
+ },
+ },
+ },
+ {
+ description: "metadata 404",
+ content: `
+- alert: Plexi_Worker_High_Signing_Latency
+ expr: |
+ sum(
+ rate(global:response_time_sum{namespace!~"test[.].+"}[15m])
+ ) by (environment, namespace)
+ /
+ sum(
+ rate(global:response_time_count{namespace!~"test[.].+"}[15m])
+ ) by (environment, namespace)
+ > 3000
+`,
+ checker: newRateCheck,
+ prometheus: newSimpleProm,
+ entries: mustParseContent(`
+- record: global:response_time_sum
+ expr: sum(response_time_sum:rate2m)
+- record: response_time_sum:rate2m
+ expr: rate(response_time_sum[2m])
+`),
+ problems: noProblems,
+ mocks: []*prometheusMock{
+ {
+ conds: []requestCondition{requireConfigPath},
+ resp: configResponse{yaml: "global:\n scrape_interval: 53s\n"},
+ },
+ {
+ conds: []requestCondition{
+ requireMetadataPath,
+ formCond{"metric", "global:response_time_sum"},
+ },
+ resp: metadataResponse{metadata: map[string][]v1.Metadata{}},
+ },
+ {
+ conds: []requestCondition{
+ requireMetadataPath,
+ formCond{"metric", "response_time_sum:rate2m"},
+ },
+ resp: httpResponse{code: http.StatusNotFound, body: "Not Found"},
+ },
+ },
+ },
+ {
+ description: "rate over non aggregate",
+ content: "- alert: my alert\n expr: rate(my:foo[5m])\n",
+ entries: mustParseContent("- record: my:foo\n expr: foo / foo\n"),
+ checker: newRateCheck,
+ prometheus: newSimpleProm,
+ problems: noProblems,
+ mocks: []*prometheusMock{
+ {
+ conds: []requestCondition{requireConfigPath},
+ resp: configResponse{yaml: "global:\n scrape_interval: 1m\n"},
+ },
+ {
+ conds: []requestCondition{
+ requireMetadataPath,
+ formCond{"metric", "my:foo"},
+ },
+ resp: metadataResponse{metadata: map[string][]v1.Metadata{}},
+ },
+ },
+ },
}
runTests(t, testCases)
}
diff --git a/internal/checks/promql_series.go b/internal/checks/promql_series.go
index 8e8916a4..afa13973 100644
--- a/internal/checks/promql_series.go
+++ b/internal/checks/promql_series.go
@@ -246,7 +246,7 @@ func (c SeriesCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Ru
slog.String("name", c.prom.Name()),
slog.String("metric", c.prom.UptimeMetric()),
)
- promUptime = &promapi.RangeQueryResult{ // nolint:exhaustruct
+ promUptime = &promapi.RangeQueryResult{ // nolint: exhaustruct
URI: c.prom.URI(),
Series: promapi.SeriesTimeRanges{
From: params.Start(),
diff --git a/internal/checks/promql_vector_matching.go b/internal/checks/promql_vector_matching.go
index 2b403358..dc23fa99 100644
--- a/internal/checks/promql_vector_matching.go
+++ b/internal/checks/promql_vector_matching.go
@@ -312,13 +312,16 @@ func (ls labelSets) overlaps(bs labelSets) bool {
return false
}
-func (ls labelSets) getFirstNonOverlap(bs labelSets) (labelSet, labelSet) {
+func (ls labelSets) getFirstNonOverlap(bs labelSets) (l, r labelSet) {
for _, a := range ls {
for _, b := range bs {
if !a.isEqual(b) {
- return a, b
+ l = a
+ r = b
+ goto DONE
}
}
}
- return labelSet{}, labelSet{} // nolint:exhaustruct
+DONE:
+ return l, r
}
diff --git a/internal/checks/promql_vector_matching_test.go b/internal/checks/promql_vector_matching_test.go
index 1ca9385b..0d055540 100644
--- a/internal/checks/promql_vector_matching_test.go
+++ b/internal/checks/promql_vector_matching_test.go
@@ -123,6 +123,62 @@ func TestVectorMatchingCheck(t *testing.T) {
},
},
},
+ {
+ description: "one to one matching / match",
+ content: "- record: foo\n expr: foo / bar\n",
+ checker: newVectorMatchingCheck,
+ prometheus: newSimpleProm,
+ problems: noProblems,
+ mocks: []*prometheusMock{
+ {
+ conds: []requestCondition{
+ requireQueryPath,
+ formCond{key: "query", value: "count(foo / bar)"},
+ },
+ resp: respondWithEmptyVector(),
+ },
+ {
+ conds: []requestCondition{
+ requireQueryPath,
+ formCond{key: "query", value: "count(foo) without(__name__)"},
+ },
+ resp: vectorResponse{
+ samples: []*model.Sample{
+ generateSample(map[string]string{
+ "instance": "aaa",
+ "job": "bbb",
+ }),
+ generateSample(map[string]string{
+ "instance": "bbb",
+ "job": "bbb",
+ }),
+ },
+ },
+ },
+ {
+ conds: []requestCondition{
+ requireQueryPath,
+ formCond{key: "query", value: "count(bar) without(__name__)"},
+ },
+ resp: vectorResponse{
+ samples: []*model.Sample{
+ generateSample(map[string]string{
+ "instance": "aaa",
+ "job": "bbb",
+ }),
+ generateSample(map[string]string{
+ "instance": "bbb",
+ "job": "bbb",
+ }),
+ generateSample(map[string]string{
+ "instance": "ccc",
+ "job": "bbb",
+ }),
+ },
+ },
+ },
+ },
+ },
{
description: "ignore missing left side",
content: "- record: foo\n expr: xxx / foo\n",
diff --git a/internal/config/config.go b/internal/config/config.go
index a37bce48..884da602 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -129,13 +129,13 @@ func getContext() *hcl.EvalContext {
}
func Load(path string, failOnMissing bool) (cfg Config, fromFile bool, err error) {
- cfg = Config{ // nolint:exhaustruct
+ cfg = Config{ // nolint: exhaustruct
CI: &CI{
MaxCommits: 20,
BaseBranch: "master",
},
- Parser: &Parser{}, // nolint:exhaustruct
- Repository: &Repository{}, // nolint:exhaustruct
+ Parser: &Parser{}, // nolint: exhaustruct
+ Repository: &Repository{}, // nolint: exhaustruct
Checks: &Checks{
Enabled: checks.CheckNames,
Disabled: []string{},
diff --git a/internal/config/parsed_rule.go b/internal/config/parsed_rule.go
index ad7bf4ce..9722dc69 100644
--- a/internal/config/parsed_rule.go
+++ b/internal/config/parsed_rule.go
@@ -145,7 +145,7 @@ func baseRules(proms []*promapi.FailoverGroup, match []Match) (rules []parsedRul
func defaultRuleMatch(match []Match, defaultStates []string) []Match {
if len(match) == 0 {
- return []Match{{State: defaultStates}} // nolint:exhaustruct
+ return []Match{{State: defaultStates}} // nolint: exhaustruct
}
dst := make([]Match, 0, len(match))
for _, m := range match {
diff --git a/internal/discovery/git_branch.go b/internal/discovery/git_branch.go
index 20369efd..519dea56 100644
--- a/internal/discovery/git_branch.go
+++ b/internal/discovery/git_branch.go
@@ -273,7 +273,7 @@ func matchEntries(before, after []Entry) (ml []matchedEntry) {
slog.String("name", a.Rule.Name()),
)
- m := matchedEntry{after: a, hasAfter: true} // nolint:exhaustruct
+ m := matchedEntry{after: a, hasAfter: true} // nolint: exhaustruct
beforeSwap := make([]Entry, 0, len(before))
var matches []Entry
var matched bool
@@ -318,7 +318,7 @@ func matchEntries(before, after []Entry) (ml []matchedEntry) {
}
for _, b := range before {
- ml = append(ml, matchedEntry{before: b, hasBefore: true}) // nolint:exhaustruct
+ ml = append(ml, matchedEntry{before: b, hasBefore: true}) // nolint: exhaustruct
}
return ml
diff --git a/internal/git/changes.go b/internal/git/changes.go
index bbda4f6d..8a9a699d 100644
--- a/internal/git/changes.go
+++ b/internal/git/changes.go
@@ -109,10 +109,10 @@ func Changes(cmd CommandRunner, baseBranch string, filter PathFilter) ([]*FileCh
}
// Rest is populated inside the next loop.
- change := &FileChange{ // nolint:exhaustruct
+ change := &FileChange{ // nolint: exhaustruct
Status: status,
- Path: PathDiff{ // nolint:exhaustruct
- After: Path{ // nolint:exhaustruct
+ Path: PathDiff{ // nolint: exhaustruct
+ After: Path{ // nolint: exhaustruct
Name: dstPath,
},
},
diff --git a/internal/promapi/cache.go b/internal/promapi/cache.go
index 8cbd5706..c7d7d8d7 100644
--- a/internal/promapi/cache.go
+++ b/internal/promapi/cache.go
@@ -22,7 +22,7 @@ func (e *endpointStats) hit() { e.hits++ }
func (e *endpointStats) miss() { e.misses++ }
func newQueryCache(maxStale time.Duration) *queryCache {
- // nolint:exhaustruct
+ // nolint: exhaustruct
return &queryCache{
entries: map[uint64]*cacheEntry{},
stats: map[string]*endpointStats{},
diff --git a/internal/promapi/errors.go b/internal/promapi/errors.go
index 71337488..67487d29 100644
--- a/internal/promapi/errors.go
+++ b/internal/promapi/errors.go
@@ -15,7 +15,7 @@ import (
"github.com/prymitive/current"
)
-func IsUnsupportedError(err error) bool {
+func isUnsupportedError(err error) bool {
var e1 APIError
if ok := errors.As(err, &e1); ok {
return e1.ErrorType == ErrAPIUnsupported
diff --git a/internal/promapi/failover.go b/internal/promapi/failover.go
index d90ffb28..27a2a974 100644
--- a/internal/promapi/failover.go
+++ b/internal/promapi/failover.go
@@ -2,8 +2,11 @@ package promapi
import (
"context"
+ "errors"
"log/slog"
"regexp"
+ "slices"
+ "sync"
"time"
"github.com/prometheus/client_golang/prometheus"
@@ -43,7 +46,32 @@ func cacheCleaner(cache *queryCache, interval time.Duration, quit chan bool) {
}
}
+type disabledChecks struct {
+ // Key is the name of the unsupported API, value is the list of checks disabled because of it.
+ apis map[string][]string
+ mtx sync.Mutex
+}
+
+func (dc *disabledChecks) disable(api, check string) {
+ dc.mtx.Lock()
+ if _, ok := dc.apis[api]; !ok {
+ dc.apis[api] = []string{}
+ }
+ if !slices.Contains(dc.apis[api], check) {
+ dc.apis[api] = append(dc.apis[api], check)
+ }
+ dc.mtx.Unlock()
+}
+
+func (dc *disabledChecks) read() map[string][]string {
+ dc.mtx.Lock()
+ defer dc.mtx.Unlock()
+ return dc.apis
+}
+
type FailoverGroup struct {
+ disabledChecks disabledChecks
+
name string
uri string
servers []*Prometheus
@@ -59,15 +87,16 @@ type FailoverGroup struct {
}
func NewFailoverGroup(name, uri string, servers []*Prometheus, strictErrors bool, uptimeMetric string, include, exclude []*regexp.Regexp, tags []string) *FailoverGroup {
- return &FailoverGroup{ // nolint:exhaustruct
- name: name,
- uri: uri,
- servers: servers,
- strictErrors: strictErrors,
- uptimeMetric: uptimeMetric,
- pathsInclude: include,
- pathsExclude: exclude,
- tags: tags,
+ return &FailoverGroup{ // nolint: exhaustruct
+ name: name,
+ uri: uri,
+ servers: servers,
+ strictErrors: strictErrors,
+ uptimeMetric: uptimeMetric,
+ pathsInclude: include,
+ pathsExclude: exclude,
+ tags: tags,
+ disabledChecks: disabledChecks{apis: map[string][]string{}}, // nolint: exhaustruct
}
}
@@ -79,6 +108,14 @@ func (fg *FailoverGroup) URI() string {
return fg.uri
}
+func (fg *FailoverGroup) DisableCheck(api, s string) {
+ fg.disabledChecks.disable(api, s)
+}
+
+func (fg *FailoverGroup) GetDisabledChecks() map[string][]string {
+ return fg.disabledChecks.read()
+}
+
func (fg *FailoverGroup) Include() []string {
sl := []string{}
for _, re := range fg.pathsInclude {
@@ -190,7 +227,7 @@ func (fg *FailoverGroup) Config(ctx context.Context, cacheTTL time.Duration) (cf
if err == nil {
return cfg, nil
}
- if !IsUnavailableError(err) {
+ if !IsUnavailableError(err) && !errors.Is(err, ErrUnsupported) {
return nil, &FailoverGroupError{err: err, uri: uri, isStrict: fg.strictErrors}
}
}
@@ -243,7 +280,7 @@ func (fg *FailoverGroup) Metadata(ctx context.Context, metric string) (metadata
if err == nil {
return metadata, nil
}
- if !IsUnavailableError(err) {
+ if !IsUnavailableError(err) && !errors.Is(err, ErrUnsupported) {
return metadata, &FailoverGroupError{err: err, uri: uri, isStrict: fg.strictErrors}
}
}
@@ -258,7 +295,7 @@ func (fg *FailoverGroup) Flags(ctx context.Context) (flags *FlagsResult, err err
if err == nil {
return flags, nil
}
- if !IsUnavailableError(err) {
+ if !IsUnavailableError(err) && !errors.Is(err, ErrUnsupported) {
return nil, &FailoverGroupError{err: err, uri: uri, isStrict: fg.strictErrors}
}
}
diff --git a/internal/promapi/prometheus.go b/internal/promapi/prometheus.go
index 8139b434..74b940d4 100644
--- a/internal/promapi/prometheus.go
+++ b/internal/promapi/prometheus.go
@@ -18,6 +18,8 @@ import (
"go.uber.org/ratelimit"
)
+var ErrUnsupported = errors.New("unsupported API")
+
type PrometheusContextKey string
const (
@@ -89,11 +91,48 @@ func sanitizeURI(s string) string {
return s
}
+type unsupporedAPIs struct {
+ mtx sync.RWMutex
+ noConfig bool
+ noFlags bool
+ noMetadata bool
+}
+
+func (ua *unsupporedAPIs) isSupported(s string) bool {
+ ua.mtx.RLock()
+ defer ua.mtx.RUnlock()
+ switch s {
+ case APIPathConfig:
+ return !ua.noConfig
+ case APIPathFlags:
+ return !ua.noFlags
+ case APIPathMetadata:
+ return !ua.noMetadata
+ default:
+ return true
+ }
+}
+
+func (ua *unsupporedAPIs) disable(s string) {
+ slog.Debug("Disabling unsupported API", slog.String("api", s))
+ ua.mtx.Lock()
+ defer ua.mtx.Unlock()
+ switch s {
+ case APIPathConfig:
+ ua.noConfig = true
+ case APIPathFlags:
+ ua.noFlags = true
+ case APIPathMetadata:
+ ua.noMetadata = true
+ }
+}
+
type Prometheus struct {
rateLimiter ratelimit.Limiter
headers map[string]string
cache *queryCache
locker *partitionLocker
+ apis *unsupporedAPIs
queries chan queryRequest
client http.Client
name string
@@ -118,7 +157,7 @@ func NewPrometheus(name, uri, publicURI string, headers map[string]string, timeo
publicURI = safeURI
}
- prom := Prometheus{ // nolint:exhaustruct
+ prom := Prometheus{ // nolint: exhaustruct
name: name,
unsafeURI: uri,
safeURI: safeURI,
@@ -129,6 +168,7 @@ func NewPrometheus(name, uri, publicURI string, headers map[string]string, timeo
locker: newPartitionLocker((&sync.Mutex{})),
rateLimiter: ratelimit.New(rl),
concurrency: concurrency,
+ apis: &unsupporedAPIs{}, // nolint: exhaustruct
}
return &prom
@@ -211,6 +251,10 @@ func processJob(prom *Prometheus, job queryRequest) queryResult {
}
}
+ if !prom.apis.isSupported(job.query.Endpoint()) {
+ return queryResult{err: ErrUnsupported} // nolint: exhaustruct
+ }
+
prometheusQueriesTotal.WithLabelValues(prom.name, job.query.Endpoint()).Inc()
prometheusQueriesRunning.WithLabelValues(prom.name, job.query.Endpoint()).Inc()
@@ -223,6 +267,16 @@ func processJob(prom *Prometheus, job queryRequest) queryResult {
return result
}
prometheusQueryErrorsTotal.WithLabelValues(prom.name, job.query.Endpoint(), errReason(result.err)).Inc()
+ if isUnsupportedError(result.err) {
+ prom.apis.disable(job.query.Endpoint())
+ slog.Warn(
+ "Looks like this server doesn't support some Prometheus API endpoints, all checks using this API will be disabled",
+ slog.String("name", prom.name),
+ slog.String("uri", prom.safeURI),
+ slog.String("api", job.query.Endpoint()),
+ )
+ return queryResult{err: ErrUnsupported} // nolint: exhaustruct
+ }
slog.Error(
"Query returned an error",
slog.Any("err", result.err),
diff --git a/internal/promapi/range.go b/internal/promapi/range.go
index 781aef60..7dc1d077 100644
--- a/internal/promapi/range.go
+++ b/internal/promapi/range.go
@@ -132,7 +132,7 @@ func (prom *Prometheus) RangeQuery(ctx context.Context, expr string, params Rang
results := make(chan queryResult, len(slices))
for _, s := range slices {
- query := queryRequest{ // nolint:exhaustruct
+ query := queryRequest{ // nolint: exhaustruct
query: rangeQuery{
prom: prom,
ctx: ctx,
@@ -165,9 +165,9 @@ func (prom *Prometheus) RangeQuery(ctx context.Context, expr string, params Rang
close(results)
}()
- merged := RangeQueryResult{ // nolint:exhaustruct
+ merged := RangeQueryResult{ // nolint: exhaustruct
URI: prom.publicURI,
- Series: SeriesTimeRanges{ // nolint:exhaustruct
+ Series: SeriesTimeRanges{ // nolint: exhaustruct
From: start,
Until: end,
Step: step,
diff --git a/internal/reporter/bitbucket_api.go b/internal/reporter/bitbucket_api.go
index 9e13e640..2497686b 100644
--- a/internal/reporter/bitbucket_api.go
+++ b/internal/reporter/bitbucket_api.go
@@ -670,7 +670,7 @@ func (bb bitBucketAPI) limitComments(src []BitBucketPendingComment) []BitBucketP
Text: fmt.Sprintf(`This pint run would create %d comment(s), which is more than %d limit configured for pint.
%d comments were skipped and won't be visibile on this PR.`, len(src), bb.maxComments, len(src)-bb.maxComments),
Severity: "NORMAL",
- Anchor: BitBucketPendingCommentAnchor{ // nolint:exhaustruct
+ Anchor: BitBucketPendingCommentAnchor{ // nolint: exhaustruct
DiffType: "EFFECTIVE",
},
})
diff --git a/internal/reporter/comments.go b/internal/reporter/comments.go
index c342936e..fe73230a 100644
--- a/internal/reporter/comments.go
+++ b/internal/reporter/comments.go
@@ -293,3 +293,36 @@ func updateDestination(ctx context.Context, s Summary, c Commenter, dst any) (er
return nil
}
+
+func makePrometheusDetailsComment(s Summary) string {
+ pds := s.GetPrometheusDetails()
+ if len(pds) == 0 {
+ return ""
+ }
+
+ var buf strings.Builder
+ buf.WriteString(`Some checks were disabled because one or more configured Prometheus server doesn't seem to support all required Prometheus APIs.
+This usually means that you're running pint against a service like Thanos or Mimir that allows to query metrics but doesn't implement all APIs documented [here](https://prometheus.io/docs/prometheus/latest/querying/api/).
+Since pint uses many of these API endpoint for querying information needed to run online checks only a real Prometheus server will allow it to run all of these checks.
+Below is the list of checks that were disabled for each Prometheus server defined in pint config file.
+
+`)
+ for _, pd := range pds {
+ buf.WriteString("- `")
+ buf.WriteString(pd.Name)
+ buf.WriteString("`\n")
+ for _, dc := range pd.DisabledChecks {
+ buf.WriteString(" - `")
+ buf.WriteString(dc.API)
+ buf.WriteString("` is unsupported, disabled checks:\n")
+ for _, name := range dc.Checks {
+ buf.WriteString(" - [")
+ buf.WriteString(name)
+ buf.WriteString("](https://cloudflare.github.io/pint/checks/")
+ buf.WriteString(name)
+ buf.WriteString(".html)\n")
+ }
+ }
+ }
+ return buf.String()
+}
diff --git a/internal/reporter/github.go b/internal/reporter/github.go
index 02f1017c..ddc32536 100644
--- a/internal/reporter/github.go
+++ b/internal/reporter/github.go
@@ -69,7 +69,7 @@ func NewGithubReporter(version, baseURL, uploadURL string, timeout time.Duration
)
ts := oauth2.StaticTokenSource(
- &oauth2.Token{AccessToken: token}, // nolint:exhaustruct
+ &oauth2.Token{AccessToken: token}, // nolint: exhaustruct
)
tc := oauth2.NewClient(context.Background(), ts)
@@ -380,6 +380,10 @@ func formatGHReviewBody(version string, summary Summary) string {
}
b.WriteString("\n\n\n\n")
+ if details := makePrometheusDetailsComment(summary); details != "" {
+ b.WriteString(details)
+ }
+
return b.String()
}
diff --git a/internal/reporter/github_test.go b/internal/reporter/github_test.go
index d2535aea..56a38104 100644
--- a/internal/reporter/github_test.go
+++ b/internal/reporter/github_test.go
@@ -2,6 +2,7 @@ package reporter_test
import (
"context"
+ "encoding/json"
"fmt"
"io"
"log/slog"
@@ -11,16 +12,18 @@ import (
"testing"
"time"
+ "github.com/google/go-cmp/cmp"
"github.com/neilotoole/slogt"
"github.com/stretchr/testify/require"
"github.com/cloudflare/pint/internal/checks"
"github.com/cloudflare/pint/internal/discovery"
"github.com/cloudflare/pint/internal/parser"
+ "github.com/cloudflare/pint/internal/promapi"
"github.com/cloudflare/pint/internal/reporter"
)
-func TestGithubReporter(t *testing.T) {
+func TestGitHubReporter(t *testing.T) {
type testCaseT struct {
httpHandler http.Handler
error func(uri string) string
@@ -30,7 +33,7 @@ func TestGithubReporter(t *testing.T) {
owner string
repo string
token string
- reports []reporter.Report
+ summary reporter.Summary
prNum int
maxComments int
timeout time.Duration
@@ -44,6 +47,10 @@ func TestGithubReporter(t *testing.T) {
expr: sum(errors) by (job)
`))
+ summaryWithDetails := reporter.NewSummary([]reporter.Report{})
+ summaryWithDetails.MarkCheckDisabled("prom1", promapi.APIPathConfig, []string{"check1", "check3", "check2"})
+ summaryWithDetails.MarkCheckDisabled("prom2", promapi.APIPathMetadata, []string{"check1"})
+
for _, tc := range []testCaseT{
{
description: "list files error",
@@ -64,7 +71,7 @@ func TestGithubReporter(t *testing.T) {
error: func(uri string) string {
return fmt.Sprintf("failed to list pull request files: GET %s/api/v3/repos/foo/bar/pulls/123/files: 400 []", uri)
},
- reports: []reporter.Report{
+ summary: reporter.NewSummary([]reporter.Report{
{
Path: discovery.Path{
Name: "foo.txt",
@@ -84,7 +91,7 @@ func TestGithubReporter(t *testing.T) {
Severity: checks.Fatal,
},
},
- },
+ }),
},
{
description: "list pull reviews timeout",
@@ -106,7 +113,7 @@ func TestGithubReporter(t *testing.T) {
error: func(_ string) string {
return "failed to list pull request reviews: context deadline exceeded"
},
- reports: []reporter.Report{
+ summary: reporter.NewSummary([]reporter.Report{
{
Path: discovery.Path{
Name: "$1",
@@ -125,7 +132,7 @@ func TestGithubReporter(t *testing.T) {
Severity: checks.Fatal,
},
},
- },
+ }),
},
{
description: "list reviews error",
@@ -146,7 +153,7 @@ func TestGithubReporter(t *testing.T) {
error: func(uri string) string {
return fmt.Sprintf("failed to list pull request reviews: GET %s/api/v3/repos/foo/bar/pulls/123/reviews: 400 []", uri)
},
- reports: []reporter.Report{
+ summary: reporter.NewSummary([]reporter.Report{
{
Path: discovery.Path{
Name: "foo.txt",
@@ -166,7 +173,7 @@ func TestGithubReporter(t *testing.T) {
Severity: checks.Fatal,
},
},
- },
+ }),
},
{
description: "no problems",
@@ -176,7 +183,7 @@ func TestGithubReporter(t *testing.T) {
prNum: 123,
maxComments: 50,
timeout: time.Second,
- reports: []reporter.Report{},
+ summary: reporter.NewSummary([]reporter.Report{}),
},
{
description: "happy path",
@@ -186,7 +193,7 @@ func TestGithubReporter(t *testing.T) {
prNum: 123,
maxComments: 50,
timeout: time.Second,
- reports: []reporter.Report{
+ summary: reporter.NewSummary([]reporter.Report{
{
Path: discovery.Path{
Name: "foo.txt",
@@ -206,7 +213,7 @@ func TestGithubReporter(t *testing.T) {
Severity: checks.Fatal,
},
},
- },
+ }),
},
{
description: "error crating review",
@@ -227,7 +234,7 @@ func TestGithubReporter(t *testing.T) {
error: func(uri string) string {
return fmt.Sprintf("failed to create pull request review: POST %s/api/v3/repos/foo/bar/pulls/123/reviews: 502 []", uri)
},
- reports: []reporter.Report{
+ summary: reporter.NewSummary([]reporter.Report{
{
Path: discovery.Path{
Name: "foo.txt",
@@ -247,7 +254,7 @@ func TestGithubReporter(t *testing.T) {
Severity: checks.Fatal,
},
},
- },
+ }),
},
{
description: "error updating existing review",
@@ -272,7 +279,7 @@ func TestGithubReporter(t *testing.T) {
error: func(uri string) string {
return fmt.Sprintf("failed to update pull request review: PUT %s/api/v3/repos/foo/bar/pulls/123/reviews/1: 502 []", uri)
},
- reports: []reporter.Report{
+ summary: reporter.NewSummary([]reporter.Report{
{
Path: discovery.Path{
Name: "foo.txt",
@@ -292,7 +299,7 @@ func TestGithubReporter(t *testing.T) {
Severity: checks.Fatal,
},
},
- },
+ }),
},
{
description: "update existing review",
@@ -313,7 +320,7 @@ func TestGithubReporter(t *testing.T) {
}
_, _ = w.Write([]byte(""))
}),
- reports: []reporter.Report{
+ summary: reporter.NewSummary([]reporter.Report{
{
Path: discovery.Path{
Name: "foo.txt",
@@ -333,7 +340,7 @@ func TestGithubReporter(t *testing.T) {
Severity: checks.Fatal,
},
},
- },
+ }),
},
{
description: "maxComments 2",
@@ -365,7 +372,7 @@ func TestGithubReporter(t *testing.T) {
}
_, _ = w.Write([]byte(""))
}),
- reports: []reporter.Report{
+ summary: reporter.NewSummary([]reporter.Report{
{
Path: discovery.Path{
Name: "foo.txt",
@@ -442,7 +449,7 @@ func TestGithubReporter(t *testing.T) {
Severity: checks.Fatal,
},
},
- },
+ }),
},
{
description: "maxComments 2, too many comments comment error",
@@ -472,7 +479,7 @@ func TestGithubReporter(t *testing.T) {
}
_, _ = w.Write([]byte(""))
}),
- reports: []reporter.Report{
+ summary: reporter.NewSummary([]reporter.Report{
{
Path: discovery.Path{
Name: "foo.txt",
@@ -549,7 +556,7 @@ func TestGithubReporter(t *testing.T) {
Severity: checks.Fatal,
},
},
- },
+ }),
},
{
description: "general comment error",
@@ -583,7 +590,7 @@ func TestGithubReporter(t *testing.T) {
error: func(uri string) string {
return fmt.Sprintf("failed to create general comment: POST %s/api/v3/repos/foo/bar/issues/123/comments: 500 []", uri)
},
- reports: []reporter.Report{
+ summary: reporter.NewSummary([]reporter.Report{
{
Path: discovery.Path{
Name: "foo.txt",
@@ -660,7 +667,7 @@ func TestGithubReporter(t *testing.T) {
Severity: checks.Fatal,
},
},
- },
+ }),
},
{
description: "modified line",
@@ -694,7 +701,7 @@ func TestGithubReporter(t *testing.T) {
}
_, _ = w.Write([]byte(""))
}),
- reports: []reporter.Report{
+ summary: reporter.NewSummary([]reporter.Report{
{
Path: discovery.Path{
Name: "foo.txt",
@@ -714,7 +721,7 @@ func TestGithubReporter(t *testing.T) {
Severity: checks.Fatal,
},
},
- },
+ }),
},
{
description: "unmodified line",
@@ -748,7 +755,7 @@ func TestGithubReporter(t *testing.T) {
}
_, _ = w.Write([]byte(""))
}),
- reports: []reporter.Report{
+ summary: reporter.NewSummary([]reporter.Report{
{
Path: discovery.Path{
Name: "foo.txt",
@@ -768,7 +775,7 @@ func TestGithubReporter(t *testing.T) {
Severity: checks.Fatal,
},
},
- },
+ }),
},
{
description: "removed line",
@@ -802,7 +809,7 @@ func TestGithubReporter(t *testing.T) {
}
_, _ = w.Write([]byte(""))
}),
- reports: []reporter.Report{
+ summary: reporter.NewSummary([]reporter.Report{
{
Path: discovery.Path{
Name: "foo.txt",
@@ -823,7 +830,7 @@ func TestGithubReporter(t *testing.T) {
Anchor: checks.AnchorBefore,
},
},
- },
+ }),
},
{
description: "review comment",
@@ -844,7 +851,7 @@ func TestGithubReporter(t *testing.T) {
}
_, _ = w.Write([]byte(""))
}),
- reports: []reporter.Report{
+ summary: reporter.NewSummary([]reporter.Report{
{
Path: discovery.Path{
Name: "foo.txt",
@@ -864,7 +871,72 @@ func TestGithubReporter(t *testing.T) {
Severity: checks.Fatal,
},
},
- },
+ }),
+ },
+ {
+ description: "prometheus details",
+ owner: "foo",
+ repo: "bar",
+ token: "something",
+ prNum: 123,
+ maxComments: 50,
+ timeout: time.Second,
+ httpHandler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ if r.Method == http.MethodPost && r.URL.Path == "/api/v3/repos/foo/bar/pulls/123/reviews" {
+ expected := `### This pull request was validated by [pint](https://github.com/cloudflare/pint).
+:heavy_check_mark: No problems found
+Stats
+
+
+| Stat | Value |
+| --- | --- |
+| Version | v0.0.0 |
+| Number of rules parsed | 0 |
+| Number of rules checked | 0 |
+| Number of problems found | 0 |
+| Number of offline checks | 0 |
+| Number of online checks | 0 |
+| Checks duration | 0 |
+
+
+Problems
+
+
+No problems reported
+
+