Skip to content

fix(coroot): preserve alerts when observations are unknown #5844

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

Draft
devantler wants to merge 4 commits into
base: main
Choose a base branch
from
+220 −3
Draft

fix(coroot): preserve alerts when observations are unknown #5844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c5b5d1d
fix(coroot): preserve alerts when observations are unknown
devantler Nov 3, 2026
3a82cd9
fix(coroot): restart the pending period when a check report is missing
devantler Nov 4, 2026
3025a6d
Merge branch 'main' into codex/coroot-unknown-5594
devantler Nov 4, 2026
9a31207
chore: Apply megalinter fixes
botantler-1[bot] Nov 4, 2026
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
4 changes: 2 additions & 2 deletions .github/workflows/publish-coroot-checkpoint-hotfix.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
patch: coroot-v1.28.1-checkpoint-record.patch
dockerfile: CorootCheckpoint.Dockerfile
image: platform-coroot
version: v1.28.1-checkpoints.1
version: v1.28.1-checkpoints.2
- component: collector
repository: coroot/coroot-cluster-agent
source: 4291458563fa8ea55615e23e0f11412db79742b6
Expand Down Expand Up @@ -126,7 +126,7 @@ jobs:
patch: coroot-v1.28.1-checkpoint-record.patch
dockerfile: CorootCheckpoint.Dockerfile
image: platform-coroot
version: v1.28.1-checkpoints.1
version: v1.28.1-checkpoints.2
- component: collector
repository: coroot/coroot-cluster-agent
source: 4291458563fa8ea55615e23e0f11412db79742b6
Expand Down
217 changes: 217 additions & 0 deletions k8s/bases/infrastructure/coroot/coroot-v1.28.1-checkpoint-record.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,3 +157,220 @@ index 0a6938f..05ac049 100644

DatabaseSize map[string]*timeseries.TimeSeries
TableSize map[DbTableKey]*timeseries.TimeSeries
diff --git a/watchers/alerts.go b/watchers/alerts.go
index 6a2b363..559e289 100644
--- a/watchers/alerts.go
+++ b/watchers/alerts.go
@@ -105,6 +105,8 @@ func (w *Alerts) Check(project *db.Project, world *model.World, from, to timeser
}
check, report := app.GetCheckWithReport(rule.Source.Check.CheckId)
if check == nil {
+ // A missing report is unobserved time too: restart any pending period.
+ delete(w.pendingAlerts, calcFingerprint(string(rule.Id), app.Id.String(), nil))
continue
}
alertsEvaluated++
@@ -137,6 +139,11 @@ func (w *Alerts) Check(project *db.Project, world *model.World, from, to timeser

func (w *Alerts) evaluateCheckAlert(project *db.Project, rule *model.AlertingRule, app *model.Application, check *model.Check, report model.AuditReportName, now timeseries.Time) {
fingerprint := calcFingerprint(string(rule.Id), app.Id.String(), nil)
+ if check.Status == model.UNKNOWN {
+ // Missing observations establish neither recovery nor a continuous breach.
+ delete(w.pendingAlerts, fingerprint)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
+ return
+ }

isFiring := check.Status > model.OK

diff --git a/watchers/alerts_test.go b/watchers/alerts_test.go
new file mode 100644
index 0000000..1042ad8
--- /dev/null
+++ b/watchers/alerts_test.go
@@ -0,0 +1,186 @@
+package watchers
+
+import (
+ "testing"
+
+ "github.com/coroot/coroot/auditor"
+ "github.com/coroot/coroot/db"
+ "github.com/coroot/coroot/model"
+ "github.com/coroot/coroot/timeseries"
+ "github.com/stretchr/testify/require"
+)
+
+func checkAlertFixture(t *testing.T) (*db.DB, *db.Project, *Alerts, *model.World, *model.Application, *model.AlertingRule) {
+ t.Helper()
+ database, err := db.NewSqlite(t.TempDir(), false)
+ require.NoError(t, err)
+ t.Cleanup(func() { require.NoError(t, database.DB().Close()) })
+ require.NoError(t, database.Migrate())
+ project := &db.Project{Id: "test-project", Name: "test-project"}
+ _, err = database.Exec("INSERT INTO project (id, name) VALUES ($1, $2)", project.Id, project.Name)
+ require.NoError(t, err)
+ rule := &model.AlertingRule{
+ Id: "test-check",
+ Name: "test check",
+ Enabled: true,
+ Severity: model.WARNING,
+ Source: model.AlertSource{
+ Type: model.AlertSourceTypeCheck,
+ Check: &model.CheckSource{CheckId: model.Checks.MemoryOOM.Id},
+ },
+ Selector: model.AppSelector{Type: model.AppSelectorTypeAll},
+ }
+ require.NoError(t, database.CreateAlertingRule(project.Id, rule))
+ now := timeseries.Now()
+ world := model.NewWorld(now.Add(-timeseries.Hour), now, timeseries.Minute, timeseries.Minute)
+ app := world.GetOrCreateApplication(model.NewApplicationId(string(project.Id), "default", model.ApplicationKindDeployment, "service"), false)
+ return database, project, NewAlerts(database, nil, nil, nil, nil), world, app, rule
+}
+
+func TestCheckAlertUnknownDoesNotRecover(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ severity model.Status
+ suppressed bool
+ keepFiring timeseries.Duration
+ }{
+ {name: "warning", severity: model.WARNING},
+ {name: "critical after keep firing expires", severity: model.CRITICAL, keepFiring: 10 * timeseries.Minute},
+ {name: "suppressed warning", severity: model.WARNING, suppressed: true},
+ {name: "suppressed critical", severity: model.CRITICAL, suppressed: true, keepFiring: 10 * timeseries.Minute},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ database, project, watcher, world, app, rule := checkAlertFixture(t)
+ rule.Severity = tc.severity
+ rule.KeepFiringFor = tc.keepFiring
+ require.NoError(t, database.UpdateAlertingRule(project.Id, rule))
+ app.Reports = []*model.AuditReport{{Name: model.AuditReportMemory, Checks: []*model.Check{{Id: rule.Source.Check.CheckId, Status: tc.severity, Message: "synthetic failure"}}}}
+ watcher.Check(project, world, world.Ctx.From, world.Ctx.To, world.Ctx.Step)
+ alerts, err := database.GetLatestAlertsByRule(project.Id, string(rule.Id))
+ require.NoError(t, err)
+ require.Len(t, alerts, 1)
+ before := alerts[0]
+ if tc.suppressed {
+ require.NoError(t, database.SuppressAlerts(project.Id, []string{before.Id}, "test"))
+ before, err = database.GetAlert(project.Id, before.Id)
+ require.NoError(t, err)
+ }
+
+ // Exercise the real auditor's missing-container path, rather than
+ // treating a hand-built OK check as a stand-in for unavailable data.
+ app.Reports = nil
+ auditor.Audit(world, project, nil, nil)
+ check, report := app.GetCheckWithReport(rule.Source.Check.CheckId)
+ require.NotNil(t, check)
+ require.Equal(t, model.UNKNOWN, check.Status)
+ watcher.evaluateCheckAlert(project, rule, app, check, report, before.UpdatedAt.Add(timeseries.Hour))
+
+ after, err := database.GetAlert(project.Id, before.Id)
+ require.NoError(t, err)
+ require.Zero(t, after.ResolvedAt, "missing telemetry is not recovery")
+ require.Equal(t, tc.suppressed, after.Suppressed, "missing telemetry must not clear suppression")
+ require.Equal(t, before.UpdatedAt, after.UpdatedAt, "no successful or failing observation was made")
+ })
+ }
+}
+
+func TestCheckAlertUnknownRestartsPendingPeriod(t *testing.T) {
+ database, project, watcher, _, app, rule := checkAlertFixture(t)
+ rule.For = 10 * timeseries.Minute
+ check := &model.Check{Id: rule.Source.Check.CheckId, Status: model.WARNING, Message: "synthetic failure"}
+ now := timeseries.Now()
+ watcher.evaluateCheckAlert(project, rule, app, check, model.AuditReportMemory, now)
+ check.Status = model.UNKNOWN
+ watcher.evaluateCheckAlert(project, rule, app, check, model.AuditReportMemory, now.Add(5*timeseries.Minute))
+ check.Status = model.WARNING
+ watcher.evaluateCheckAlert(project, rule, app, check, model.AuditReportMemory, now.Add(11*timeseries.Minute))
+ alerts, err := database.GetLatestAlertsByRule(project.Id, string(rule.Id))
+ require.NoError(t, err)
+ require.Empty(t, alerts, "unobserved time must not count toward a continuous breach")
+ watcher.evaluateCheckAlert(project, rule, app, check, model.AuditReportMemory, now.Add(21*timeseries.Minute))
+ alerts, err = database.GetLatestAlertsByRule(project.Id, string(rule.Id))
+ require.NoError(t, err)
+ require.Len(t, alerts, 1)
+}
+
+func TestCheckAlertOKStillRecoversAfterKeepFiring(t *testing.T) {
+ database, project, watcher, _, app, rule := checkAlertFixture(t)
+ rule.KeepFiringFor = 10 * timeseries.Minute
+ check := &model.Check{Id: rule.Source.Check.CheckId, Status: model.WARNING, Message: "synthetic failure"}
+ watcher.evaluateCheckAlert(project, rule, app, check, model.AuditReportMemory, timeseries.Now())
+ alerts, err := database.GetLatestAlertsByRule(project.Id, string(rule.Id))
+ require.NoError(t, err)
+ require.Len(t, alerts, 1)
+ before := alerts[0]
+ check.Status = model.OK
+ watcher.evaluateCheckAlert(project, rule, app, check, model.AuditReportMemory, before.UpdatedAt.Add(timeseries.Minute))
+ after, err := database.GetAlert(project.Id, before.Id)
+ require.NoError(t, err)
+ require.Zero(t, after.ResolvedAt)
+ watcher.evaluateCheckAlert(project, rule, app, check, model.AuditReportMemory, before.UpdatedAt.Add(11*timeseries.Minute))
+ after, err = database.GetAlert(project.Id, before.Id)
+ require.NoError(t, err)
+ require.NotZero(t, after.ResolvedAt)
+}
+
+func TestCheckAlertDisabledRuleStillRetiresUnknown(t *testing.T) {
+ database, project, watcher, world, app, rule := checkAlertFixture(t)
+ app.Reports = []*model.AuditReport{{Name: model.AuditReportMemory, Checks: []*model.Check{{Id: rule.Source.Check.CheckId, Status: model.WARNING, Message: "synthetic failure"}}}}
+ watcher.Check(project, world, world.Ctx.From, world.Ctx.To, world.Ctx.Step)
+ alerts, err := database.GetLatestAlertsByRule(project.Id, string(rule.Id))
+ require.NoError(t, err)
+ require.Len(t, alerts, 1)
+ app.Reports = nil
+ auditor.Audit(world, project, nil, nil)
+ rule.Enabled = false
+ require.NoError(t, database.UpdateAlertingRule(project.Id, rule))
+ watcher.Check(project, world, world.Ctx.From, world.Ctx.To, world.Ctx.Step)
+ after, err := database.GetAlert(project.Id, alerts[0].Id)
+ require.NoError(t, err)
+ require.NotZero(t, after.ResolvedAt)
+}
+
+func TestCheckAlertMissingReportIsNotRecovery(t *testing.T) {
+ database, project, watcher, world, app, rule := checkAlertFixture(t)
+ rule.Source.Check.CheckId = model.Checks.PostgresAvailability.Id
+ require.NoError(t, database.UpdateAlertingRule(project.Id, rule))
+ app.Reports = []*model.AuditReport{{Name: model.AuditReportPostgres, Checks: []*model.Check{{Id: rule.Source.Check.CheckId, Status: model.WARNING, Message: "synthetic failure"}}}}
+ watcher.Check(project, world, world.Ctx.From, world.Ctx.To, world.Ctx.Step)
+ alerts, err := database.GetLatestAlertsByRule(project.Id, string(rule.Id))
+ require.NoError(t, err)
+ require.Len(t, alerts, 1)
+ app.Reports = nil
+ auditor.Audit(world, project, nil, nil)
+ check, _ := app.GetCheckWithReport(rule.Source.Check.CheckId)
+ require.Nil(t, check)
+ watcher.Check(project, world, world.Ctx.From, world.Ctx.To, world.Ctx.Step)
+ after, err := database.GetAlert(project.Id, alerts[0].Id)
+ require.NoError(t, err)
+ require.Zero(t, after.ResolvedAt)
+}
+
+func TestCheckAlertMissingReportRestartsPendingPeriod(t *testing.T) {
+ database, project, watcher, world, app, rule := checkAlertFixture(t)
+ rule.Source.Check.CheckId = model.Checks.PostgresAvailability.Id
+ rule.For = 10 * timeseries.Minute
+ require.NoError(t, database.UpdateAlertingRule(project.Id, rule))
+ check := &model.Check{Id: rule.Source.Check.CheckId, Status: model.WARNING, Message: "synthetic failure"}
+ now := timeseries.Now()
+ watcher.evaluateCheckAlert(project, rule, app, check, model.AuditReportPostgres, now)
+ fingerprint := calcFingerprint(string(rule.Id), app.Id.String(), nil)
+ require.Contains(t, watcher.pendingAlerts, fingerprint)
+ app.Reports = nil
+ auditor.Audit(world, project, nil, nil)
+ missing, _ := app.GetCheckWithReport(rule.Source.Check.CheckId)
+ require.Nil(t, missing)
+ watcher.Check(project, world, world.Ctx.From, world.Ctx.To, world.Ctx.Step)
+ require.NotContains(t, watcher.pendingAlerts, fingerprint, "a missing report must not keep the pending period running")
+ watcher.evaluateCheckAlert(project, rule, app, check, model.AuditReportPostgres, now.Add(11*timeseries.Minute))
+ alerts, err := database.GetLatestAlertsByRule(project.Id, string(rule.Id))
+ require.NoError(t, err)
+ require.Empty(t, alerts, "the breach must satisfy the full For period again after a missing report")
+ watcher.evaluateCheckAlert(project, rule, app, check, model.AuditReportPostgres, now.Add(22*timeseries.Minute))
+ alerts, err = database.GetLatestAlertsByRule(project.Id, string(rule.Id))
+ require.NoError(t, err)
+ require.Len(t, alerts, 1)
+}
2 changes: 1 addition & 1 deletion scripts/capture-openbao-replacement/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ workflow runs it in `preflight` mode and then runs both judges for the server it
The expected servers and release come from the reviewed render, never from the cluster.

| Exit | Line | Meaning |
|---|---|---|
| --- | --- | --- |
| 0 | `OPENBAO_REPLACEMENT_EVIDENCE=CAPTURED` | `health.json` and `quorum.json` are in `--out`, readable only by the owner. |
| 2 | `OPENBAO_REPLACEMENT_EVIDENCE=UNVERIFIED stage=<stage>` | A read failed, so nothing was written. Invalid arguments also exit 2. |

Expand Down
Loading