Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 33 additions & 0 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3161,6 +3161,39 @@ export async function countRecentAuditEventsForActorAndTarget(env: Env, actor: s
return row.count;
}

/** Same as {@link countRecentAuditEventsForActorAndTarget} but additionally scoped to a specific `outcome` +
* `detail`. Backs the PagerDuty cooldown (#9695): a repeat page is suppressed only by rows that ACTUALLY paged
* (outcome "completed", detail "triggered"), never by the suppression rows or the auto-resolve rows that share
* the same eventType/targetKey — otherwise a single page (or even a `denied` non-page) self-renewed the window
* every cron tick and the repo never paged again. */
export async function countRecentAuditEventsForActorTargetAndOutcome(
env: Env,
actor: string,
eventType: string,
targetKey: string,
outcome: string,
detail: string,
sinceIso: string,
): Promise<number> {
const db = getDb(env.DB);
const [row] = await db
.select({ count: sql<number>`count(*)` })
.from(auditEvents)
.where(
and(
eq(auditEvents.actor, actor),
eq(auditEvents.eventType, eventType),
eq(auditEvents.targetKey, targetKey),
eq(auditEvents.outcome, outcome),
eq(auditEvents.detail, detail),
gte(auditEvents.createdAt, sinceIso),
),
);
/* v8 ignore next -- count(*) always returns exactly one row; the empty-array guard only satisfies the destructure type. */
if (!row) return 0;
return row.count;
}

/** Shared by every `targetKey` literal-prefix `LIKE` scan below ({@link countRecentAuditEventsForActorInRepo},
* {@link findHottestReviewTargetForRepo}) so a repo name containing a SQL `LIKE` wildcard (`%`/`_`) is always
* matched literally, never as a pattern -- e.g. `owner/foo_bar` must never spuriously match `owner/fooXbar#...`'s
Expand Down
16 changes: 11 additions & 5 deletions src/services/notify-pagerduty.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { countRecentAuditEventsForActorAndTarget, recordAuditEvent } from "../db/repositories";
import { countRecentAuditEventsForActorTargetAndOutcome, recordAuditEvent } from "../db/repositories";
import { errorMessage } from "../utils/json";
import { meetsSeverityThreshold, resolveSeverityThreshold, type LoopoverSeverity } from "./severity-threshold";

Expand All @@ -21,6 +21,12 @@ import { meetsSeverityThreshold, resolveSeverityThreshold, type LoopoverSeverity
// every cron tick doesn't re-page every tick.

const PAGERDUTY_EVENTS_URL = "https://events.pagerduty.com/v2/enqueue";

// The audit `detail` written for a real page vs an auto-resolve. Both share outcome "completed", so the cooldown
// must key off `detail` to count only rows that ACTUALLY paged (#9695). Exported + used at both the write and
// the read site so the two spellings can never drift.
export const PAGERDUTY_AUDIT_DETAIL_TRIGGERED = "triggered";
export const PAGERDUTY_AUDIT_DETAIL_RESOLVED = "resolved";
// PagerDuty routing/integration keys are 32 lowercase hex characters.
const ROUTING_KEY_RE = /^[a-f0-9]{32}$/i;
const DEFAULT_MIN_SEVERITY: PagerDutySeverity = "error";
Expand Down Expand Up @@ -182,8 +188,8 @@ export async function triggerPagerDutyIncident(
// window can reach back across the deploy boundary. Querying both actors costs one extra indexed count and
// removes the whole risk category rather than requiring a precisely-timed follow-up cleanup.
const [recentPagesNewActor, recentPagesLegacyActor] = await Promise.all([
countRecentAuditEventsForActorAndTarget(env, "loopover", "external_notification.pagerduty", params.dedupKey, cooldownSinceIso),
countRecentAuditEventsForActorAndTarget(env, "gittensory", "external_notification.pagerduty", params.dedupKey, cooldownSinceIso),
countRecentAuditEventsForActorTargetAndOutcome(env, "loopover", "external_notification.pagerduty", params.dedupKey, "completed", PAGERDUTY_AUDIT_DETAIL_TRIGGERED, cooldownSinceIso),
countRecentAuditEventsForActorTargetAndOutcome(env, "gittensory", "external_notification.pagerduty", params.dedupKey, "completed", PAGERDUTY_AUDIT_DETAIL_TRIGGERED, cooldownSinceIso),
]);
const recentPages = recentPagesNewActor + recentPagesLegacyActor;
if (recentPages > 0) {
Expand Down Expand Up @@ -211,7 +217,7 @@ export async function triggerPagerDutyIncident(
signal: AbortSignal.timeout(5000),
});
if (!response.ok) throw new Error(`pagerduty_events_http_${response.status}`);
await auditPagerDutyNotification(env, { repoFullName: params.repoFullName, dedupKey: params.dedupKey }, "completed", "triggered", { source: resolution.source });
await auditPagerDutyNotification(env, { repoFullName: params.repoFullName, dedupKey: params.dedupKey }, "completed", PAGERDUTY_AUDIT_DETAIL_TRIGGERED, { source: resolution.source });
} catch (error) {
const message = errorMessage(error);
console.warn(JSON.stringify({ event: "pagerduty_trigger_failed", repo: params.repoFullName, message: message.slice(0, 200) }));
Expand Down Expand Up @@ -251,7 +257,7 @@ export async function resolvePagerDutyIncident(
signal: AbortSignal.timeout(5000),
});
if (!response.ok) throw new Error(`pagerduty_events_http_${response.status}`);
await auditPagerDutyNotification(env, { repoFullName: params.repoFullName, dedupKey: params.dedupKey }, "completed", "resolved", { source: resolution.source });
await auditPagerDutyNotification(env, { repoFullName: params.repoFullName, dedupKey: params.dedupKey }, "completed", PAGERDUTY_AUDIT_DETAIL_RESOLVED, { source: resolution.source });
} catch (error) {
const message = errorMessage(error);
console.warn(JSON.stringify({ event: "pagerduty_resolve_failed", repo: params.repoFullName, message: message.slice(0, 200) }));
Expand Down
54 changes: 54 additions & 0 deletions test/unit/notify-pagerduty.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,60 @@ describe("triggerPagerDutyIncident — cooldown gate (alert fatigue control #2)"
expect(calls).toHaveLength(1);
});

it("REGRESSION: a recent suppression (denied/cooldown_active) row does NOT itself suppress the next page (#9695)", async () => {
const calls = stubFetch();
const env = enabledEnv();
const fiveMinutesAgo = new Date(Date.now() - 5 * 60 * 1000).toISOString();
// Before #9695 this `denied` row was counted (the counter did not filter outcome/detail), so a single
// suppression self-renewed the window and the repo could never page again.
await recordAuditEvent(env, {
eventType: "external_notification.pagerduty",
actor: "loopover",
targetKey: "ops_anomaly:acme/widgets",
outcome: "denied",
detail: "cooldown_active",
metadata: {},
createdAt: fiveMinutesAgo,
});
await trigger(env);
expect(calls).toHaveLength(1); // it pages, because no ACTUAL page happened in the window
});

it("REGRESSION: a recent auto-resolve (completed/resolved) row does NOT suppress the next page (#9695)", async () => {
const calls = stubFetch();
const env = enabledEnv();
const fiveMinutesAgo = new Date(Date.now() - 5 * 60 * 1000).toISOString();
// resolved shares outcome "completed" with a real page, so an outcome-only filter would wrongly count it.
await recordAuditEvent(env, {
eventType: "external_notification.pagerduty",
actor: "loopover",
targetKey: "ops_anomaly:acme/widgets",
outcome: "completed",
detail: "resolved",
metadata: {},
createdAt: fiveMinutesAgo,
});
await trigger(env);
expect(calls).toHaveLength(1); // a re-page for a flapping condition is not blocked by its own resolve
});

it("REGRESSION: a recent failed page (error) does NOT suppress its own retry (#9695)", async () => {
const calls = stubFetch();
const env = enabledEnv();
const fiveMinutesAgo = new Date(Date.now() - 5 * 60 * 1000).toISOString();
await recordAuditEvent(env, {
eventType: "external_notification.pagerduty",
actor: "loopover",
targetKey: "ops_anomaly:acme/widgets",
outcome: "error",
detail: "The page failed",
metadata: {},
createdAt: fiveMinutesAgo,
});
await trigger(env);
expect(calls).toHaveLength(1); // nobody was paged, so the retry must go through
});

it("REGRESSION: a recent page recorded under the pre-rebrand 'loopover' actor still suppresses a duplicate within the cooldown window", async () => {
const calls = stubFetch();
const env = enabledEnv();
Expand Down