Merge pull request #1050 from cloudflare/truncate

Don't list all possible prometheus servers

internal/checks/base_test.go

@@ -93,12 +93,15 @@ type problemsFn func(string) []checks.Problem
 
 type newPrometheusFn func(string) *promapi.FailoverGroup
 
-type newCtxFn func() context.Context
+type newCtxFn func(string) context.Context
+
+type otherPromsFn func(string) []*promapi.FailoverGroup
 
 type checkTest struct {
  description string
  content string
  prometheus newPrometheusFn
+ otherProms otherPromsFn
  ctx newCtxFn
  checker newCheckFn
  entries []discovery.Entry
@@ -130,20 +133,28 @@ func runTests(t *testing.T, testCases []checkTest) {
  }
 
  var proms []*promapi.FailoverGroup
+ reg := prometheus.NewRegistry()
  prom := tc.prometheus(uri)
  if prom != nil {
  proms = append(proms, prom)
- reg := prometheus.NewRegistry()
  prom.StartWorkers(reg)
  defer prom.Close(reg)
  }
 
+ if tc.otherProms != nil {
+ for _, op := range tc.otherProms(uri) {
+ proms = append(proms, op)
+ op.StartWorkers(reg)
+ defer op.Close(reg)
+ }
+ }
+
  entries, err := parseContent(tc.content)
  require.NoError(t, err, "cannot parse rule content")
  for _, entry := range entries {
  ctx := context.WithValue(context.Background(), promapi.AllPrometheusServers, proms)
  if tc.ctx != nil {
- ctx = tc.ctx()
+ ctx = tc.ctx(uri)
  }
  problems := tc.checker(prom).Check(ctx, entry.Path, entry.Rule, tc.entries)
  require.Equal(t, tc.problems(uri), problems)

internal/checks/promql_series.go

@@ -5,6 +5,7 @@ import (
  "fmt"
  "log/slog"
  "regexp"
+ "strconv"
  "strings"
  "time"
 
@@ -573,7 +574,7 @@ func (c SeriesCheck) checkOtherServer(ctx context.Context, query string) string
  buf.WriteString(query)
  buf.WriteString("` was found on other prometheus servers:\n\n")
 
- var matches int
+ var matches, skipped int
  for _, prom := range servers {
  slog.Debug("Checking if metric exists on any other Prometheus server", slog.String("check", c.Reporter()), slog.String("selector", query))
 
@@ -591,6 +592,10 @@ func (c SeriesCheck) checkOtherServer(ctx context.Context, query string) string
 
  if series > 0 {
  matches++
+ if matches > 10 {
+ skipped++
+ continue
+ }
  buf.WriteString("- [")
  buf.WriteString(prom.Name())
  buf.WriteString("](")
@@ -600,6 +605,11 @@ func (c SeriesCheck) checkOtherServer(ctx context.Context, query string) string
  buf.WriteString(")\n")
  }
  }
+ if skipped > 0 {
+ buf.WriteString("- and ")
+ buf.WriteString(strconv.Itoa(skipped))
+ buf.WriteString(" other server(s).\n")
+ }
 
  buf.WriteString("\nYou might be trying to deploy this rule to the wrong Prometheus server instance.\n")
 

internal/checks/promql_series_test.go

@@ -437,7 +437,7 @@ func TestSeriesCheck(t *testing.T) {
  {
  description: "#2 series never present, custom range",
  content: "- record: foo\n expr: sum(notfound)\n",
- ctx: func() context.Context {
+ ctx: func(_ string) context.Context {
  s := checks.PromqlSeriesSettings{
  LookbackRange: "3d",
  LookbackStep: "6m",
@@ -669,7 +669,7 @@ func TestSeriesCheck(t *testing.T) {
  description: "#2 series never present but metric ignored",
  content: "- record: foo\n expr: sum(notfound)\n",
  checker: newSeriesCheck,
- ctx: func() context.Context {
+ ctx: func(_ string) context.Context {
  s := checks.PromqlSeriesSettings{
  IgnoreMetrics: []string{"foo", "bar", "not.+"},
  }
@@ -1413,7 +1413,7 @@ func TestSeriesCheck(t *testing.T) {
  description: "#4 metric was present but disappeared over 1h ago / ignored",
  content: "- record: foo\n expr: sum(found{job=\"foo\", instance=\"bar\"})\n",
  checker: newSeriesCheck,
- ctx: func() context.Context {
+ ctx: func(_ string) context.Context {
  s := checks.PromqlSeriesSettings{
  IgnoreMetrics: []string{"foo", "found", "not.+"},
  }
@@ -1990,7 +1990,7 @@ func TestSeriesCheck(t *testing.T) {
  description: "#5 metric was present but not with label value",
  content: "- record: foo\n expr: sum(found{notfound=\"notfound\", instance=~\".+\", not!=\"negative\", instance!~\"bad\"})\n",
  checker: newSeriesCheck,
- ctx: func() context.Context {
+ ctx: func(_ string) context.Context {
  s := checks.PromqlSeriesSettings{
  IgnoreMetrics: []string{"foo", "bar", "found"},
  }
@@ -3771,6 +3771,129 @@ func TestSeriesCheck(t *testing.T) {
  }
  },
  },
+ {
+ description: "series not present on other servers",
+ content: "- record: foo\n expr: notfound\n",
+ checker: newSeriesCheck,
+ prometheus: newSimpleProm,
+ otherProms: func(uri string) []*promapi.FailoverGroup {
+ var proms []*promapi.FailoverGroup
+ for i := range 5 {
+ proms = append(proms, simpleProm(fmt.Sprintf("prom%d", i), uri+"/other", time.Second, false))
+ }
+ return proms
+ },
+ problems: func(uri string) []checks.Problem {
+ return []checks.Problem{
+ {
+ Lines: parser.LineRange{
+ First: 2,
+ Last: 2,
+ },
+ Reporter: checks.SeriesCheckName,
+ Text: noMetricText("prom", uri, "notfound", "1w"),
+ Details: checks.SeriesCheckCommonProblemDetails,
+ Severity: checks.Bug,
+ },
+ }
+ },
+ mocks: []*prometheusMock{
+ {
+ conds: []requestCondition{requestPathCond{path: "/other/api/v1/query"}},
+ resp: respondWithEmptyVector(),
+ },
+ {
+ conds: []requestCondition{requireQueryPath},
+ resp: respondWithEmptyVector(),
+ },
+ {
+ conds: []requestCondition{requireRangeQueryPath},
+ resp: respondWithEmptyMatrix(),
+ },
+ },
+ },
+ {
+ description: "series present on other servers",
+ content: "- record: foo\n expr: notfound\n",
+ checker: newSeriesCheck,
+ prometheus: newSimpleProm,
+ otherProms: func(uri string) []*promapi.FailoverGroup {
+ var proms []*promapi.FailoverGroup
+ for i := range 5 {
+ proms = append(proms, simpleProm(fmt.Sprintf("prom%d", i), uri+"/other", time.Second, false))
+ }
+ return proms
+ },
+ problems: func(uri string) []checks.Problem {
+ return []checks.Problem{
+ {
+ Lines: parser.LineRange{
+ First: 2,
+ Last: 2,
+ },
+ Reporter: checks.SeriesCheckName,
+ Text: noMetricText("prom", uri, "notfound", "1w"),
+ Details: fmt.Sprintf("`notfound` was found on other prometheus servers:\n\n- [prom0](%s/other/graph?g0.expr=notfound)\n- [prom1](%s/other/graph?g0.expr=notfound)\n- [prom2](%s/other/graph?g0.expr=notfound)\n- [prom3](%s/other/graph?g0.expr=notfound)\n- [prom4](%s/other/graph?g0.expr=notfound)\n\nYou might be trying to deploy this rule to the wrong Prometheus server instance.\n", uri, uri, uri, uri, uri),
+ Severity: checks.Bug,
+ },
+ }
+ },
+ mocks: []*prometheusMock{
+ {
+ conds: []requestCondition{requestPathCond{path: "/other/api/v1/query"}},
+ resp: respondWithSingleInstantVector(),
+ },
+ {
+ conds: []requestCondition{requireQueryPath},
+ resp: respondWithEmptyVector(),
+ },
+ {
+ conds: []requestCondition{requireRangeQueryPath},
+ resp: respondWithEmptyMatrix(),
+ },
+ },
+ },
+ {
+ description: "series present on other servers / 15",
+ content: "- record: foo\n expr: notfound\n",
+ checker: newSeriesCheck,
+ prometheus: newSimpleProm,
+ otherProms: func(uri string) []*promapi.FailoverGroup {
+ var proms []*promapi.FailoverGroup
+ for i := range 15 {
+ proms = append(proms, simpleProm(fmt.Sprintf("prom%d", i), uri+"/other", time.Second, false))
+ }
+ return proms
+ },
+ problems: func(uri string) []checks.Problem {
+ return []checks.Problem{
+ {
+ Lines: parser.LineRange{
+ First: 2,
+ Last: 2,
+ },
+ Reporter: checks.SeriesCheckName,
+ Text: noMetricText("prom", uri, "notfound", "1w"),
+ Details: fmt.Sprintf("`notfound` was found on other prometheus servers:\n\n- [prom0](%s/other/graph?g0.expr=notfound)\n- [prom1](%s/other/graph?g0.expr=notfound)\n- [prom2](%s/other/graph?g0.expr=notfound)\n- [prom3](%s/other/graph?g0.expr=notfound)\n- [prom4](%s/other/graph?g0.expr=notfound)\n- [prom5](%s/other/graph?g0.expr=notfound)\n- [prom6](%s/other/graph?g0.expr=notfound)\n- [prom7](%s/other/graph?g0.expr=notfound)\n- [prom8](%s/other/graph?g0.expr=notfound)\n- [prom9](%s/other/graph?g0.expr=notfound)\n- and 5 other server(s).\n\nYou might be trying to deploy this rule to the wrong Prometheus server instance.\n", uri, uri, uri, uri, uri, uri, uri, uri, uri, uri),
+ Severity: checks.Bug,
+ },
+ }
+ },
+ mocks: []*prometheusMock{
+ {
+ conds: []requestCondition{requestPathCond{path: "/other/api/v1/query"}},
+ resp: respondWithSingleInstantVector(),
+ },
+ {
+ conds: []requestCondition{requireQueryPath},
+ resp: respondWithEmptyVector(),
+ },
+ {
+ conds: []requestCondition{requireRangeQueryPath},
+ resp: respondWithEmptyMatrix(),
+ },
+ },
+ },
  }
  runTests(t, testCases)
 }