diff --git a/CLAUDE.md b/CLAUDE.md index d421103a..1a02e3b4 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*. **That spike is PACED as of row 34 (2026-08-15)** — `ScadaBridge:SiteEventLog:MaxPurgedRowsPerTick` (default **250 000**, `0` = unlimited escape hatch) is a per-tick delete ceiling **shared** by the retention purge and the cap trim, and a ceiling-limited tick reschedules after `PurgeCatchUpInterval` (default **5 min**) instead of the daily `PurgeInterval` so retention still converges (~11 ticks ≈ 55 min for the worst case, vs 11 days without the acceleration). Measured worst case, unpaced: a 1 GB event log holds **~2.73 M rows at 393 B/row**, and expiring all of them in one tick took 285 s and queued **2 731 947 oplog rows — 2.7x LocalDb's `MaxOplogRows` default of 1 000 000**, i.e. enough to make the maintenance tick prune the oplog and flag a **full ten-table snapshot resync**. **The subtlety that makes pacing the right fix rather than a bigger `MaxOplogRows`:** the *resident* backlog is `produced − drained`, so a healthy peer blunts it — on a loopback pair the same 2.73 M burst peaked at only **391 k** and drained in 6.0 min. The overrun is therefore **conditional on peer throughput** (breached outright with the peer down; breached at ~1.5 M if the peer runs at half loopback speed), and a 250 k ceiling makes the outcome independent of the peer instead. Drain speed itself is not a bottleneck (7.6–19.8 k tombstones/s measured), so a full 250 k tick clears far inside row 33's 15-minute sustained dwell — verified at 2x a tick (500 k drained in 46 s). The purge loop is now a delay loop rather than a `PeriodicTimer`, so its cadence anchors to the END of the previous purge. Ceiling hits log a rate-limited (15 min) Warning and always increment `scadabridge.site.event_log.purge_ceiling_hits` — read the counter's shape: a run that stops is a backlog converging, hits that never stop mean the ceiling is mis-sized for that site's event rate. Measurement + decision record: `docs/plans/2026-08-15-site-events-purge-burst-measurement.md`. 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*. **That spike is PACED as of row 34 (2026-08-15)** — `ScadaBridge:SiteEventLog:MaxPurgedRowsPerTick` (default **250 000**, `0` = unlimited escape hatch) is a per-tick delete ceiling **shared** by the retention purge and the cap trim, and a ceiling-limited tick reschedules after `PurgeCatchUpInterval` (default **5 min**) instead of the daily `PurgeInterval` so retention still converges (~11 ticks ≈ 55 min for the worst case, vs 11 days without the acceleration). Measured worst case, unpaced: a 1 GB event log holds **~2.73 M rows at 393 B/row**, and expiring all of them in one tick took 285 s and queued **2 731 947 oplog rows — 2.7x LocalDb's `MaxOplogRows` default of 1 000 000**, i.e. enough to make the maintenance tick prune the oplog and flag a **full ten-table snapshot resync**. **The subtlety that makes pacing the right fix rather than a bigger `MaxOplogRows`:** the *resident* backlog is `produced − drained`, so a healthy peer blunts it — on a loopback pair the same 2.73 M burst peaked at only **391 k** and drained in 6.0 min. The overrun is therefore **conditional on peer throughput** (breached outright with the peer down; breached at ~1.5 M if the peer runs at half loopback speed), and a 250 k ceiling makes the outcome independent of the peer instead. Drain speed itself is not a bottleneck (7.6–19.8 k tombstones/s measured), so a full 250 k tick clears far inside row 33's 15-minute sustained dwell — verified at 2x a tick (500 k drained in 46 s). The purge loop is now a delay loop rather than a `PeriodicTimer`, so its cadence anchors to the END of the previous purge. Ceiling hits log a rate-limited (15 min) Warning and always increment `scadabridge.site.event_log.purge_ceiling_hits` — read the counter's shape: a run that stops is a backlog converging, hits that never stop mean the ceiling is mis-sized for that site's event rate. Measurement + decision record: `docs/plans/2026-08-15-site-events-purge-burst-measurement.md`. 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-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/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.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/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/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/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!)); + } +} 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); + } +}