[HDX-4997] Alert detail page with evaluation history - #2835
Conversation
🦋 Changeset detectedLatest commit: ff57796 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds a feature-gated alert-detail route for inspecting query results and evaluation history.
Confidence Score: 4/5The PR is not yet safe to merge because the chart and evaluation-history surfaces still apply different bounds to the selected time range. The evaluation and annotation hooks floor both range endpoints to minute boundaries, while the chart receives the exact picker range, so evaluations or annotations near either boundary can disagree with the displayed chart data. Files Needing Attention: packages/app/src/api.ts, packages/app/src/AlertDetailPage.tsx
|
| Filename | Overview |
|---|---|
| packages/app/src/api.ts | Adds infinite evaluation-history pagination, but its minute-rounded bounds remain inconsistent with the exact range used by chart queries. |
| packages/app/src/AlertDetailPage.tsx | Introduces the feature-gated alert-detail page and coordinates its chart, history strip, event stream, controls, and loading states. |
| packages/app/src/components/alerts/AlertDetailChart.tsx | Builds alert charts for saved searches and supported dashboard tiles, including thresholds and state annotations. |
| packages/app/src/components/alerts/AlertHistoryCards.tsx | Adds errored evaluation-window styling and per-window error detail modals. |
| packages/app/tests/e2e/features/alerts.spec.ts | Extends full-stack alert coverage for navigation, charting, evaluation history, pagination, and error displays. |
Sequence Diagram
sequenceDiagram
participant User
participant Detail as Alert Detail Page
participant API as Alert API
participant CH as Chart Query
User->>Detail: Select time range
Detail->>CH: Query chart data for selected range
Detail->>API: Fetch evaluation-history page
API-->>Detail: Evaluations and nextBefore cursor
CH-->>Detail: Time-series data
Detail-->>User: Chart, history strip, and event stream
User->>Detail: Scroll to sentinel
Detail->>API: Fetch older page using nextBefore
API-->>Detail: Older evaluations
Reviews (5): Last reviewed commit: "feat(app): alert detail page with evalua..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -0,0 +1,396 @@ | |||
| import * as React from 'react'; | |||
There was a problem hiding this comment.
This new 396-line component combines row rendering, expansion behavior, error details, and infinite-scroll states, exceeding the repository's 300-line component limit and increasing the cost of maintaining these independent responsibilities.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Deep Review✅ No critical issues found. No P0/P1 defects were confirmed against the diff — the feature is flag-gated (default off), the infinite-scroll error/retry loop-prevention holds (the sentinel is unmounted on
🟡 P2 -- recommended
🔵 P3 nitpicks (4)
Reviewers (2 returned of 12 dispatched): testing, learnings-researcher. Dispatched but not returned before synthesis: correctness, maintainability, project-standards, agent-native, performance, api-contract, reliability, adversarial, kieran-typescript, julik-frontend-races. Testing gaps:
|
50d2a33 to
8b31195
Compare
b61e454 to
90f9343
Compare
90f9343 to
d6aaed8
Compare
a4b448b to
c32b78e
Compare
d6aaed8 to
f821dd0
Compare
…#2833) Linear Issue: [HDX-4997](https://linear.app/clickhouse/issue/HDX-4997/record-alert-evaluation-errors-in-alerthistory-and-show-them-on-the) ## Stack (1/3) This is the base of a 3-PR stack that splits #2786 for reviewability: 1. **→ this PR** — evaluations read model + endpoint (api, common-utils) 2. #2834 — persist evaluation errors/analytics in the alert task (api) 3. #2835 — alert detail page UI (app) PRs 2 and 3 both base on this branch but are independent of each other; once this merges they can land in either order (GitHub retargets them to `main` automatically when this branch is deleted on merge). ## Why To surface alert evaluation history (including failures) on a per-alert detail page, we need a read model over `AlertHistory` that can answer "what happened in each evaluation window?" — including windows that errored, per-group results for group-by alerts, and evaluation analytics. Today `AlertHistory` only stores OK/ALERT rows and there is no per-alert evaluations API. ## What - **Types (`common-utils`)** for evaluation errors (`AlertError`/`AlertErrorType` incl. `QUERY_TIMEOUT`), per-window evaluations with per-group breakdown (capped at `ALERT_EVALUATION_GROUPS_LIMIT`, firing-first), and evaluation analytics (`queryDurationMs`, `webhookDurationMs`, `backfilledBuckets`). - **`AlertHistory` schema** gains optional `errors` + `analytics` fields, and `AlertState` gains `ERROR` (only ever used on history rows). - **`GET /alerts/:id/evaluations`**: per-window evaluation history scoped to a `startTime`/`endTime` range (clamped to the 31d retention window), grouped across group-by groups newest-first, with a hard-bounded scan of at most ~(limit+1) intervals per request and a server-provided `nextBefore` cursor that always advances past the scanned slice so paging progresses across gaps instead of stalling. - Windows with ERROR rows surface their errors (deduped, newest-first) and rank as ERROR; firing-transition annotations exclude ERROR rows. Nothing writes ERROR rows or analytics yet — the alert task's write side lands in PR 2 of the stack. ## Testing - `packages/api` + `packages/common-utils`: `ci:lint` (eslint + tsc), `ci:unit` green - Integration: `alertHistory.int.test.ts` (new, 80 cases), `routers/api/alerts.int.test.ts`, and the full `*alerts.int*` set pass locally (278 tests)
Datadog-style alert status page at /alerts/:id, reachable via a Details link on each alerts-page row (the alert name keeps linking to its saved search / dashboard tile): - Header with state badge, silence/ack, source link, and a time picker; the alert's underlying query charted over the selected range with threshold reference lines and firing/recovery annotations. - Widened evaluation-history strip (60 windows); chart, strip, and event stream all follow the picker's exact range. - Evaluation event stream: one parent row per window labeled with the evaluated bucket start (matching the chart's x-axis), state, latest value, breaches, backfilled buckets, query/webhook durations, and error labels; per-group child rows for group-by alerts; older windows load via an infinite-scroll sentinel using the endpoint's nextBefore cursor. A failed page fetch unmounts the sentinel (whose effect would otherwise refire forever) and renders an explicit retry affordance; a failed initial page shows a failure message instead of the empty state. - Alerts-page history strip renders errored evaluation windows as striped-red segments with full error details in a modal on click. - The Details link and /alerts/:id route are gated behind NEXT_PUBLIC_ENABLE_ALERT_DETAILS (default off) — enabled in dev (.env.development) and CI (e2e webserver) only while the feature bakes; self-hosted deployments opt in via docker-compose. Includes unit tests and full-stack e2e coverage seeded directly in MongoDB.
f821dd0 to
ff57796
Compare
github-actions
Bot
added
the
review/tier-4
🔴 Tier 4 — CriticalTouches 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:
Review process: Deep review from a domain expert. Synchronous walkthrough may be required. Stats
|
E2E Test Results✅ All tests passed • 279 passed • 1 skipped • 1100s
Tests ran across 4 shards in parallel. |
1 participant
Linear Issue: HDX-4997
Stack (3/3)
Splits #2786 for reviewability. Now based on
main(#2833 merged); independent of the write-side PR (#2834) — the UI renders whatever the endpoint returns, and error/analytics columns simply stay empty until #2834 lands.Why
Alerts today are a list row with a 20-segment history strip — there's no place to see why an alert fired or failed, what value breached the threshold, or how evaluations behaved over time. This adds a Datadog-style per-alert status page so users can debug alerts without spelunking in the database.
What
Alert status page at
/alerts/:id, reachable via a Details link on each alerts-page row (the alert name keeps linking to its saved search / dashboard tile):nextBeforecursor. A failed page fetch unmounts the sentinel (whose effect would otherwise refire forever) and renders an explicit retry affordance; a failed initial page shows a failure message instead of the empty state./alerts/:idroute are gated behindNEXT_PUBLIC_ENABLE_ALERT_DETAILS(default off) — enabled in dev (.env.development) and CI (e2e webserver) only while the feature bakes; self-hosted deployments opt in via docker-compose.Testing
packages/app:ci:lint(eslint + tsc + stylelint),ci:unitgreen (2703 tests, incl. newAlertEvaluationsTable/AlertHistoryCardsunit tests)tests/e2e/features/alerts.spec.ts) seeded directly in MongoDB