Skip to content

Commit

Permalink
Automatically disable unsupported checks
Browse files Browse the repository at this point in the history
  • Loading branch information
prymitive committed Jan 6, 2025
1 parent 7bc11b1 commit 2e138d3
Show file tree
Hide file tree
Showing 22 changed files with 406 additions and 112 deletions.
16 changes: 16 additions & 0 deletions cmd/pint/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions internal/checks/alerts_absent.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package checks

import (
"context"
"errors"
"fmt"
"time"

Expand Down Expand Up @@ -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,
Expand Down
14 changes: 1 addition & 13 deletions internal/checks/alerts_absent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
5 changes: 5 additions & 0 deletions internal/checks/alerts_external_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package checks

import (
"context"
"errors"
"fmt"

"github.com/cloudflare/pint/internal/discovery"
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 0 additions & 3 deletions internal/checks/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions internal/checks/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
5 changes: 5 additions & 0 deletions internal/checks/labels_conflict.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package checks

import (
"context"
"errors"
"fmt"

"github.com/cloudflare/pint/internal/discovery"
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 1 addition & 13 deletions internal/checks/labels_conflict_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
5 changes: 5 additions & 0 deletions internal/checks/promql_counter.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package checks

import (
"context"
"errors"
"fmt"

v1 "github.com/prometheus/client_golang/api/prometheus/v1"
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions internal/checks/promql_range_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package checks

import (
"context"
"errors"
"fmt"
"time"

Expand Down Expand Up @@ -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,
Expand Down
14 changes: 1 addition & 13 deletions internal/checks/promql_range_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
11 changes: 11 additions & 0 deletions internal/checks/promql_rate.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package checks

import (
"context"
"errors"
"fmt"
"slices"
"time"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 0 additions & 9 deletions internal/checks/promql_rate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion internal/promapi/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
61 changes: 49 additions & 12 deletions internal/promapi/failover.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ package promapi

import (
"context"
"errors"
"log/slog"
"regexp"
"slices"
"sync"
"time"

"github.com/prometheus/client_golang/prometheus"
Expand Down Expand Up @@ -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
Expand All @@ -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
}
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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}
}
}
Expand Down Expand Up @@ -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}
}
}
Expand All @@ -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}
}
}
Expand Down
Loading

0 comments on commit 2e138d3

Please sign in to comment.