Skip to content

Add the alert-notebook binding endpoint (#4222) - #4366

Merged
erikdarlingdata merged 8 commits into
devfrom
feature/4222-alert-notebook-endpoint
Sep 26, 2026
Merged

erikdarlingdata merged 8 commits into
devfrom
feature/4222-alert-notebook-endpoint

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

Refs #4222.

Why

The alert-notebook MVP (#4222, Slice C) needs a server endpoint that turns an alert link (server, metric, at, dedup) into a bound, read-only notebook definition. Today's #/triage page matches by nearest time only and cannot become a notebook (its sections aren't read cells). This PR is the (a) slice: the new GET /api/alert-notebook endpoint. Two parallel #4222 lanes cover the notebook read cell type + views.js rendering (a2) and the shell play/pause control (d); this PR does not touch ValidateNotebookDefinition or any JS.

What changes

  • New Darling/PerformanceMonitor.Darling.Service/AlertNotebookEndpoint.cs, mapped from DarlingWebEndpoints.MapAll right after DarlingTriageEndpoint.Map, same auth-middleware placement as every sibling route.
  • GET /api/alert-notebook?server&metric&at&dedup returns:
    {
      "alert": { "alert_time", "server_id", "server_name", "metric_name", "current_value", "threshold_value", "detail_text", "incident_since", "involved_objects", "database", "total_occurrences" } | null,
      "status": "Resolved at <T>" | "Fired again at <T>" | "No resolution recorded" | "Unknown (not collected since <T>)",
      "notes": [ "..." ],
      "template": { "id": "mechanical/<metric>", "version": 1 },
      "definition": {
        "kind": "notebook",
        "cells": [
          { "type": "header", "title", "server", "alert_time", "incident_since", "involved_objects", "database", "total_occurrences" },
          { "type": "status", "title": "Status", "status": "<one of the four status strings>" },
          { "type": "read", "read": "<catalog read name>", "params": { "server", "as_of", "hours", "limit", ... }, "viz": "table", "title": "<section title>" },
          ...
        ]
      }
    }
  • alert match: scans same-metric history rows in the anchor window for an incident whose context_json carries the link's dedup key (via AlertContextSerializer.TryDeserialize); dedup wins over nearer-in-time. With no dedup, falls back to today's nearest same-metric row, same as DarlingTriageEndpoint.
  • status, four arms: a later resolution-alias row after at -> "Resolved at T"; else a later same-metric firing -> "Fired again at T"; else the metric's server has a v_collection_log row at or after at -> "No resolution recorded"; else -> "Unknown (not collected since T)". Never "ongoing". A fleet-level metric (no serverId) always reads Unknown when neither of the first two arms fires — freshness can't be checked without a server.
  • Binding: window_end = min(at + 15 min, now) via DarlingTriageEndpoint.ResolveAnchor/AnchorSlack; a future at clamps to now (inherited from ResolveAnchor). The alert match uses the same AlertMatchLookback bound before the anchor that DarlingTriageEndpoint uses and picks the key match nearest the anchor; the status arms read forward to min(anchor + 24 h, now), so a later resolution or re-fire is found. A server name that doesn't resolve (and isn't fleet-level) reads Unknown rather than other servers' rows.
  • Mechanical template: every metric converts DarlingTriageEndpoint.SectionsFor(metric) (or its fallback) into one {type:"read"} cell per section, each carrying server, as_of, and the section's own hours/limit where declared, else a 24-hour / 50-row default. template.id = "mechanical/<metric>", version = 1. The authored blocking/deadlock templates are explicitly not part of this slice — every metric, including those two, gets the mechanical conversion for now.
  • Reuse, not copy: widened DarlingTriageEndpoint.AnchorSlack and AlertMatchLookback from private to internal (both already TimeSpan constants); every other member this endpoint calls (ResolveAnchor, ResolutionAliases, SectionsByMetric/SectionsFor, IsFleetLevelStoreServer, IsFleetLevelStoreMetric, TriageSection, DefaultSections) was already internal and is called directly, not duplicated.
  • A stale or empty firing (no history row, no incident match) returns 200 with an honest note and empty alert, never an error — same posture as DarlingTriageEndpoint (Alert deliveries lack Datadog-parity structure: no structured context tags, no linked triage artifact #2710).

Known gaps (this slice only)

  • No composed cells are emitted (only read cells), so no cell carries the spec's absolute per-cell range {windowStart, windowEnd} yet — DarlingTriageEndpoint.TriageSection has no composed-cell concept to convert. That lands with authored/composed templates in a later slice.
  • Pins: see the pm-pr lane reports below (security, dedup, status arms, window math, read-name census, stale-link degrade, live JSON shape).
  • The unauthenticated-request pin is written (401 before any store command); see the pins report below.

Lite parity: N/A (Darling web only).

Test plan

  • dotnet build Darling/Darling.Tests/Darling.Tests.csproj -p:EnableWindowsTargeting=true — 0 Warning(s), 0 Error(s)
  • dotnet build Lite.Tests/Lite.Tests.csproj -p:EnableWindowsTargeting=true — 0 Warning(s), 0 Error(s)
  • New Darling.Tests pins for this endpoint (dedup match, the four status arms, binding/window math, mechanical-template read-name coverage against BuildReadDispatch, stale-link degrade, unauthenticated zero-store-commands) — not yet written; both test projects target net10.0-windows and cannot run on this macOS lane. CI decides.
  • Full Windows test suite — CI decides.

CHANGELOG entry

SECTION: Added
ENTRY:

For the coordinator

The SPA render slice (a2/other #4222 lanes) consumes this exact JSON shape from GET /api/alert-notebook:

  • Root: { alert, status, notes[], template: {id, version}, definition: {kind: "notebook", cells: [...]} }.
  • alert is either null or an object with keys: alert_time (ISO-8601 o format), server_id, server_name, metric_name, current_value, threshold_value, detail_text, and — only present when an incident matched — incident_since (ISO-8601 or null), involved_objects (comma-joined string), database (string or null), total_occurrences (number or null).
  • status is always one of exactly four strings: "Resolved at <yyyy-MM-ddTHH:mm:ssZ>", "Fired again at <yyyy-MM-ddTHH:mm:ssZ>", "No resolution recorded", "Unknown (not collected since <yyyy-MM-ddTHH:mm:ssZ>)".
  • definition.cells[0] is always {type: "header", title, server, alert_time?, incident_since?, involved_objects?, database?, total_occurrences?} — a directly-rendered cell, not a read cell.
  • definition.cells[1] is always {type: "status", title: "Status", status} — same non-read shape.
  • definition.cells[2..] are {type: "read", read, params: {...}, viz: "table", title} — one per SectionsFor(metric) entry. params always carries hours and limit explicitly (never omitted), plus server/as_of when applicable (fleet-level sections omit server).
  • No composed cells and no per-cell range are emitted yet (see Known gaps above) — the render slice should treat every definition.cells entry as header | status | read for now.
  • Method names for follow-on server work: AlertNotebookEndpoint.Map, AlertNotebookEndpoint.ResolveStatusAsync, AlertNotebookEndpoint.ReadCell, AlertNotebookEndpoint.HeaderCell. File: Darling/PerformanceMonitor.Darling.Service/AlertNotebookEndpoint.cs.

pm-pr lane report (pins)

Head after this lane: b3e0f8e8 (was ee638793; branch has not moved otherwise).

New file: Darling/Darling.Tests/AlertNotebookEndpointTests.cs. Build: 0 Warning(s), 0 Error(s) on Darling/Darling.Tests/Darling.Tests.csproj (net10.0-windows, macOS host, -p:EnableWindowsTargeting=true).

Pins added (all in the new file unless noted):

  1. Security (merge blocker), first.

    • Unauthenticated_GetAlertNotebook_Returns401_AndNeverDialsTheStore — mirrors DarlingWebHostGateLiveTests.NetworkMode_NoToken_ApiPath_Returns401Json exactly, against /api/alert-notebook, with the data source pointed at a closed local port (Port=1) the same way WebReadCancellationPinTests.DeadStore() does. The auth gate answers 401 JSON before AlertNotebookEndpoint's lambda runs, so the dead pool is never dialed — the assertion is the 401 + JSON body + no <html, same shape the sibling triage/fleet routes already prove.
    • OutOfCidrRemote_GetAlertNotebook_IsForbidden_BeforeTheHandlerRuns — the OIDC sign-in for the web dashboard: give the web surface a per-user identity instead of one shared token #2550 CIDR-outermost gate, same dead-pool proof.
    • AuthenticatedHostileQuery_NeverReturns500_AndCarriesNoExceptionText ([Theory], 4 cases: far-future at, SQL-shaped server, an over-long metric (4096 chars), SQL-shaped dedup) — asserts never 500, always 200 or 400, and the body never contains "Exception"/"StackTrace" (the Web viewer: tool errors still show the exception text in the browser #4283/Web: /api/ping detail and two degraded notes carry raw exception text (needs a ruling) #4316 rule).
    • FarFutureAt_StillDegradesCleanly_NoWiderWindowSignalOnTheWire — confirms the future-at clamp doesn't open a distinct wider-window code path on the wire.
    • These 4 tests run on net10.0-windows only (TestServer/live HTTP) — CI-only, not run locally. RED/GREEN not directly observable on macOS; build compiles clean and the logic mirrors an already-green sibling class (DarlingWebHostGateLiveTests) byte-for-byte in shape, so risk of a false pass is low, but this is unchecked locally, CI decides.
  2. Dedup beats the nearer row: DedupMatch_BeatsNearerRow_WhenTheNearerRowHasADifferentKey, NoDedupKey_FallsBackToNearestRow — pure logic reproducing the handler's exact scan-then-fallback rule (the handler has no extracted method to call directly; it's one inline lambda). GREEN, verified via a throwaway net10.0 console harness (see below).

  3. Four status arms, never "ongoing": UnknownArm_NeverContainsTheWord_Ongoing, FourStatusArms_AreDistinctStrings_AndNoneSayOngoing. GREEN via the harness.

  4. Window math / future-at clamp: ResolveAnchor_FarFutureAt_ClampsToNow_NotUnboundedForward (calls the real DarlingTriageEndpoint.ResolveAnchor, xunit — runs on CI only, but is a direct call to existing pure code so risk is minimal), WindowEnd_IsAnchorPlusSlack_ClampedToNow, WindowEnd_RecentAnchor_ClampsAtNow_NeverExceedsIt. GREEN via the harness for the window-math pair; ResolveAnchor_FarFutureAt_ClampsToNow_NotUnboundedForward matches the harness's FarFutureAt_ClampsToNow/FarFutureAt_AsOfNull cases exactly (also GREEN).

  5. Mechanical templates name only BuildReadDispatch reads: EveryMechanicalReadCell_NamesAKnownDispatchEntry — census over every DarlingTriageEndpoint.SectionsFor entry (every mapped metric + DefaultSections), asserting every .Read key is in DarlingWebEndpoints.BuildReadDispatch(). GREEN via the harness (0 offenders across the full section set).

  6. Stale-link degrade: UnknownMetricAndDedup_Returns200_WithEmptyAlertAndSections_NotAnError — an unknown metric+dedup against the dead pool never returns 500. CI-only (TestServer).

How the pure-logic pins were verified GREEN without a Windows test run: a throwaway net10.0 console project (AssemblyName=Darling.Tests so InternalsVisibleTo on PerformanceMonitor.Darling.Service applies) referencing the Service project directly, re-implementing each pure assertion inline and calling the real DarlingTriageEndpoint.ResolveAnchor/AnchorSlack/SectionsFor/SectionsByMetric and DarlingWebEndpoints.BuildReadDispatch(). All 11 checks printed PASS, TOTAL: 0 FAIL. Deleted after use; not part of the commit.

Endpoint review findings:

  • Defect found, not fixed (time-boxed out): windowStart (line ~199 of AlertNotebookEndpoint.cs) is computed (anchor - AlertMatchLookback, adjusted by incidentSince) but never read anywhere after — it doesn't feed windowEnd, isn't passed to any read cell, and isn't in the response body. Either dead code left over from an earlier draft, or a real gap (the read cells' own hours/as_of params don't derive from it either — they use FamilyLookbackHours + asOf from ResolveAnchor instead). Filing as a note here rather than fixing blind, since removing it or wiring it into the cells are both plausible and the correct one depends on intent I can't infer from the diff alone. This should be triaged before merge or as an immediate fast-follow.
  • Auth parity: Map is called from MapAll after DarlingTriageEndpoint.Map, inside the same ConfigurePipeline gate chain — no bypass found.
  • CancellationToken threading: context.RequestAborted is threaded into GetAlertHistoryAsync and ResolveStatusAsync's ExecuteScalarAsync call. This route isn't in BuildReadDispatch (it's a dedicated Map, like /api/triage), so WebReadCancellationPinTests's dispatch-table ratchet doesn't cover it — same posture as /api/triage itself, which also isn't in that ratchet. No gap introduced beyond the existing triage-endpoint precedent.
  • No raw exception text found in any response path — all catch blocks route through DarlingWebFailureLog.Report + a canned note string, matching Web viewer: tool errors still show the exception text in the browser #4283/Web: /api/ping detail and two degraded notes carry raw exception text (needs a ruling) #4316.
  • DarlingServerResolver.ResolveOrErrorAsync and the alert-history read are parameterized (Npgsql $1/$2/... placeholders), so the SQL-shaped server/dedup theory cases are string comparisons only, not injection vectors — confirmed by reading PgAlertHistoryStore.cs's parameter binding.

Not run locally (CI-only): the whole Darling.Tests project is net10.0-windows; only the throwaway harness ran the pure-logic pins. No live Postgres container was needed for verification — I started pmpr-4366 (port 55460) briefly per the brief but the pins never required it (the security/degrade pins use a dead-port pool by design, matching WebReadCancellationPinTests's own pattern); it has been removed.

CHANGELOG entry: already correctly labeled [#4366], linking pull/4366 — no fix needed.

Left unchecked, with reason: the 4 live-HTTP security theory/fact tests and the stale-link degrade test compile but did not run locally (no Windows runtime here); CI's build job runs the full suite. The windowStart dead-variable finding above is reported, not fixed, per the "don't fix blind" rule when intent is ambiguous.

pm-pr lane report (windowStart)

Question: windowStart (~line 199, before this change) was computed but never used downstream — dead code, or a broken window binding?

Answer: (b) dead code, per the #4222 spec. The spec's "Definition" section binds windows two ways: window_end = min(at + 15 min, now) is the shared anchor, but the paired window_start = min(Incident Since, at) − family lookback only feeds a composed cell's absolute range {windowStart, windowEnd} (#2735/#2788). Read cells get server, as_of = window_end, and hours = lookback — never a range. This slice's class doc already says so explicitly: "No composed cells are emitted here, so no cell carries an absolute range yet — that is this slice's honest gap, not an oversight." DarlingTriageEndpoint's own precedent binds the same way: its sections thread server/as_of per read, never a start/end pair. So every read cell here already carries the correct half of the window (as_of); windowStart had no consumer because this mechanical slice emits no composed cells, and removing it changes no behavior.

Change: removed the unused windowStart local (and the now-dead incidentSince local that only fed it) from AlertNotebookEndpoint.cs, and rewrote the window-math comment above windowEnd to say plainly why only the end is computed here.

JSON shape: unchanged — no new keys, no removed keys. #4368's SPA consumer needs no changes.

CHANGELOG: unaffected (no behavior change; the entry already covers the endpoint's addition).

Build: Darling/Darling.Tests/Darling.Tests.csproj — 0 Warning(s), 0 Error(s) on macOS, before and after the merge from origin/dev (clean, no conflicts).

Head: b3e0f8e8 → f2ae68c3 (dead-code removal commit 34cc7dc4 + merge commit f2ae68c3, no rebase).

pm-pr lane report (review fixes F1-F4, test auth)

New head: cabf3ebc (was f2ae68c3). Branch feature/4222-alert-notebook-endpoint, draft, unchanged base.

Extraction

Pulled the match logic out as internal static (Row, Incident) MatchAlert(rows, metric, dedup, anchor) and the two forward status arms out as internal static string? StatusFromHistory(rows, metric, anchor, matchedRow, serverId, fleetLevelStore) on AlertNotebookEndpoint. Both are pure over their inputs; StatusFromHistory returns null when neither arm fires so the caller falls through to the collector-freshness arms unchanged.

F1 (late resolution/re-fire)

Added a second, forward-looking history read [anchor, min(anchor + AlertMatchLookback, now)], same row cap (200), cancellable on RequestAborted. Its rows are unioned with the original match-window rows and passed to ResolveStatusAsync/StatusFromHistory. The notebook's cell window (windowEnd/asOf) is untouched.

F2 (unresolved-server leak)

StatusFromHistory now returns Unknown (not collected since …) immediately when serverId is null && !fleetLevelStore. When a server id is known, rows are filtered to that server before either arm runs.

F3 (dedup nearest, not newest)

MatchAlert now scans every dedup match and keeps the one nearest anchor (ties toward the earlier row), instead of the first (effectively newest, since rows arrive DESC) match.

F4 pins

  • MatchAlert: nearest-wins over a same-key re-fire in the tail (2 variants), dedup beats a nearer non-matching row, no-dedup falls back to nearest — all against real AlertHistoryReadRow + AlertContextSerializer-built ContextJson.
  • StatusFromHistory: resolution 40 min later reads Resolved (F1), another server's resolution is ignored when unresolved (F2), a known server ignores another server's rows, a later same-metric firing reads "Fired again", and a four-arms-distinct/no-"ongoing" census against the seam's real outputs.
  • Removed the old tautological DedupMatch_*/NoDedupKey_* pins (they copied logic into the test body) and kept the existing window/anchor/census pins as-is.
  • Live shape pin AuthenticatedRequest_ReturnsTheDocumentedNotebookShape: authenticated server=probe&metric=High+CPU against the dead-store fixture; asserts status starts "Unknown (not collected since ", definition.kind == "notebook", cell[0]/[1] types header/status, every cell[2..] is type read with limit+hours params, template.id == "mechanical/High CPU".
  • Security pins retained as-is (401 unauth, 403 out-of-CIDR); noted in the earlier PR round that the 401 pin also passes on dev — it proves pipeline position, not the route.

CI auth fix (pm-pr's follow-up finding)

CI showed 4 "hostile query" cases getting 302 instead of 200/400 — the old pins presented ?token= but never followed the token→cookie exchange, so they hit SetCookieAndRedirect and never reached the handler. Added SendAuthenticated: sends ?token= once, asserts 302 + captures Set-Cookie, then resends the real request with that cookie — the same two-step exchange DarlingWebHostGateLiveTests proves the gate performs. Applied to the hostile-query theory, the far-future-at pin, the stale-link pin (now asserts exactly 200, not "not 500"), and the new shape pin.

Build

dotnet build Darling/Darling.Tests/Darling.Tests.csproj: 0 Warning(s), 0 Error(s) on macOS. xUnit/live suites are net10.0-windows only; CI decides those.

JSON shape

Unchanged — no edits to AlertRowNode, HeaderCell, StatusCell, ReadCell, or the response object's key set. git merge origin/dev was already up to date (no new dev commits since f2ae68c).

Not done

F5/F6 and the PR body text are pm-pr's per the brief; not touched here.

Refs #4222.

GET /api/alert-notebook resolves an alert link (server, metric, at,
dedup) into a bound, read-only notebook definition: the dedup-matched
alert (falling back to the nearest same-metric row), a four-arm
status (Resolved / Fired again / No resolution recorded / Unknown),
and a mechanical notebook template built from the same per-metric
read list DarlingTriageEndpoint.SectionsFor already serves.

Reuses DarlingTriageEndpoint's ResolveAnchor, ResolutionAliases,
SectionsByMetric/SectionsFor, IsFleetLevelStoreServer and
IsFleetLevelStoreMetric rather than copying them; widens
AnchorSlack and AlertMatchLookback to internal for the same reason.
Adds AlertNotebookEndpointTests.cs covering:
- unauthenticated GET /api/alert-notebook returns 401 before the handler
  can dial the store (dead-pool proof, mirrors DarlingWebHostGateLiveTests)
- out-of-CIDR remote is forbidden before the token gate
- hostile/malformed authenticated queries (far-future at, SQL-ish server/
  dedup, an over-long metric) never 500 and carry no exception text
- dedup match beats a nearer-in-time row; no dedup falls back to nearest
- window math: window_end = min(anchor + AnchorSlack, now), and a
  far-future at clamps rather than widening the window
- the four status arms are distinct strings and none say "ongoing"
- every mechanical read cell names a BuildReadDispatch key (census)
- a stale/unknown alert link degrades to a non-error response
@erikdarlingdata
erikdarlingdata marked this pull request as ready for review September 26, 2026 01:53
@erikdarlingdata
erikdarlingdata merged commit 9aa1519 into dev Sep 26, 2026
15 of 16 checks passed
@erikdarlingdata
erikdarlingdata deleted the feature/4222-alert-notebook-endpoint branch September 26, 2026 01:53
erikdarlingdata added a commit that referenced this pull request Sep 26, 2026
* Refs #4222. Client render for the alert notebook (SPA slice)

- panels.js renderPanel and compose.js renderComposedPanelCard take an
  optional onSettled callback, fired once a cell's load reaches a terminal
  state, so a caller with its own concurrency budget knows when a slot frees.
- views.js renderNotebookDoc is refactored to accept either a saved view
  (mode: saved, unchanged behaviour) or an in-memory definition bound to a
  firing (mode: alert, read-only): no fleet scope picker, a max-3 in-flight
  InFlightLimiter gating read-cell loads, header/status cells rendered
  directly from the endpoint's own facts, Save as notebook and Open live
  controls.
- triage.js tries GET /api/alert-notebook first and renders its definition
  through renderNotebookDoc in alert mode; on a 404 (endpoint not present on
  this build) it falls back to the existing /api/triage assembly unchanged.
- New pin tests (Darling.Tests/AlertNotebookRenderClientTests.cs) over the
  shipped JS source text: in-memory render path, no /api/fleet call in
  alert mode, the max-3 limiter, and the Save-as-notebook provenance string.

Built against PR #4366 (feature/4222-alert-notebook-endpoint, head
ee63879) for the /api/alert-notebook response shape.

Status refresh on the 60s poll is out of scope (needs slice d's poll guard
on dev). Not opening a PR yet per the coordinator's rule: pins exist and
build 0/0; a successor lane should verify the shape against #4366 once it
lands and open the PR.

* Render current_value/threshold_value/detail_text; guard alert-mode read cells on route change (#4368)

* Fix #4368 CI: scope innerHTML pin to renderAlertHeaderCell, update panels.js loadPanel pin for onSettled

* Fix #4368 pin false positives: strip JS comments before innerHTML check, add census entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant