[HDX-4997] Persist alert evaluation errors and analytics in AlertHistory

[wrn14897]

Linear Issue: HDX-4997

Stack (2/3)

Splits #2786 for reviewability. Now based on main (#2833 merged); independent of the UI PR (3/3).

1. [HDX-4997] Alert evaluations read model + GET /alerts/:id/evaluations #2833 — evaluations read model + endpoint (api, common-utils) — merged
2. → this PR — persist evaluation errors/analytics in the alert task (api)
3. [HDX-4997] Alert detail page with evaluation history #2835 — alert detail page UI (app)

Why

When an alert evaluation fails (ClickHouse query error/timeout, webhook failure), the only persisted signal is alert.executionErrors — a latest-only snapshot wiped by the next successful run. There is no durable, per-window record of which evaluations failed, so the alert detail page (and any postmortem) can't show failure history.

What

• Failed evaluations are recorded as ERROR-state AlertHistory rows carrying error type/message/timestamp, upserted per evaluation window so per-tick retries collapse into a single row; rows expire with the existing 30d TTL.
• Webhook/notification failures also produce an ERROR row alongside the normal evaluation rows; a stale ERROR row from a failed earlier tick is removed when a clean same-window retry succeeds.
• Retry/backfill semantics are untouched: ERROR rows are excluded from the due-ness gate, the retry date-range computation, and consecutive-window counting — recording an error never marks the window as evaluated, so the failed window is still retried every tick and backfilled on recovery.
• Query timeouts are classified as QUERY_TIMEOUT (client request timeout/abort, server-side TIMEOUT_EXCEEDED/159, socket timeouts — walking the cause chain since the query client wraps failures) with an actionable message that includes the configured evaluation timeout.
• Evaluation analytics (queryDurationMs, webhookDurationMs, backfilledBuckets) are recorded on every history row the evaluation writes, including ERROR rows.

Testing

• packages/api: ci:lint (eslint + tsc + openapi), ci:unit green (incl. new errors.test.ts timeout-classification unit tests)
• Integration: checkAlerts.int.test.ts full suite passes locally (160 tests), including the new error-recording / QUERY_TIMEOUT / webhook-failure / backfill-analytics cases

[changeset-bot]

🦋 Changeset detected

Latest commit: 34732ec

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@hyperdx/common-utils	Minor
@hyperdx/api	Minor
@hyperdx/app	Minor
@hyperdx/otel-collector	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview	Aug 11, 2026 4:53pm
hyperdx-storybook	Ready	Preview	Aug 11, 2026 4:53pm

[greptile-apps]

Greptile Summary

The PR persists alert evaluation and notification failures in AlertHistory and adds evaluation-level analytics while preserving retry and backfill behavior.

• Classifies client, server, and socket query timeouts as QUERY_TIMEOUT.
• Upserts one ERROR history row per alert evaluation window.
• Excludes ERROR rows from scheduling and consecutive-window calculations.
• Removes stale query-error rows after successful same-window retries or backfills.
• Records query duration, webhook duration, and backfilled-bucket counts.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains in the eligible follow-up review scope.

No blocking failure remains.

Important Files Changed

Filename	Overview
packages/api/src/tasks/checkAlerts/index.ts	Classifies query timeouts, captures evaluation analytics, persists failures against scheduled windows, and excludes ERROR rows from evaluation scheduling inputs.
packages/api/src/tasks/checkAlerts/providers/default.ts	Extends provider persistence to upsert ERROR histories and remove stale query-error records after successful retries or backfills.
packages/api/src/tasks/checkAlerts/errors.ts	Adds bounded cause-chain timeout detection for ClickHouse client, server, and socket errors.
packages/api/src/tasks/checkAlerts/providers/index.ts	Expands the provider contract with evaluation-window, analytics, and evaluated-range metadata.
packages/common-utils/src/clickhouse/index.ts	Exposes the configured request timeout through a read-only client accessor.
packages/api/src/tasks/checkAlerts/tests/checkAlerts.int.test.ts	Adds integration coverage for error persistence, retry and backfill behavior, stale-error cleanup, timeout classification, and analytics.
packages/api/src/tasks/checkAlerts/tests/errors.test.ts	Adds focused timeout-classification coverage, including wrapped causes and cycle/depth safeguards.

Sequence Diagram

sequenceDiagram
  participant Scheduler
  participant AlertTask
  participant ClickHouse
  participant History as AlertHistory
  participant Webhook
  Scheduler->>AlertTask: Evaluate alert window
  AlertTask->>ClickHouse: Run alert query
  alt Query fails
    ClickHouse-->>AlertTask: Error or timeout
    AlertTask->>History: Upsert ERROR row for window
  else Query succeeds
    ClickHouse-->>AlertTask: Evaluation results
    AlertTask->>Webhook: Send transition notification
    AlertTask->>History: Persist normal rows and analytics
    AlertTask->>History: Remove superseded query ERROR rows
    opt Notification fails
      AlertTask->>History: Upsert WEBHOOK_ERROR row
    end
  end

Loading

_{Reviews (7): Last reviewed commit: "Merge branch 'main' into warren/HDX-4997..." | Re-trigger Greptile}

[github-actions]

PLACEHOLDER

[github-actions]

material for review below

[wrn14897]

Addressed the stale-ERROR-row cleanup gap in 06c74cc:

• updateAlertState now receives the evaluated date range and, on a clean save, deletes ERROR rows with createdAt ∈ (rangeStart, currentWindowStart] — covering windows recovered via backfill, not just a same-window retry.
• The lower bound is deliberately exclusive: an ERROR row at exactly the previous anchor belongs to an already-evaluated window (e.g. a webhook failure recorded alongside its normal rows) that is never retried, so it survives as a truthful record. Same reasoning keeps the row for a never-backfilled failed window (no anchor → one-window lookback never re-covers the failed data), which the existing "keeps ERROR rows from older windows" test pins.
• The cleanup now also runs when the evaluation succeeded but this run's webhook failed, so stale query-failure rows are cleared before the fresh WEBHOOK_ERROR row is upserted.
• New integration test: a failed window recovered via a later tick's backfill has its ERROR row cleared, while a webhook-failure ERROR row at the anchor window survives (pins the boundary). Full checkAlerts int suite passes locally (274 tests).

[github-actions]

Deep Review

✅ No critical (P0/P1) issues found. The core design is sound: ERROR rows are consistently excluded from the due-ness gate (alertHistory.ts:494), the retry date-range computation, and consecutive-window counting (index.ts:1599, index.ts:1700), so recording a failure never marks a window evaluated. The QUERY_TIMEOUT enum is in sync across its source of truth (common-utils/types.ts), the OpenAPI JSDoc, and openapi.json; the timeout-message wiring reads requestTimeoutMs (default 3600000, never NaN) from the client configured with sourceTimeoutMs; and the changeset is present and well-formed.

🟡 P2 -- recommended

• packages/api/src/tasks/checkAlerts/providers/default.ts:478 -- upsertErrorHistory upserts on {alert, createdAt, state: ERROR} but no unique index backs that key, so two overlapping evaluations of the same window can both miss the match and each insert a row, defeating the one-row-per-window guarantee.
  • Fix: Add a partial unique index on {alert, createdAt, state} (scoped to state: ERROR) in alertHistory.ts so concurrent upserts collapse instead of duplicating.
  • _{performance, learnings-researcher}
• packages/api/src/tasks/checkAlerts/providers/default.ts:432 -- the new stale-row deleteMany and the upsertErrorHistory call run awaited-but-unguarded after Alert.updateOne has already set the success state; if either Mongo op throws, the exception unwinds into processAlert's outer catch, which records an ERROR history row at the same evaluationWindowStart, and the evaluations view ranks ERROR above OK/ALERT so a window that actually succeeded renders as failed.
  • Fix: Wrap the stale-row cleanup and error-row upsert in try/catch with a logged warning, matching the Promise.allSettled tolerance already used for the history create calls.

🔵 P3 nitpicks (2)

• packages/api/src/tasks/checkAlerts/errors.ts:18 -- timeout classification depends on the literal @clickhouse/client message strings 'Timeout error.' and 'The user aborted a request.', which will silently stop matching if the upstream client rewords them on a version bump.
  • Fix: Add a regression test that constructs the error from the installed client version (or asserts the constants against it) so a wording change fails CI instead of silently reclassifying timeouts as generic query errors.
• packages/api/src/tasks/checkAlerts/index.ts:1268 -- evaluationAnalytics is assigned by shared object reference to every history record before persistence; safe today because AlertHistory.create snapshots the values, but a future in-place mutation of one record's analytics would silently affect all rows from the same evaluation.
  • Fix: Assign a shallow copy ({ ...evaluationAnalytics }) per record to keep the rows independent.

---

Reviewers (returned at synthesis time): api-contract, performance, learnings-researcher, plus orchestrator code analysis.

Testing gaps:

• No contract/snapshot test asserts openapi.json stays in sync with the common-utils AlertErrorType enum, so a future enum addition could drift silently. (api-contract)
• The stale-ERROR-row deleteMany is untested under a large backfill gap (many missed windows between the previous anchor and the current tick). (performance)
• Coverage caveat: correctness, adversarial, reliability, testing, maintainability, project-standards, kieran-typescript, and agent-native reviewers were dispatched but had not returned when synthesis was forced; the two P2 findings above rest on direct code analysis and were independently corroborated by the returned performance and learnings reviewers.

[github-actions]

🔴 Tier 4 — Critical

Touches authentication, tenancy data models, the public API or shipped database config — or substantially changes background tasks, the OTel pipeline, image build, or release CI.

Why this tier:

• Critical-path files (1) — tenancy, public API, or shipped database config:
  • packages/api/src/routers/external-api/v2/alerts.ts
• Background tasks or delivery pipeline substantially modified — 318 lines (bar: 30):
  • packages/api/src/tasks/checkAlerts/errors.ts
  • packages/api/src/tasks/checkAlerts/index.ts
  • packages/api/src/tasks/checkAlerts/providers/default.ts
  • packages/api/src/tasks/checkAlerts/providers/index.ts
• Cross-layer change: touches backend (packages/api) + shared utils (packages/common-utils)

Review process: Deep review from a domain expert. Synchronous walkthrough may be required.
SLA: Schedule synchronous review within 2 business days.

Stats

• Production files changed: 7
• Production lines changed: 330 (+ 790 in test files, excluded from tier calculation)
• Critical-path lines changed: 320
• Branch: warren/HDX-4997-alert-error-persistence
• Author: wrn14897

To override this classification, remove the review/tier-4 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

[github-actions]

E2E Test Results

✅ All tests passed • 277 passed • 1 skipped • 1042s

Status	Count
✅ Passed	277
❌ Failed	0
⚠️ Flaky	0
⏭️ Skipped	1

Tests ran across 4 shards in parallel.

View full report →

[pulpdrew]

LGTM