diff --git a/docs/changelog.md b/docs/changelog.md index 6616cb2b..1d53efab 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -8,6 +8,8 @@ - `promql/series` check was refactored and will now detect a range of problems. See [promql/series](checks/promql/series.md) for details. - `promql/regexp` severity is now `Bug` instead of a `Warning`. +- `promql/rate` check will no longer produce warnings, it will only + report issues that cause queries to never return anything. ## v0.14.0 diff --git a/docs/checks/promql/rate.md b/docs/checks/promql/rate.md index 5346adb4..9f979327 100644 --- a/docs/checks/promql/rate.md +++ b/docs/checks/promql/rate.md @@ -9,15 +9,7 @@ grand_parent: Documentation This check inspects `rate()` and `irate()` functions and warns if used duration is too low. It does so by first getting global `scrape_interval` value for selected Prometheus servers and comparing duration to it. -Reported issue depends on a few factors: - -For `rate()` function: -- If duration is less than 2x `scrape_interval` it will report a bug. -- If duration is between 2x and 4x `scrape_interval` it will report a warning. - -For `irate()` function: -- If duration is less than 2x `scrape_interval` it will report a bug. -- If duration is between 2x and 3x `scrape_interval` it will report a warning. +It will report a bug if duration is less than 2x `scrape_interval`. ## Configuration diff --git a/docs/checks/promql/series.md b/docs/checks/promql/series.md index 99752fee..5ae61047 100644 --- a/docs/checks/promql/series.md +++ b/docs/checks/promql/series.md @@ -28,7 +28,7 @@ of different issues. Here are some usual cases. ## Your query is using ALERTS or ALERTS_FOR_STATE metrics -Prometheus itself exposes metrics about active alerts [see docs here](https://prometheus.io/docs/prometheus/latest/configuration/alerting_rules/#inspecting-alerts-during-runtime. +Prometheus itself exposes [metrics about active alerts](https://prometheus.io/docs/prometheus/latest/configuration/alerting_rules/#inspecting-alerts-during-runtime). And it's possible to use those metrics in recording or alerting rules. If pint finds a query using either `ALERTS{alertname="..."}` or `ALERTS_FOR_STATE{alertname="..."}` selector it will check if there's diff --git a/internal/checks/promql_rate.go b/internal/checks/promql_rate.go index b28a2bcb..c53d0a8d 100644 --- a/internal/checks/promql_rate.go +++ b/internal/checks/promql_rate.go @@ -69,14 +69,11 @@ func (c RateCheck) Check(ctx context.Context, rule parser.Rule, entries []discov func (c RateCheck) checkNode(node *parser.PromQLNode, cfg *promapi.ConfigResult) (problems []exprProblem) { if n, ok := node.Node.(*promParser.Call); ok && (n.Func.Name == "rate" || n.Func.Name == "irate") { var minIntervals int - var recIntervals int switch n.Func.Name { case "rate": minIntervals = 2 - recIntervals = 4 case "irate": minIntervals = 2 - recIntervals = 3 } for _, arg := range n.Args { if m, ok := arg.(*promParser.MatrixSelector); ok { @@ -88,14 +85,6 @@ func (c RateCheck) checkNode(node *parser.PromQLNode, cfg *promapi.ConfigResult) severity: Bug, } problems = append(problems, p) - } else if m.Range < cfg.Config.Global.ScrapeInterval*time.Duration(recIntervals) { - p := exprProblem{ - expr: node.Expr, - text: fmt.Sprintf("duration for %s() is recommended to be at least %d x scrape_interval, %s is using %s scrape_interval", - n.Func.Name, recIntervals, promText(c.prom.Name(), cfg.URI), output.HumanizeDuration(cfg.Config.Global.ScrapeInterval)), - severity: Warning, - } - problems = append(problems, p) } } } diff --git a/internal/checks/promql_rate_test.go b/internal/checks/promql_rate_test.go index 51ab6498..45a8ac37 100644 --- a/internal/checks/promql_rate_test.go +++ b/internal/checks/promql_rate_test.go @@ -16,10 +16,6 @@ func durationMustText(name, uri, fun, multi, using string) string { return fmt.Sprintf(`duration for %s() must be at least %s x scrape_interval, prometheus %q at %s is using %s scrape_interval`, fun, multi, name, uri, using) } -func durationRecommenedText(name, uri, fun, multi, using string) string { - return fmt.Sprintf("duration for %s() is recommended to be at least %s x scrape_interval, prometheus %q at %s is using %s scrape_interval", fun, multi, name, uri, using) -} - func TestRateCheck(t *testing.T) { testCases := []checkTest{ { @@ -54,17 +50,7 @@ func TestRateCheck(t *testing.T) { description: "rate < 4x scrape_interval", content: "- record: foo\n expr: rate(foo[3m])\n", checker: newRateCheck, - problems: func(uri string) []checks.Problem { - return []checks.Problem{ - { - Fragment: "rate(foo[3m])", - Lines: []int{2}, - Reporter: "promql/rate", - Text: durationRecommenedText("prom", uri, "rate", "4", "1m"), - Severity: checks.Warning, - }, - } - }, + problems: noProblems, mocks: []*prometheusMock{ { conds: []requestCondition{requireConfigPath}, @@ -110,17 +96,7 @@ func TestRateCheck(t *testing.T) { description: "irate < 3x scrape_interval", content: "- record: foo\n expr: irate(foo[2m])\n", checker: newRateCheck, - problems: func(uri string) []checks.Problem { - return []checks.Problem{ - { - Fragment: "irate(foo[2m])", - Lines: []int{2}, - Reporter: "promql/rate", - Text: durationRecommenedText("prom", uri, "irate", "3", "1m"), - Severity: checks.Warning, - }, - } - }, + problems: noProblems, mocks: []*prometheusMock{ { conds: []requestCondition{requireConfigPath}, @@ -164,18 +140,8 @@ func TestRateCheck(t *testing.T) { - record: foo expr: irate({__name__="foo"}[2m]) `, - checker: newRateCheck, - problems: func(uri string) []checks.Problem { - return []checks.Problem{ - { - Fragment: `irate({__name__="foo"}[2m])`, - Lines: []int{3}, - Reporter: "promql/rate", - Text: durationRecommenedText("prom", uri, "irate", "3", "1m"), - Severity: checks.Warning, - }, - } - }, + checker: newRateCheck, + problems: noProblems, mocks: []*prometheusMock{ { conds: []requestCondition{requireConfigPath}, @@ -189,18 +155,8 @@ func TestRateCheck(t *testing.T) { - record: foo expr: irate({__name__=~"(foo|bar)_total"}[2m]) `, - checker: newRateCheck, - problems: func(uri string) []checks.Problem { - return []checks.Problem{ - { - Fragment: `irate({__name__=~"(foo|bar)_total"}[2m])`, - Lines: []int{3}, - Reporter: "promql/rate", - Text: durationRecommenedText("prom", uri, "irate", "3", "1m"), - Severity: checks.Warning, - }, - } - }, + checker: newRateCheck, + problems: noProblems, mocks: []*prometheusMock{ { conds: []requestCondition{requireConfigPath}, @@ -238,13 +194,6 @@ func TestRateCheck(t *testing.T) { checker: newRateCheck, problems: func(uri string) []checks.Problem { return []checks.Problem{ - { - Fragment: "rate(foo[3m])", - Lines: []int{2}, - Reporter: "promql/rate", - Text: durationRecommenedText("prom", uri, "rate", "4", "1m"), - Severity: checks.Warning, - }, { Fragment: "rate(bar[1m])", Lines: []int{2}, @@ -358,28 +307,6 @@ func TestRateCheck(t *testing.T) { }, }, }, - { - description: "irate < 3 x default 1m", - content: "- record: foo\n expr: irate(foo[2m])\n", - checker: newRateCheck, - problems: func(uri string) []checks.Problem { - return []checks.Problem{ - { - Fragment: "irate(foo[2m])", - Lines: []int{2}, - Reporter: "promql/rate", - Text: durationRecommenedText("prom", uri, "irate", "3", "1m"), - Severity: checks.Warning, - }, - } - }, - mocks: []*prometheusMock{ - { - conds: []requestCondition{requireConfigPath}, - resp: configResponse{yaml: "global: {}\n"}, - }, - }, - }, } runTests(t, testCases) }