From a2f0086fa1933adeacfeda6ddc03096c5972de93 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 15 Aug 2026 21:40:48 -0400 Subject: [PATCH 1/2] =?UTF-8?q?fix(health):=20rows=2080+73=20=E2=80=94=20u?= =?UTF-8?q?nconfigured=20replication=20reports=20null,=20reconciler=20pres?= =?UTF-8?q?ence=20surfaced?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Component-HealthMonitoring.md | 55 +++++- .../Components/Pages/Monitoring/Health.razor | 24 +++ .../Messages/Health/SiteHealthReport.cs | 49 ++++- .../CentralControlGrpc/CentralControl.cs | 129 +++++++++---- .../Grpc/CentralControlDtoMapper.cs | 2 + .../Protos/central_control.proto | 6 + .../ISiteHealthCollector.cs | 34 +++- .../LocalDbReplicationStatusReporter.cs | 34 ++++ .../ServiceCollectionExtensions.cs | 17 +- .../SiteHealthCollector.cs | 28 ++- .../Actors/AkkaHostedService.cs | 49 ++++- .../SiteServiceRegistration.cs | 31 ++- .../Pages/HealthPageTests.cs | 115 +++++++++++ .../Grpc/CentralControlDtoMapperTests.cs | 7 + .../LocalDbReplicationStatusReporterTests.cs | 182 ++++++++++++++++++ .../StartupReconcilerHealthFieldTests.cs | 79 ++++++++ .../SiteLocalDbWiringTests.cs | 13 +- .../UnreplicatedSiteLocalDbHealthTests.cs | 134 +++++++++++++ 18 files changed, 921 insertions(+), 67 deletions(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests/LocalDbReplicationStatusReporterTests.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests/StartupReconcilerHealthFieldTests.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.Host.Tests/UnreplicatedSiteLocalDbHealthTests.cs diff --git a/docs/requirements/Component-HealthMonitoring.md b/docs/requirements/Component-HealthMonitoring.md index e33f8c40..a9d353b6 100644 --- a/docs/requirements/Component-HealthMonitoring.md +++ b/docs/requirements/Component-HealthMonitoring.md @@ -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. diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/Health.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/Health.razor index c2b0f5be..21f5c5f2 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/Health.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/Health.razor @@ -267,6 +267,14 @@ Replication backlog } + @if (IsStartupReconcilerMissing(state)) + { + + No self-heal + + } @if (!state.IsOnline && state.LastStatusChangeAt is { } changedAt) { offline since @changedAt.ToString("u") @@ -566,6 +574,22 @@ && state.LocalDbBacklogAboveSince is { } since && DateTimeOffset.UtcNow - since >= HealthOptions.Value.LocalDbBacklogSustainedFor; + // Register row 73: the "No self-heal" badge. + // + // Warning-level treatment, deliberately not an error and never an offline-style state: + // the node still receives config through LocalDb replication while it is up, so what it + // has lost is recovery from a deploy that landed while it was DOWN. That is degraded, + // not down — which is exactly what a warning badge says. + // + // Tri-state, and only the explicit false is badged. Null means "not reported": the + // central self-report row (where a site reconciler is meaningless) and any site node on + // a pre-row-73 build both send null, and badging those would fire on every site the + // moment central is upgraded ahead of the sites. Offline sites are skipped for the same + // reason the backlog badge skips them — the Offline badge is the signal that matters. + private bool IsStartupReconcilerMissing(SiteHealthState state) => + state.IsOnline + && state.LatestReport?.StartupReconcilerActive == false; + // Online central nodes, from the same ClusterNodes list the Nodes column renders. // Fewer than two means no standby, which disables the manual-failover control. This is // a display-side guard only — AkkaManualFailoverService re-checks against live cluster diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Health/SiteHealthReport.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Health/SiteHealthReport.cs index ec43d5a1..346dd034 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Health/SiteHealthReport.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Health/SiteHealthReport.cs @@ -103,12 +103,23 @@ public record SiteHealthReport( /// or when replication is not wired on this node. /// /// + /// /// Nullable on purpose, and is NOT the same as null. - /// Replication ships default-OFF, so a site node with no peer configured reports - /// — that is a healthy, expected state, not an outage. Only - /// a node that was configured with a peer and is reporting - /// is degraded, and distinguishing those two cases is the operator's job, not this - /// field's. + /// Null means "no reading" — replication is not configured on this node + /// (the default-OFF posture), or the reporter has not run yet. + /// means "configured, and the peer session is down", + /// which is a genuine fault worth investigating. + /// + /// + /// The site side must never publish for an unconfigured node + /// (deferred-work register row 80): the replication engine is registered + /// unconditionally and answers Connected=false while inert, so passing that + /// through made every deliberately-unreplicated site look like a broken replicated + /// one, and starved central's null branches (conditional KPI emission, + /// sustained-badge anchor exclusion) of the null they were built for. + /// LocalDbReplicationStatusReporter gates on the Host-supplied + /// SiteLocalDbSetup.ReplicationIsConfigured to keep that distinction honest. + /// /// public bool? LocalDbReplicationConnected { get; init; } @@ -134,6 +145,34 @@ public record SiteHealthReport( /// 0 so existing producers/tests that don't populate it stay valid. /// public long SiteStreamAlarmDropCount { get; init; } + + /// + /// Whether this node spawned its SiteReconciliationActor at boot — the per-node + /// startup self-heal that lets a standby which was DOWN during a deploy recover its + /// deployed inventory (and, since register row 67, restart the Instance Actors a + /// boot-time load missed). + /// + /// + /// + /// Tri-state, and the distinction is the whole point (deferred-work register row 73). + /// Null = not reported: a central self-report, where the notion does not apply, + /// or a site node running a pre-row-73 build. = the + /// reconciler is running. = the node booted WITHOUT it, + /// because no IDeploymentConfigFetcher was resolvable. + /// + /// + /// That false case used to be invisible: AkkaHostedService logged a single boot + /// Warning and spawned nothing, so a misconfigured site reported itself perfectly + /// healthy forever while being unable to self-heal. Central badges it rather than + /// failing the site — replication remains the primary path, so the node is degraded, + /// not down. + /// + /// + /// Stamped once at boot by the Host (which owns the spawn decision) via + /// ISiteHealthCollector.SetStartupReconcilerActive; it is not polled. + /// + /// + public bool? StartupReconcilerActive { get; init; } } /// diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/CentralControlGrpc/CentralControl.cs b/src/ZB.MOM.WW.ScadaBridge.Communication/CentralControlGrpc/CentralControl.cs index f40a9138..44fb952b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/CentralControlGrpc/CentralControl.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/CentralControlGrpc/CentralControl.cs @@ -72,7 +72,7 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Grpc { "ZXkYASABKAkSQQoFdmFsdWUYAiABKAsyMi5zY2FkYWJyaWRnZS5jZW50cmFs", "Y29udHJvbC52MS5UYWdRdWFsaXR5Q291bnRzRHRvOgI4ASJQChFOb2RlU3Rh", "dHVzTGlzdER0bxI7CgVub2RlcxgBIAMoCzIsLnNjYWRhYnJpZGdlLmNlbnRy", - "YWxjb250cm9sLnYxLk5vZGVTdGF0dXNEdG8iig4KE1NpdGVIZWFsdGhSZXBv", + "YWxjb250cm9sLnYxLk5vZGVTdGF0dXNEdG8iyQ4KE1NpdGVIZWFsdGhSZXBv", "cnREdG8SDwoHc2l0ZV9pZBgBIAEoCRIXCg9zZXF1ZW5jZV9udW1iZXIYAiAB", "KAMSNAoQcmVwb3J0X3RpbWVzdGFtcBgDIAEoCzIaLmdvb2dsZS5wcm90b2J1", "Zi5UaW1lc3RhbXAScAoYZGF0YV9jb25uZWN0aW9uX3N0YXR1c2VzGAQgAygL", @@ -106,42 +106,43 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Grpc { "YmxlVmFsdWUSQgoebG9jYWxfZGJfcmVwbGljYXRpb25fY29ubmVjdGVkGBsg", "ASgLMhouZ29vZ2xlLnByb3RvYnVmLkJvb2xWYWx1ZRI7ChZsb2NhbF9kYl9v", "cGxvZ19iYWNrbG9nGBwgASgLMhsuZ29vZ2xlLnByb3RvYnVmLkludDY0VmFs", - "dWUacgobRGF0YUNvbm5lY3Rpb25TdGF0dXNlc0VudHJ5EgsKA2tleRgBIAEo", - "CRJCCgV2YWx1ZRgCIAEoDjIzLnNjYWRhYnJpZGdlLmNlbnRyYWxjb250cm9s", - "LnYxLkNvbm5lY3Rpb25IZWFsdGhFbnVtOgI4ARpxChhUYWdSZXNvbHV0aW9u", - "Q291bnRzRW50cnkSCwoDa2V5GAEgASgJEkQKBXZhbHVlGAIgASgLMjUuc2Nh", - "ZGFicmlkZ2UuY2VudHJhbGNvbnRyb2wudjEuVGFnUmVzb2x1dGlvblN0YXR1", - "c0R0bzoCOAEaQgogU3RvcmVBbmRGb3J3YXJkQnVmZmVyRGVwdGhzRW50cnkS", - "CwoDa2V5GAEgASgJEg0KBXZhbHVlGAIgASgFOgI4ASJjChZTaXRlSGVhbHRo", - "UmVwb3J0QWNrRHRvEg8KB3NpdGVfaWQYASABKAkSFwoPc2VxdWVuY2VfbnVt", - "YmVyGAIgASgDEhAKCGFjY2VwdGVkGAMgASgIEg0KBWVycm9yGAQgASgJIosB", - "CgxIZWFydGJlYXREdG8SDwoHc2l0ZV9pZBgBIAEoCRIVCg1ub2RlX2hvc3Ru", - "YW1lGAIgASgJEhEKCWlzX2FjdGl2ZRgDIAEoCBItCgl0aW1lc3RhbXAYBCAB", - "KAsyGi5nb29nbGUucHJvdG9idWYuVGltZXN0YW1wEhEKCXN5bnRoZXRpYxgF", - "IAEoCCq9AQoUQ29ubmVjdGlvbkhlYWx0aEVudW0SIQodQ09OTkVDVElPTl9I", - "RUFMVEhfVU5TUEVDSUZJRUQQABIfChtDT05ORUNUSU9OX0hFQUxUSF9DT05O", - "RUNURUQQARIiCh5DT05ORUNUSU9OX0hFQUxUSF9ESVNDT05ORUNURUQQAhIg", - "ChxDT05ORUNUSU9OX0hFQUxUSF9DT05ORUNUSU5HEAMSGwoXQ09OTkVDVElP", - "Tl9IRUFMVEhfRVJST1IQBDKhBgoVQ2VudHJhbENvbnRyb2xTZXJ2aWNlEoMB", - "ChJTdWJtaXROb3RpZmljYXRpb24SNC5zY2FkYWJyaWRnZS5jZW50cmFsY29u", - "dHJvbC52MS5Ob3RpZmljYXRpb25TdWJtaXREdG8aNy5zY2FkYWJyaWRnZS5j", - "ZW50cmFsY29udHJvbC52MS5Ob3RpZmljYXRpb25TdWJtaXRBY2tEdG8SkgEK", - "F1F1ZXJ5Tm90aWZpY2F0aW9uU3RhdHVzEjkuc2NhZGFicmlkZ2UuY2VudHJh", - "bGNvbnRyb2wudjEuTm90aWZpY2F0aW9uU3RhdHVzUXVlcnlEdG8aPC5zY2Fk", - "YWJyaWRnZS5jZW50cmFsY29udHJvbC52MS5Ob3RpZmljYXRpb25TdGF0dXNS", - "ZXNwb25zZUR0bxJHChFJbmdlc3RBdWRpdEV2ZW50cxIbLnNpdGVzdHJlYW0u", - "QXVkaXRFdmVudEJhdGNoGhUuc2l0ZXN0cmVhbS5Jbmdlc3RBY2sSUAoVSW5n", - "ZXN0Q2FjaGVkVGVsZW1ldHJ5EiAuc2l0ZXN0cmVhbS5DYWNoZWRUZWxlbWV0", - "cnlCYXRjaBoVLnNpdGVzdHJlYW0uSW5nZXN0QWNrEoABCg1SZWNvbmNpbGVT", - "aXRlEjYuc2NhZGFicmlkZ2UuY2VudHJhbGNvbnRyb2wudjEuUmVjb25jaWxl", - "U2l0ZVJlcXVlc3REdG8aNy5zY2FkYWJyaWRnZS5jZW50cmFsY29udHJvbC52", - "MS5SZWNvbmNpbGVTaXRlUmVzcG9uc2VEdG8SfQoQUmVwb3J0U2l0ZUhlYWx0", - "aBIyLnNjYWRhYnJpZGdlLmNlbnRyYWxjb250cm9sLnYxLlNpdGVIZWFsdGhS", - "ZXBvcnREdG8aNS5zY2FkYWJyaWRnZS5jZW50cmFsY29udHJvbC52MS5TaXRl", - "SGVhbHRoUmVwb3J0QWNrRHRvElAKCUhlYXJ0YmVhdBIrLnNjYWRhYnJpZGdl", - "LmNlbnRyYWxjb250cm9sLnYxLkhlYXJ0YmVhdER0bxoWLmdvb2dsZS5wcm90", - "b2J1Zi5FbXB0eUIrqgIoWkIuTU9NLldXLlNjYWRhQnJpZGdlLkNvbW11bmlj", - "YXRpb24uR3JwY2IGcHJvdG8z")); + "dWUSPQoZc3RhcnR1cF9yZWNvbmNpbGVyX2FjdGl2ZRgdIAEoCzIaLmdvb2ds", + "ZS5wcm90b2J1Zi5Cb29sVmFsdWUacgobRGF0YUNvbm5lY3Rpb25TdGF0dXNl", + "c0VudHJ5EgsKA2tleRgBIAEoCRJCCgV2YWx1ZRgCIAEoDjIzLnNjYWRhYnJp", + "ZGdlLmNlbnRyYWxjb250cm9sLnYxLkNvbm5lY3Rpb25IZWFsdGhFbnVtOgI4", + "ARpxChhUYWdSZXNvbHV0aW9uQ291bnRzRW50cnkSCwoDa2V5GAEgASgJEkQK", + "BXZhbHVlGAIgASgLMjUuc2NhZGFicmlkZ2UuY2VudHJhbGNvbnRyb2wudjEu", + "VGFnUmVzb2x1dGlvblN0YXR1c0R0bzoCOAEaQgogU3RvcmVBbmRGb3J3YXJk", + "QnVmZmVyRGVwdGhzRW50cnkSCwoDa2V5GAEgASgJEg0KBXZhbHVlGAIgASgF", + "OgI4ASJjChZTaXRlSGVhbHRoUmVwb3J0QWNrRHRvEg8KB3NpdGVfaWQYASAB", + "KAkSFwoPc2VxdWVuY2VfbnVtYmVyGAIgASgDEhAKCGFjY2VwdGVkGAMgASgI", + "Eg0KBWVycm9yGAQgASgJIosBCgxIZWFydGJlYXREdG8SDwoHc2l0ZV9pZBgB", + "IAEoCRIVCg1ub2RlX2hvc3RuYW1lGAIgASgJEhEKCWlzX2FjdGl2ZRgDIAEo", + "CBItCgl0aW1lc3RhbXAYBCABKAsyGi5nb29nbGUucHJvdG9idWYuVGltZXN0", + "YW1wEhEKCXN5bnRoZXRpYxgFIAEoCCq9AQoUQ29ubmVjdGlvbkhlYWx0aEVu", + "dW0SIQodQ09OTkVDVElPTl9IRUFMVEhfVU5TUEVDSUZJRUQQABIfChtDT05O", + "RUNUSU9OX0hFQUxUSF9DT05ORUNURUQQARIiCh5DT05ORUNUSU9OX0hFQUxU", + "SF9ESVNDT05ORUNURUQQAhIgChxDT05ORUNUSU9OX0hFQUxUSF9DT05ORUNU", + "SU5HEAMSGwoXQ09OTkVDVElPTl9IRUFMVEhfRVJST1IQBDKhBgoVQ2VudHJh", + "bENvbnRyb2xTZXJ2aWNlEoMBChJTdWJtaXROb3RpZmljYXRpb24SNC5zY2Fk", + "YWJyaWRnZS5jZW50cmFsY29udHJvbC52MS5Ob3RpZmljYXRpb25TdWJtaXRE", + "dG8aNy5zY2FkYWJyaWRnZS5jZW50cmFsY29udHJvbC52MS5Ob3RpZmljYXRp", + "b25TdWJtaXRBY2tEdG8SkgEKF1F1ZXJ5Tm90aWZpY2F0aW9uU3RhdHVzEjku", + "c2NhZGFicmlkZ2UuY2VudHJhbGNvbnRyb2wudjEuTm90aWZpY2F0aW9uU3Rh", + "dHVzUXVlcnlEdG8aPC5zY2FkYWJyaWRnZS5jZW50cmFsY29udHJvbC52MS5O", + "b3RpZmljYXRpb25TdGF0dXNSZXNwb25zZUR0bxJHChFJbmdlc3RBdWRpdEV2", + "ZW50cxIbLnNpdGVzdHJlYW0uQXVkaXRFdmVudEJhdGNoGhUuc2l0ZXN0cmVh", + "bS5Jbmdlc3RBY2sSUAoVSW5nZXN0Q2FjaGVkVGVsZW1ldHJ5EiAuc2l0ZXN0", + "cmVhbS5DYWNoZWRUZWxlbWV0cnlCYXRjaBoVLnNpdGVzdHJlYW0uSW5nZXN0", + "QWNrEoABCg1SZWNvbmNpbGVTaXRlEjYuc2NhZGFicmlkZ2UuY2VudHJhbGNv", + "bnRyb2wudjEuUmVjb25jaWxlU2l0ZVJlcXVlc3REdG8aNy5zY2FkYWJyaWRn", + "ZS5jZW50cmFsY29udHJvbC52MS5SZWNvbmNpbGVTaXRlUmVzcG9uc2VEdG8S", + "fQoQUmVwb3J0U2l0ZUhlYWx0aBIyLnNjYWRhYnJpZGdlLmNlbnRyYWxjb250", + "cm9sLnYxLlNpdGVIZWFsdGhSZXBvcnREdG8aNS5zY2FkYWJyaWRnZS5jZW50", + "cmFsY29udHJvbC52MS5TaXRlSGVhbHRoUmVwb3J0QWNrRHRvElAKCUhlYXJ0", + "YmVhdBIrLnNjYWRhYnJpZGdlLmNlbnRyYWxjb250cm9sLnYxLkhlYXJ0YmVh", + "dER0bxoWLmdvb2dsZS5wcm90b2J1Zi5FbXB0eUIrqgIoWkIuTU9NLldXLlNj", + "YWRhQnJpZGdlLkNvbW11bmljYXRpb24uR3JwY2IGcHJvdG8z")); descriptor = pbr::FileDescriptor.FromGeneratedCode(descriptorData, new pbr::FileDescriptor[] { global::Google.Protobuf.WellKnownTypes.EmptyReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.TimestampReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.WrappersReflection.Descriptor, global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.SitestreamReflection.Descriptor, }, new pbr::GeneratedClrTypeInfo(new[] {typeof(global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.ConnectionHealthEnum), }, null, new pbr::GeneratedClrTypeInfo[] { @@ -159,7 +160,7 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Grpc { new pbr::GeneratedClrTypeInfo(typeof(global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.ConnectionEndpointMapDto), global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.ConnectionEndpointMapDto.Parser, new[]{ "Entries" }, null, null, null, new pbr::GeneratedClrTypeInfo[] { null, }), new pbr::GeneratedClrTypeInfo(typeof(global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.TagQualityMapDto), global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.TagQualityMapDto.Parser, new[]{ "Entries" }, null, null, null, new pbr::GeneratedClrTypeInfo[] { null, }), new pbr::GeneratedClrTypeInfo(typeof(global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.NodeStatusListDto), global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.NodeStatusListDto.Parser, new[]{ "Nodes" }, null, null, null, null), - new pbr::GeneratedClrTypeInfo(typeof(global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.SiteHealthReportDto), global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.SiteHealthReportDto.Parser, new[]{ "SiteId", "SequenceNumber", "ReportTimestamp", "DataConnectionStatuses", "TagResolutionCounts", "ScriptErrorCount", "AlarmEvaluationErrorCount", "StoreAndForwardBufferDepths", "DeadLetterCount", "DeployedInstanceCount", "EnabledInstanceCount", "DisabledInstanceCount", "NodeRole", "NodeHostname", "DataConnectionEndpoints", "DataConnectionTagQuality", "ParkedMessageCount", "ClusterNodes", "SiteAuditWriteFailures", "AuditRedactionFailure", "SiteAuditBacklog", "SiteEventLogWriteFailures", "OldestParkedMessageAgeSeconds", "ScriptQueueDepth", "ScriptBusyThreads", "ScriptOldestBusyAgeSeconds", "LocalDbReplicationConnected", "LocalDbOplogBacklog" }, null, null, null, new pbr::GeneratedClrTypeInfo[] { null, null, null, }), + new pbr::GeneratedClrTypeInfo(typeof(global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.SiteHealthReportDto), global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.SiteHealthReportDto.Parser, new[]{ "SiteId", "SequenceNumber", "ReportTimestamp", "DataConnectionStatuses", "TagResolutionCounts", "ScriptErrorCount", "AlarmEvaluationErrorCount", "StoreAndForwardBufferDepths", "DeadLetterCount", "DeployedInstanceCount", "EnabledInstanceCount", "DisabledInstanceCount", "NodeRole", "NodeHostname", "DataConnectionEndpoints", "DataConnectionTagQuality", "ParkedMessageCount", "ClusterNodes", "SiteAuditWriteFailures", "AuditRedactionFailure", "SiteAuditBacklog", "SiteEventLogWriteFailures", "OldestParkedMessageAgeSeconds", "ScriptQueueDepth", "ScriptBusyThreads", "ScriptOldestBusyAgeSeconds", "LocalDbReplicationConnected", "LocalDbOplogBacklog", "StartupReconcilerActive" }, null, null, null, new pbr::GeneratedClrTypeInfo[] { null, null, null, }), new pbr::GeneratedClrTypeInfo(typeof(global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.SiteHealthReportAckDto), global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.SiteHealthReportAckDto.Parser, new[]{ "SiteId", "SequenceNumber", "Accepted", "Error" }, null, null, null, null), new pbr::GeneratedClrTypeInfo(typeof(global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.HeartbeatDto), global::ZB.MOM.WW.ScadaBridge.Communication.Grpc.HeartbeatDto.Parser, new[]{ "SiteId", "NodeHostname", "IsActive", "Timestamp", "Synthetic" }, null, null, null, null) })); @@ -4266,6 +4267,7 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Grpc { ScriptOldestBusyAgeSeconds = other.ScriptOldestBusyAgeSeconds; LocalDbReplicationConnected = other.LocalDbReplicationConnected; LocalDbOplogBacklog = other.LocalDbOplogBacklog; + StartupReconcilerActive = other.StartupReconcilerActive; _unknownFields = pb::UnknownFieldSet.Clone(other._unknownFields); } @@ -4642,6 +4644,27 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Grpc { } + /// Field number for the "startup_reconciler_active" field. + public const int StartupReconcilerActiveFieldNumber = 29; + private static readonly pb::FieldCodec _single_startupReconcilerActive_codec = pb::FieldCodec.ForStructWrapper(234); + private bool? startupReconcilerActive_; + /// + /// Whether this node spawned its SiteReconciliationActor at boot. Absent means + /// "not reported" — a central self-report (where it is meaningless) or a site + /// node on a pre-row-73 build. false means the node booted WITHOUT the startup + /// self-heal path, which is a misconfiguration a healthy-looking site would + /// otherwise hide entirely. + /// + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + [global::System.CodeDom.Compiler.GeneratedCode("protoc", null)] + public bool? StartupReconcilerActive { + get { return startupReconcilerActive_; } + set { + startupReconcilerActive_ = value; + } + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] [global::System.CodeDom.Compiler.GeneratedCode("protoc", null)] public override bool Equals(object other) { @@ -4685,6 +4708,7 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Grpc { if (!pbc::ProtobufEqualityComparers.BitwiseNullableDoubleEqualityComparer.Equals(ScriptOldestBusyAgeSeconds, other.ScriptOldestBusyAgeSeconds)) return false; if (LocalDbReplicationConnected != other.LocalDbReplicationConnected) return false; if (LocalDbOplogBacklog != other.LocalDbOplogBacklog) return false; + if (StartupReconcilerActive != other.StartupReconcilerActive) return false; return Equals(_unknownFields, other._unknownFields); } @@ -4720,6 +4744,7 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Grpc { if (scriptOldestBusyAgeSeconds_ != null) hash ^= pbc::ProtobufEqualityComparers.BitwiseNullableDoubleEqualityComparer.GetHashCode(ScriptOldestBusyAgeSeconds); if (localDbReplicationConnected_ != null) hash ^= LocalDbReplicationConnected.GetHashCode(); if (localDbOplogBacklog_ != null) hash ^= LocalDbOplogBacklog.GetHashCode(); + if (startupReconcilerActive_ != null) hash ^= StartupReconcilerActive.GetHashCode(); if (_unknownFields != null) { hash ^= _unknownFields.GetHashCode(); } @@ -4837,6 +4862,9 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Grpc { if (localDbOplogBacklog_ != null) { _single_localDbOplogBacklog_codec.WriteTagAndValue(output, LocalDbOplogBacklog); } + if (startupReconcilerActive_ != null) { + _single_startupReconcilerActive_codec.WriteTagAndValue(output, StartupReconcilerActive); + } if (_unknownFields != null) { _unknownFields.WriteTo(output); } @@ -4946,6 +4974,9 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Grpc { if (localDbOplogBacklog_ != null) { _single_localDbOplogBacklog_codec.WriteTagAndValue(ref output, LocalDbOplogBacklog); } + if (startupReconcilerActive_ != null) { + _single_startupReconcilerActive_codec.WriteTagAndValue(ref output, StartupReconcilerActive); + } if (_unknownFields != null) { _unknownFields.WriteTo(ref output); } @@ -5034,6 +5065,9 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Grpc { if (localDbOplogBacklog_ != null) { size += _single_localDbOplogBacklog_codec.CalculateSizeWithTag(LocalDbOplogBacklog); } + if (startupReconcilerActive_ != null) { + size += _single_startupReconcilerActive_codec.CalculateSizeWithTag(StartupReconcilerActive); + } if (_unknownFields != null) { size += _unknownFields.CalculateSize(); } @@ -5147,6 +5181,11 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Grpc { LocalDbOplogBacklog = other.LocalDbOplogBacklog; } } + if (other.startupReconcilerActive_ != null) { + if (startupReconcilerActive_ == null || other.StartupReconcilerActive != false) { + StartupReconcilerActive = other.StartupReconcilerActive; + } + } _unknownFields = pb::UnknownFieldSet.MergeFrom(_unknownFields, other._unknownFields); } @@ -5305,6 +5344,13 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Grpc { } break; } + case 234: { + bool? value = _single_startupReconcilerActive_codec.Read(input); + if (startupReconcilerActive_ == null || value != false) { + StartupReconcilerActive = value; + } + break; + } } } #endif @@ -5463,6 +5509,13 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Grpc { } break; } + case 234: { + bool? value = _single_startupReconcilerActive_codec.Read(ref input); + if (startupReconcilerActive_ == null || value != false) { + StartupReconcilerActive = value; + } + break; + } } } } diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/CentralControlDtoMapper.cs b/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/CentralControlDtoMapper.cs index d675845b..96a9322a 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/CentralControlDtoMapper.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/CentralControlDtoMapper.cs @@ -475,6 +475,7 @@ public static class CentralControlDtoMapper ScriptOldestBusyAgeSeconds = msg.ScriptOldestBusyAgeSeconds, LocalDbReplicationConnected = msg.LocalDbReplicationConnected, LocalDbOplogBacklog = msg.LocalDbOplogBacklog, + StartupReconcilerActive = msg.StartupReconcilerActive, }; foreach (var (name, health) in msg.DataConnectionStatuses) @@ -649,6 +650,7 @@ public static class CentralControlDtoMapper ScriptOldestBusyAgeSeconds = dto.ScriptOldestBusyAgeSeconds, LocalDbReplicationConnected = dto.LocalDbReplicationConnected, LocalDbOplogBacklog = dto.LocalDbOplogBacklog, + StartupReconcilerActive = dto.StartupReconcilerActive, }; } diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/Protos/central_control.proto b/src/ZB.MOM.WW.ScadaBridge.Communication/Protos/central_control.proto index 7faa1170..2c2832c0 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/Protos/central_control.proto +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/Protos/central_control.proto @@ -228,6 +228,12 @@ message SiteHealthReportDto { // Absent means UNKNOWN, never zero — a failed backlog read rendered as 0 // would report a broken replication pair as perfectly healthy. google.protobuf.Int64Value local_db_oplog_backlog = 28; + // Whether this node spawned its SiteReconciliationActor at boot. Absent means + // "not reported" — a central self-report (where it is meaningless) or a site + // node on a pre-row-73 build. false means the node booted WITHOUT the startup + // self-heal path, which is a misconfiguration a healthy-looking site would + // otherwise hide entirely. + google.protobuf.BoolValue startup_reconciler_active = 29; } // Central -> Site: health report ack, so a lost report is observable. diff --git a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/ISiteHealthCollector.cs b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/ISiteHealthCollector.cs index 2495f6a4..c0747983 100644 --- a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/ISiteHealthCollector.cs +++ b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/ISiteHealthCollector.cs @@ -162,14 +162,38 @@ public interface ISiteHealthCollector /// service. Point-in-time: values are NOT reset on . /// /// - /// Whether a sync session is currently running. is the - /// normal state on a node with no peer configured — replication ships default-OFF. + /// Whether a sync session is currently running, or when there + /// is no reading to give — which is the state of a node with replication UNCONFIGURED + /// (register row 80). means "replication is configured on this + /// node and the peer session is currently down", i.e. a genuine fault worth a badge; + /// reporting that on a deliberately-unreplicated site reads as a fault that isn't one. /// /// - /// Unacked oplog entries, or when unknown. Pass null through - /// unchanged: a failed poll rendered as 0 would report a broken pair as healthy. + /// Unacked oplog entries, or when unknown (failed poll) or not + /// applicable (replication unconfigured). Pass null through unchanged: a failed poll + /// rendered as 0 would report a broken pair as healthy. /// - void SetLocalDbReplicationStatus(bool connected, long? oplogBacklog) + void SetLocalDbReplicationStatus(bool? connected, long? oplogBacklog) + { + // Default no-op so test fakes do not need to be updated. + } + + /// + /// Record whether this node spawned its SiteReconciliationActor at boot, for the + /// StartupReconcilerActive field of every subsequent + /// (deferred-work register row 73). + /// + /// + /// Called ONCE at boot by the Host, which owns the spawn decision — this is a latched + /// boot fact, not a polled gauge, so there is no reporter hosted service behind it. + /// A collector that is never told (central's self-report, or any test host) leaves the + /// field null, which reads as "not reported" rather than "reconciler absent". + /// + /// + /// when the reconciler was created; when + /// it was skipped because no IDeploymentConfigFetcher was available. + /// + void SetStartupReconcilerActive(bool active) { // Default no-op so test fakes do not need to be updated. } diff --git a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/LocalDbReplicationStatusReporter.cs b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/LocalDbReplicationStatusReporter.cs index 39f14ff1..d36cfa3d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/LocalDbReplicationStatusReporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/LocalDbReplicationStatusReporter.cs @@ -22,6 +22,23 @@ namespace ZB.MOM.WW.ScadaBridge.HealthMonitoring; /// report a replication pair that cannot read its own oplog as perfectly healthy. /// /// +/// An UNCONFIGURED node reports null for both fields (register row 80). The +/// replication engine is registered unconditionally and sits inert without a peer, so +/// ISyncStatus answers Connected=false / OplogBacklog=0 on a site +/// that was never meant to replicate at all. Publishing that pair is actively misleading: +/// a dashboard reader sees "connected: false" on a healthy unreplicated site and reads a +/// fault, and the central null-branch machinery (conditional KPI emission, sustained-badge +/// anchor exclusion) never receives the null it was built for. The +/// replicationConfiguredProvider gate therefore short-circuits the probe and pushes +/// (null, null) — "no reading", per the documented nullable semantics. +/// +/// +/// The gate is supplied by the Host, not read from configuration here, mirroring +/// SiteEventLogReplicationCheck: the Host owns +/// SiteLocalDbSetup.ReplicationIsConfigured (the PeerAddress-OR-ApiKey rule) and +/// HealthMonitoring must not learn to read LocalDb configuration itself. +/// +/// /// Cadence. 30 s, matching and /// SiteAuditBacklogReporter. Any exception during a probe is logged and swallowed; /// the next tick retries. @@ -34,6 +51,7 @@ public sealed class LocalDbReplicationStatusReporter : IHostedService, IDisposab private readonly Func _connectedProvider; private readonly Func _oplogBacklogProvider; + private readonly Func _replicationConfiguredProvider; private readonly ISiteHealthCollector _collector; private readonly ILogger _logger; private readonly TimeSpan _refreshInterval; @@ -43,18 +61,26 @@ public sealed class LocalDbReplicationStatusReporter : IHostedService, IDisposab /// Initializes a new instance of . /// Reads whether a peer sync session is currently running. /// Reads the unacked oplog backlog, or null when unknown. + /// + /// Reads whether replication is configured on this node at all (the Host's + /// SiteLocalDbSetup.ReplicationIsConfigured). When it answers false, both fields + /// are published as null rather than as the engine's inert false/0 defaults. + /// /// The site health collector receiving the snapshot. /// Logger instance. /// Poll interval override; defaults to 30 s. public LocalDbReplicationStatusReporter( Func connectedProvider, Func oplogBacklogProvider, + Func replicationConfiguredProvider, ISiteHealthCollector collector, ILogger logger, TimeSpan? refreshInterval = null) { _connectedProvider = connectedProvider ?? throw new ArgumentNullException(nameof(connectedProvider)); _oplogBacklogProvider = oplogBacklogProvider ?? throw new ArgumentNullException(nameof(oplogBacklogProvider)); + _replicationConfiguredProvider = replicationConfiguredProvider + ?? throw new ArgumentNullException(nameof(replicationConfiguredProvider)); _collector = collector ?? throw new ArgumentNullException(nameof(collector)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _refreshInterval = refreshInterval ?? DefaultRefreshInterval; @@ -114,6 +140,14 @@ public sealed class LocalDbReplicationStatusReporter : IHostedService, IDisposab { try { + if (!_replicationConfiguredProvider()) + { + // Replication unconfigured: "no reading", NOT "disconnected with an empty + // backlog". Publishing the inert engine's false/0 here is register row 80. + _collector.SetLocalDbReplicationStatus(null, null); + return; + } + _collector.SetLocalDbReplicationStatus( _connectedProvider(), _oplogBacklogProvider()); } diff --git a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/ServiceCollectionExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/ServiceCollectionExtensions.cs index d8d3b457..7c548269 100644 --- a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/ServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/ServiceCollectionExtensions.cs @@ -170,19 +170,33 @@ public static class ServiceCollectionExtensions /// a reference on the replication library, matching /// . Pass the backlog through as /// nullable — coercing null to 0 would report an unreadable oplog as a healthy one. + /// + /// is the register-row-80 gate: the + /// replication engine is registered unconditionally and answers false/0 + /// while inert, so without it a deliberately-unreplicated site publishes + /// "disconnected, empty backlog" instead of the documented "no reading" null. The + /// caller supplies it — the Host owns SiteLocalDbSetup.ReplicationIsConfigured, + /// exactly as it supplies SiteEventLogReplicationCheck to SiteEventLogging. + /// /// /// The service collection to register into. /// Given the root provider, returns a reader for "a sync session is running". /// Given the root provider, returns a reader for the unacked backlog (null = unknown). + /// + /// Given the root provider, returns a reader for "replication is configured on this node". + /// False makes the reporter publish null for BOTH fields. + /// /// The same for chaining. public static IServiceCollection AddLocalDbReplicationHealthBridge( this IServiceCollection services, Func> connectedProvider, - Func> oplogBacklogProvider) + Func> oplogBacklogProvider, + Func> replicationConfiguredProvider) { ArgumentNullException.ThrowIfNull(services); ArgumentNullException.ThrowIfNull(connectedProvider); ArgumentNullException.ThrowIfNull(oplogBacklogProvider); + ArgumentNullException.ThrowIfNull(replicationConfiguredProvider); if (services.Any(d => d.ServiceType == typeof(LocalDbReplicationHealthBridgeMarker))) { @@ -193,6 +207,7 @@ public static class ServiceCollectionExtensions services.AddHostedService(sp => new LocalDbReplicationStatusReporter( connectedProvider(sp), oplogBacklogProvider(sp), + replicationConfiguredProvider(sp), sp.GetRequiredService(), sp.GetRequiredService>())); diff --git a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/SiteHealthCollector.cs b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/SiteHealthCollector.cs index e3341850..72dc91b6 100644 --- a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/SiteHealthCollector.cs +++ b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/SiteHealthCollector.cs @@ -20,8 +20,18 @@ public class SiteHealthCollector : ISiteHealthCollector private long _siteEventLogWriteFailures; // One volatile tuple rather than two independent fields: a torn read that paired a // fresh Connected with a stale backlog would be indistinguishable from a real state. - // Null = the reporter has not yet run (or replication is not wired). - private volatile Tuple? _localDbReplicationStatus; + // Null = the reporter has not yet run. Once it HAS run, an unreplicated node latches + // (null, null) — "no reading" — rather than the inert engine's (false, 0); see + // LocalDbReplicationStatusReporter and register row 80. + private volatile Tuple? _localDbReplicationStatus; + // Latched once at boot by the Host (register row 73). A tri-state int rather than a + // bool? because `volatile` cannot be applied to Nullable (a struct) and this is + // written on the startup thread but read on the report-sender thread: + // 0 = never told -> report null ("not reported": central self-report, or a test host) + // 1 = reconciler spawned + // -1 = reconciler SKIPPED (no IDeploymentConfigFetcher) + // Null must stay distinct from -1: "we don't know" is not "self-heal is missing". + private volatile int _startupReconcilerActive; private readonly ConcurrentDictionary _connectionStatuses = new(); private readonly ConcurrentDictionary _tagResolutionCounts = new(); private readonly ConcurrentDictionary _connectionEndpoints = new(); @@ -109,11 +119,17 @@ public class SiteHealthCollector : ISiteHealthCollector } /// - public void SetLocalDbReplicationStatus(bool connected, long? oplogBacklog) + public void SetLocalDbReplicationStatus(bool? connected, long? oplogBacklog) { _localDbReplicationStatus = Tuple.Create(connected, oplogBacklog); } + /// + public void SetStartupReconcilerActive(bool active) + { + _startupReconcilerActive = active ? 1 : -1; + } + /// public void UpdateConnectionHealth(string connectionName, ConnectionHealth health) { @@ -256,6 +272,7 @@ public class SiteHealthCollector : ISiteHealthCollector // Single read of the volatile tuple — two reads could straddle a reporter tick and // pair a fresh Connected with a stale backlog. var localDbReplication = _localDbReplicationStatus; + var startupReconciler = _startupReconcilerActive; // Snapshot current connection and tag resolution state var connectionStatuses = new Dictionary(_connectionStatuses); @@ -307,7 +324,10 @@ public class SiteHealthCollector : ISiteHealthCollector // with an empty backlog". LocalDbReplicationConnected = localDbReplication?.Item1, LocalDbOplogBacklog = localDbReplication?.Item2, - SiteStreamAlarmDropCount = Interlocked.Read(ref _siteStreamAlarmDropCount) + SiteStreamAlarmDropCount = Interlocked.Read(ref _siteStreamAlarmDropCount), + // Tri-state: 0 stays null ("not reported"), which must never collapse into + // false ("self-heal is missing"). Register row 73. + StartupReconcilerActive = startupReconciler == 0 ? null : startupReconciler > 0 }; } } diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Actors/AkkaHostedService.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Actors/AkkaHostedService.cs index 4f4aee45..de97d92b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Actors/AkkaHostedService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Actors/AkkaHostedService.cs @@ -1029,6 +1029,14 @@ akka {{ // singleton, which starts the Instance Actor if the boot-time load missed it. Going // through the PROXY (not a local ActorOf) keeps Instance Actors under the singleton // regardless of which node's reconcile pass found the gap. + // + // Whichever branch is taken is LATCHED onto the site health report as + // StartupReconcilerActive (deferred-work register row 73). Before that, the skip + // was a single boot Warning and nothing else: a node that silently lost its entire + // self-heal path went on reporting itself perfectly healthy forever, and the only + // evidence was one line scrolled off the top of a log. Central badges the false + // case at Warning rather than failing the site — replication is still the primary + // path, so the node is degraded, not down. if (deploymentConfigFetcher != null) { var reconcileLogger = _serviceProvider.GetRequiredService() @@ -1045,15 +1053,50 @@ akka {{ null, dmProxy)), "site-reconciliation"); + siteHealthCollector?.SetStartupReconcilerActive(true); _logger.LogInformation( "SiteReconciliationActor created (per-node startup self-heal) for site {SiteId} node {Node}", _nodeOptions.SiteId, _nodeOptions.NodeName); } else { - _logger.LogWarning( - "No IDeploymentConfigFetcher available; SiteReconciliationActor not created — " - + "startup self-heal disabled (replication remains the primary path)"); + siteHealthCollector?.SetStartupReconcilerActive(false); + + // Raised from Warning to Error (register row 73). This is a misconfiguration, + // not a degraded-but-expected state: no configuration makes the fetcher + // legitimately absent on a site node, and the consequence is that neither the + // file heal nor the row-67 runtime heal will ever run on this node. + _logger.LogError( + "No IDeploymentConfigFetcher available; SiteReconciliationActor not created for " + + "site {SiteId} node {Node} — startup self-heal is DISABLED (replication " + + "remains the primary path). Surfaced centrally as StartupReconcilerActive=false.", + _nodeOptions.SiteId, _nodeOptions.NodeName); + + // Also record it where an operator reviewing the site after the fact will find + // it — the boot log line is gone by then, but the site event log is queryable + // from the Central UI. Best-effort: a failed event write must never keep the + // node from finishing startup, which is why this is fire-and-forget with an + // observed continuation rather than an await. + var bootEventLogger = _serviceProvider + .GetService(); + _ = bootEventLogger?.LogEventAsync( + "deployment", + "Error", + null, + "AkkaHostedService", + "Startup self-heal disabled: SiteReconciliationActor was not created " + + "because no IDeploymentConfigFetcher is registered.", + $"Node {_nodeOptions.NodeName} of site {_nodeOptions.SiteId} cannot " + + "reconcile its deployed inventory at boot. Config deploys still reach " + + "this node via LocalDb replication when it is up; a deploy that lands " + + "while it is DOWN will not be recovered on restart.") + .ContinueWith( + t => _logger.LogWarning( + t.Exception, + "Failed to write the startup-self-heal-disabled site event."), + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted, + TaskScheduler.Default); } // Audit Log — site-side telemetry actor that drains the SQLite diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs b/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs index 60c0889c..b33b727e 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs @@ -137,14 +137,27 @@ public static class SiteServiceRegistration sp => () => sp.GetRequiredService().FailedWriteCount); // LocalDb replication — bridge ISyncStatus onto the site health report. Registered - // unconditionally alongside the engine: on a node with no peer this reports - // Connected=false with a real 0 backlog, which is the healthy default-OFF state. - // OplogBacklog is passed through NULLABLE on purpose — null means the poll failed - // or no provider is wired, and flattening it to 0 would render a replication pair - // that cannot read its own oplog as perfectly healthy. + // unconditionally alongside the engine, but what it PUBLISHES is gated on whether + // this node is configured to replicate at all (register row 80). + // + // The engine is inert without a peer, so ISyncStatus answers Connected=false with a + // real 0 backlog on a site that was never meant to replicate. Publishing that pair + // contradicts the documented nullable semantics ("null means no reading, NOT + // disconnected with an empty backlog"): a dashboard reader sees connected:false on a + // healthy unreplicated site and reads a fault, and central's null-branch machinery + // (conditional KPI emission, sustained-badge anchor exclusion) never receives the + // null it was built for — so rig site-b/site-c emitted a misleading + // localDbReplicationConnected=0 series. Unconfigured now publishes null/null. + // + // Delegating to SiteLocalDbSetup.ReplicationIsConfigured keeps the + // PeerAddress-OR-ApiKey rule in ONE place and mirrors SiteEventLogReplicationCheck + // below — HealthMonitoring must not learn to read LocalDb configuration itself. + // Read once here (configuration is fixed for the process lifetime) and captured. + var localDbReplicationConfigured = SiteLocalDbSetup.ReplicationIsConfigured(config); services.AddLocalDbReplicationHealthBridge( sp => () => sp.GetRequiredService().Connected, - sp => () => sp.GetRequiredService().OplogBacklog); + sp => () => sp.GetRequiredService().OplogBacklog, + _ => () => localDbReplicationConfigured); // Audit Log — site-side hot-path writer + telemetry collaborators. // The SiteAuditTelemetryActor itself is registered by AkkaHostedService @@ -217,9 +230,9 @@ public static class SiteServiceRegistration // backlog genuinely rises at purge time and reads like a replication fault to anyone who // does not know a purge just ran. Delegating to SiteLocalDbSetup.ReplicationIsConfigured // keeps the PeerAddress-OR-ApiKey rule in one place — SiteEventLogging must not learn to - // read LocalDb configuration itself. - var replicationConfigured = SiteLocalDbSetup.ReplicationIsConfigured(config); - SiteEventLogReplicationCheck replicationCheck = () => replicationConfigured; + // read LocalDb configuration itself. Shares the single read taken for the LocalDb + // replication health bridge above, so the two surfaces can never disagree. + SiteEventLogReplicationCheck replicationCheck = () => localDbReplicationConfigured; services.AddSingleton(replicationCheck); // Health checks — the shared ZB.MOM.WW.Health probes, mapped by MapZbHealth on the site's diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/HealthPageTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/HealthPageTests.cs index 4ee067cf..862765bb 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/HealthPageTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/HealthPageTests.cs @@ -14,6 +14,7 @@ using ZB.MOM.WW.ScadaBridge.CentralUI.Services; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Audit; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Health; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Notification; using ZB.MOM.WW.ScadaBridge.Commons.Types; using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit; @@ -443,6 +444,120 @@ public class HealthPageTests : BunitContext Assert.Contains("LocalDb Replication Backlog", cut.Markup)); } + // ----- Register row 73: the "No self-heal" badge ----------------------------------- + // + // Tri-state, and only the explicit false is badged. Null means "not reported" — the + // central self-report row, and every site node on a pre-row-73 build — so badging null + // would light up the whole fleet the moment central is upgraded ahead of the sites. + + [Fact] + public void Renders_StartupReconcilerBadge_WhenTheSiteReportsItIsMissing() + { + SeedStates(new SiteHealthState + { + SiteId = "site-a", + IsOnline = true, + LastHeartbeatAt = DateTimeOffset.UtcNow, + LatestReport = ReportWithReconciler("site-a", active: false), + }); + + var cut = RenderHealthPage(); + + cut.WaitForAssertion(() => + Assert.Contains("data-test=\"startup-reconciler-badge\"", cut.Markup)); + } + + [Fact] + public void DoesNotRender_StartupReconcilerBadge_WhenTheReconcilerIsRunning() + { + SeedStates(new SiteHealthState + { + SiteId = "site-a", + IsOnline = true, + LastHeartbeatAt = DateTimeOffset.UtcNow, + LatestReport = ReportWithReconciler("site-a", active: true), + }); + + var cut = RenderHealthPage(); + + cut.WaitForAssertion(() => Assert.Contains("site-a", cut.Markup)); + Assert.DoesNotContain("data-test=\"startup-reconciler-badge\"", cut.Markup); + } + + [Fact] + public void DoesNotRender_StartupReconcilerBadge_WhenNotReported() + { + // Null = "not reported", never "missing". This is central's own self-report row and + // any site that has not yet been upgraded. + SeedStates(new SiteHealthState + { + SiteId = "site-a", + IsOnline = true, + LastHeartbeatAt = DateTimeOffset.UtcNow, + LatestReport = ReportWithReconciler("site-a", active: null), + }); + + var cut = RenderHealthPage(); + + cut.WaitForAssertion(() => Assert.Contains("site-a", cut.Markup)); + Assert.DoesNotContain("data-test=\"startup-reconciler-badge\"", cut.Markup); + } + + [Fact] + public void DoesNotRender_StartupReconcilerBadge_WhenTheSiteHasNoReportYet() + { + SeedStates(new SiteHealthState + { + SiteId = "site-a", + IsOnline = true, + LastHeartbeatAt = DateTimeOffset.UtcNow, + LatestReport = null, + }); + + var cut = RenderHealthPage(); + + cut.WaitForAssertion(() => Assert.Contains("site-a", cut.Markup)); + Assert.DoesNotContain("data-test=\"startup-reconciler-badge\"", cut.Markup); + } + + [Fact] + public void DoesNotRender_StartupReconcilerBadge_ForAnOfflineSite() + { + // Same rule as the replication-backlog badge: the Offline badge is the signal that + // matters, and stacking a stale warning on top of it is noise. + SeedStates(new SiteHealthState + { + SiteId = "site-a", + IsOnline = false, + LastHeartbeatAt = DateTimeOffset.UtcNow.AddMinutes(-30), + LatestReport = ReportWithReconciler("site-a", active: false), + }); + + var cut = RenderHealthPage(); + + cut.WaitForAssertion(() => Assert.Contains("site-a", cut.Markup)); + Assert.DoesNotContain("data-test=\"startup-reconciler-badge\"", cut.Markup); + } + + private static SiteHealthReport ReportWithReconciler(string siteId, bool? active) => + new( + SiteId: siteId, + SequenceNumber: 1, + ReportTimestamp: DateTimeOffset.UtcNow, + DataConnectionStatuses: + new Dictionary(), + TagResolutionCounts: new Dictionary(), + ScriptErrorCount: 0, + AlarmEvaluationErrorCount: 0, + StoreAndForwardBufferDepths: new Dictionary(), + DeadLetterCount: 0, + DeployedInstanceCount: 0, + EnabledInstanceCount: 0, + DisabledInstanceCount: 0) + { + StartupReconcilerActive = active, + }; + [Fact] public void Renders_OfflineSince_ForOfflineSiteWithStatusChange() { diff --git a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/Grpc/CentralControlDtoMapperTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/Grpc/CentralControlDtoMapperTests.cs index 369841a9..a4ca0909 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/Grpc/CentralControlDtoMapperTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/Grpc/CentralControlDtoMapperTests.cs @@ -368,6 +368,7 @@ public class CentralControlDtoMapperTests Assert.Null(roundTripped.ScriptOldestBusyAgeSeconds); Assert.Null(roundTripped.LocalDbReplicationConnected); Assert.Null(roundTripped.LocalDbOplogBacklog); + Assert.Null(roundTripped.StartupReconcilerActive); } [Fact] @@ -407,12 +408,16 @@ public class CentralControlDtoMapperTests LocalDbOplogBacklog = 0, OldestParkedMessageAgeSeconds = 0d, ScriptOldestBusyAgeSeconds = 0d, + // Register row 73: false ("this node booted with no self-heal") is the value + // central badges on, so it must not decay into null ("not reported") en route. + StartupReconcilerActive = false, }; var roundTripped = CentralControlDtoMapper.FromDto(Wire(CentralControlDtoMapper.ToDto(original))); Assert.False(roundTripped.LocalDbReplicationConnected); Assert.Equal(0L, roundTripped.LocalDbOplogBacklog); + Assert.False(roundTripped.StartupReconcilerActive); Assert.Equal(0d, roundTripped.OldestParkedMessageAgeSeconds); Assert.Equal(0d, roundTripped.ScriptOldestBusyAgeSeconds); } @@ -548,6 +553,7 @@ public class CentralControlDtoMapperTests ScriptOldestBusyAgeSeconds = 12.25d, LocalDbReplicationConnected = true, LocalDbOplogBacklog = 250L, + StartupReconcilerActive = true, }; /// @@ -596,6 +602,7 @@ public class CentralControlDtoMapperTests Assert.Equal(expected.ScriptOldestBusyAgeSeconds, actual.ScriptOldestBusyAgeSeconds); Assert.Equal(expected.LocalDbReplicationConnected, actual.LocalDbReplicationConnected); Assert.Equal(expected.LocalDbOplogBacklog, actual.LocalDbOplogBacklog); + Assert.Equal(expected.StartupReconcilerActive, actual.StartupReconcilerActive); } /// diff --git a/tests/ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests/LocalDbReplicationStatusReporterTests.cs b/tests/ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests/LocalDbReplicationStatusReporterTests.cs new file mode 100644 index 00000000..c350de37 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests/LocalDbReplicationStatusReporterTests.cs @@ -0,0 +1,182 @@ +using Microsoft.Extensions.Logging.Abstractions; + +namespace ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests; + +/// +/// Deferred-work register row 80 — must +/// distinguish "replication is not configured on this node" (null, no reading) from +/// "replication is configured and the peer session is down" (false, a genuine fault). +/// +/// +/// The replication engine is registered unconditionally on every site node and sits inert +/// without a peer, so ISyncStatus answers Connected=false / OplogBacklog=0 +/// on a site that was never meant to replicate. Passing that straight through made rig +/// site-b/site-c publish "disconnected, empty backlog" — a healthy unreplicated site reading +/// as a broken replicated one, and central's null branches (conditional KPI emission, +/// sustained-badge anchor exclusion, per register row 33) never receiving the null they exist +/// for. The gate is supplied by the Host from SiteLocalDbSetup.ReplicationIsConfigured, +/// mirroring SiteEventLogReplicationCheck. +/// +public class LocalDbReplicationStatusReporterTests +{ + private static LocalDbReplicationStatusReporter Build( + SiteHealthCollector collector, + bool replicationConfigured, + bool connected = false, + long? backlog = 0L) => + new( + connectedProvider: () => connected, + oplogBacklogProvider: () => backlog, + replicationConfiguredProvider: () => replicationConfigured, + collector: collector, + logger: NullLogger.Instance, + refreshInterval: TimeSpan.FromHours(1)); + + [Fact] + public void Probe_ReplicationUnconfigured_PublishesNullForBothFields() + { + // The row-80 regression. The inert engine's false/0 must NOT reach the report. + var collector = new SiteHealthCollector(); + using var reporter = Build(collector, replicationConfigured: false); + + reporter.Probe(); + var report = collector.CollectReport("TestSite"); + + Assert.Null(report.LocalDbReplicationConnected); + Assert.Null(report.LocalDbOplogBacklog); + } + + [Fact] + public void Probe_ReplicationUnconfigured_DoesNotReadTheSyncStatusProviders() + { + // The gate short-circuits rather than reading and discarding: an unconfigured node + // should not be polling a replication engine it never uses. Also guards against a + // future provider that throws when replication is off. + var connectedReads = 0; + var backlogReads = 0; + var collector = new SiteHealthCollector(); + using var reporter = new LocalDbReplicationStatusReporter( + connectedProvider: () => { connectedReads++; return false; }, + oplogBacklogProvider: () => { backlogReads++; return 0L; }, + replicationConfiguredProvider: () => false, + collector: collector, + logger: NullLogger.Instance, + refreshInterval: TimeSpan.FromHours(1)); + + reporter.Probe(); + + Assert.Equal(0, connectedReads); + Assert.Equal(0, backlogReads); + } + + [Fact] + public void Probe_ReplicationConfiguredButDisconnected_PublishesFalse_NotNull() + { + // The other half of the contract, and the reason the fix is a gate rather than a + // blanket "never report false": on a node that IS configured to replicate, a down + // peer session is a real fault and must stay visible as false. + var collector = new SiteHealthCollector(); + using var reporter = Build( + collector, replicationConfigured: true, connected: false, backlog: 4200L); + + reporter.Probe(); + var report = collector.CollectReport("TestSite"); + + Assert.False(report.LocalDbReplicationConnected); + Assert.Equal(4200L, report.LocalDbOplogBacklog); + } + + [Fact] + public void Probe_ReplicationConfiguredAndConnected_PublishesTrue() + { + var collector = new SiteHealthCollector(); + using var reporter = Build( + collector, replicationConfigured: true, connected: true, backlog: 0L); + + reporter.Probe(); + var report = collector.CollectReport("TestSite"); + + Assert.True(report.LocalDbReplicationConnected); + Assert.Equal(0L, report.LocalDbOplogBacklog); + } + + [Fact] + public void Probe_ReplicationConfigured_PreservesANullBacklogAsUnknown() + { + // A failed oplog poll on a REPLICATED node is "unknown", never 0 — rendering it as a + // healthy empty backlog would report a pair that cannot read its own oplog as fine. + // Connected stays false (the session really is down); only the backlog is unknown. + var collector = new SiteHealthCollector(); + using var reporter = Build( + collector, replicationConfigured: true, connected: false, backlog: null); + + reporter.Probe(); + var report = collector.CollectReport("TestSite"); + + Assert.False(report.LocalDbReplicationConnected); + Assert.Null(report.LocalDbOplogBacklog); + } + + [Fact] + public void Probe_ProviderThrows_LeavesThePreviousSnapshotIntact() + { + // Health reporting must never take the node down, and a failed probe must not + // silently downgrade a good reading to null. + var shouldThrow = false; + var collector = new SiteHealthCollector(); + using var reporter = new LocalDbReplicationStatusReporter( + connectedProvider: () => shouldThrow + ? throw new InvalidOperationException("sync status unavailable") + : true, + oplogBacklogProvider: () => 7L, + replicationConfiguredProvider: () => true, + collector: collector, + logger: NullLogger.Instance, + refreshInterval: TimeSpan.FromHours(1)); + + reporter.Probe(); + shouldThrow = true; + reporter.Probe(); + + var report = collector.CollectReport("TestSite"); + Assert.True(report.LocalDbReplicationConnected); + Assert.Equal(7L, report.LocalDbOplogBacklog); + } + + [Fact] + public void Probe_GateFlippedOn_StopsReportingNull() + { + // Turning replication on for a previously-unreplicated site is a both-nodes-together + // restart in practice, but the gate is read per probe, so pin that it is not latched. + var configured = false; + var collector = new SiteHealthCollector(); + using var reporter = new LocalDbReplicationStatusReporter( + connectedProvider: () => true, + oplogBacklogProvider: () => 12L, + replicationConfiguredProvider: () => configured, + collector: collector, + logger: NullLogger.Instance, + refreshInterval: TimeSpan.FromHours(1)); + + reporter.Probe(); + Assert.Null(collector.CollectReport("TestSite").LocalDbReplicationConnected); + + configured = true; + reporter.Probe(); + + var report = collector.CollectReport("TestSite"); + Assert.True(report.LocalDbReplicationConnected); + Assert.Equal(12L, report.LocalDbOplogBacklog); + } + + [Fact] + public void Constructor_NullReplicationConfiguredProvider_Throws() + { + Assert.Throws(() => new LocalDbReplicationStatusReporter( + connectedProvider: () => false, + oplogBacklogProvider: () => 0L, + replicationConfiguredProvider: null!, + collector: new SiteHealthCollector(), + logger: NullLogger.Instance)); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests/StartupReconcilerHealthFieldTests.cs b/tests/ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests/StartupReconcilerHealthFieldTests.cs new file mode 100644 index 00000000..e17db263 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests/StartupReconcilerHealthFieldTests.cs @@ -0,0 +1,79 @@ +namespace ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests; + +/// +/// Deferred-work register row 73 — SiteHealthReport.StartupReconcilerActive surfaces +/// whether a site node spawned its SiteReconciliationActor at boot. +/// +/// +/// The skip used to be a single boot Warning and nothing else, so a node that lost its whole +/// self-heal path went on reporting itself perfectly healthy. The field is tri-state and the +/// three cases must stay distinct: null = not reported (central self-report, or a pre-row-73 +/// site build), true = running, false = skipped because no IDeploymentConfigFetcher +/// was available. Collapsing null into false would badge every site the moment central is +/// upgraded ahead of them. +/// +public class StartupReconcilerHealthFieldTests +{ + [Fact] + public void CollectReport_NeverTold_LeavesTheFieldNull() + { + // "Not reported" — NOT "the reconciler is missing". This is the state of central's + // own self-report and of every test host that never calls the setter. + var collector = new SiteHealthCollector(); + + var report = collector.CollectReport("TestSite"); + + Assert.Null(report.StartupReconcilerActive); + } + + [Fact] + public void CollectReport_ReconcilerSpawned_ReportsTrue() + { + var collector = new SiteHealthCollector(); + + collector.SetStartupReconcilerActive(true); + + Assert.True(collector.CollectReport("TestSite").StartupReconcilerActive); + } + + [Fact] + public void CollectReport_ReconcilerSkipped_ReportsFalse_NotNull() + { + // The case the row exists for: false must be distinguishable from "not reported", + // because false is what central badges. + var collector = new SiteHealthCollector(); + + collector.SetStartupReconcilerActive(false); + + var report = collector.CollectReport("TestSite"); + Assert.NotNull(report.StartupReconcilerActive); + Assert.False(report.StartupReconcilerActive); + } + + [Fact] + public void CollectReport_IsPointInTime_NotDrainedOnCollect() + { + // A latched boot fact, not a per-interval counter: every subsequent report must + // carry it, or the badge would flicker off after the first report. + var collector = new SiteHealthCollector(); + collector.SetStartupReconcilerActive(false); + + collector.CollectReport("TestSite"); + var second = collector.CollectReport("TestSite"); + + Assert.False(second.StartupReconcilerActive); + } + + [Fact] + public void SetStartupReconcilerActive_IsNotWriteOnce() + { + // Nothing in the Host calls this twice today, but a latch that silently ignored a + // correction would be a nasty way to find that out. + var collector = new SiteHealthCollector(); + + collector.SetStartupReconcilerActive(false); + collector.SetStartupReconcilerActive(true); + + Assert.True(collector.CollectReport("TestSite").StartupReconcilerActive); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbWiringTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbWiringTests.cs index fc4a296c..bfb2bb49 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbWiringTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteLocalDbWiringTests.cs @@ -178,6 +178,18 @@ public class SiteLocalDbWiringTests : IDisposable // End-to-end through the REAL collector: probe once, then collect. This is what // catches a bridge that is registered but wired to the wrong provider — the // registration test above would pass either way. + // + // This fixture IS a replicating node: it carries LocalDb:Replication:ApiKey (the + // passive half of the rig's site-a pair), and configured-ness is PeerAddress OR + // ApiKey. So false/0 is the correct, meaningful answer here — "replication is on, + // no session is up yet, and the oplog really is empty". Register row 80 is about + // the OTHER case, which is pinned by UnreplicatedSiteLocalDbHealthTests: a node + // with NEITHER key must report null/null, because the engine's inert false/0 there + // means "not applicable", not "disconnected". + // + // Do not weaken this assertion to null without checking which fixture you are in — + // conflating "no PeerAddress" with "replication unconfigured" is precisely the + // mistake that produced row 80. var reporter = _host.Services.GetServices() .OfType() .Single(); @@ -186,7 +198,6 @@ public class SiteLocalDbWiringTests : IDisposable reporter.Probe(); var report = collector.CollectReport("TestSite"); - // No peer configured: not connected, and a REAL 0 backlog rather than null. Assert.False(report.LocalDbReplicationConnected); Assert.Equal(0L, report.LocalDbOplogBacklog); } diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/UnreplicatedSiteLocalDbHealthTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/UnreplicatedSiteLocalDbHealthTests.cs new file mode 100644 index 00000000..cc4ccc71 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/UnreplicatedSiteLocalDbHealthTests.cs @@ -0,0 +1,134 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using ZB.MOM.WW.LocalDb.Replication; +using ZB.MOM.WW.ScadaBridge.HealthMonitoring; +using ZB.MOM.WW.ScadaBridge.Host.Actors; + +namespace ZB.MOM.WW.ScadaBridge.Host.Tests; + +/// +/// Deferred-work register row 80 — a site node with replication UNCONFIGURED must report +/// LocalDbReplicationConnected and LocalDbOplogBacklog as null. +/// +/// +/// +/// The sibling fixture is a replicating node +/// (it sets LocalDb:Replication:ApiKey, modelling the passive half of the rig's +/// site-a pair), so false/0 is the right answer there. This fixture is the other posture — +/// the rig's site-b/site-c, and the shipping default — with NEITHER PeerAddress nor +/// ApiKey. +/// +/// +/// The replication engine is registered unconditionally and idles, so ISyncStatus +/// still answers Connected=false with a real 0 backlog. Publishing that pair +/// is the row-80 bug: a dashboard reader sees connected: false on a perfectly healthy +/// unreplicated site and reads a fault, and central's null branches — the conditional KPI +/// emission and the sustained-badge anchor exclusion built for register row 33 — never +/// receive the null they exist for, so unreplicated sites emitted a misleading +/// localDbReplicationConnected=0 series. +/// +/// +/// Built from the REAL composition root for the same reason the sibling fixture is: the +/// defect is in the wiring, and a hand-assembled ServiceCollection would prove +/// nothing about what the Host actually passes. +/// +/// +public class UnreplicatedSiteLocalDbHealthTests : IDisposable +{ + private readonly WebApplication _host; + private readonly string _tempDbPath; + private readonly string _tempSiteDbPath; + private readonly string _tempTrackingDbPath; + + public UnreplicatedSiteLocalDbHealthTests() + { + var stamp = Guid.NewGuid(); + _tempDbPath = Path.Combine(Path.GetTempPath(), $"scadabridge_unrep_localdb_{stamp}.db"); + _tempSiteDbPath = Path.Combine(Path.GetTempPath(), $"scadabridge_unrep_site_{stamp}.db"); + _tempTrackingDbPath = Path.Combine(Path.GetTempPath(), $"scadabridge_unrep_track_{stamp}.db"); + + var builder = WebApplication.CreateBuilder(); + builder.Configuration.Sources.Clear(); + builder.Configuration.AddInMemoryCollection(new Dictionary + { + ["ScadaBridge:Node:Role"] = "Site", + ["ScadaBridge:Node:NodeName"] = "node-a", + ["ScadaBridge:Node:NodeHostname"] = "test-site-b", + ["ScadaBridge:Node:SiteId"] = "TestSiteB", + ["ScadaBridge:Node:RemotingPort"] = "0", + ["ScadaBridge:Node:GrpcPort"] = "0", + ["ScadaBridge:Database:SiteDbPath"] = _tempSiteDbPath, + ["ScadaBridge:OperationTracking:ConnectionString"] = $"Data Source={_tempTrackingDbPath}", + ["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551", + ["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@localhost:2552", + + // The point of this fixture: LocalDb:Path only. No PeerAddress, no ApiKey — + // SiteLocalDbSetup.ReplicationIsConfigured is false, matching rig site-b/site-c. + ["LocalDb:Path"] = _tempDbPath, + }); + + builder.Services.AddGrpc(); + builder.Services.AddSingleton(); + + SiteServiceRegistration.Configure(builder.Services, builder.Configuration); + + AkkaHostedServiceRemover.RemoveAkkaHostedServiceOnly(builder.Services); + + _host = builder.Build(); + } + + public void Dispose() + { + (_host as IDisposable)?.Dispose(); + foreach (var path in new[] { _tempDbPath, _tempSiteDbPath, _tempTrackingDbPath }) + { + try { File.Delete(path); } catch { /* best effort */ } + } + + GC.SuppressFinalize(this); + } + + [Fact] + public void UnreplicatedSite_HealthReport_ReportsNullForBothLocalDbFields() + { + // The row-80 regression, end-to-end through the real container and the real + // collector. Before the fix this produced false/0. + var reporter = _host.Services.GetServices() + .OfType() + .Single(); + var collector = _host.Services.GetRequiredService(); + + reporter.Probe(); + var report = collector.CollectReport("TestSiteB"); + + Assert.Null(report.LocalDbReplicationConnected); + Assert.Null(report.LocalDbOplogBacklog); + } + + [Fact] + public void UnreplicatedSite_StillRegistersTheHealthBridge() + { + // The fix is a GATE on what the reporter publishes, not a skipped registration. + // Keeping the hosted service present means turning replication on is a config + // change rather than a different service graph, and it keeps one code path for + // both postures. + var hosted = _host.Services.GetServices(); + + Assert.Contains(hosted, h => h is LocalDbReplicationStatusReporter); + } + + [Fact] + public void UnreplicatedSite_ReplicationEngine_StillResolvesAndIdles() + { + // Establishes the premise the row-80 fix rests on: the engine really does answer + // false/0 here. If this ever starts returning null on its own, the gate becomes + // redundant rather than wrong — but the test above still pins the observable + // contract either way. + var status = _host.Services.GetRequiredService(); + + Assert.False(status.Connected); + Assert.Equal(0L, status.OplogBacklog); + } +} From 8b761841bc02268af45685493dc3b81f57f90266 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 15 Aug 2026 21:41:01 -0400 Subject: [PATCH 2/2] =?UTF-8?q?fix(logging):=20row=2071=20=E2=80=94=20stop?= =?UTF-8?q?=20logging=20the=20standby's=20by-design=20active-node=20verdic?= =?UTF-8?q?t=20at=20ERROR?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A healthy standby logged "Health check active-node with status Unhealthy … 'not the site pair primary (standby)'" at Error on every probe — every 10s on a site node, and (discovered while fixing this) every 5s on a central standby, which Traefik health-probes at that cadence. Correct-by-design state at an alarming severity: it floods the log and trains operators to ignore ERR. Mechanism: DefaultHealthCheckService logs one line per check per probe and picks the level from the verdict — Debug/Warning/Error for Healthy/Degraded/Unhealthy. Both active-node checks answer Unhealthy on the standby because 503 IS the contract /health/active publishes. There is no IHealthCheckPublisher in this repo, so each line is one inbound HTTP probe. Fix: one Serilog exclusion filter in LoggerConfigurationFactory. A level override could not work — Serilog's MinimumLevel.Override only RAISES a per-source floor, and a LogEvent's Level is immutable, so neither an override nor an enricher can rewrite Error as Information. Changing the check to report Degraded was rejected: it would alter the HTTP status that /health/active publishes, which is load-bearing for Traefik and the family dashboard. The constraint (genuine failures must still log at Error) is preserved by keying the predicate on the check NAME rather than the logger category: localdb, akka-cluster, database and required-singletons keep their Error lines, and the active-node check has no error path of its own — the active node always answers Healthy — so the only event this can ever suppress is a standby reporting that it is a standby. The verdict stays fully observable via /health/active's own HTTP status, the readiness probe's cluster view, and the health dashboard. Also corrects a stale CLAUDE.md claim found while verifying what consumes this check: "/health/active is central-only (site nodes map no /health/* at all)" is wrong — SiteServiceRegistration registers akka-cluster + localdb + the role-scoped SitePairActiveNodeHealthCheck, and Program.cs's Site branch calls MapZbHealth() on the HTTP/1.1 listener. Fail-before evidence: with the filter behaviourally disabled, StandbyActiveNodeVerdict_IsNotLogged fails "Collection was not empty" while the other 9 tests in the file — the must-still-log constraints — pass either way, which is what proves the filter is narrow rather than a blanket suppression. Tests: Host 518 green (10 new). --- CLAUDE.md | 4 +- docs/requirements/Component-Host.md | 54 +++++++ .../LoggerConfigurationFactory.cs | 79 +++++++++ .../StandbyHealthCheckLogNoiseTests.cs | 152 ++++++++++++++++++ 4 files changed, 287 insertions(+), 2 deletions(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StandbyHealthCheckLogNoiseTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index fd96eff5..6c45e958 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -83,7 +83,7 @@ spec for each is `docs/requirements/Component-.md`, and `README.md` carrie - `site_events.id` changed from autoincrement INTEGER to an application-minted **GUID**. Last-writer-wins keys on the primary key, so two nodes independently minting `id=1,2,3…` would destroy each other's events rather than merge them. The event-log read path uses a composite `(timestamp, id)` keyset cursor with an **opaque string** continuation token; `EventLogEntry.Id` and both `ContinuationToken`s are `string`/`string?` on the site↔central Akka DTOs. - `ScadaBridge:OperationTracking:ConnectionString`, `ScadaBridge:SiteEventLog:DatabasePath` and — as of Phase 2 — `ScadaBridge:StoreAndForward:SqliteDbPath` + `ScadaBridge:Database:SiteDbPath` are all **migration-only** — nothing reads them but `SiteLocalDbLegacyMigrator`, which copies a pre-Phase-1 file in once (deterministic `mig-{NodeName}-{legacyId}` event ids, `INSERT OR IGNORE`, runs AFTER `RegisterReplicated` so migrated rows replicate) and renames it `.migrated`. Delete the keys once a node has migrated. - This incidentally fixes a data-loss bug: both legacy databases defaulted to CWD-relative paths **outside** the mounted volume and were discarded on every container recreate. - - **Replication is default-OFF and opt-in** via `LocalDb:Replication:PeerAddress` + a matching `ApiKey` on both nodes. `LocalDbSyncAuthInterceptor` is **fail-closed**: no configured key means no sync stream is accepted at all, so a key typo does not degrade to unauthenticated replication — the pair simply stops converging. The sync endpoint shares the existing site gRPC h2c listener (8083); no new port. Rig posture: **site-a replicated, site-b/site-c deliberately not**, so both states are proven side-by-side. Status surfaces on the site health report as `LocalDbReplicationConnected` / `LocalDbOplogBacklog` (both nullable — null means "no data", NOT "disconnected with an empty backlog") and as `localdb_*` Prometheus series. **A transient backlog spike is expected daily and is not a fault:** the `site_events` retention purge (and the storage-cap trim) is CDC-captured like any other write — there is deliberately no purge-exemption path — so the backlog jumps by the deleted batch size and drains as the peer acks it; the purge logs an Information note saying so on a replication-enabled node (`SiteEventLogReplicationCheck`, supplied by the Host from `SiteLocalDbSetup.ReplicationIsConfigured` so SiteEventLogging never reads LocalDb config itself). Healthy = `LocalDbReplicationConnected` stays true and the backlog returns to ~0; see `docs/deployment/topology-guide.md` → *Reading the replication backlog*. Note `ZbTelemetryOptions.Meters` is an **allowlist** (`SiteServiceRegistration.ObservedMeters`); an unlisted meter exports nothing, silently. + - **Replication is default-OFF and opt-in** via `LocalDb:Replication:PeerAddress` + a matching `ApiKey` on both nodes. `LocalDbSyncAuthInterceptor` is **fail-closed**: no configured key means no sync stream is accepted at all, so a key typo does not degrade to unauthenticated replication — the pair simply stops converging. The sync endpoint shares the existing site gRPC h2c listener (8083); no new port. Rig posture: **site-a replicated, site-b/site-c deliberately not**, so both states are proven side-by-side. Status surfaces on the site health report as `LocalDbReplicationConnected` / `LocalDbOplogBacklog` (both nullable — null means "no data", NOT "disconnected with an empty backlog") and as `localdb_*` Prometheus series. **An UNCONFIGURED node reports null for both** — the code only started matching that documented contract with register row 80: the engine is registered unconditionally and answers `Connected=false`/`0` while inert, and publishing that made site-b/site-c look like broken replicated sites and starved central's null branches (row 33's conditional KPI emission + sustained-badge anchor exclusion) of the null they exist for. `LocalDbReplicationStatusReporter` now gates on a Host-supplied `SiteLocalDbSetup.ReplicationIsConfigured` flag, so `false` means exactly one thing: **configured, peer session down** = a real fault. Watch the trap that caused it — configured-ness is `PeerAddress` **OR** `ApiKey`, so "no PeerAddress" is NOT "unreplicated" (the pre-existing Host wiring test fixture is a passive-half replicated node for precisely this reason). **A transient backlog spike is expected daily and is not a fault:** the `site_events` retention purge (and the storage-cap trim) is CDC-captured like any other write — there is deliberately no purge-exemption path — so the backlog jumps by the deleted batch size and drains as the peer acks it; the purge logs an Information note saying so on a replication-enabled node (`SiteEventLogReplicationCheck`, supplied by the Host from `SiteLocalDbSetup.ReplicationIsConfigured` so SiteEventLogging never reads LocalDb config itself). Healthy = `LocalDbReplicationConnected` stays true and the backlog returns to ~0; see `docs/deployment/topology-guide.md` → *Reading the replication backlog*. Note `ZbTelemetryOptions.Meters` is an **allowlist** (`SiteServiceRegistration.ObservedMeters`); an unlisted meter exports nothing, silently. - Design: **scadaproj** `docs/plans/2026-07-19-scadabridge-localdb-design.md` (that doc lives in the umbrella repo, not here); Phase 1 plan + Phase 2 gate are here under `docs/plans/`. Phase 2 **deleted** `SiteReplicationActor`, its `ReplicationMessages`, StoreAndForward's `ReplicationService`, and `StoreAndForwardStorage.ReplaceAllAsync` — do not reintroduce them: - CDC replication does all three jobs now: config deploys reach the standby as ordinary row changes — **the standby makes no fetch at all** during a deploy (`SiteReconciliationActor`'s node-STARTUP fetch when central reports gaps is a different, surviving path) — and buffer mutations replicate via triggers on `sf_messages`. `ReplaceAllAsync` was a destructive delete-all-then-insert-all resync and is **unsafe to reintroduce**: a mass DELETE on a replicated table would be captured and shipped to the peer. LocalDb's snapshot resync merges per row under LWW and never deletes, which is also why the old N1 directional-authority guard is gone — there is no wipe left to gate. @@ -180,7 +180,7 @@ spec for each is `docs/requirements/Component-.md`, and `README.md` carrie - Failure detection: 2s heartbeat, 10s threshold. Total failover ~25s (drill-measured 2026-07-21 under auto-down: active-crash TAKEOVER in 28s, standby-crash removal in 27s with 0 routing blips — `docker/failover-drill.sh`). - CoordinatedShutdown for graceful singleton handover. - Automatic dual-node recovery from persistent storage. -- **Active/standby is decided by `ActiveNodeEvaluator.SelfIsOldestUp`, never by cluster leadership** — see the Architecture note above. `/health/active` is **central-only** (site nodes map no `/health/*` at all) and backs both Traefik's active-node routing and `IActiveNodeGate`, so the proxy and the Inbound API always agree on which node is active. Central reaches a site by dialling `GrpcNodeAAddress`/`GrpcNodeBAddress` explicitly and flipping on error; whichever node answers, the site-internal `ClusterSingletonProxy` lands the work on the active node for free, so central still never needs to track which *site* node is active. +- **Active/standby is decided by `ActiveNodeEvaluator.SelfIsOldestUp`, never by cluster leadership** — see the Architecture note above. **Central's** `/health/active` backs both Traefik's active-node routing and `IActiveNodeGate`, so the proxy and the Inbound API always agree on which node is active. **Site nodes DO map `/health/*` too** — the "site nodes map no `/health/*` at all" claim that stood here was stale: `SiteServiceRegistration` registers `akka-cluster` + `localdb` (Ready) and `SitePairActiveNodeHealthCheck` (Active, role-scoped to `site-{SiteId}`, deliberately NOT central's unscoped shared check), and `Program.cs`'s Site branch calls `MapZbHealth()` on the HTTP/1.1 listener so the family overview dashboard can probe every node identically. Nothing in this repo probes a site's `/health/active` (Traefik health-probes central only), and there is **no `IHealthCheckPublisher` anywhere**, so those checks run only on an inbound HTTP probe. Consequence worth knowing: the standby's correct-by-design `active-node` Unhealthy verdict is logged by `DefaultHealthCheckService` at **Error** on every probe, which `LoggerConfigurationFactory` now suppresses with one narrowly-keyed Serilog filter (register row 71 — see `Component-Host.md` → *Standby health-check log noise*; genuine failures of every other check still log at Error). Central reaches a site by dialling `GrpcNodeAAddress`/`GrpcNodeBAddress` explicitly and flipping on error; whichever node answers, the site-internal `ClusterSingletonProxy` lands the work on the active node for free, so central still never needs to track which *site* node is active. - **Seed-node ordering: every node lists ITSELF first (decision 2026-07-22) — the boot-alone gap is CLOSED.** Only `seed-nodes[0]` may self-join to form a new cluster (Akka runs `FirstSeedNodeProcess` for it, `JoinSeedNodeProcess` — which can never form one — for everyone else). All 14 shipped node appsettings now lead with the node's own address, so any node can cold-start alone and become operational unattended (~5s, `seed-node-timeout`); `StartupValidator` fails the boot if the ordering is broken (compares host AND port; Akka does no DNS canonicalisation). Two nodes cold-starting together while mutually reachable converge on ONE cluster via the `InitJoin` handshake — they split only under a genuine boot-time partition, the same class auto-down accepts. **An external self-form timer (`Cluster.Join(SelfAddress)` after a window) was implemented and REJECTED:** it sits outside the join handshake, so on a routine standby restart — where the peer is alive but the join is stalled behind removal of the node's own stale incarnation — it fires mid-join and permanently splits the pair (measured: still split after 90s). Regression tests: `SelfFirstSeedBootstrapTests`. The keep-oldest active-crash total outage was separately closed by the auto-down decision. See `docs/requirements/Component-ClusterInfrastructure.md` → Seed Node Ordering. - **Simultaneous-cold-start split-brain guard (opt-in, Gitea #33) — the residual self-first cost, closed.** Self-first-on-both means a *truly simultaneous* cold start (shared power/hypervisor event) races `FirstSeedNodeProcess` on BOTH nodes → two 1-node clusters that never merge (in-process loopback tests converge and hide it; real parallel VM starts don't — OtOpcUa reproduced it live). Ported from OtOpcUa (`lmxopcua` `d1dac87f`): a **dark switch `ScadaBridge:Cluster:BootstrapGuard:Enabled` (default OFF, guard-off byte-identical)**. On: `BuildHocon` emits an EMPTY seed list (Akka does not auto-join) and `ClusterBootstrapCoordinator` (`IHostedService`, registered in BOTH the Central and Site composition roots) issues ONE reachability-gated `JoinSeedNodes` — the pure `ClusterBootstrapGuard` core makes the lower canonical `host:port` the **founder** (self-first, forms immediately), the higher node TCP-probes the founder up to `PartnerProbeSeconds` (25s) and joins **peer-first** if reachable else **self-first** (cold-start-alone preserved). Decided BEFORE a single join, **never re-forms mid-handshake** (that was the rejected self-form-timer's flaw); case-insensitive founder tie-break; probe timings validated `>0` when enabled. Residual accepted trade: founder dies in the probe→join window → higher node hangs unjoined, coordinator WARNs, a restart recovers it. Tests: `ClusterBootstrapGuardTests` (pure) + `ClusterBootstrapCoordinatorTests` (real-ActorSystem, incl. both-cold-start-together-form-one-cluster). **ENABLED on the docker rig (all 8 nodes) 2026-08-02** after `deploy.sh`'s simultaneous recreate split site pairs twice on 2026-08-01 with the guard off; live gate PASS — two consecutive simultaneous-start trials (a full redeploy + a full-topology `docker compose restart`), all four pairs converged (every lower-address node founded self-first, every higher node probed-then-joined peer-first), zero splits. Default remains OFF elsewhere (wonder-app-vd03, docker-env2, Host defaults). See `docs/requirements/Component-ClusterInfrastructure.md` → Simultaneous cold start. diff --git a/docs/requirements/Component-Host.md b/docs/requirements/Component-Host.md index 25fee106..ea7f14af 100644 --- a/docs/requirements/Component-Host.md +++ b/docs/requirements/Component-Host.md @@ -195,6 +195,60 @@ Akka actors are constructed by the actor system, not by the DI container, so the --- +### Boot-time facts the Host supplies to Health Monitoring + +Health Monitoring reports two things the Host alone knows, and in both cases the Host reads the +configuration and hands over a captured value rather than teaching the component to read it. This +is the `SiteEventLogReplicationCheck` pattern generalised: configuration-reading rules live in one +place, and a component that merely *reports* a fact never grows a dependency on where it comes +from. + +**LocalDb replication configured-ness** (`SiteServiceRegistration`, register row 80). The +replication engine is registered unconditionally and idles without a peer, so `ISyncStatus` answers +`Connected = false` with a real `0` backlog on a node that was never meant to replicate. Passing +that through made every deliberately-unreplicated site publish "disconnected, empty backlog". The +Host now evaluates `SiteLocalDbSetup.ReplicationIsConfigured(config)` **once** and passes it into +`AddLocalDbReplicationHealthBridge` as a third provider; an unconfigured node publishes `null` for +both LocalDb health fields. The same captured value feeds `SiteEventLogReplicationCheck`, so the +two surfaces can never disagree about whether this node replicates. Note the rule is `PeerAddress` +**OR** `ApiKey` — only the dialling half sets a peer address. + +**Startup reconciler presence** (`AkkaHostedService`, register row 73). The Host decides whether to +spawn `SiteReconciliationActor` — it needs an `IDeploymentConfigFetcher`, and skips the actor when +none is resolvable. That skip was previously a lone boot Warning, so a node with no self-heal path +at all reported itself perfectly healthy. Both branches now call +`ISiteHealthCollector.SetStartupReconcilerActive(...)`, latching the fact onto every subsequent +health report as tri-state `StartupReconcilerActive`; the skip branch additionally logs at **Error** +(raised from Warning — 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 +renders it as a *No self-heal* warning badge. See +[Component-HealthMonitoring.md](Component-HealthMonitoring.md) → *Startup Reconciler Presence*. + +### Standby health-check log noise + +`LoggerConfigurationFactory` installs one Serilog exclusion filter, for the by-design standby +verdict of the `active-node` probe (register row 71). + +`DefaultHealthCheckService` logs one line per check per probe and picks the level from the verdict — +Debug / Warning / **Error** for Healthy / Degraded / Unhealthy. Both active-node checks +(`SitePairActiveNodeHealthCheck` on a site, the shared `ActiveNodeHealthCheck` on central) answer +Unhealthy on the standby, because 503 is exactly the contract `/health/active` publishes to Traefik +and the family dashboard. A perfectly healthy standby therefore logged ERR on every probe — every +10 s on a site node, every 5 s on a central node behind Traefik's active-node probe — which floods +the log and trains operators to ignore ERR. + +Serilog's `MinimumLevel.Override` can only *raise* a per-source floor and a `LogEvent`'s level is +immutable, so neither an override nor an enricher can rewrite the event; excluding it is the only +surgical option. The predicate matches on `Level == Error` **and** the health-check logger category +**and** `HealthCheckName == "active-node"`. Keying on the check name rather than the category is +what preserves the constraint: a failing `localdb`, `akka-cluster`, `database` or +`required-singletons` check still logs at Error, and the active-node check has no error path of its +own (the active node always answers Healthy), so the only event this can ever suppress is a standby +reporting that it is a standby. The verdict remains fully observable through the HTTP status of +`/health/active` itself, the readiness probe's cluster view, and the health dashboard. + +--- + ## Component Registration Matrix | Component | Central | Site | DI (`AddXxx`) | Actors (`AddXxxActors`) | Endpoints (`MapXxx`) | diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/LoggerConfigurationFactory.cs b/src/ZB.MOM.WW.ScadaBridge.Host/LoggerConfigurationFactory.cs index 8b906f1d..8275ee6b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/LoggerConfigurationFactory.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/LoggerConfigurationFactory.cs @@ -85,12 +85,91 @@ public static class LoggerConfigurationFactory return new LoggerConfiguration() .ReadFrom.Configuration(configuration) .MinimumLevel.Is(minimumLevel) + // Register row 71. Drop ONLY the framework's Error line for the by-design + // standby verdict of the active-node probe. See IsExpectedStandbyActiveNodeProbe + // for why this cannot hide a genuine failure. + .Filter.ByExcluding(IsExpectedStandbyActiveNodeProbe) .Enrich.WithProperty("SiteId", siteId) .Enrich.WithProperty("NodeHostname", nodeHostname) .Enrich.WithProperty("NodeRole", nodeRole) .Enrich.With(new TraceContextEnricher()); } + /// + /// Logger category that emits the per-probe "Health check {HealthCheckName} with status + /// {HealthStatus} …" line. It is DefaultHealthCheckService, not + /// HealthCheckPublisherHostedService: this host registers no + /// IHealthCheckPublisher, so the checks run only on an inbound HTTP probe of + /// /health/*. + /// + internal const string HealthCheckServiceSourceContext = + "Microsoft.Extensions.Diagnostics.HealthChecks.DefaultHealthCheckService"; + + /// Name both the site and central /health/active probes register under. + internal const string ActiveNodeCheckName = "active-node"; + + /// + /// Suppresses the standby node's correct-by-design active-node Error line + /// (deferred-work register row 71). + /// + /// + /// + /// The problem. DefaultHealthCheckService logs one line per check per probe, + /// picking the level from the verdict — Debug for Healthy, Warning for Degraded, + /// Error for Unhealthy. Both active-node checks + /// (SitePairActiveNodeHealthCheck on a site, the shared ActiveNodeHealthCheck + /// on central) answer Unhealthy on the standby, because 503 is exactly the contract + /// /health/active publishes to Traefik and the family dashboard. So a perfectly + /// healthy standby logged ERR on every probe — every 10 s on a site node, every 5 s on a + /// central node behind Traefik's 5 s active-node probe. That floods the standby's log and + /// trains operators to ignore ERR. + /// + /// + /// Why a filter and not a level override. Serilog's MinimumLevel.Override + /// can only RAISE a per-source floor; it cannot rewrite one Error event as Information. + /// A LogEvent's level is immutable, so an enricher cannot do it either. Excluding + /// the single by-design event is the surgical option, and it leaves the verdict fully + /// observable where it belongs: the HTTP status of /health/active itself, the + /// cluster view on the readiness probe, and the health dashboard. + /// + /// + /// Why this cannot hide a genuine failure. The predicate is keyed on the check + /// NAME, not on the category, so a failing localdb, akka-cluster, + /// database, or required-singletons check still logs at Error untouched. + /// And the active-node check is a pure "am I the oldest Up member" verdict with no error + /// path of its own — the ACTIVE node always answers Healthy, so the only event this can + /// ever match is a standby reporting that it is a standby. Genuine loss of the pair + /// surfaces on the readiness checks, which are deliberately out of scope here. + /// + /// + /// Matching on rather than on the HealthStatus + /// property avoids depending on how the enum renders into the event; the level→verdict + /// mapping above already makes Error equivalent to Unhealthy. + /// + /// + /// The candidate log event. + /// when the event should be dropped. + internal static bool IsExpectedStandbyActiveNodeProbe(LogEvent logEvent) + { + if (logEvent is null || logEvent.Level != LogEventLevel.Error) + { + return false; + } + + return MatchesScalar(logEvent, "SourceContext", HealthCheckServiceSourceContext) + && MatchesScalar(logEvent, "HealthCheckName", ActiveNodeCheckName); + } + + /// Tests one structured property for an exact scalar string value. + /// The event whose properties are inspected. + /// The structured property name. + /// The value the property must equal. + /// on an exact ordinal match. + private static bool MatchesScalar(LogEvent logEvent, string propertyName, string expected) => + logEvent.Properties.TryGetValue(propertyName, out var value) + && value is ScalarValue { Value: string actual } + && string.Equals(actual, expected, StringComparison.Ordinal); + /// /// Parses a Serilog name, falling back to /// for null/blank/unrecognised values. diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StandbyHealthCheckLogNoiseTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StandbyHealthCheckLogNoiseTests.cs new file mode 100644 index 00000000..17d914d1 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StandbyHealthCheckLogNoiseTests.cs @@ -0,0 +1,152 @@ +using Microsoft.Extensions.Configuration; +using Serilog; +using Serilog.Events; + +namespace ZB.MOM.WW.ScadaBridge.Host.Tests; + +/// +/// Deferred-work register row 71 — a standby node logged the correct-by-design +/// active-node health-check verdict at ERROR on every probe. +/// +/// +/// +/// DefaultHealthCheckService picks the log level from the verdict (Debug/Warning/Error +/// for Healthy/Degraded/Unhealthy). Both active-node checks answer Unhealthy on the standby +/// because 503 is the contract /health/active publishes, so a healthy standby logged +/// ERR every 10 s on a site node and every 5 s on a central node behind Traefik's probe. +/// +/// +/// The constraint on the fix is that a GENUINE failure must still log at Error, which is why +/// the filter is keyed on the check NAME rather than on the logger category. +/// +/// +public class StandbyHealthCheckLogNoiseTests +{ + private const string HealthCheckCategory = + "Microsoft.Extensions.Diagnostics.HealthChecks.DefaultHealthCheckService"; + + private const string Template = + "Health check {HealthCheckName} with status {HealthStatus} completed after " + + "{ElapsedMilliseconds}ms with message '{HealthCheckDescription}'"; + + private static ILogger BuildLogger(InMemorySink sink) + { + var config = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["ScadaBridge:Logging:MinimumLevel"] = "Debug", + }) + .Build(); + + return LoggerConfigurationFactory + .Build(config, "Site", "site-a", "site-a-b") + .WriteTo.Sink(sink) + .CreateLogger(); + } + + /// Emits one event shaped exactly like the framework's per-check line. + private static void EmitHealthCheckLine( + ILogger logger, string category, LogEventLevel level, string checkName, string status) => + logger + .ForContext("SourceContext", category) + .Write(level, Template, checkName, status, 1.0, "some message"); + + [Fact] + public void StandbyActiveNodeVerdict_IsNotLogged() + { + // The row-71 regression: "Health check active-node with status Unhealthy … 'not the + // site pair primary (standby)'" at Error, once per probe, forever. + var sink = new InMemorySink(); + var logger = BuildLogger(sink); + + EmitHealthCheckLine( + logger, HealthCheckCategory, LogEventLevel.Error, "active-node", "Unhealthy"); + + Assert.Empty(sink.LogEvents); + } + + [Theory] + [InlineData("localdb")] + [InlineData("akka-cluster")] + [InlineData("database")] + [InlineData("required-singletons")] + public void GenuineCheckFailure_StillLogsAtError(string checkName) + { + // The constraint. The filter is name-scoped, so every OTHER check on the same logger + // category keeps its Error line — a blanket category override would have silenced + // these too, which is exactly the trap this test exists to catch. + var sink = new InMemorySink(); + var logger = BuildLogger(sink); + + EmitHealthCheckLine( + logger, HealthCheckCategory, LogEventLevel.Error, checkName, "Unhealthy"); + + var evt = Assert.Single(sink.LogEvents); + Assert.Equal(LogEventLevel.Error, evt.Level); + } + + [Fact] + public void ActiveNodeHealthyVerdict_IsUnaffected() + { + // The active node reports Healthy, which the framework logs at Debug. Nothing about + // the filter should touch it — it only ever matches Error. + var sink = new InMemorySink(); + var logger = BuildLogger(sink); + + EmitHealthCheckLine( + logger, HealthCheckCategory, LogEventLevel.Debug, "active-node", "Healthy"); + + Assert.Single(sink.LogEvents); + } + + [Fact] + public void ActiveNodeDegradedVerdict_IsUnaffected() + { + // Degraded logs at Warning. If a future check shape ever reports Degraded for + // active-node, that should still be visible — the filter is Error-only. + var sink = new InMemorySink(); + var logger = BuildLogger(sink); + + EmitHealthCheckLine( + logger, HealthCheckCategory, LogEventLevel.Warning, "active-node", "Degraded"); + + Assert.Single(sink.LogEvents); + } + + [Fact] + public void AnErrorMentioningActiveNode_FromAnotherCategory_IsNotSuppressed() + { + // The predicate requires BOTH the health-check category and the check name, so + // application code logging about the active node keeps its Error. + var sink = new InMemorySink(); + var logger = BuildLogger(sink); + + EmitHealthCheckLine( + logger, + "ZB.MOM.WW.ScadaBridge.Host.Actors.AkkaHostedService", + LogEventLevel.Error, + "active-node", + "Unhealthy"); + + Assert.Single(sink.LogEvents); + } + + [Fact] + public void OrdinaryErrorsWithNoHealthCheckProperties_AreNotSuppressed() + { + var sink = new InMemorySink(); + var logger = BuildLogger(sink); + + logger.Error("something genuinely broke"); + + Assert.Single(sink.LogEvents); + } + + [Fact] + public void Predicate_RejectsANullEvent() + { + // Defensive: Serilog never hands the filter a null, but the predicate is internal + // and reachable, and returning "suppress" for null would be the wrong default. + Assert.False(LoggerConfigurationFactory.IsExpectedStandbyActiveNodeProbe(null!)); + } +}