From 72a6dd9aadfb5ede76abf9c9b96c4e3041521091 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Sat, 7 Nov 2026 11:42:44 +0100 Subject: [PATCH] fix(alerts): last_evaluated_at reflects every evaluation, not only errors listRules only read state rows that carried an error, so healthy rules always reported a null last evaluation. The time is now the newest across all of a rule's group rows (anomaly __total__ heartbeats included); the error still comes from the newest row that has one. The document returned by a rule update (v2 PATCH) gets the same state, so list, retrieve and update agree. --- apps/api/src/routes/v2/alerts.http.test.ts | 29 +++++ .../services/alerts/AlertRulesService.test.ts | 110 ++++++++++++++++++ .../src/services/alerts/AlertRulesService.ts | 82 +++++++++---- 3 files changed, 200 insertions(+), 21 deletions(-) diff --git a/apps/api/src/routes/v2/alerts.http.test.ts b/apps/api/src/routes/v2/alerts.http.test.ts index a3fe57110b..e2aea42df6 100644 --- a/apps/api/src/routes/v2/alerts.http.test.ts +++ b/apps/api/src/routes/v2/alerts.http.test.ts @@ -404,6 +404,35 @@ describe("v2 alerts over HTTP", () => { await harness.dispose() }) + it("returns last_evaluated_at for a healthy rule from list, retrieve and PATCH", async () => { + const harness = makeHarness() + const key = await harness.bootstrapKey(["alerts:write"]) + const { rule } = await createWebhookAndRule(harness, key.secret) + expect(rule.status).toBe(200) + expect(rule.body.last_evaluated_at).toBeNull() + + await executeSql( + harness.testDb, + `insert into alert_rule_states (org_id, rule_id, group_key, last_evaluated_at, updated_at) + select org_id, id, '__total__', timestamptz '2026-07-31 12:00:00+00', timestamptz '2026-07-31 12:00:00+00' + from alert_rules where org_id = 'org_alerts_e2e'`, + ) + + const evaluatedAt = "2026-07-31T12:00:00.000Z" + const listed = await harness.request("GET", "/v2/alerts/rules", key.secret) + expect(listed.body.data[0].last_evaluated_at).toBe(evaluatedAt) + expect(listed.body.data[0].last_evaluation_error).toBeNull() + const retrieved = await harness.request("GET", `/v2/alerts/rules/${rule.body.id}`, key.secret) + expect(retrieved.body.last_evaluated_at).toBe(evaluatedAt) + const patched = await harness.request("PATCH", `/v2/alerts/rules/${rule.body.id}`, key.secret, { + name: "Checkout errors", + }) + expect(patched.status).toBe(200) + expect(patched.body.last_evaluated_at).toBe(evaluatedAt) + + await harness.dispose() + }) + it("paginates delivery history beyond the first 100 rows", async () => { const harness = makeHarness() const key = await harness.bootstrapKey(["alerts:write"]) diff --git a/packages/backend/src/services/alerts/AlertRulesService.test.ts b/packages/backend/src/services/alerts/AlertRulesService.test.ts index 3d68274881..af8c1be9b8 100644 --- a/packages/backend/src/services/alerts/AlertRulesService.test.ts +++ b/packages/backend/src/services/alerts/AlertRulesService.test.ts @@ -61,6 +61,27 @@ const seedDestination = (testDb: TestDb) => [DESTINATION, ORG, msToDate(NOW), USER], ) +const seedState = ( + testDb: TestDb, + state: { groupKey: string; evaluatedAt: number; error?: string; errorKind?: string }, +) => + executeSql( + testDb, + `INSERT INTO alert_rule_states + (org_id, rule_id, group_key, last_evaluated_at, last_error, last_error_kind, updated_at) + VALUES ($1, $2, $3, $4, $5, $6, $4)`, + [ + ORG, + RULE, + state.groupKey, + msToDate(state.evaluatedAt), + state.error ?? null, + state.errorKind ?? null, + ], + ) + +const iso = (ms: number) => new Date(ms).toISOString() + const request = new AlertRuleUpsertRequest({ name: "Checkout error rate", severity: "critical", @@ -126,4 +147,93 @@ describe("AlertRulesService", () => { assert.strictEqual(error.destinationId, unknown) }).pipe(Effect.provide(makeLayer(testDb))) }) + + describe("evaluation state on listed rules", () => { + const listOnlyRule = ( + testDb: TestDb, + states: ReadonlyArray[1]>, + ruleRequest: AlertRuleUpsertRequest = request, + ) => + Effect.gen(function* () { + yield* Effect.promise(() => seedDestination(testDb)) + const rules = yield* AlertRulesService + yield* rules.createRule(ORG, USER, ADMIN_ROLES, ruleRequest) + yield* Effect.forEach(states, (state) => Effect.promise(() => seedState(testDb, state))) + const listed = yield* rules.listRules(ORG) + assert.strictEqual(listed.rules.length, 1) + return listed.rules[0] + }).pipe(Effect.provide(makeLayer(testDb))) + + it.effect("reports the last evaluation of a healthy rule", () => + Effect.gen(function* () { + const rule = yield* listOnlyRule(createTestDb(createdDbs), [ + { groupKey: "__total__", evaluatedAt: NOW + 60_000 }, + ]) + assert.strictEqual(rule?.lastEvaluatedAt, iso(NOW + 60_000)) + assert.strictEqual(rule?.lastEvaluationError, null) + assert.strictEqual(rule?.lastEvaluationErrorKind, null) + }), + ) + + it.effect("keeps the error and its time on an erroring rule", () => + Effect.gen(function* () { + const rule = yield* listOnlyRule(createTestDb(createdDbs), [ + { + groupKey: "__total__", + evaluatedAt: NOW + 60_000, + error: "timed out", + errorKind: "query_too_slow", + }, + ]) + assert.strictEqual(rule?.lastEvaluatedAt, iso(NOW + 60_000)) + assert.strictEqual(rule?.lastEvaluationError, "timed out") + assert.strictEqual(rule?.lastEvaluationErrorKind, "query_too_slow") + }), + ) + + it.effect("reports the newest time across group rows, with the error of the failing group", () => + Effect.gen(function* () { + const rule = yield* listOnlyRule(createTestDb(createdDbs), [ + { + groupKey: "checkout", + evaluatedAt: NOW + 60_000, + error: "bad query", + errorKind: "invalid_query", + }, + { groupKey: "payments", evaluatedAt: NOW + 180_000 }, + { groupKey: "cart", evaluatedAt: NOW + 120_000 }, + ]) + assert.strictEqual(rule?.lastEvaluatedAt, iso(NOW + 180_000)) + assert.strictEqual(rule?.lastEvaluationError, "bad query") + assert.strictEqual(rule?.lastEvaluationErrorKind, "invalid_query") + }), + ) + + it.effect("reports the heartbeat row of an anomaly rule", () => + Effect.gen(function* () { + const rule = yield* listOnlyRule( + createTestDb(createdDbs), + [{ groupKey: "__total__", evaluatedAt: NOW + 240_000 }], + new AlertRuleUpsertRequest({ + name: "Checkout anomalies", + signalType: "anomaly", + comparator: "lt", + windowMinutes: 5, + serviceNames: ["checkout"], + destinationIds: [DESTINATION], + }), + ) + assert.strictEqual(rule?.signalType, "anomaly") + assert.strictEqual(rule?.lastEvaluatedAt, iso(NOW + 240_000)) + }), + ) + + it.effect("leaves a never-evaluated rule without a time", () => + Effect.gen(function* () { + const rule = yield* listOnlyRule(createTestDb(createdDbs), []) + assert.strictEqual(rule?.lastEvaluatedAt, null) + assert.strictEqual(rule?.lastEvaluationError, null) + }), + ) + }) }) diff --git a/packages/backend/src/services/alerts/AlertRulesService.ts b/packages/backend/src/services/alerts/AlertRulesService.ts index 0a20ea1b74..353bef4b7a 100644 --- a/packages/backend/src/services/alerts/AlertRulesService.ts +++ b/packages/backend/src/services/alerts/AlertRulesService.ts @@ -59,6 +59,47 @@ import * as Policy from "@maple/domain/policy" const decodeErrorKindOption = Schema.decodeUnknownOption(AlertEvaluationErrorKind) const MAX_ACTIVE_ALERT_RULES_PER_ORG = 100 +interface EvaluationStateRow { + readonly ruleId: string + readonly lastError: string | null + readonly lastErrorKind: string | null + readonly lastEvaluatedAt: number | null +} + +// evaluatedAt is the newest tick across every group row (anomaly `__total__` heartbeats included). +// The error comes from the newest row still carrying one; a successful tick clears its row's error. +const evaluationStatesByRule = (rows: ReadonlyArray) => { + const evaluatedAtByRule = new Map() + const errorRowByRule = new Map() + for (const state of rows) { + const at = state.lastEvaluatedAt + const newest = evaluatedAtByRule.get(state.ruleId) + if (at !== null && (newest === undefined || at > newest)) evaluatedAtByRule.set(state.ruleId, at) + if (state.lastError === null) continue + const existing = errorRowByRule.get(state.ruleId) + if (existing === undefined || (at ?? 0) > (existing.lastEvaluatedAt ?? 0)) { + errorRowByRule.set(state.ruleId, state) + } + } + const ruleIds = new Set([...evaluatedAtByRule.keys(), ...errorRowByRule.keys()]) + return new Map( + Arr.map(Arr.fromIterable(ruleIds), (ruleId) => { + const errorRow = errorRowByRule.get(ruleId) + return [ + ruleId, + { + error: errorRow?.lastError ?? null, + errorKind: + errorRow === undefined + ? null + : Option.getOrNull(decodeErrorKindOption(errorRow.lastErrorKind)), + evaluatedAt: evaluatedAtByRule.get(ruleId) ?? null, + }, + ] + }), + ) +} + export interface AlertRulesServiceApi { readonly listRules: ( orgId: OrgId, @@ -416,6 +457,22 @@ export const makeAlertRulePersistence = (options: { } }) + const loadEvaluationStates = Effect.fn("AlertRulesService.loadEvaluationStates")(function* ( + orgId: OrgId, + ruleId?: AlertRuleId, + ) { + const stateRows = yield* dbExecute((db) => + db.run( + PG.from(AlertRuleStates) + .select("ruleId", "lastError", "lastErrorKind", "lastEvaluatedAt") + .where(($) => + ruleId === undefined ? [$.orgId.eq(orgId)] : [$.orgId.eq(orgId), $.ruleId.eq(ruleId)], + ), + ), + ) + return evaluationStatesByRule(stateRows) + }) + const upsertRuleRow = Effect.fn("AlertRulesService.upsertRuleRow")(function* ( orgId: OrgId, userId: UserId, @@ -429,7 +486,8 @@ export const makeAlertRulePersistence = (options: { message: "Alert rule row was not readable after it was saved", }) } - const document = yield* rowToRuleDocument(row) + const stateByRule = yield* loadEvaluationStates(orgId, ruleId) + const document = yield* rowToRuleDocument(row, stateByRule.get(ruleId)) return txid === undefined ? document : new AlertRuleDocument({ ...document, txid }) }) @@ -442,26 +500,8 @@ export const makeAlertRulePersistence = (options: { .orderBy(["createdAt", "desc"], ["id", "desc"]), ), ) - const stateRows = yield* dbExecute((db) => - db.run( - PG.from(AlertRuleStates) - .select("ruleId", "lastError", "lastErrorKind", "lastEvaluatedAt") - .where(($) => [$.orgId.eq(orgId)]), - ), - ) - const errorByRule = new Map() - for (const state of stateRows) { - if (state.lastError == null) continue - const existing = errorByRule.get(state.ruleId) - if (existing == null || (state.lastEvaluatedAt ?? 0) > (existing.evaluatedAt ?? 0)) { - errorByRule.set(state.ruleId, { - error: state.lastError, - errorKind: Option.getOrNull(decodeErrorKindOption(state.lastErrorKind)), - evaluatedAt: state.lastEvaluatedAt, - }) - } - } - const rules = yield* Effect.forEach(rows, (row) => rowToRuleDocument(row, errorByRule.get(row.id))) + const stateByRule = yield* loadEvaluationStates(orgId) + const rules = yield* Effect.forEach(rows, (row) => rowToRuleDocument(row, stateByRule.get(row.id))) return new AlertRulesListResponse({ rules }) })