Skip to content

Repository navigation

Fix keep_firing_for handling #2012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
prymitive merged 1 commit into from
Oct 21, 2026
+365 −74
Merged

Fix keep_firing_for handling #2012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Changelog

## v0.89.4

### Fixed

- Fixed `alerts/count` check undercounting alerts for rules with
`keep_firing_for` - [#2004](https://github.com/cloudflare/pint/issues/2004).

## v0.89.3

### Fixed
Expand Down
23 changes: 19 additions & 4 deletions internal/checks/alerts_count.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,27 @@ func (c AlertsCheck) Check(ctx context.Context, entry *discovery.Entry, _ []*dis
keepFiringForDur = entry.Rule.AlertingRule.KeepFiringFor.Value
}

var alerts int
// An alert fires once a range is longer than `for`.
// If `keep_firing_for` is set it will delay when the alert resolves.
var (
alerts int
firingFP uint64
firingUntil time.Time
)
for _, r := range qr.Series.Ranges {
// If `keepFiringFor` is not defined its Duration will be 0
if r.End.Sub(r.Start) > (forDur + keepFiringForDur) {
alerts++
if r.Fingerprint == firingFP && !r.Start.After(firingUntil) {
firingUntil = r.End.Add(time.Second + keepFiringForDur)
// Separate range of an already firing alert and within keep_firing_for value.
// This does not count as a new alert.
continue
}
if r.End.Sub(r.Start) <= forDur {
// Too short to fire due to `for` value, wouldn't fire at all.
continue
}
alerts++
firingFP = r.Fingerprint
firingUntil = r.End.Add(time.Second + keepFiringForDur)
}

if alerts < c.minCount {
Expand Down
210 changes: 197 additions & 13 deletions internal/checks/alerts_count_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,25 @@ func newAlertsCheck(prom *promapi.FailoverGroup) checks.RuleChecker {

func TestAlertsCountCheck(t *testing.T) {
content := "- alert: Foo Is Down\n expr: up{job=\"foo\"} == 0\n"
now := time.Now()

testCases := []checkTest{
{
// A recording rule is not checked, so no problems are reported.
description: "ignores recording rules",
content: "- record: foo\n expr: up == 0\n",
checker: newAlertsCheck,
prometheus: newSimpleProm,
},
{
// The expression does not parse, so no problems are reported.
description: "ignores rules with syntax errors",
content: "- alert: Foo Is Down\n expr: sum(\n",
checker: newAlertsCheck,
prometheus: newSimpleProm,
},
{
// The server rejects the query, so the check reports a problem with the error.
description: "bad request",
content: content,
checker: newAlertsCheck,
Expand All @@ -47,6 +51,7 @@ func TestAlertsCountCheck(t *testing.T) {
problems: true,
},
{
// The optional server is unreachable, so queries report a warning problem.
description: "connection refused / upstream not required / warning",
content: content,
checker: newAlertsCheck,
Expand All @@ -56,7 +61,7 @@ func TestAlertsCountCheck(t *testing.T) {
problems: true,
},
{
description: "empty response",
description: "empty response / 0 alerts",
content: content,
checker: newAlertsCheck,
prometheus: newSimpleProm,
Expand All @@ -72,7 +77,7 @@ func TestAlertsCountCheck(t *testing.T) {
problems: true,
},
{
description: "multiple alerts",
description: "multiple alerts / seven ranges for one series / 7 alerts",
content: content,
checker: newAlertsCheck,
prometheus: newSimpleProm,
Expand Down Expand Up @@ -147,7 +152,7 @@ func TestAlertsCountCheck(t *testing.T) {
problems: true,
},
{
description: "for: 10m",
description: "for: 10m / 2 ranges over 10m / 2 alerts",
content: "- alert: Foo Is Down\n for: 10m\n expr: up{job=\"foo\"} == 0\n",
checker: newAlertsCheck,
prometheus: newSimpleProm,
Expand Down Expand Up @@ -209,7 +214,7 @@ func TestAlertsCountCheck(t *testing.T) {
problems: true,
},
{
description: "minCount=2",
description: "minCount=2 / 2 alerts reported",
content: "- alert: Foo Is Down\n for: 10m\n expr: up{job=\"foo\"} == 0\n",
checker: func(prom *promapi.FailoverGroup) checks.RuleChecker {
return checks.NewAlertsCheck(prom, time.Hour*24, time.Minute, time.Minute*5, 2, "rule comment", checks.Information)
Expand Down Expand Up @@ -273,7 +278,7 @@ func TestAlertsCountCheck(t *testing.T) {
problems: true,
},
{
description: "minCount=2 severity=bug",
description: "minCount=2 severity=bug / 2 alerts reported as bug",
content: "- alert: Foo Is Down\n for: 10m\n expr: up{job=\"foo\"} == 0\n",
checker: func(prom *promapi.FailoverGroup) checks.RuleChecker {
return checks.NewAlertsCheck(prom, time.Hour*24, time.Minute, time.Minute*5, 2, "", checks.Bug)
Expand Down Expand Up @@ -337,7 +342,7 @@ func TestAlertsCountCheck(t *testing.T) {
problems: true,
},
{
description: "minCount=10",
description: "minCount=10 / 2 alerts below minCount, nothing reported",
content: "- alert: Foo Is Down\n for: 10m\n expr: up{job=\"foo\"} == 0\n",
checker: func(prom *promapi.FailoverGroup) checks.RuleChecker {
return checks.NewAlertsCheck(prom, time.Hour*24, time.Minute, time.Minute*5, 10, "", checks.Information)
Expand Down Expand Up @@ -400,7 +405,7 @@ func TestAlertsCountCheck(t *testing.T) {
},
},
{
description: "{__name__=}",
description: "{__name__=} / three ranges / 3 alerts",
content: `
- alert: foo
expr: '{__name__="up", job="foo"} == 0'
Expand Down Expand Up @@ -447,7 +452,7 @@ func TestAlertsCountCheck(t *testing.T) {
problems: true,
},
{
description: "{__name__=~}",
description: "{__name__=~} / three ranges / 3 alerts",
content: `
- alert: foo
expr: '{__name__=~"(up|foo)", job="foo"} == 0'
Expand Down Expand Up @@ -494,7 +499,7 @@ func TestAlertsCountCheck(t *testing.T) {
problems: true,
},
{
description: "uptime query error",
description: "uptime query error / gap detection skipped / 3 alerts",
content: "- alert: Foo Is Down\n expr: up{job=\"foo\"} == 0\n",
checker: newAlertsCheck,
prometheus: newSimpleProm,
Expand Down Expand Up @@ -538,7 +543,7 @@ func TestAlertsCountCheck(t *testing.T) {
problems: true,
},
{
description: "keep_firing_for: 10m",
description: "keep_firing_for: 10m / 6 ranges 1h apart / 6 alerts",
content: "- alert: Foo Is Down\n keep_firing_for: 10m\n expr: up{job=\"foo\"} == 0\n",
checker: newAlertsCheck,
prometheus: newSimpleProm,
Expand Down Expand Up @@ -600,7 +605,7 @@ func TestAlertsCountCheck(t *testing.T) {
problems: true,
},
{
description: "for: 10m + keep_firing_for: 10m",
description: "for: 10m + keep_firing_for: 10m / 2 ranges over 10m / 2 alerts",
content: "- alert: Foo Is Down\n for: 10m\n keep_firing_for: 10m\n expr: up{job=\"foo\"} == 0\n",
checker: newAlertsCheck,
prometheus: newSimpleProm,
Expand Down Expand Up @@ -661,10 +666,189 @@ func TestAlertsCountCheck(t *testing.T) {
},
problems: true,
},
{
description: "keep_firing_for=30m merges 15m gaps between ranges / 2 alerts",
content: "- alert: Foo Is Down\n keep_firing_for: 30m\n expr: up{job=\"foo\"} == 0\n",
checker: newAlertsCheck,
prometheus: newSimpleProm,
mocks: []*prometheusMock{
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: `up{job="foo"} == 0`},
},
resp: matrixResponse{
samples: []*model.SampleStream{
generateSampleStream(
map[string]string{"job": "foo"},
now.Add(time.Hour*-20),
now.Add(time.Hour*-20).Add(time.Minute*5),
time.Minute,
),
generateSampleStream(
map[string]string{"job": "foo"},
now.Add(time.Hour*-20).Add(time.Minute*20),
now.Add(time.Hour*-20).Add(time.Minute*25),
time.Minute,
),
generateSampleStream(
map[string]string{"job": "foo"},
now.Add(time.Hour*-20).Add(time.Minute*40),
now.Add(time.Hour*-20).Add(time.Minute*45),
time.Minute,
),
generateSampleStream(
map[string]string{"job": "foo"},
now.Add(time.Hour*-16),
now.Add(time.Hour*-16).Add(time.Minute*5),
time.Minute,
),
},
},
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: "count(\nup\n)"},
},
resp: respondWithSingleRangeVector1D(),
},
},
problems: true,
},
{
description: "keep_firing_for=2m covers a 2m gap but not wider gaps / 3 alerts",
content: "- alert: Foo Is Down\n keep_firing_for: 2m\n expr: up{job=\"foo\"} == 0\n",
checker: newAlertsCheck,
prometheus: newSimpleProm,
mocks: []*prometheusMock{
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: `up{job="foo"} == 0`},
},
resp: matrixResponse{
samples: []*model.SampleStream{
generateSampleStream(
map[string]string{"job": "foo"},
now.Add(time.Hour*-20),
now.Add(time.Hour*-20),
time.Minute,
),
generateSampleStream(
map[string]string{"job": "foo"},
now.Add(time.Hour*-20).Add(time.Minute*2),
now.Add(time.Hour*-20).Add(time.Minute*2),
time.Minute,
),
generateSampleStream(
map[string]string{"job": "foo"},
now.Add(time.Hour*-20).Add(time.Minute*8),
now.Add(time.Hour*-20).Add(time.Minute*8),
time.Minute,
),
generateSampleStream(
map[string]string{"job": "foo"},
now.Add(time.Hour*-20).Add(time.Minute*12),
now.Add(time.Hour*-20).Add(time.Minute*12),
time.Minute,
),
},
},
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: "count(\nup\n)"},
},
// Prometheus is up the whole window, so any gap is real.
resp: matrixResponse{
samples: []*model.SampleStream{
generateSampleStream(
map[string]string{},
now.Add(time.Hour*-24),
now,
time.Minute,
),
},
},
},
},
problems: true,
},
{
// The ranges at +40m and +70m start before keep_firing_for: 30m
// runs out, so foo has one alert. The last 2m range is too short
// for `for`. job=bar fires its own alert.
description: "for=10m + keep_firing_for=30m gives one alert per series / 2 alerts",
content: "- alert: Foo Is Down\n for: 10m\n keep_firing_for: 30m\n expr: up{job=\"foo\"} == 0\n",
checker: newAlertsCheck,
prometheus: newSimpleProm,
mocks: []*prometheusMock{
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: `up{job="foo"} == 0`},
},
resp: matrixResponse{
samples: []*model.SampleStream{
generateSampleStream(
map[string]string{"job": "bar"},
now.Add(time.Hour*-20),
now.Add(time.Hour*-20).Add(time.Minute*15),
time.Minute,
),
generateSampleStream(
map[string]string{"job": "foo"},
now.Add(time.Hour*-20),
now.Add(time.Hour*-20).Add(time.Minute*15),
time.Minute,
),
generateSampleStream(
map[string]string{"job": "foo"},
now.Add(time.Hour*-20).Add(time.Minute*40),
now.Add(time.Hour*-20).Add(time.Minute*42),
time.Minute,
),
generateSampleStream(
map[string]string{"job": "foo"},
now.Add(time.Hour*-20).Add(time.Minute*70),
now.Add(time.Hour*-20).Add(time.Minute*85),
time.Minute,
),
generateSampleStream(
map[string]string{"job": "foo"},
now.Add(time.Hour*-16),
now.Add(time.Hour*-16).Add(time.Minute*2),
time.Minute,
),
},
},
},
{
conds: []requestCondition{
requireRangeQueryPath,
formCond{key: "query", value: "count(\nup\n)"},
},
// Prometheus is up the whole window, so any gap is real.
resp: matrixResponse{
samples: []*model.SampleStream{
generateSampleStream(
map[string]string{},
now.Add(time.Hour*-24),
now,
time.Minute,
),
},
},
},
},
problems: true,
},
{
// Two 7m ranges separated by 3m, Prometheus was down during those 3m.
// Since Prometheus was down there's no gap, so ranges merge into ~17m, exceeding for: 10m.
description: "ranges merge when separated by Prometheus downtime",
description: "Prometheus down during gap / ranges merge / 1 alert",
content: "- alert: Foo Is Down\n for: 10m\n expr: up{job=\"foo\"} == 0\n",
checker: newAlertsCheck,
prometheus: newSimpleProm,
Expand Down Expand Up @@ -724,7 +908,7 @@ func TestAlertsCountCheck(t *testing.T) {
{
// Two 7m ranges but Prometheus was up the entire time.
// There's a gap between them, so they don't merge.
description: "ranges stay separate when gap is confirmed by uptime",
description: "Prometheus up during gap / ranges stay separate / 0 alerts",
content: "- alert: Foo Is Down\n for: 10m\n expr: up{job=\"foo\"} == 0\n",
checker: newAlertsCheck,
prometheus: newSimpleProm,
Expand Down
Loading
Loading