Skip to content

Every alert went to every channel because the model had one routing decision, made at install time: a sparse routes table sends self-monitor, reports, agent-jobs and performance pages where their audiences are, the taxonomy is written down, and the ledger says where each post went (#3598) - #3668

Merged
erikdarlingdata merged 2 commits into
devfrom
feat/3598-routes
Sep 19, 2026

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

The lie

The notification model had one routing decision in it, made at install time. config.config_notification is a single row — CHECK (id = 1) — holding one destination per channel type, and WebhookAlertService.TrySendWebhookAlertsAsync fanned every alert to every configured channel without consulting what the alert was. One stream interleaved four audiences: the monitor's own health (Compression Job Stuck, Retention Held), scheduled prose (Collector Cost Digest, Fleet Sweep Rollup), Agent job failures, and the deadlock and blocking pages the channel exists for. The product knew the answer at send time — the metric name is in hand at every call site — and threw it away; a reader did the routing in their head on every post. The generic-webhook template branching on {{metric}} in an external router was the only escape hatch, and it forfeited the native Slack rendering.

The fix — V131, a sparse routes table layered over the parent row

config.config_notification_routes: route_id (identity), metric_match (a family name or an exact metric name), one destination per channel type under the parent row's own column names — teams_url, slack_url, generic_url, pagerduty_routing_key, smtp_recipients, all NOT NULL DEFAULT '' — enabled, modified_at, and a generated configured_channels presence column. The config_collector_schedules-over-code-defaults shape: empty means inherit, so a store with zero rows behaves byte for byte as it did and the rung is a no-op for every existing install. A V117-shaped statement-level trigger bumps config_version on every write, so the running service hot-reloads a route on its next sweep (design point 5, pinned live).

The taxonomy is written down (design point 1). AlertFamily in Notifications is the closed enum — self-monitor, reports, agent-jobs, performance — with the census of every delivered metric name each family owns, the two dynamic prefixes (Custom: and Analysis: → performance), and the two delivered recoveries paired with their firing. A census test walks every metric constant the engines declare plus every string literal at an AlertOutcome(/FireAsync( fire site (read from source) and asserts each is classified explicitly, that no census entry is an orphan, and that every AlertSeverity.ForMetric arm is in the census — so a new alert cannot ship unclassified. The fall-through is performance, deliberately: an unclassified alert lands where every alert landed yesterday, never in a report channel someone stopped watching.

Resolution (NotificationRouter.Resolve, pure): per channel, an enabled route matching the metric exactly — its own name first, then the firing a recovery pairs with — then an enabled route matching its family, then the parent default; within a level the lowest route_id with a non-empty column. "Per channel" is the load-bearing phrase: an exact route that sets only Slack takes Slack and lets Teams fall to the family route and then to the parent. A route can enable a channel the parent lacks — a PagerDuty key on the performance route alone, with none on the parent, is how "only pages page" is spelled. Resolution runs after the cooldown and the repeat budget in both the webhook fan-out and the email path (design point 2): it changes where a post lands, never whether it is sent, and a throttled alert never consults a route.

Recoveries follow their firing (design point 4). The shared engine's condition-cleared notices never reach a channel — AlertResolution is a history row by design — so the only recoveries a route sees are the two the self-alert evaluator fires under their own names: Server Restored routes as Server Unreachable, AG Replica Reconnected as AG Replica Disconnected. An exact route on the firing catches the clear; a route spelling the recovery's own name outranks it.

The ledger says where it went (design point 3). AlertContextDto gains a trailing Route member beside #3635's Severity — {Family, RouteId, Destinations:[{Channel, RouteId, Source}]} — written by DarlingAlertDeliverer and DarlingFindingAlertSender from the decision the fan-out actually used, no schema change. get_alert_history surfaces it as route on both SKUs (Lite's is always null; it has no routes and says so).

Secrets. Four of the five destination columns are the same bearer secrets they are on the parent row, so they join the fail-closed viewer/mcp column carve (ViewerRestrictedConfigTables, mirrored in provision-roles.sql). The generated configured_channels column exists precisely for the roles denied the URLs: it discloses which channels a route sets and nothing else. mcp gets exactly the two writes that move no destination — UPDATE (enabled, modified_at) and DELETE — matching the parent's posture (the one config_notification write mcp holds is the cooldown column); authoring a route is the Viewer's Settings window.

Surfaces

  • Viewer: Settings → Notifications → Manage Notification Routes… (a MuteRulesWindow-shaped grid + edit dialog whose family list is filled from AlertFamily.All, never a copy). A read-only seat sees the routes with presence, not URLs.
  • MCP: get_notification_routes (taxonomy + routes, destinations withheld, resolution order stated), set_notification_route_enabled, delete_notification_route. Lite gets get_notification_routes too (taxonomy + routes_supported: false). Not folded into get_alert_settings: that payload's read→modify→write invariant (every emitted key writable) is pinned, and mute rules have their own read tool for the same reason.
  • Docs: Darling README notification-routes section with the four families' membership and the resolution order; the generic-webhook workaround retired to "still works"; the PostgreSQL runbook's alerting section says where its eight alerts land.

Why this shape

Store-only ([JsonIgnore] on DarlingConfig.NotificationRoutes), like mute rules: a route is live tuning through the Settings grid, and a darling.json copy would be overwritten by the first store reload exactly as Alerts is. Reaches the shared services through a default interface member on IAlertSettings (NotificationRoutes => Array.Empty<>(), the ICollectorSchemaInfo.RequiredPgExtensions precedent), so Lite's adapter opts into nothing and only Darling's store-backed adapter overrides it. Proxies, the generic channel's headers/template and the PagerDuty region stay on the parent: they say how a channel type is reached, not where a family lands.

What it does NOT do

  • A route cannot silence a channel the parent has — empty means inherit, per the issue's sketch. The way to keep reports off PagerDuty is to put the PagerDuty key on the performance route and off the parent. A sink sentinel is the natural next rung and is deliberately not here.
  • The email column only redirects recipients; host, from and credentials live on the parent, so email is delivered only where the parent's SMTP is configured.
  • No web-dashboard editor for routes (so no viewer-role write); mcp cannot author or re-point a route.
  • Analysis: … findings are performance (they carry a severity and a remedy over a monitored server's data). Their hash-suffixed names make exact routes impractical; if that audience differs, a fifth family is a one-line census change.
  • Lite has no routes table; it resolves every firing to its parent channels and publishes the taxonomy.

Tests

Executed on this Mac via a throwaway harness (net10.0-windows, WindowsDesktop framework entry removed): NotificationRoutingTests (15) — census in both directions + severity arms, zero-routes ⇒ every channel Same() as the settings member for every delivered metric, exact > family > default per channel, route-enables-a-channel, disabled skipped / lowest id wins, recovery pairing, case/trim, ledger DTO round trip and pre-#3598 rows, and a real loopback fan-out through DarlingAlertDeliverer (parent endpoint gets the page whose body equals BuildSlackPayload's own render on the fixed clock; the self-monitor endpoint gets the self-alert; fire+clear both land on the exact route's endpoint; four ledger rows carry the route). NotificationRoutesRungTests (rung DDL/trigger/no-ACL, viewer carve classification vs parent, mcp grant shape, MCP-store non-secret read, service/viewer/DDL column parity, validation, MCP descriptions) and NotificationRoutesLivePostgresTests (DARLING_TEST_PG, timescale/timescaledb:2.28.1-pg18: full ladder 1→131 applies, generated column populates, beacon bumps on INSERT/UPDATE/DELETE, StoreConfigProvider reads the row, ApplyToConfig swaps it by reference, the resolver routes on it). Plus the neighbouring pins re-run green: DarlingManagedRolesTests, ProvisionRolesAclDriftTests, McpConfigReadAvoidsSecretColumnsTests, McpPageContractTests, McpLatestSnapshotStampTests, EngineCapabilityMissTests, AsOfWindowAnchorTests, DocCommentHygieneTests, AlertDeliveryChannelTests, DarlingMcpAlertToolsTests, AlertHistoryRowSeverityTests, and Lite's CrossAppMcpToolInventoryPinTests (tool censuses 88/156). The viewer probe's top-arm pin and the WPF windows are first executed in CI.

Closes #3598

…ble: self-monitor, reports, agent-jobs and performance pages stop interleaving on one stream, the taxonomy is written down, and the ledger says where each post went (#3598)
…documents the two route writes as web-less, so the tool-catalog parity pin holds (#3598)
@erikdarlingdata

Copy link
Copy Markdown
Owner Author

Coordinator review under the review-dark window (the bot cannot post on dev until the #3650 fix reaches main). Read the three hunks the self-review named as riskiest: (1) NotificationRouter.Resolve — exact → recovery-pair → family → parent, enabled routes only, lowest route_id within a level, per channel; the default arm returns the settings member untouched only when the parent channel is enabled AND non-blank, which is exactly the old XConfigured predicate, so zero routes is byte-identical by construction and the pin proves it. (2) The fan-out replaces the four gates 1:1 with route.X.Destination is { } url after the cooldown/budget decision — one resolution per firing, as the issue's design point 2 requires. (3) V131 is V117's trigger shape on a table whose generated configured_channels gives the viewer its column for free; ladder 129 → 130 → 131 is correct on the branch and the live test walked it on PG18/Timescale. Two of the self-review's reviewer questions are real and recorded on #3598 as follow-ups rather than scope tonight: a per-channel SINK so a family can be silenced on one channel (today routes can only add/redirect — 'empty = inherit' per the issue's spec), and email's parent to still being required when routes carry the only recipients. Arming.

@erikdarlingdata
erikdarlingdata merged commit aad299e into dev Sep 19, 2026
6 of 8 checks passed
@erikdarlingdata
erikdarlingdata deleted the feat/3598-routes branch September 19, 2026 00:25
erikdarlingdata added a commit that referenced this pull request Sep 19, 2026
…spliced once

Fifty-nine PRs merged to dev today across the coordinator's lanes and the wave-2
worker's; each lane returned its entry to a buffer instead of touching this file,
so that fifty-plus PRs did not each rebase the same twenty lines. This is the one
splice. Every entry is one line (the archiver's compact() and the pins read them
that way); riders fold into their parent's entry (#3599 under #3590, #3619 under
#3611, #3623 under #3616, #3640 under #3633; #3617 test-only and #3661 re-cut as
#3666 carry none); #3657's entry is in because it MERGED to dev - the twin to main
is what is still pending.

[Unreleased] gains a `### Added` above `### Fixed` (Keep-a-Changelog order) for the
six new capabilities: per-user theme colours (#3606 / #3577 arm B), routed alert
families (#3668 / #3598), the PostgreSQL logging audit tool (#3643 / #3607), the
service-side wait sampler (#3645 / #3604), and the log-event classifier with its
temp-file / autovacuum parser families (#3646 / #3601, #3664 / #3602 #3603). The
other forty-eight are honesty fixes to existing surfaces and append to `### Fixed`
after the wave-1 bullets, in PR-number order.

Thirty-six reference definitions added for the issues the new entries cite and the
index did not yet define; the [Unreleased] group is one ascending run again, which
moves [#3557] into its slot (the one deleted line). Nothing under ## [3.8.0] or
older is touched; the archive script was not run. One editorial touch: the #3585
entry ended in a dangling "Darling" and now reads "Darling only." (the tool exists
only in the Darling MCP host).

tools/changelog/changelog_archive.py verify: all PASS (1420 bold entries, floor
1,329; 1,358 distinct refs resolve; CRLF throughout; 356,539 bytes under the
750 KiB ceiling). ChangelogIndexAndArchiveTests: 5/5 pass via a net10.0 harness.
erikdarlingdata added a commit that referenced this pull request Sep 19, 2026
…spliced once (#3672)

Fifty-nine PRs merged to dev today across the coordinator's lanes and the wave-2
worker's; each lane returned its entry to a buffer instead of touching this file,
so that fifty-plus PRs did not each rebase the same twenty lines. This is the one
splice. Every entry is one line (the archiver's compact() and the pins read them
that way); riders fold into their parent's entry (#3599 under #3590, #3619 under
#3611, #3623 under #3616, #3640 under #3633; #3617 test-only and #3661 re-cut as
#3666 carry none); #3657's entry is in because it MERGED to dev - the twin to main
is what is still pending.

[Unreleased] gains a `### Added` above `### Fixed` (Keep-a-Changelog order) for the
six new capabilities: per-user theme colours (#3606 / #3577 arm B), routed alert
families (#3668 / #3598), the PostgreSQL logging audit tool (#3643 / #3607), the
service-side wait sampler (#3645 / #3604), and the log-event classifier with its
temp-file / autovacuum parser families (#3646 / #3601, #3664 / #3602 #3603). The
other forty-eight are honesty fixes to existing surfaces and append to `### Fixed`
after the wave-1 bullets, in PR-number order.

Thirty-six reference definitions added for the issues the new entries cite and the
index did not yet define; the [Unreleased] group is one ascending run again, which
moves [#3557] into its slot (the one deleted line). Nothing under ## [3.8.0] or
older is touched; the archive script was not run. One editorial touch: the #3585
entry ended in a dangling "Darling" and now reads "Darling only." (the tool exists
only in the Darling MCP host).

tools/changelog/changelog_archive.py verify: all PASS (1420 bold entries, floor
1,329; 1,358 distinct refs resolve; CRLF throughout; 356,539 bytes under the
750 KiB ceiling). ChangelogIndexAndArchiveTests: 5/5 pass via a net10.0 harness.
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