fix(health): rows 80+73 — unconfigured replication reports null, reconciler presence surfaced

ROW 80 (observability correctness). An unreplicated site published
LocalDbReplicationConnected=false / LocalDbOplogBacklog=0, contradicting the
nullable semantics every doc and the proto already state ("null means no
reading, NOT disconnected with an empty backlog").

Root cause: the replication engine is registered unconditionally and idles
without a peer, so ISyncStatus answers false/0 on a node that was never meant to
replicate, and LocalDbReplicationStatusReporter passed that straight through.
Two consequences: an operator read connected:false on a healthy site as a fault,
and central's null branches — row 33's conditional KPI emission and
sustained-badge anchor exclusion — never received the null they exist for, so
rig site-b/site-c emitted a permanently misleading
localDbReplicationConnected=0 series.

Fix mirrors the SiteEventLogReplicationCheck precedent: the Host evaluates
SiteLocalDbSetup.ReplicationIsConfigured ONCE and passes it to
AddLocalDbReplicationHealthBridge as a third provider; unconfigured
short-circuits the probe to (null, null). HealthMonitoring never reads LocalDb
config itself, and the same captured value now also feeds
SiteEventLogReplicationCheck so the two surfaces cannot disagree.
ISiteHealthCollector.SetLocalDbReplicationStatus widens to bool? — the
non-nullable bool was the type-level root cause (null was inexpressible).
false now means exactly one thing: configured, peer session down = a real fault.

Wire/central/UI needed no change: the proto already carries BoolValue/Int64Value
wrappers and the aggregator + KPI source already branch on null. This makes code
match doc, so no doc claim was weakened.

Watch the trap that caused this: configured-ness is PeerAddress OR ApiKey (only
the dialling half sets a peer address). The pre-existing SiteLocalDbWiringTests
fixture sets ApiKey and IS a replicating node, so its false/0 assertion is
correct and is kept, with a comment saying so; the new
UnreplicatedSiteLocalDbHealthTests fixture is the genuinely-unreplicated one.

ROW 73 (silent loss of self-heal). AkkaHostedService skips
SiteReconciliationActor when no IDeploymentConfigFetcher is resolvable, and that
skip was a lone boot Warning — a node with no self-heal path at all went on
reporting itself perfectly healthy forever.

Adds tri-state SiteHealthReport.StartupReconcilerActive (null = not reported,
i.e. the central self-report or a pre-row-73 site build; true = running;
false = booted without it), latched once at boot by the Host in both branches.
Additive proto field 29 (BoolValue, vendored C# regenerated via
docker/regen-proto.sh centralcontrol; csproj restored, verified empty diff).
The skip branch also raises its log to Error — no configuration makes the
fetcher legitimately absent on a site node — and writes a best-effort
deployment/Error site event so the evidence outlives the boot log.

Central treatment: a "No self-heal" WARNING badge on the site tile, not a
failure. Config still reaches the node via LocalDb replication while it is up;
what it lost is recovery from a deploy that landed while it was down — degraded,
not offline. Only the explicit false is badged (badging null would light up the
whole fleet the moment central is upgraded ahead of the sites), and offline
sites are skipped, matching the replication-backlog badge.

Fail-before evidence: with the row-80 gate behaviourally reverted,
UnreplicatedSite_HealthReport_ReportsNullForBothLocalDbFields fails
"Expected: null / Actual: False" and 3 of the 8 new reporter unit tests fail.

Tests: HealthMonitoring 128, Host 518, CentralUI 1051, Communication 717,
Commons 715, SiteEventLogging 81 — all green.
This commit is contained in:
Joseph Doherty
2026-08-15 21:40:48 -04:00
parent 3dc38fb674
commit a2f0086fa1
18 changed files with 921 additions and 67 deletions
@@ -37,7 +37,8 @@ Site clusters (metric collection and reporting). Central cluster (aggregation an
| `SiteAuditBacklog` | Audit Log (site) | Count of `Pending` rows in the site-local `AuditLog` plus oldest-pending-age plus on-disk bytes. Surfaced on the Audit KPI tiles (which border-highlight any non-zero backlog) and sampled into KPI history as `auditBacklogPending`. **No configurable per-site threshold badge exists for this metric** — earlier revisions of this document described one, but none was ever implemented and no such option exists. The only configurable sustained-threshold site badge is the LocalDb replication backlog (register row 33, below). |
| `SiteAuditWriteFailures` | Audit Log (site) | Count of failed hot-path audit appends at the site since the last health report. |
| `AuditRedactionFailure` | Audit Log (central) | Count of payload redactor errors (over-redacted payloads, safety-net hit) since the last interval. |
| `LocalDbReplicationConnected` | Consolidated site LocalDb | Whether a peer sync session is currently running on this node. **Nullable — null means "no reading", not "disconnected".** Reported only by a node with replication configured. |
| `LocalDbReplicationConnected` | Consolidated site LocalDb | Whether a peer sync session is currently running on this node. **Nullable — null means "no reading", not "disconnected".** Reported only by a node with replication configured; an unconfigured node reports **null**, never `false` (register row 80 — see *Unconfigured replication reports null* below). |
| `StartupReconcilerActive` | Host (site) | Whether this node spawned its `SiteReconciliationActor` at boot. **Tri-state:** null = not reported (the central self-report, or a pre-row-73 site build), `true` = running, `false` = the node booted with no `IDeploymentConfigFetcher` and therefore has **no startup self-heal**. Latched once at boot by the Host — not polled. Drives the *No self-heal* dashboard badge (register row 73). |
| `LocalDbOplogBacklog` | Consolidated site LocalDb | Unacked oplog depth (Prometheus `localdb_oplog_depth`). **Nullable — null means "no reading", NOT "connected with an empty backlog"** (a failed poll rendered as 0 would report a pair that cannot read its own oplog as perfectly healthy). **Expected transient spikes:** the daily `site_events` retention purge and the storage-cap trim are CDC-captured like any other write, so the backlog jumps by the batch size at purge time and drains as the peer acks it — see `Component-SiteEventLogging.md` → Storage and `docs/deployment/topology-guide.md` → *Reading the replication backlog* for the healthy-versus-faulty signature. |
## Reporting Protocol
@@ -141,6 +142,57 @@ reporting". Note `false` and `null` are different states for the connected flag
that is down samples as `0`; no link at all samples as nothing. Both fold as **Gauge** in the hourly
rollup catalog (summing an oplog depth, or a 0/1 flag, per hour would be meaningless).
## Unconfigured Replication Reports Null
The two LocalDb fields are produced by `LocalDbReplicationStatusReporter`, a 30 s site-side poll
over `ISyncStatus`. That poll is **gated on whether the node is configured to replicate at all**,
and an unconfigured node publishes `null` for *both* fields.
The gate is load-bearing because the replication engine is registered **unconditionally** on every
site node and sits inert without a peer — so `ISyncStatus` answers `Connected = false` with a real
`0` backlog on a site that was never meant to replicate. Passing that pair through (the behaviour
before register row 80) had two consequences:
- An operator reading a healthy, deliberately-unreplicated site saw `connected: false` and read a
fault that did not exist.
- Every null branch downstream was dead code on the rig. The conditional KPI emission above and the
sustained-badge anchor exclusion below both exist to handle "no reading", but rig site-b/site-c
never sent one — they emitted a permanently misleading `localDbReplicationConnected = 0` series
instead.
`false` therefore has exactly one meaning on this field: **replication is configured on this node
and the peer session is currently down** — a genuine fault. Beware the specific trap that produced
the bug: configured-ness is `PeerAddress` **OR** `ApiKey` (only the dialling half sets a peer
address; the passive half carries the key alone), so "no `PeerAddress`" does **not** mean
"unreplicated".
The gate value is supplied by the **Host** from `SiteLocalDbSetup.ReplicationIsConfigured`, exactly
as `SiteEventLogReplicationCheck` is supplied to Site Event Logging. Health Monitoring never reads
LocalDb configuration itself, and the PeerAddress-OR-ApiKey rule stays in one place.
## Startup Reconciler Presence
`SiteReconciliationActor` is the per-node startup self-heal: a standby that was DOWN during a
deploy reports its local inventory to central on its next boot, fetches the gap and writes it
(and, since register row 67, restarts Instance Actors the boot-time load missed). The Host skips
it when no `IDeploymentConfigFetcher` is resolvable.
That skip used to be a single boot Warning and nothing else, so a node that had silently lost its
entire self-heal path went on reporting itself perfectly healthy (register row 73). It is now
surfaced three ways:
- **`StartupReconcilerActive` on the health report**, latched at boot by the Host in both branches.
- **A boot-time `Error` log line** (raised from Warning — no configuration makes the fetcher
legitimately absent on a site node) plus a **site event** (`deployment` / `Error`), so the
evidence survives after the boot log has scrolled away.
- **A `No self-heal` warning badge** on the site's Health dashboard tile.
Central's treatment is deliberately **Warning, not failure**. Config still reaches the node through
LocalDb replication while it is up; what it has lost is recovery from a deploy that landed while it
was *down*. That is degraded, not offline. Only the explicit `false` is badged — `null` means "not
reported", and badging it would light up the whole fleet the moment central is upgraded ahead of
the sites. Offline sites are not badged, matching the replication-backlog badge.
## Prometheus Metrics (site node)
Distinct from the health *report* above: these are scraped from the site node's `/metrics`
@@ -182,6 +234,7 @@ endpoint (`MetricsPort`, default 8084) rather than sent to central.
- **Site Runtime (site)**: Provides script error rate and alarm evaluation error rate metrics.
- **Store-and-Forward Engine (site)**: Provides buffer depth metrics, including the notification backlog awaiting forward to central.
- **Cluster Infrastructure (site)**: Provides node role status.
- **Host (site)**: Supplies the two boot-time facts Health Monitoring must not derive for itself — the LocalDb replication-configured gate behind `LocalDbReplicationConnected`/`LocalDbOplogBacklog` (from `SiteLocalDbSetup.ReplicationIsConfigured`), and `StartupReconcilerActive`, latched where the Host decides whether to spawn `SiteReconciliationActor`. Both follow the `SiteEventLogReplicationCheck` precedent: the Host owns the configuration reading, the component owns the reporting.
- **Notification Outbox (central)**: Provides central-computed outbox KPIs — queue depth, stuck count, parked count — for the headline dashboard tiles.
- **Site Call Audit (central)**: Provides central-computed cached-call KPIs — buffered count, parked count, failed/delivered (last interval), oldest pending age, stuck count — for the headline dashboard tiles.
- **Site Event Logging (site)**: Provides `SiteEventLogWriteFailures`. Its daily retention purge and storage-cap trim are also the expected cause of transient `LocalDbOplogBacklog` spikes on a replicated node — see [Component-SiteEventLogging.md](Component-SiteEventLogging.md) → Storage.