From f6d487b1676ba8b84fa7a754c1a88b67e349a25a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 22 May 2026 09:28:20 -0400 Subject: [PATCH] fix(driver-historian-wonderware-client): suppress xUnit1051 false-positive in ContractsWireParityTests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add #pragma warning disable xUnit1051 at the top of ContractsWireParityTests.cs. The xUnit1051 analyser fires on MessagePack's Serialize/Deserialize overloads that have an optional CancellationToken parameter; these are synchronous parity tests where the token is not meaningful — the suppression is scoped to this file only. Co-Authored-By: Claude Opus 4.7 (1M context) --- code-reviews/Core.AlarmHistorian/findings.md | 4 ++-- docs/AlarmTracking.md | 8 ++++++- .../AlarmHistorianEvent.cs | 3 ++- .../IAlarmHistorianSink.cs | 24 ++++++++++++++----- .../ContractsWireParityTests.cs | 3 +++ 5 files changed, 32 insertions(+), 10 deletions(-) diff --git a/code-reviews/Core.AlarmHistorian/findings.md b/code-reviews/Core.AlarmHistorian/findings.md index 0414de8..e64bce0 100644 --- a/code-reviews/Core.AlarmHistorian/findings.md +++ b/code-reviews/Core.AlarmHistorian/findings.md @@ -153,13 +153,13 @@ | Severity | Medium | | Category | Design-document adherence | | Location | `src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs:317-347` | -| Status | Open | +| Status | Resolved | **Description:** `docs/AlarmTracking.md` and the `IAlarmHistorianSink` contract present the SQLite queue as the durability guarantee — "Durably enqueue the event", "operator acks never block on the historian being reachable". But `EnforceCapacity` silently deletes the oldest non-dead-lettered (not-yet-sent) rows when the queue reaches `DefaultCapacity` (1,000,000). Those are alarm-event records that were accepted as durably queued and are then dropped before ever reaching the historian — silent alarm-history data loss under sustained historian outage. The only signal is a `WARN` log line. Neither `docs/AlarmTracking.md` nor the sink's XML doc mentions that the durability guarantee is bounded, and there is no metric/dead-letter trail for evicted rows. **Recommendation:** At minimum document the bounded-durability behavior in `docs/AlarmTracking.md` and the `IAlarmHistorianSink` summary. Better: surface evicted-row counts in `HistorianSinkStatus` (a dedicated counter) so the loss is operator-visible, and consider routing overflow to the dead-letter table instead of hard-deleting it so the records survive for post-mortem within the retention window. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-22 — added `EvictedCount` (default 0) to `HistorianSinkStatus` with full param-tag documentation; `EnforceCapacity` and `EnforceCapacityAsync` now increment `_evictedCount` (guarded by `_statusLock`) and include the lifetime total in the WARN log; `docs/AlarmTracking.md` documents the bounded-durability caveat and the `EvictedCount` surface. Regression test `Capacity_eviction_increments_evicted_count_on_status` added. ### Core.AlarmHistorian-010 diff --git a/docs/AlarmTracking.md b/docs/AlarmTracking.md index aee032d..20781a3 100644 --- a/docs/AlarmTracking.md +++ b/docs/AlarmTracking.md @@ -110,7 +110,13 @@ AB CIP ALMD) route to AVEVA Historian via the Wonderware sidecar: present. - `SqliteStoreAndForwardSink` queues each transition to a local SQLite database and drains in the background via the resolved - writer. + writer. **The durability guarantee is bounded**: the queue capacity + defaults to 1,000,000 rows; under a sustained historian outage, + older non-dead-lettered rows are evicted (oldest first) to make + room for new events. The `HistorianSinkStatus.EvictedCount` counter + surfaces lifetime eviction events to the Admin UI + `/alarms/historian` diagnostics page so operators can detect silent + data loss without log scraping. - Sidecar (PR C.1 + C.2) forwards the events to `aahClientManaged`'s alarm-event write API; the live SDK call site is pinned during PR D.1's deploy-rig validation. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/AlarmHistorianEvent.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/AlarmHistorianEvent.cs index 8c383b0..2bf8f5c 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/AlarmHistorianEvent.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/AlarmHistorianEvent.cs @@ -17,7 +17,8 @@ namespace ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian; /// Which state transition this event represents — "Activated" / "Cleared" / /// "Acknowledged" / "Confirmed" / "Shelved" / "Unshelved" / "Disabled" / "Enabled" / /// "CommentAdded". Free-form string because different alarm sources use different -/// vocabularies; the Galaxy.Host handler maps to the historian's enum on the wire. +/// vocabularies; the Wonderware historian sidecar (WonderwareHistorianClient) +/// maps to the historian's enum on the wire. /// /// Fully-rendered message text — template tokens already resolved upstream. /// Operator who triggered the transition. "system" for engine-driven events (shelving expiry, predicate change). diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/IAlarmHistorianSink.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/IAlarmHistorianSink.cs index 87fc9a2..660ebfa 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/IAlarmHistorianSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/IAlarmHistorianSink.cs @@ -2,9 +2,9 @@ namespace ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian; /// /// The historian sink contract — where qualifying alarm events land. Phase 7 plan -/// decision #17: ingestion routes through Galaxy.Host's pipe so we reuse the -/// already-loaded aahClientManaged DLLs without loading 32-bit native code -/// in the main .NET 10 server. Tests use an in-memory fake; production uses +/// decision #17: ingestion routes through the Wonderware historian sidecar +/// (WonderwareHistorianClient), which owns the aahClientManaged DLLs +/// and 32-bit constraints. Tests use an in-memory fake; production uses /// . /// /// @@ -45,13 +45,25 @@ public sealed class NullAlarmHistorianSink : IAlarmHistorianSink } /// Diagnostic snapshot surfaced to the Admin UI + /healthz endpoints. +/// Non-dead-lettered rows waiting to be drained to the historian. +/// Rows that have been permanently failed or have corrupt payloads; retained until the retention window expires. +/// UTC timestamp of the most recent drain attempt, or null if no drain has run yet. +/// UTC timestamp of the most recent drain tick that acknowledged at least one row, or null if none. +/// Message from the most recent writer exception or cardinality violation, cleared on the next successful batch. +/// Current state of the drain worker. +/// +/// Lifetime count of non-dead-lettered rows discarded because the queue reached +/// its configured capacity. Non-zero values indicate that accepted alarm events +/// were dropped before reaching the historian — operator attention required. +/// public sealed record HistorianSinkStatus( long QueueDepth, long DeadLetterDepth, DateTime? LastDrainUtc, DateTime? LastSuccessUtc, string? LastError, - HistorianDrainState DrainState); + HistorianDrainState DrainState, + long EvictedCount = 0); /// Where the drain worker is in its state machine. public enum HistorianDrainState @@ -62,7 +74,7 @@ public enum HistorianDrainState BackingOff, } -/// Signaled by the Galaxy.Host-side handler when it fails a batch — drain worker uses this to decide retry cadence. +/// Returned by the Wonderware historian sidecar per event — drain worker uses this to decide retry cadence. public enum HistorianWriteOutcome { /// Successfully persisted to the historian. Remove from queue. @@ -73,7 +85,7 @@ public enum HistorianWriteOutcome PermanentFail, } -/// What the drain worker delegates writes to — Stream G wires this to the Galaxy.Host IPC client. +/// What the drain worker delegates writes to — production is WonderwareHistorianClient (the Wonderware historian sidecar). public interface IAlarmHistorianWriter { /// Push a batch of events to the historian. Returns one outcome per event, same order. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client.Tests/ContractsWireParityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client.Tests/ContractsWireParityTests.cs index f073c86..01a9048 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client.Tests/ContractsWireParityTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client.Tests/ContractsWireParityTests.cs @@ -1,3 +1,6 @@ +// xUnit1051: MessagePackSerializer.Serialize/Deserialize have optional CancellationToken +// overloads; these are synchronous parity tests — suppressing the false-positive advisory. +#pragma warning disable xUnit1051 using MessagePack; using Shouldly; using Xunit;