From e73dea735ca74c79cf1b223438295408810a7969 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 13 Jul 2026 22:59:05 -0700 Subject: [PATCH] feat(observability): shared severity-threshold resolver for Sentry + PagerDuty Every self-host observability capture point fired unconditionally before this: captureError/captureReviewFailure always forwarded to Sentry, and forwardStructuredLogToSentry hardcoded an error/fatal-only cutoff with no way to configure it. The one severity-threshold precedent (PagerDuty's PAGERDUTY_MIN_SEVERITY/PAGERDUTY_REPO_MIN_SEVERITY) was scoped to paging only, not general Sentry/log verbosity. Extracts a shared resolveSeverityThreshold (src/services/severity-threshold.ts): global env var + per-repo JSON-map override, the same precedence every other alerting channel (Discord, PagerDuty) already uses. Wires it into all three sentry.ts capture paths (SENTRY_MIN_SEVERITY / SENTRY_REPO_MIN_SEVERITY) and retrofits notify-pagerduty.ts's own resolver to delegate to the shared one instead of keeping a parallel copy. Default threshold (error) is byte-identical to every path's pre-existing behavior; an operator can lower a specific repo's threshold to warning/info for full visibility while actively debugging it, without raising Sentry noise everywhere else. Regenerated the self-host env reference for the 2 new vars. Closes #5119 --- .../src/lib/selfhost-env-reference.ts | 5 - src/env.d.ts | 11 ++ src/selfhost/sentry.ts | 66 ++++++++++-- src/services/notify-pagerduty.ts | 24 ++--- src/services/severity-threshold.ts | 63 +++++++++++ test/unit/selfhost-sentry.test.ts | 101 +++++++++++++++++- test/unit/severity-threshold.test.ts | 69 ++++++++++++ 7 files changed, 310 insertions(+), 29 deletions(-) create mode 100644 src/services/severity-threshold.ts create mode 100644 test/unit/severity-threshold.test.ts diff --git a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts index 2155e0e845..f0451f4148 100644 --- a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts +++ b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts @@ -353,10 +353,6 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ name: "PAGERDUTY_COOLDOWN_MINUTES", firstReference: "src/services/notify-pagerduty.ts", }, - { - name: "PAGERDUTY_MIN_SEVERITY", - firstReference: "src/services/notify-pagerduty.ts", - }, { name: "PAGERDUTY_ROUTING_KEY", firstReference: "src/services/notify-pagerduty.ts", @@ -573,7 +569,6 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [ "| `OTEL_TRACES_SAMPLER` | `src/selfhost/otel.ts` |", "| `OTEL_TRACES_SAMPLER_ARG` | `src/selfhost/otel.ts` |", "| `PAGERDUTY_COOLDOWN_MINUTES` | `src/services/notify-pagerduty.ts` |", - "| `PAGERDUTY_MIN_SEVERITY` | `src/services/notify-pagerduty.ts` |", "| `PAGERDUTY_ROUTING_KEY` | `src/services/notify-pagerduty.ts` |", "| `PGPOOL_MAX` | `src/selfhost/queue-common.ts` |", "| `PGVECTOR_ENABLED` | `src/server.ts` |", diff --git a/src/env.d.ts b/src/env.d.ts index 30da857d71..f8e24c3e0d 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -256,6 +256,17 @@ declare global { * Defaults to 60 when unset. This is on top of PagerDuty's own `dedup_key` coalescing (which prevents * duplicate *incidents*, not duplicate *pages* for a still-open one). */ PAGERDUTY_COOLDOWN_MINUTES?: string; + /** Sentry noise control (#5119): the minimum severity (`info` < `warning` < `error` < `critical`) that + * reaches Sentry from captureError/captureReviewFailure/forwardStructuredLogToSentry (src/selfhost/sentry.ts), + * for any repo not present in SENTRY_REPO_MIN_SEVERITY (a JSON `{repoFullName: severity}` map, same + * deliberately-untyped pattern as PAGERDUTY_REPO_MIN_SEVERITY). Defaults to `error` when unset — matches + * every capture path's pre-#5119 behavior exactly, so an operator who never touches these vars sees no + * change. Lower a specific repo's threshold via the map (e.g. to `info`) for full visibility while + * actively debugging it, without raising Sentry noise everywhere else. Shares one severity-threshold + * resolver with PAGERDUTY_MIN_SEVERITY (src/services/severity-threshold.ts) — not a parallel concept. */ + SENTRY_MIN_SEVERITY?: string; + /** Per-repo override map for SENTRY_MIN_SEVERITY — see its doc comment for the shape and precedence. */ + SENTRY_REPO_MIN_SEVERITY?: string; GITTENSORY_CONTRIBUTOR_ISSUE_TOKEN?: string; PRODUCT_USAGE_HASH_SALT?: string; /** Server-to-server API bearer token — bypasses per-repo write checks (src/auth/security.ts). */ diff --git a/src/selfhost/sentry.ts b/src/selfhost/sentry.ts index 2fba90fdc9..5d98c55cc8 100644 --- a/src/selfhost/sentry.ts +++ b/src/selfhost/sentry.ts @@ -14,6 +14,7 @@ import { } from "./otel"; import { hashedInstallationIdWith } from "./review-tracing"; import { queueDeadLetterReviveIntervalMs } from "./queue-common"; +import { meetsSeverityThreshold, resolveSeverityThreshold, type LoopoverSeverity } from "../services/severity-threshold"; type SentryNs = typeof import("@sentry/node"); type SentryClient = NonNullable>; @@ -432,6 +433,41 @@ export async function buildSentryOpenTelemetryBridge(): Promise | undefined): string { + if (!context) return ""; + const repo = typeof context.repo === "string" ? context.repo : typeof context.repository === "string" ? context.repository : undefined; + return repo ?? ""; +} + +/** Resolve the minimum severity Sentry capture for `repoFullName`: SENTRY_REPO_MIN_SEVERITY (a JSON + * `{repoFullName: severity}` map) wins, else the global SENTRY_MIN_SEVERITY, else `"error"` -- the quietest + * safe default, matching today's de facto behavior (every capture path below was already error/fatal-only + * before this resolver existed). Reads the real Node `process.env` directly: captureError/ + * captureReviewFailure/forwardStructuredLogToSentry take no `env` parameter (initSentry's own env argument is + * not retained), so this is the only env self-host functions in this file can reach at capture time. */ +function resolveSentryMinSeverity(repoFullName: string): LoopoverSeverity { + const processEnv = (globalThis as unknown as { process?: { env?: Record } }).process?.env ?? {}; + return resolveSeverityThreshold(processEnv as unknown as Env, repoFullName, "SENTRY_MIN_SEVERITY", "SENTRY_REPO_MIN_SEVERITY"); +} + +/** Map a structured log's own `level` field (Sentry-native `debug`/`info`/`warning`/`warn`/`error`/`fatal`) onto + * the shared 4-tier {@link LoopoverSeverity} taxonomy for threshold comparison. `debug` folds into `info` (the + * taxonomy has no separate debug tier, matching PagerDutySeverity's shape for consistency -- #5119). A level + * that ISN'T one of these recognized severity words (e.g. `"audit"` -- a log CATEGORY, not a severity grade) + * is treated as the quietest tier (`info`), never promoted to `error` -- matching this function's pre-#5119 + * behavior of silently skipping anything that wasn't literally `error`/`fatal`. */ +function normalizeLoopoverSeverity(level: string): LoopoverSeverity { + const lower = level.toLowerCase(); + if (lower === "critical" || lower === "fatal") return "critical"; + if (lower === "error") return "error"; + if (lower === "warning" || lower === "warn") return "warning"; + return "info"; +} + /** Name a captured Error before capture so its Sentry issue title reads "eventName: message" instead of the * generic "Error: message" (or a caught exception's own class name, e.g. "HttpError: ..."). Mirrors * forwardStructuredLogToSentry's `errorEvent.name = event` below, but never mutates the caught value: some @@ -449,7 +485,9 @@ function namedCaptureError(error: unknown, eventName?: string): Error { return namedError; } -/** Capture an error with optional structured context. No-op when Sentry is off. `eventName`, when given, becomes +/** Capture an error with optional structured context. No-op when Sentry is off OR the repo's resolved severity + * threshold (#5119) is above `error` (the fixed grade every call here represents) -- suppressed from Sentry, + * still visible in Workers Logs/stdout via the console call that led here. `eventName`, when given, becomes * the Sentry issue title's prefix (see {@link namedCaptureError}) AND the grouping fingerprint (#5010) -- * Sentry's default stack-trace-based grouping fragments the SAME logical failure into separate issues whenever * it is captured from more than one call site (e.g. two different functions each constructing the identical @@ -461,6 +499,7 @@ export function captureError( eventName?: string, ): void { if (!active || !Sentry) return; + if (!meetsSeverityThreshold("error", resolveSentryMinSeverity(contextRepoFullName(context)))) return; Sentry.withScope((scope) => { setOtelTraceScope(scope); if (context) { const safeContext = hashedInstallationContext(context); scope.setContext("gittensory", safeContext); applyOperationalTags(scope, safeContext); } @@ -470,7 +509,8 @@ export function captureError( } /** Capture a failed review at ERROR level, tagged by repo/PR/SHA for triage. A review that cannot be produced is a - * real failure the maintainer must SEE — not a warning that hides in the noise. No-op when off. `eventName`, when + * real failure the maintainer must SEE — not a warning that hides in the noise. No-op when off OR the repo's + * resolved severity threshold (#5119) is above `error` (this always captures at error grade). `eventName`, when * given, becomes the Sentry issue title's prefix AND the grouping fingerprint -- see {@link captureError}'s * identical discipline and #5010. */ export function captureReviewFailure( @@ -479,6 +519,7 @@ export function captureReviewFailure( eventName?: string, ): void { if (!active || !Sentry) return; + if (!meetsSeverityThreshold("error", resolveSentryMinSeverity(contextRepoFullName(context)))) return; Sentry.withScope((scope) => { scope.setLevel("error"); setOtelTraceScope(scope); @@ -562,10 +603,14 @@ function summarizeLogFields(obj: Record): string { .join(", "); } -/** Forward a structured console line to Sentry when it is an ERROR-level log. The engine logs operational - * failures (orb_broker_unavailable, gate-check errors, relay drops, …) as JSON strings, often via console.error. - * No-op when Sentry is off, the line isn't a JSON object string, or its level isn't error/fatal — routine logs - * (audit/info/no-level: job_complete, regate_sweep_throttled, …) are intentionally skipped. */ +/** Forward a structured console line to Sentry when its level meets the repo's resolved severity threshold + * (#5119, default `error` — matches this function's pre-#5119 hardcoded error/fatal-only behavior byte for + * byte). The engine logs operational failures (orb_broker_unavailable, gate-check errors, relay drops, …) as + * JSON strings, often via console.error. No-op when Sentry is off, the line isn't a JSON object string, or it + * carries no level at all (and isn't from the error sink) — a log with no severity signal is a data-completeness + * gap, not a below-threshold decision, so it is always skipped regardless of any repo's configured threshold. + * An operator can lower a specific repo's threshold (SENTRY_REPO_MIN_SEVERITY) to `warning` or `info` to see + * routine logs from that repo while actively debugging it, without raising Sentry noise everywhere else. */ export function forwardStructuredLogToSentry(line: unknown, fromErrorSink = false): void { if (!active || !Sentry) return; if (typeof line !== "string" || line.charCodeAt(0) !== 123 /* "{" */) return; @@ -579,11 +624,14 @@ export function forwardStructuredLogToSentry(line: unknown, fromErrorSink = fals const safeObj = hashedInstallationContext(obj); // A console.error sink is error-level by DEFAULT even when the JSON omits an explicit level (many engine error // logs do) — that's how those errors reach Sentry instead of printing to stderr and vanishing. An EXPLICIT level - // always wins, so a deliberate level:"warn" emitted via console.error is still skipped. + // always wins over the error-sink default. const explicitLevel = typeof obj.level === "string" ? obj.level : undefined; const level = explicitLevel ?? (fromErrorSink ? "error" : undefined); - if (level !== "error" && level !== "fatal") return; - const severity = level === "fatal" ? "fatal" : "error"; + if (!level) return; // no severity signal at all — never forwarded, independent of any threshold + const loopoverSeverity = normalizeLoopoverSeverity(level); + if (!meetsSeverityThreshold(loopoverSeverity, resolveSentryMinSeverity(contextRepoFullName(safeObj)))) return; + // Sentry's own native level string (setLevel below) — critical maps back to "fatal", its Sentry-native spelling. + const severity = loopoverSeverity === "critical" ? "fatal" : loopoverSeverity === "warning" ? "warning" : loopoverSeverity === "info" ? "info" : "error"; const event = typeof obj.event === "string" ? obj.event : undefined; // Lead the Sentry title with the real failure detail (message → error), not just the event slug, so an operator // sees WHAT broke straight from the issue list instead of having to open the context blob. diff --git a/src/services/notify-pagerduty.ts b/src/services/notify-pagerduty.ts index 76ce013e4a..073aeaf4a0 100644 --- a/src/services/notify-pagerduty.ts +++ b/src/services/notify-pagerduty.ts @@ -1,5 +1,6 @@ import { countRecentAuditEventsForActorAndTarget, recordAuditEvent } from "../db/repositories"; import { errorMessage } from "../utils/json"; +import { meetsSeverityThreshold, resolveSeverityThreshold, type LoopoverSeverity } from "./severity-threshold"; // PagerDuty Events API v2 (https://developer.pagerduty.com/docs/events-api-v2/overview/). Experimental, // default-OFF (LOOPOVER_ENABLE_PAGERDUTY) — a self-host operator opts in per #4937's paging epic. @@ -88,23 +89,18 @@ export function resolvePagerDutyRoutingKey(env: Env, repoFullName: string): Page : { status: "disabled", reason: fallback ? "invalid_global_key" : "missing_global_key" }; } -export type PagerDutySeverity = "critical" | "error" | "warning" | "info"; - -const SEVERITY_RANK: Record = { info: 0, warning: 1, error: 2, critical: 3 }; - -function isPagerDutySeverity(value: unknown): value is PagerDutySeverity { - return value === "critical" || value === "error" || value === "warning" || value === "info"; -} +/** @deprecated alias of {@link LoopoverSeverity} -- kept so existing imports (ops-wire.ts's + * classifyAnomalySeverity/worstAnomaly) don't need a rename. Shares the codebase's one severity-threshold + * concept (#5119) instead of a PagerDuty-only copy. */ +export type PagerDutySeverity = LoopoverSeverity; /** Resolve the minimum severity that pages for `repoFullName`: per-repo map entry, else the global override, * else {@link DEFAULT_MIN_SEVERITY} — the quietest safe default, so an operator who never touches these vars - * still only gets paged for active-incident-grade conditions, never routine calibration nudges. */ + * still only gets paged for active-incident-grade conditions, never routine calibration nudges. Delegates to + * the shared {@link resolveSeverityThreshold} resolver (#5119) so PagerDuty and Sentry share one + * severity-threshold concept, not two parallel ones. */ export function resolvePagerDutyMinSeverity(env: Env, repoFullName: string): PagerDutySeverity { - const map = repoJsonMap(env, "PAGERDUTY_REPO_MIN_SEVERITY"); - const mapped = map[repoFullName.toLowerCase()]; - if (isPagerDutySeverity(mapped)) return mapped; - const global = envString(env, "PAGERDUTY_MIN_SEVERITY"); - return isPagerDutySeverity(global) ? global : DEFAULT_MIN_SEVERITY; + return resolveSeverityThreshold(env, repoFullName, "PAGERDUTY_MIN_SEVERITY", "PAGERDUTY_REPO_MIN_SEVERITY", DEFAULT_MIN_SEVERITY); } /** Coerce a JSON-map value or raw env string to a positive minute count; anything else (absent, zero, @@ -171,7 +167,7 @@ export async function triggerPagerDutyIncident( } const minSeverity = resolvePagerDutyMinSeverity(env, params.repoFullName); - if (SEVERITY_RANK[params.severity] < SEVERITY_RANK[minSeverity]) { + if (!meetsSeverityThreshold(params.severity, minSeverity)) { await auditPagerDutyNotification(env, { repoFullName: params.repoFullName, dedupKey: params.dedupKey }, "denied", "below_min_severity", { severity: params.severity, minSeverity, diff --git a/src/services/severity-threshold.ts b/src/services/severity-threshold.ts new file mode 100644 index 0000000000..b517201c4a --- /dev/null +++ b/src/services/severity-threshold.ts @@ -0,0 +1,63 @@ +// Shared severity-threshold resolver (#5119): global-default + per-repo-override precedence for gating how +// much observability noise an operator's ops channels receive. notify-pagerduty.ts's own +// resolvePagerDutyMinSeverity was the original, single-purpose version of this (paging only); sentry.ts's +// capture paths now share the SAME resolver so there is one severity-threshold concept in the codebase, not +// two parallel ones. + +export type LoopoverSeverity = "critical" | "error" | "warning" | "info"; + +export const SEVERITY_RANK: Record = { info: 0, warning: 1, error: 2, critical: 3 }; + +export function isLoopoverSeverity(value: unknown): value is LoopoverSeverity { + return value === "critical" || value === "error" || value === "warning" || value === "info"; +} + +/** True when `severity` meets or exceeds `threshold` (higher {@link SEVERITY_RANK}) -- the shared "should this + * actually fire" comparison every severity-gated channel (PagerDuty pages, Sentry captures) makes. */ +export function meetsSeverityThreshold(severity: LoopoverSeverity, threshold: LoopoverSeverity): boolean { + return SEVERITY_RANK[severity] >= SEVERITY_RANK[threshold]; +} + +function envString(env: Env, name: string): string | undefined { + const fromEnv = (env as unknown as Record)[name]; + if (typeof fromEnv === "string" && fromEnv.trim().length > 0) return fromEnv.trim(); + /* v8 ignore next 2 -- process.env is the self-host Node fallback; Worker/D1 tests pass values on Env. */ + const processEnv = (globalThis as unknown as { process?: { env?: Record } }).process?.env; + const fromProcess = processEnv?.[name]; + return typeof fromProcess === "string" && fromProcess.trim().length > 0 ? fromProcess.trim() : undefined; +} + +/** Parse a `{repoFullName: value}` JSON map off `envName`, lower-casing repo keys. Malformed/absent -> `{}`. */ +function repoJsonMap(env: Env, envName: string): Record { + const raw = envString(env, envName); + if (!raw) return {}; + try { + const parsed = JSON.parse(raw) as unknown; + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return {}; + const out: Record = {}; + for (const [repo, value] of Object.entries(parsed)) out[repo.toLowerCase()] = value; + return out; + } catch { + return {}; + } +} + +/** Resolve the minimum severity threshold for `repoFullName`: a valid `repoMapVarName` JSON-map entry wins, + * else a valid `globalVarName` override, else `fallback` (default `"error"` -- the quietest safe default, so + * an operator who never touches these vars keeps today's de facto behavior). Mirrors + * {@link resolveDiscordWebhook}/{@link resolvePagerDutyRoutingKey}'s exact per-repo-override-wins-over-global + * precedence. `repoFullName` may be `""` for a non-repo-scoped event -- the (empty) map lookup simply misses + * and falls through to the global threshold, which is the correct behavior for global-only events. */ +export function resolveSeverityThreshold( + env: Env, + repoFullName: string, + globalVarName: string, + repoMapVarName: string, + fallback: LoopoverSeverity = "error", +): LoopoverSeverity { + const map = repoJsonMap(env, repoMapVarName); + const mapped = map[repoFullName.toLowerCase()]; + if (isLoopoverSeverity(mapped)) return mapped; + const global = envString(env, globalVarName); + return isLoopoverSeverity(global) ? global : fallback; +} diff --git a/test/unit/selfhost-sentry.test.ts b/test/unit/selfhost-sentry.test.ts index 8e80a2de17..e8756c475c 100644 --- a/test/unit/selfhost-sentry.test.ts +++ b/test/unit/selfhost-sentry.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { hostname } from "node:os"; // Mock @sentry/node so the dynamic import inside initSentry() resolves to spies. Hoisted so vi.mock can see it. @@ -1187,6 +1187,105 @@ describe("forwardStructuredLogToSentry — central console.log → Sentry error }); }); +describe("severity-threshold gating (#5119) — captureError/captureReviewFailure/forwardStructuredLogToSentry", () => { + const clearThresholdEnv = () => { + delete process.env.SENTRY_MIN_SEVERITY; + delete process.env.SENTRY_REPO_MIN_SEVERITY; + }; + afterEach(clearThresholdEnv); + + it("captureError: default threshold (error) changes nothing — an error-grade capture still fires", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + captureError(new Error("boom"), { repo: "acme/widgets" }); + expect(mocks.captureException).toHaveBeenCalledTimes(1); + }); + + it("captureError: a repo threshold above error (critical) suppresses the capture", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + process.env.SENTRY_REPO_MIN_SEVERITY = JSON.stringify({ "acme/widgets": "critical" }); + captureError(new Error("boom"), { repo: "acme/widgets" }); + expect(mocks.captureException).not.toHaveBeenCalled(); + }); + + it("captureError: a repo threshold above error does not suppress a DIFFERENT repo's capture", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + process.env.SENTRY_REPO_MIN_SEVERITY = JSON.stringify({ "acme/widgets": "critical" }); + captureError(new Error("boom"), { repo: "other/repo" }); + expect(mocks.captureException).toHaveBeenCalledTimes(1); + }); + + it("captureError: the global threshold applies when context carries no repo", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + process.env.SENTRY_MIN_SEVERITY = "critical"; + captureError(new Error("boom")); + expect(mocks.captureException).not.toHaveBeenCalled(); + }); + + it("captureError: reads `repository` when `repo` is absent from context", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + process.env.SENTRY_REPO_MIN_SEVERITY = JSON.stringify({ "acme/widgets": "critical" }); + captureError(new Error("boom"), { repository: "acme/widgets" }); + expect(mocks.captureException).not.toHaveBeenCalled(); + }); + + it("captureReviewFailure: default threshold (error) changes nothing — still fires", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + captureReviewFailure(new Error("rev"), { repo: "acme/widgets" }); + expect(mocks.captureException).toHaveBeenCalledTimes(1); + }); + + it("captureReviewFailure: a repo threshold above error (critical) suppresses the capture", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + process.env.SENTRY_REPO_MIN_SEVERITY = JSON.stringify({ "acme/widgets": "critical" }); + captureReviewFailure(new Error("rev"), { repo: "acme/widgets" }); + expect(mocks.captureException).not.toHaveBeenCalled(); + }); + + it("forwardStructuredLogToSentry: default threshold (error) still forwards error/fatal and still skips warning/info — byte-identical to pre-#5119 behavior", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + forwardStructuredLogToSentry(JSON.stringify({ level: "error", event: "x", repo: "acme/widgets" })); + forwardStructuredLogToSentry(JSON.stringify({ level: "fatal", event: "y", repo: "acme/widgets" })); + forwardStructuredLogToSentry(JSON.stringify({ level: "warning", event: "z", repo: "acme/widgets" })); + forwardStructuredLogToSentry(JSON.stringify({ level: "info", event: "w", repo: "acme/widgets" })); + expect(mocks.captureException).toHaveBeenCalledTimes(2); + }); + + it("forwardStructuredLogToSentry: lowering a repo's threshold to info surfaces its warning/info-grade logs (#5119's core deliverable)", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + process.env.SENTRY_REPO_MIN_SEVERITY = JSON.stringify({ "acme/widgets": "info" }); + forwardStructuredLogToSentry(JSON.stringify({ level: "warning", event: "z", repo: "acme/widgets" })); + expect(mocks.captureException).toHaveBeenCalledTimes(1); + expect(mocks.scope.setLevel).toHaveBeenLastCalledWith("warning"); + forwardStructuredLogToSentry(JSON.stringify({ level: "info", event: "w", repo: "acme/widgets" })); + expect(mocks.captureException).toHaveBeenCalledTimes(2); + expect(mocks.scope.setLevel).toHaveBeenLastCalledWith("info"); + }); + + it("forwardStructuredLogToSentry: lowering ONE repo's threshold does not affect a different repo's default gating", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + process.env.SENTRY_REPO_MIN_SEVERITY = JSON.stringify({ "acme/widgets": "info" }); + forwardStructuredLogToSentry(JSON.stringify({ level: "warning", event: "z", repo: "other/repo" })); + expect(mocks.captureException).not.toHaveBeenCalled(); + }); + + it("forwardStructuredLogToSentry: an unrecognized level (a log CATEGORY like \"audit\", not a severity) never promotes to error grade even with a lowered threshold", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + process.env.SENTRY_MIN_SEVERITY = "info"; + forwardStructuredLogToSentry(JSON.stringify({ level: "audit", event: "job_complete", repo: "acme/widgets" })); + expect(mocks.captureException).toHaveBeenCalledTimes(1); + expect(mocks.scope.setLevel).toHaveBeenLastCalledWith("info"); + }); + + it("forwardStructuredLogToSentry: raising the global threshold to critical suppresses ordinary error-grade logs but still lets fatal through", async () => { + await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); + process.env.SENTRY_MIN_SEVERITY = "critical"; + forwardStructuredLogToSentry(JSON.stringify({ level: "error", event: "x" })); + expect(mocks.captureException).not.toHaveBeenCalled(); + forwardStructuredLogToSentry(JSON.stringify({ level: "fatal", event: "y" })); + expect(mocks.captureException).toHaveBeenCalledTimes(1); + }); +}); + describe("installStructuredLogForwarding — central console sink instrumentation (#1468)", () => { const makeConsole = () => { const base = { log: vi.fn(), error: vi.fn() }; diff --git a/test/unit/severity-threshold.test.ts b/test/unit/severity-threshold.test.ts new file mode 100644 index 0000000000..6318ab27d5 --- /dev/null +++ b/test/unit/severity-threshold.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from "vitest"; +import { isLoopoverSeverity, meetsSeverityThreshold, resolveSeverityThreshold } from "../../src/services/severity-threshold"; +import { createTestEnv } from "../helpers/d1"; + +const withEnv = (over: Record = {}): Env => Object.assign(createTestEnv(), over) as Env; + +describe("isLoopoverSeverity", () => { + it("accepts exactly the four recognized severities", () => { + for (const value of ["critical", "error", "warning", "info"]) expect(isLoopoverSeverity(value)).toBe(true); + }); + it("rejects anything else, including near-misses and non-strings", () => { + for (const value of ["fatal", "warn", "debug", "", undefined, null, 1, {}]) expect(isLoopoverSeverity(value)).toBe(false); + }); +}); + +describe("meetsSeverityThreshold", () => { + it("a severity meets an equal or lower threshold", () => { + expect(meetsSeverityThreshold("error", "error")).toBe(true); + expect(meetsSeverityThreshold("error", "warning")).toBe(true); + expect(meetsSeverityThreshold("critical", "info")).toBe(true); + }); + it("a severity below the threshold does not meet it", () => { + expect(meetsSeverityThreshold("warning", "error")).toBe(false); + expect(meetsSeverityThreshold("info", "critical")).toBe(false); + }); +}); + +describe("resolveSeverityThreshold", () => { + it("a valid repo-map entry wins over the global override", () => { + const env = withEnv({ SENTRY_REPO_MIN_SEVERITY: JSON.stringify({ "acme/widgets": "info" }), SENTRY_MIN_SEVERITY: "critical" }); + expect(resolveSeverityThreshold(env, "acme/widgets", "SENTRY_MIN_SEVERITY", "SENTRY_REPO_MIN_SEVERITY")).toBe("info"); + }); + + it("repo-map lookup is case-insensitive", () => { + const env = withEnv({ SENTRY_REPO_MIN_SEVERITY: JSON.stringify({ "acme/widgets": "warning" }) }); + expect(resolveSeverityThreshold(env, "ACME/Widgets", "SENTRY_MIN_SEVERITY", "SENTRY_REPO_MIN_SEVERITY")).toBe("warning"); + }); + + it("an invalid/absent repo entry falls back to a valid global override", () => { + expect(resolveSeverityThreshold(withEnv({ SENTRY_MIN_SEVERITY: "info" }), "acme/widgets", "SENTRY_MIN_SEVERITY", "SENTRY_REPO_MIN_SEVERITY")).toBe("info"); + const env = withEnv({ SENTRY_REPO_MIN_SEVERITY: JSON.stringify({ "acme/widgets": "not-a-severity" }), SENTRY_MIN_SEVERITY: "critical" }); + expect(resolveSeverityThreshold(env, "acme/widgets", "SENTRY_MIN_SEVERITY", "SENTRY_REPO_MIN_SEVERITY")).toBe("critical"); + }); + + it("no repo entry + no/invalid global → defaults to the caller-supplied fallback (error unless overridden)", () => { + expect(resolveSeverityThreshold(withEnv(), "acme/widgets", "SENTRY_MIN_SEVERITY", "SENTRY_REPO_MIN_SEVERITY")).toBe("error"); + expect(resolveSeverityThreshold(withEnv({ SENTRY_MIN_SEVERITY: "not-a-severity" }), "acme/widgets", "SENTRY_MIN_SEVERITY", "SENTRY_REPO_MIN_SEVERITY")).toBe("error"); + expect(resolveSeverityThreshold(withEnv(), "acme/widgets", "SENTRY_MIN_SEVERITY", "SENTRY_REPO_MIN_SEVERITY", "critical")).toBe("critical"); + }); + + it("an empty repoFullName (non-repo-scoped event) falls through an unrelated repo map to the global fallback", () => { + const env = withEnv({ SENTRY_REPO_MIN_SEVERITY: JSON.stringify({ "acme/widgets": "info" }), SENTRY_MIN_SEVERITY: "warning" }); + expect(resolveSeverityThreshold(env, "", "SENTRY_MIN_SEVERITY", "SENTRY_REPO_MIN_SEVERITY")).toBe("warning"); + }); + + it("ignores malformed or non-object repo-map values and falls back to the global override", () => { + expect(resolveSeverityThreshold(withEnv({ SENTRY_REPO_MIN_SEVERITY: "{not json", SENTRY_MIN_SEVERITY: "info" }), "acme/widgets", "SENTRY_MIN_SEVERITY", "SENTRY_REPO_MIN_SEVERITY")).toBe("info"); + expect(resolveSeverityThreshold(withEnv({ SENTRY_REPO_MIN_SEVERITY: "[]", SENTRY_MIN_SEVERITY: "info" }), "acme/widgets", "SENTRY_MIN_SEVERITY", "SENTRY_REPO_MIN_SEVERITY")).toBe("info"); + }); + + it("uses process.env as a self-host fallback when the runtime Env object does not carry the var", () => { + process.env.SENTRY_MIN_SEVERITY = "info"; + try { + expect(resolveSeverityThreshold(withEnv(), "acme/widgets", "SENTRY_MIN_SEVERITY", "SENTRY_REPO_MIN_SEVERITY")).toBe("info"); + } finally { + delete process.env.SENTRY_MIN_SEVERITY; + } + }); +});