diff --git a/CLAUDE.md b/CLAUDE.md index fd96eff5..6c45e958 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -83,7 +83,7 @@ spec for each is `docs/requirements/Component-.md`, and `README.md` carrie - `site_events.id` changed from autoincrement INTEGER to an application-minted **GUID**. Last-writer-wins keys on the primary key, so two nodes independently minting `id=1,2,3…` would destroy each other's events rather than merge them. The event-log read path uses a composite `(timestamp, id)` keyset cursor with an **opaque string** continuation token; `EventLogEntry.Id` and both `ContinuationToken`s are `string`/`string?` on the site↔central Akka DTOs. - `ScadaBridge:OperationTracking:ConnectionString`, `ScadaBridge:SiteEventLog:DatabasePath` and — as of Phase 2 — `ScadaBridge:StoreAndForward:SqliteDbPath` + `ScadaBridge:Database:SiteDbPath` are all **migration-only** — nothing reads them but `SiteLocalDbLegacyMigrator`, which copies a pre-Phase-1 file in once (deterministic `mig-{NodeName}-{legacyId}` event ids, `INSERT OR IGNORE`, runs AFTER `RegisterReplicated` so migrated rows replicate) and renames it `.migrated`. Delete the keys once a node has migrated. - This incidentally fixes a data-loss bug: both legacy databases defaulted to CWD-relative paths **outside** the mounted volume and were discarded on every container recreate. - - **Replication is default-OFF and opt-in** via `LocalDb:Replication:PeerAddress` + a matching `ApiKey` on both nodes. `LocalDbSyncAuthInterceptor` is **fail-closed**: no configured key means no sync stream is accepted at all, so a key typo does not degrade to unauthenticated replication — the pair simply stops converging. The sync endpoint shares the existing site gRPC h2c listener (8083); no new port. Rig posture: **site-a replicated, site-b/site-c deliberately not**, so both states are proven side-by-side. Status surfaces on the site health report as `LocalDbReplicationConnected` / `LocalDbOplogBacklog` (both nullable — null means "no data", NOT "disconnected with an empty backlog") and as `localdb_*` Prometheus series. **A transient backlog spike is expected daily and is not a fault:** the `site_events` retention purge (and the storage-cap trim) is CDC-captured like any other write — there is deliberately no purge-exemption path — so the backlog jumps by the deleted batch size and drains as the peer acks it; the purge logs an Information note saying so on a replication-enabled node (`SiteEventLogReplicationCheck`, supplied by the Host from `SiteLocalDbSetup.ReplicationIsConfigured` so SiteEventLogging never reads LocalDb config itself). Healthy = `LocalDbReplicationConnected` stays true and the backlog returns to ~0; see `docs/deployment/topology-guide.md` → *Reading the replication backlog*. Note `ZbTelemetryOptions.Meters` is an **allowlist** (`SiteServiceRegistration.ObservedMeters`); an unlisted meter exports nothing, silently. + - **Replication is default-OFF and opt-in** via `LocalDb:Replication:PeerAddress` + a matching `ApiKey` on both nodes. `LocalDbSyncAuthInterceptor` is **fail-closed**: no configured key means no sync stream is accepted at all, so a key typo does not degrade to unauthenticated replication — the pair simply stops converging. The sync endpoint shares the existing site gRPC h2c listener (8083); no new port. Rig posture: **site-a replicated, site-b/site-c deliberately not**, so both states are proven side-by-side. Status surfaces on the site health report as `LocalDbReplicationConnected` / `LocalDbOplogBacklog` (both nullable — null means "no data", NOT "disconnected with an empty backlog") and as `localdb_*` Prometheus series. **An UNCONFIGURED node reports null for both** — the code only started matching that documented contract with register row 80: the engine is registered unconditionally and answers `Connected=false`/`0` while inert, and publishing that made site-b/site-c look like broken replicated sites and starved central's null branches (row 33's conditional KPI emission + sustained-badge anchor exclusion) of the null they exist for. `LocalDbReplicationStatusReporter` now gates on a Host-supplied `SiteLocalDbSetup.ReplicationIsConfigured` flag, so `false` means exactly one thing: **configured, peer session down** = a real fault. Watch the trap that caused it — configured-ness is `PeerAddress` **OR** `ApiKey`, so "no PeerAddress" is NOT "unreplicated" (the pre-existing Host wiring test fixture is a passive-half replicated node for precisely this reason). **A transient backlog spike is expected daily and is not a fault:** the `site_events` retention purge (and the storage-cap trim) is CDC-captured like any other write — there is deliberately no purge-exemption path — so the backlog jumps by the deleted batch size and drains as the peer acks it; the purge logs an Information note saying so on a replication-enabled node (`SiteEventLogReplicationCheck`, supplied by the Host from `SiteLocalDbSetup.ReplicationIsConfigured` so SiteEventLogging never reads LocalDb config itself). Healthy = `LocalDbReplicationConnected` stays true and the backlog returns to ~0; see `docs/deployment/topology-guide.md` → *Reading the replication backlog*. Note `ZbTelemetryOptions.Meters` is an **allowlist** (`SiteServiceRegistration.ObservedMeters`); an unlisted meter exports nothing, silently. - Design: **scadaproj** `docs/plans/2026-07-19-scadabridge-localdb-design.md` (that doc lives in the umbrella repo, not here); Phase 1 plan + Phase 2 gate are here under `docs/plans/`. Phase 2 **deleted** `SiteReplicationActor`, its `ReplicationMessages`, StoreAndForward's `ReplicationService`, and `StoreAndForwardStorage.ReplaceAllAsync` — do not reintroduce them: - CDC replication does all three jobs now: config deploys reach the standby as ordinary row changes — **the standby makes no fetch at all** during a deploy (`SiteReconciliationActor`'s node-STARTUP fetch when central reports gaps is a different, surviving path) — and buffer mutations replicate via triggers on `sf_messages`. `ReplaceAllAsync` was a destructive delete-all-then-insert-all resync and is **unsafe to reintroduce**: a mass DELETE on a replicated table would be captured and shipped to the peer. LocalDb's snapshot resync merges per row under LWW and never deletes, which is also why the old N1 directional-authority guard is gone — there is no wipe left to gate. @@ -180,7 +180,7 @@ spec for each is `docs/requirements/Component-.md`, and `README.md` carrie - Failure detection: 2s heartbeat, 10s threshold. Total failover ~25s (drill-measured 2026-07-21 under auto-down: active-crash TAKEOVER in 28s, standby-crash removal in 27s with 0 routing blips — `docker/failover-drill.sh`). - CoordinatedShutdown for graceful singleton handover. - Automatic dual-node recovery from persistent storage. -- **Active/standby is decided by `ActiveNodeEvaluator.SelfIsOldestUp`, never by cluster leadership** — see the Architecture note above. `/health/active` is **central-only** (site nodes map no `/health/*` at all) and backs both Traefik's active-node routing and `IActiveNodeGate`, so the proxy and the Inbound API always agree on which node is active. Central reaches a site by dialling `GrpcNodeAAddress`/`GrpcNodeBAddress` explicitly and flipping on error; whichever node answers, the site-internal `ClusterSingletonProxy` lands the work on the active node for free, so central still never needs to track which *site* node is active. +- **Active/standby is decided by `ActiveNodeEvaluator.SelfIsOldestUp`, never by cluster leadership** — see the Architecture note above. **Central's** `/health/active` backs both Traefik's active-node routing and `IActiveNodeGate`, so the proxy and the Inbound API always agree on which node is active. **Site nodes DO map `/health/*` too** — the "site nodes map no `/health/*` at all" claim that stood here was stale: `SiteServiceRegistration` registers `akka-cluster` + `localdb` (Ready) and `SitePairActiveNodeHealthCheck` (Active, role-scoped to `site-{SiteId}`, deliberately NOT central's unscoped shared check), and `Program.cs`'s Site branch calls `MapZbHealth()` on the HTTP/1.1 listener so the family overview dashboard can probe every node identically. Nothing in this repo probes a site's `/health/active` (Traefik health-probes central only), and there is **no `IHealthCheckPublisher` anywhere**, so those checks run only on an inbound HTTP probe. Consequence worth knowing: the standby's correct-by-design `active-node` Unhealthy verdict is logged by `DefaultHealthCheckService` at **Error** on every probe, which `LoggerConfigurationFactory` now suppresses with one narrowly-keyed Serilog filter (register row 71 — see `Component-Host.md` → *Standby health-check log noise*; genuine failures of every other check still log at Error). Central reaches a site by dialling `GrpcNodeAAddress`/`GrpcNodeBAddress` explicitly and flipping on error; whichever node answers, the site-internal `ClusterSingletonProxy` lands the work on the active node for free, so central still never needs to track which *site* node is active. - **Seed-node ordering: every node lists ITSELF first (decision 2026-07-22) — the boot-alone gap is CLOSED.** Only `seed-nodes[0]` may self-join to form a new cluster (Akka runs `FirstSeedNodeProcess` for it, `JoinSeedNodeProcess` — which can never form one — for everyone else). All 14 shipped node appsettings now lead with the node's own address, so any node can cold-start alone and become operational unattended (~5s, `seed-node-timeout`); `StartupValidator` fails the boot if the ordering is broken (compares host AND port; Akka does no DNS canonicalisation). Two nodes cold-starting together while mutually reachable converge on ONE cluster via the `InitJoin` handshake — they split only under a genuine boot-time partition, the same class auto-down accepts. **An external self-form timer (`Cluster.Join(SelfAddress)` after a window) was implemented and REJECTED:** it sits outside the join handshake, so on a routine standby restart — where the peer is alive but the join is stalled behind removal of the node's own stale incarnation — it fires mid-join and permanently splits the pair (measured: still split after 90s). Regression tests: `SelfFirstSeedBootstrapTests`. The keep-oldest active-crash total outage was separately closed by the auto-down decision. See `docs/requirements/Component-ClusterInfrastructure.md` → Seed Node Ordering. - **Simultaneous-cold-start split-brain guard (opt-in, Gitea #33) — the residual self-first cost, closed.** Self-first-on-both means a *truly simultaneous* cold start (shared power/hypervisor event) races `FirstSeedNodeProcess` on BOTH nodes → two 1-node clusters that never merge (in-process loopback tests converge and hide it; real parallel VM starts don't — OtOpcUa reproduced it live). Ported from OtOpcUa (`lmxopcua` `d1dac87f`): a **dark switch `ScadaBridge:Cluster:BootstrapGuard:Enabled` (default OFF, guard-off byte-identical)**. On: `BuildHocon` emits an EMPTY seed list (Akka does not auto-join) and `ClusterBootstrapCoordinator` (`IHostedService`, registered in BOTH the Central and Site composition roots) issues ONE reachability-gated `JoinSeedNodes` — the pure `ClusterBootstrapGuard` core makes the lower canonical `host:port` the **founder** (self-first, forms immediately), the higher node TCP-probes the founder up to `PartnerProbeSeconds` (25s) and joins **peer-first** if reachable else **self-first** (cold-start-alone preserved). Decided BEFORE a single join, **never re-forms mid-handshake** (that was the rejected self-form-timer's flaw); case-insensitive founder tie-break; probe timings validated `>0` when enabled. Residual accepted trade: founder dies in the probe→join window → higher node hangs unjoined, coordinator WARNs, a restart recovers it. Tests: `ClusterBootstrapGuardTests` (pure) + `ClusterBootstrapCoordinatorTests` (real-ActorSystem, incl. both-cold-start-together-form-one-cluster). **ENABLED on the docker rig (all 8 nodes) 2026-08-02** after `deploy.sh`'s simultaneous recreate split site pairs twice on 2026-08-01 with the guard off; live gate PASS — two consecutive simultaneous-start trials (a full redeploy + a full-topology `docker compose restart`), all four pairs converged (every lower-address node founded self-first, every higher node probed-then-joined peer-first), zero splits. Default remains OFF elsewhere (wonder-app-vd03, docker-env2, Host defaults). See `docs/requirements/Component-ClusterInfrastructure.md` → Simultaneous cold start. diff --git a/docs/requirements/Component-Host.md b/docs/requirements/Component-Host.md index 25fee106..ea7f14af 100644 --- a/docs/requirements/Component-Host.md +++ b/docs/requirements/Component-Host.md @@ -195,6 +195,60 @@ Akka actors are constructed by the actor system, not by the DI container, so the --- +### Boot-time facts the Host supplies to Health Monitoring + +Health Monitoring reports two things the Host alone knows, and in both cases the Host reads the +configuration and hands over a captured value rather than teaching the component to read it. This +is the `SiteEventLogReplicationCheck` pattern generalised: configuration-reading rules live in one +place, and a component that merely *reports* a fact never grows a dependency on where it comes +from. + +**LocalDb replication configured-ness** (`SiteServiceRegistration`, register row 80). The +replication engine is registered unconditionally and idles without a peer, so `ISyncStatus` answers +`Connected = false` with a real `0` backlog on a node that was never meant to replicate. Passing +that through made every deliberately-unreplicated site publish "disconnected, empty backlog". The +Host now evaluates `SiteLocalDbSetup.ReplicationIsConfigured(config)` **once** and passes it into +`AddLocalDbReplicationHealthBridge` as a third provider; an unconfigured node publishes `null` for +both LocalDb health fields. The same captured value feeds `SiteEventLogReplicationCheck`, so the +two surfaces can never disagree about whether this node replicates. Note the rule is `PeerAddress` +**OR** `ApiKey` — only the dialling half sets a peer address. + +**Startup reconciler presence** (`AkkaHostedService`, register row 73). The Host decides whether to +spawn `SiteReconciliationActor` — it needs an `IDeploymentConfigFetcher`, and skips the actor when +none is resolvable. That skip was previously a lone boot Warning, so a node with no self-heal path +at all reported itself perfectly healthy. Both branches now call +`ISiteHealthCollector.SetStartupReconcilerActive(...)`, latching the fact onto every subsequent +health report as tri-state `StartupReconcilerActive`; the skip branch additionally logs at **Error** +(raised from Warning — no configuration makes the fetcher legitimately absent on a site node) and +writes a best-effort `deployment`/`Error` site event so the evidence outlives the boot log. Central +renders it as a *No self-heal* warning badge. See +[Component-HealthMonitoring.md](Component-HealthMonitoring.md) → *Startup Reconciler Presence*. + +### Standby health-check log noise + +`LoggerConfigurationFactory` installs one Serilog exclusion filter, for the by-design standby +verdict of the `active-node` probe (register row 71). + +`DefaultHealthCheckService` logs one line per check per probe and picks the level from the verdict — +Debug / Warning / **Error** for Healthy / Degraded / Unhealthy. Both active-node checks +(`SitePairActiveNodeHealthCheck` on a site, the shared `ActiveNodeHealthCheck` on central) answer +Unhealthy on the standby, because 503 is exactly the contract `/health/active` publishes to Traefik +and the family dashboard. A perfectly healthy standby therefore logged ERR on every probe — every +10 s on a site node, every 5 s on a central node behind Traefik's active-node probe — which floods +the log and trains operators to ignore ERR. + +Serilog's `MinimumLevel.Override` can only *raise* a per-source floor and a `LogEvent`'s level is +immutable, so neither an override nor an enricher can rewrite the event; excluding it is the only +surgical option. The predicate matches on `Level == Error` **and** the health-check logger category +**and** `HealthCheckName == "active-node"`. Keying on the check name rather than the category is +what preserves the constraint: a failing `localdb`, `akka-cluster`, `database` or +`required-singletons` check still logs at Error, and the active-node check has no error path of its +own (the active node always answers Healthy), so the only event this can ever suppress is a standby +reporting that it is a standby. The verdict remains fully observable through the HTTP status of +`/health/active` itself, the readiness probe's cluster view, and the health dashboard. + +--- + ## Component Registration Matrix | Component | Central | Site | DI (`AddXxx`) | Actors (`AddXxxActors`) | Endpoints (`MapXxx`) | diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/LoggerConfigurationFactory.cs b/src/ZB.MOM.WW.ScadaBridge.Host/LoggerConfigurationFactory.cs index 8b906f1d..8275ee6b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/LoggerConfigurationFactory.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/LoggerConfigurationFactory.cs @@ -85,12 +85,91 @@ public static class LoggerConfigurationFactory return new LoggerConfiguration() .ReadFrom.Configuration(configuration) .MinimumLevel.Is(minimumLevel) + // Register row 71. Drop ONLY the framework's Error line for the by-design + // standby verdict of the active-node probe. See IsExpectedStandbyActiveNodeProbe + // for why this cannot hide a genuine failure. + .Filter.ByExcluding(IsExpectedStandbyActiveNodeProbe) .Enrich.WithProperty("SiteId", siteId) .Enrich.WithProperty("NodeHostname", nodeHostname) .Enrich.WithProperty("NodeRole", nodeRole) .Enrich.With(new TraceContextEnricher()); } + /// + /// Logger category that emits the per-probe "Health check {HealthCheckName} with status + /// {HealthStatus} …" line. It is DefaultHealthCheckService, not + /// HealthCheckPublisherHostedService: this host registers no + /// IHealthCheckPublisher, so the checks run only on an inbound HTTP probe of + /// /health/*. + /// + internal const string HealthCheckServiceSourceContext = + "Microsoft.Extensions.Diagnostics.HealthChecks.DefaultHealthCheckService"; + + /// Name both the site and central /health/active probes register under. + internal const string ActiveNodeCheckName = "active-node"; + + /// + /// Suppresses the standby node's correct-by-design active-node Error line + /// (deferred-work register row 71). + /// + /// + /// + /// The problem. DefaultHealthCheckService logs one line per check per probe, + /// picking the level from the verdict — Debug for Healthy, Warning for Degraded, + /// Error for Unhealthy. Both active-node checks + /// (SitePairActiveNodeHealthCheck on a site, the shared ActiveNodeHealthCheck + /// on central) answer Unhealthy on the standby, because 503 is exactly the contract + /// /health/active publishes to Traefik and the family dashboard. So a perfectly + /// healthy standby logged ERR on every probe — every 10 s on a site node, every 5 s on a + /// central node behind Traefik's 5 s active-node probe. That floods the standby's log and + /// trains operators to ignore ERR. + /// + /// + /// Why a filter and not a level override. Serilog's MinimumLevel.Override + /// can only RAISE a per-source floor; it cannot rewrite one Error event as Information. + /// A LogEvent's level is immutable, so an enricher cannot do it either. Excluding + /// the single by-design event is the surgical option, and it leaves the verdict fully + /// observable where it belongs: the HTTP status of /health/active itself, the + /// cluster view on the readiness probe, and the health dashboard. + /// + /// + /// Why this cannot hide a genuine failure. The predicate is keyed on the check + /// NAME, not on the category, so a failing localdb, akka-cluster, + /// database, or required-singletons check still logs at Error untouched. + /// And the active-node check is a pure "am I the oldest Up member" verdict with no error + /// path of its own — the ACTIVE node always answers Healthy, so the only event this can + /// ever match is a standby reporting that it is a standby. Genuine loss of the pair + /// surfaces on the readiness checks, which are deliberately out of scope here. + /// + /// + /// Matching on rather than on the HealthStatus + /// property avoids depending on how the enum renders into the event; the level→verdict + /// mapping above already makes Error equivalent to Unhealthy. + /// + /// + /// The candidate log event. + /// when the event should be dropped. + internal static bool IsExpectedStandbyActiveNodeProbe(LogEvent logEvent) + { + if (logEvent is null || logEvent.Level != LogEventLevel.Error) + { + return false; + } + + return MatchesScalar(logEvent, "SourceContext", HealthCheckServiceSourceContext) + && MatchesScalar(logEvent, "HealthCheckName", ActiveNodeCheckName); + } + + /// Tests one structured property for an exact scalar string value. + /// The event whose properties are inspected. + /// The structured property name. + /// The value the property must equal. + /// on an exact ordinal match. + private static bool MatchesScalar(LogEvent logEvent, string propertyName, string expected) => + logEvent.Properties.TryGetValue(propertyName, out var value) + && value is ScalarValue { Value: string actual } + && string.Equals(actual, expected, StringComparison.Ordinal); + /// /// Parses a Serilog name, falling back to /// for null/blank/unrecognised values. diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StandbyHealthCheckLogNoiseTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StandbyHealthCheckLogNoiseTests.cs new file mode 100644 index 00000000..17d914d1 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StandbyHealthCheckLogNoiseTests.cs @@ -0,0 +1,152 @@ +using Microsoft.Extensions.Configuration; +using Serilog; +using Serilog.Events; + +namespace ZB.MOM.WW.ScadaBridge.Host.Tests; + +/// +/// Deferred-work register row 71 — a standby node logged the correct-by-design +/// active-node health-check verdict at ERROR on every probe. +/// +/// +/// +/// DefaultHealthCheckService picks the log level from the verdict (Debug/Warning/Error +/// for Healthy/Degraded/Unhealthy). Both active-node checks answer Unhealthy on the standby +/// because 503 is the contract /health/active publishes, so a healthy standby logged +/// ERR every 10 s on a site node and every 5 s on a central node behind Traefik's probe. +/// +/// +/// The constraint on the fix is that a GENUINE failure must still log at Error, which is why +/// the filter is keyed on the check NAME rather than on the logger category. +/// +/// +public class StandbyHealthCheckLogNoiseTests +{ + private const string HealthCheckCategory = + "Microsoft.Extensions.Diagnostics.HealthChecks.DefaultHealthCheckService"; + + private const string Template = + "Health check {HealthCheckName} with status {HealthStatus} completed after " + + "{ElapsedMilliseconds}ms with message '{HealthCheckDescription}'"; + + private static ILogger BuildLogger(InMemorySink sink) + { + var config = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["ScadaBridge:Logging:MinimumLevel"] = "Debug", + }) + .Build(); + + return LoggerConfigurationFactory + .Build(config, "Site", "site-a", "site-a-b") + .WriteTo.Sink(sink) + .CreateLogger(); + } + + /// Emits one event shaped exactly like the framework's per-check line. + private static void EmitHealthCheckLine( + ILogger logger, string category, LogEventLevel level, string checkName, string status) => + logger + .ForContext("SourceContext", category) + .Write(level, Template, checkName, status, 1.0, "some message"); + + [Fact] + public void StandbyActiveNodeVerdict_IsNotLogged() + { + // The row-71 regression: "Health check active-node with status Unhealthy … 'not the + // site pair primary (standby)'" at Error, once per probe, forever. + var sink = new InMemorySink(); + var logger = BuildLogger(sink); + + EmitHealthCheckLine( + logger, HealthCheckCategory, LogEventLevel.Error, "active-node", "Unhealthy"); + + Assert.Empty(sink.LogEvents); + } + + [Theory] + [InlineData("localdb")] + [InlineData("akka-cluster")] + [InlineData("database")] + [InlineData("required-singletons")] + public void GenuineCheckFailure_StillLogsAtError(string checkName) + { + // The constraint. The filter is name-scoped, so every OTHER check on the same logger + // category keeps its Error line — a blanket category override would have silenced + // these too, which is exactly the trap this test exists to catch. + var sink = new InMemorySink(); + var logger = BuildLogger(sink); + + EmitHealthCheckLine( + logger, HealthCheckCategory, LogEventLevel.Error, checkName, "Unhealthy"); + + var evt = Assert.Single(sink.LogEvents); + Assert.Equal(LogEventLevel.Error, evt.Level); + } + + [Fact] + public void ActiveNodeHealthyVerdict_IsUnaffected() + { + // The active node reports Healthy, which the framework logs at Debug. Nothing about + // the filter should touch it — it only ever matches Error. + var sink = new InMemorySink(); + var logger = BuildLogger(sink); + + EmitHealthCheckLine( + logger, HealthCheckCategory, LogEventLevel.Debug, "active-node", "Healthy"); + + Assert.Single(sink.LogEvents); + } + + [Fact] + public void ActiveNodeDegradedVerdict_IsUnaffected() + { + // Degraded logs at Warning. If a future check shape ever reports Degraded for + // active-node, that should still be visible — the filter is Error-only. + var sink = new InMemorySink(); + var logger = BuildLogger(sink); + + EmitHealthCheckLine( + logger, HealthCheckCategory, LogEventLevel.Warning, "active-node", "Degraded"); + + Assert.Single(sink.LogEvents); + } + + [Fact] + public void AnErrorMentioningActiveNode_FromAnotherCategory_IsNotSuppressed() + { + // The predicate requires BOTH the health-check category and the check name, so + // application code logging about the active node keeps its Error. + var sink = new InMemorySink(); + var logger = BuildLogger(sink); + + EmitHealthCheckLine( + logger, + "ZB.MOM.WW.ScadaBridge.Host.Actors.AkkaHostedService", + LogEventLevel.Error, + "active-node", + "Unhealthy"); + + Assert.Single(sink.LogEvents); + } + + [Fact] + public void OrdinaryErrorsWithNoHealthCheckProperties_AreNotSuppressed() + { + var sink = new InMemorySink(); + var logger = BuildLogger(sink); + + logger.Error("something genuinely broke"); + + Assert.Single(sink.LogEvents); + } + + [Fact] + public void Predicate_RejectsANullEvent() + { + // Defensive: Serilog never hands the filter a null, but the predicate is internal + // and reachable, and returning "suppress" for null would be the wrong default. + Assert.False(LoggerConfigurationFactory.IsExpectedStandbyActiveNodeProbe(null!)); + } +}