From 39db75083da35ba2890989b940e283f89e7b9b30 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 19:23:40 -0400 Subject: [PATCH 1/4] feat(drivers): every driver reports the authored tags it skipped (Gitea #541) A driver builds its RawPath -> definition table at init and tolerates a per-tag miss: an unmappable TagConfig, an unparseable address, a capability-matrix rejection. That tolerance is right -- one bad tag must not take down the other few thousand. What was missing is any statement of the fact afterwards. A skip is not an error and happens before any read, so the driver stays Healthy, errors/5 min stays 0, Last error stays empty, and Last read advances live off the tags that did map. On the wonder Makino in #540 that reading held while focas-z-34184 ignored 20 of its 22 tags. New optional capability ISkippedTagReporter, backed by a shared SkippedTagLedger so the reset/commit/cap rules are one implementation rather than one per driver. The report -- authored count, exact skipped count, and a capped sample of RawPath + reason -- rides DriverHealthChanged to a new Tags column on /hosts, over the Phase 5 telemetry stream. No table: a driver-only node has no ConfigDb to write to, the lesson from #521. The issue named eight drivers. It is twelve -- every driver in DriverTypeNames.All has an authored-tag skip path. Calculation (no scriptId), MTConnect (no readable DataItem id), Galaxy (no attributeRef) and OpcUaClient (no nodeId) were unlisted, and OpcUaClient's was the quietest of the twelve: NamespaceMap drops such an entry with a bare `continue` and not even a log line. SkippedTagReporterFleetTests pins the invariant reflectively over every IDriver in src/Drivers/, so a thirteenth cannot re-introduce the silence. Sql drops a tag at TWO points and its report is the union of both: a blob that does not map, AND an identifier the 8.1 catalog gate rejects against the live catalog. The second is what schema drift produces -- a renamed column, a dropped view -- months after a deployment sealed green, and reporting only the first would have that driver claim "0 skipped" while polling half its tags, which is worse than reporting nothing because the column would then be actively reassuring. Deliberately additive to driver state, never a change of it. Latching non-Healthy for a config problem would satisfy both halves of the #527 comms-loss conjunction -- 90 s without a successful read AND non-Healthy -- and a driver whose tags were ALL skipped has no successful reads by construction, so it would reconnect-loop forever against a healthy device. Three details that are load-bearing rather than incidental: - The skip digest is part of the publish fingerprint. A skipped tag leaves every other fingerprint component identical to a clean driver's, so without it PublishHealthSnapshot's dedup swallows exactly the publish carrying the signal -- the trap that already bit the rediscovery and host-status channels. The digest covers the sample, not just the counts: a redeploy that fixes one tag and breaks another leaves both counts unchanged. - The count is exact; only the sample is capped, and the tooltip says so when it is showing fewer than it counted. A truncated list that looked complete would have an operator fix twenty tags and believe the job done. - The ledger stages and commits, so an init that throws part-way leaves the previous report standing. A partial snapshot would read as "we now skip fewer tags", the opposite of the truth. The proto field needs no companion presence flag, unlike host_statuses: a singular message field carries real presence in proto3, and a has_ flag would collide with protoc's generated HasSkippedTags. Verified red-first three ways: dropping SkippedTagDigest from the fingerprint fails the two dedup tests, removing the seam from one driver fails the fleet guard, and dropping the catalog-gate leg fails the Sql union test. Full solution builds with zero new warnings against 4e22f024. --- docs/drivers/README.md | 37 +++ .../Messages/Drivers/DriverHealthChanged.cs | 18 +- .../Protos/telemetry.proto | 21 ++ .../IDriverHealthPublisher.cs | 12 +- .../ISkippedTagReporter.cs | 176 ++++++++++++ .../AbCipDriver.cs | 11 +- .../AbLegacyDriver.cs | 11 +- .../CalculationDriver.cs | 13 +- .../FocasDriver.cs | 18 +- .../GalaxyDriver.cs | 25 +- .../MTConnectDriver.cs | 17 +- .../ModbusDriver.cs | 13 +- .../MqttDriver.cs | 19 +- .../OpcUaClientDriver.cs | 33 ++- .../ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs | 13 +- .../ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs | 51 +++- .../TwinCATDriver.cs | 13 +- .../Components/Pages/Hosts.razor | 27 +- .../Hosts/HostsDriverView.cs | 34 ++- .../Telemetry/TelemetryProtoMapCentral.cs | 25 +- .../Grpc/TelemetryProtoMapNode.cs | 16 ++ .../Drivers/AkkaDriverHealthPublisher.cs | 6 +- .../Drivers/DriverInstanceActor.cs | 47 ++- .../InterfaceIndependenceTests.cs | 1 + .../SkippedTagLedgerTests.cs | 210 ++++++++++++++ .../SkippedTagReporterFleetTests.cs | 92 ++++++ .../FocasSkippedTagReportingTests.cs | 170 +++++++++++ .../SqlCatalogGateDriverTests.cs | 47 +++ .../Hosts/HostsDriverViewTests.cs | 92 ++++++ .../TelemetryProtoMapCentralTests.cs | 56 ++++ .../DriverInstanceActorHostStatusTests.cs | 3 +- ...iverInstanceActorRediscoverySignalTests.cs | 3 +- .../DriverInstanceActorSkippedTagTests.cs | 269 ++++++++++++++++++ ...InstanceActorSubscriptionReconcileTests.cs | 3 +- 34 files changed, 1572 insertions(+), 30 deletions(-) create mode 100644 src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISkippedTagReporter.cs create mode 100644 tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/SkippedTagLedgerTests.cs create mode 100644 tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/SkippedTagReporterFleetTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasSkippedTagReportingTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorSkippedTagTests.cs diff --git a/docs/drivers/README.md b/docs/drivers/README.md index 68d7ceca..0a06ffff 100644 --- a/docs/drivers/README.md +++ b/docs/drivers/README.md @@ -11,6 +11,7 @@ OtOpcUa is a multi-driver OPC UA server. The Core (`ZB.MOM.WW.OtOpcUa.Core` + `C - `IAlarmSource` — driver-emitted OPC UA A&C events - `IHistoryProvider` — driver-side raw / processed / at-time / events HistoryRead (see [HistoricalDataAccess.md](../v1/HistoricalDataAccess.md)) - `IRediscoverable` — driver-initiated address-space rebuild notifications +- `ISkippedTagReporter` — authored tags the driver could not build into its poll table (see [Skipped tags](#skipped-tags) below) - `IHistorianDataSource` — server-side historian read backend registration (the HistorianGateway backend), distinct from the driver-side `IHistoryProvider` HistoryRead path Each driver opts into only the capabilities it supports. Every async capability call at the Server dispatch layer goes through `CapabilityInvoker` (`Core/Resilience/CapabilityInvoker.cs`), which wraps it in a Polly pipeline keyed on `(DriverInstanceId, HostName, DriverCapability)`. The `OTOPCUA0001` analyzer enforces the wrap at build time. Drivers themselves never depend on Polly; they just implement the capability interface and let the Core wrap it. @@ -21,6 +22,42 @@ Driver **factories** are registered at startup in `DriverFactoryRegistry` (`src/ > > **Stability tier** never came from that registry anyway: it is the `DriverTier tier = DriverTier.A` parameter on `DriverFactoryRegistry.cs:46`, and **no factory in `src/Drivers/` passes a tier**, so every shipped driver runs Tier A. The Tier-C recycle protections that [docs/v2/driver-stability.md](../v2/driver-stability.md) describes — `MemoryTracking`, `MemoryRecycle`, `ScheduledRecycleScheduler`, `IDriverSupervisor` — are **gone**, not merely dormant: they required an out-of-process driver Host that no longer exists anywhere (Galaxy is gRPC-to-`mxaccessgw`, FOCAS is in-process), `IDriverSupervisor` had no implementations, and the trackers were only ever constructed in tests. The `Tier` column below is therefore uniformly A, and the tier's remaining effect is `DriverResilienceOptions.GetTierDefaults` — the per-capability timeout / retry / breaker policy set, which **is** live. +## Skipped tags + +Every driver builds its `RawPath → definition` table at `InitializeAsync` and **tolerates a per-tag miss** — an unmappable `TagConfig`, an unparseable address, a capability-matrix rejection, a `TagConfig` naming no readable id. The tag is logged, skipped, never enters the poll table, and every read of it returns `BadNodeIdUnknown`. That tolerance is deliberate: one bad tag must not take down the other few thousand on the same device. + +**What that costs, and what closes it (Gitea [#541](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/541)).** A skip is not an error, and it happens before any read. So a driver ignoring part or all of its configuration reads, on `/hosts`: + +| Column | What it shows | Why | +|---|---|---| +| Status | `Healthy` | nothing faulted — a config miss is not a fault | +| Errors/5 min | `0` | the counter counts transitions into Faulted | +| Last error | `—` | never set | +| Last read | advancing live | the tags that *did* map are polling fine | + +On the wonder Makino in [#540](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/540) that reading held for weeks while `focas-z-34184` ignored **20 of its 22 tags**. + +Each driver therefore implements `ISkippedTagReporter`, accumulating into the shared `SkippedTagLedger` during its tag-table build. The report — authored count, exact skipped count, and a capped sample of `RawPath` + reason — rides `DriverHealthChanged` to the **Tags** column on `/hosts`, alongside the per-host connectivity of [#521](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/521) and by the same transport (the Phase 5 telemetry stream, **not** a table — a driver-only node has no ConfigDb to write to). + +Reading the Tags column: + +| Cell | Meaning | +|---|---| +| `—` | the driver cannot report skips. **Not** the same as reporting none. | +| `12 tags` | the driver reports, and skipped nothing. | +| `20 / 22 skipped` (amber) | partly working — some tags poll, some do not. | +| `22 / 22 skipped` (red) | polling **nothing**. The state most likely to look healthy, because it has no failing reads to report. | + +Three properties worth knowing: + +- **A skip never changes driver state.** Latching non-Healthy for a config problem would satisfy both halves of the [#527](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/527) comms-loss conjunction — *90 s without a successful read AND non-Healthy* — and a driver whose tags were all skipped has no successful reads by construction, so it would reconnect-loop forever against a device that is perfectly fine. The signal is additive to the state, not a change of it. +- **The count is exact; only the sample is capped** (`SkippedTagLedger.MaxSample`). The tooltip says explicitly when it is showing fewer than it counted — a truncated list that looked complete would have an operator fix twenty tags and believe the job done. +- **The report is replaced per build, never accumulated**, and an init that throws part-way leaves the previous report standing rather than publishing the partial one it reached. A truncated snapshot would read as *"we now skip fewer tags"*, the opposite of the truth. + +**Sql is the one driver that drops a tag at two points**, and its report is the union: a `TagConfig` that does not map (`BuildTagTable`) *and* an identifier the §8.1 catalog gate rejects against the live catalog (`ApplyCatalogGateAsync`). The second is what schema drift produces — a renamed column, a dropped view — months after a deployment sealed green, and reporting only the first would have the driver claim `0 skipped` while polling half its tags. In both cases the tag keeps its node and publishes `BadNodeIdUnknown`; it is only absent from the *polled* table. + +`SkippedTagReporterFleetTests` pins the invariant reflectively over every `IDriver` in `src/Drivers/`, so a thirteenth driver cannot re-introduce the silence. + ## Ground-truth driver list | Driver | Project path | Tier | Wire / library | Capabilities | Notable quirk | diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Drivers/DriverHealthChanged.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Drivers/DriverHealthChanged.cs index 4c7eb74e..bf239d44 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Drivers/DriverHealthChanged.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Drivers/DriverHealthChanged.cs @@ -44,6 +44,21 @@ namespace ZB.MOM.WW.OtOpcUa.Commons.Messages.Drivers; /// Nothing dedups on record equality — DriverInstanceActor keeps its own flattened fingerprint /// precisely because of this — but do not introduce such a comparison without fixing it here first. /// +/// +/// What this driver skipped when it last built its tag table, from +/// ISkippedTagReporter.GetSkippedTags(); null when the driver does not implement that +/// capability. All twelve production drivers do. +/// Why this is on the health snapshot (Gitea #541). A skip happens at init, is not an +/// error, and does not stop the driver's OTHER tags from reading — so State stays Healthy, +/// ErrorCount5Min stays 0, LastError stays null and LastSuccessfulReadUtc keeps +/// advancing while the driver ignores part or all of its configuration. On the Makino in Gitea #540 +/// that combination held while 20 of 22 tags were ignored. This field is what contradicts it. +/// Additive, never a state change. See ISkippedTagReporter for why a skipped tag +/// must not latch the driver non-Healthy — it would satisfy both halves of the #527 comms-loss +/// conjunction and reconnect-loop a healthy device. +/// Value-equality caveat — the same one carries: the +/// record's generated Equals compares the sample list by REFERENCE. +/// public sealed record DriverHealthChanged( string ClusterId, string DriverInstanceId, @@ -54,7 +69,8 @@ public sealed record DriverHealthChanged( DateTime PublishedUtc, DateTime? RediscoveryNeededUtc = null, string? RediscoveryReason = null, - IReadOnlyList? HostStatuses = null) + IReadOnlyList? HostStatuses = null, + SkippedTagReport? SkippedTags = null) { /// /// DPS topic name. Both the runtime AkkaDriverHealthPublisher and the AdminUI diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Protos/telemetry.proto b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Protos/telemetry.proto index a683b6cd..a5529c2f 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Protos/telemetry.proto +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Protos/telemetry.proto @@ -76,6 +76,27 @@ message DriverHealth { // than letting an empty list silently claim the driver has no probe. bool has_host_statuses = 10; repeated HostConnectivity host_statuses = 11; + // Authored tags the driver could not build into its poll table (Gitea #541). The same two facts + // host_statuses needs a flag for — "the driver cannot report skips" vs "it reports none" — are + // distinguished here by the field's own presence: a singular MESSAGE field has real presence in + // proto3, which a repeated field does not. So no companion has_ flag (and one would collide: protoc + // already generates a HasSkippedTags property for this field). + SkippedTags skipped_tags = 12; +} + +// Mirrors ZB.MOM.WW.OtOpcUa.Core.Abstractions.SkippedTagReport. +message SkippedTags { + int32 authored_count = 1; + // The TRUE total, which may exceed sample.length — the detail list is capped so a deployment that + // misconfigures thousands of tags cannot inflate a message published on every state change. + int32 skipped_count = 2; + repeated SkippedTag sample = 3; +} + +// Mirrors ZB.MOM.WW.OtOpcUa.Core.Abstractions.SkippedTag. +message SkippedTag { + string raw_path = 1; + string reason = 2; } // Mirrors ZB.MOM.WW.OtOpcUa.Core.Abstractions.HostConnectivityStatus. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverHealthPublisher.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverHealthPublisher.cs index 265d1f4f..c1edf53a 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverHealthPublisher.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverHealthPublisher.cs @@ -28,6 +28,12 @@ public interface IDriverHealthPublisher /// AbLegacy instance owning several PLCs) surface ONE unreachable device that would otherwise be /// invisible behind an aggregate-Healthy driver row. /// + /// + /// Authored tags the driver could not build into its poll table, from + /// ; null when the driver is not an + /// . Contradicts the "Healthy, 0 errors, reads advancing" reading + /// of a driver that is in fact ignoring part or all of its configuration (Gitea #541). + /// void Publish( string clusterId, string driverInstanceId, @@ -35,7 +41,8 @@ public interface IDriverHealthPublisher int errorCount5Min, DateTime? rediscoveryNeededUtc = null, string? rediscoveryReason = null, - IReadOnlyList? hostStatuses = null); + IReadOnlyList? hostStatuses = null, + SkippedTagReport? skippedTags = null); } /// @@ -57,6 +64,7 @@ public sealed class NullDriverHealthPublisher : IDriverHealthPublisher int errorCount5Min, DateTime? rediscoveryNeededUtc = null, string? rediscoveryReason = null, - IReadOnlyList? hostStatuses = null) + IReadOnlyList? hostStatuses = null, + SkippedTagReport? skippedTags = null) { /* no-op */ } } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISkippedTagReporter.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISkippedTagReporter.cs new file mode 100644 index 00000000..dec8a980 --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISkippedTagReporter.cs @@ -0,0 +1,176 @@ +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +/// +/// Optional driver capability: report the authored tags this driver decided, at initialise time, +/// that it cannot poll — so an operator can see that a driver reporting Healthy is in fact +/// ignoring some or all of its configuration. +/// +/// +/// +/// Why this exists (Gitea #541). Every driver builds its RawPath → definition +/// table at initialise and tolerates a per-tag miss — an unmappable TagConfig, an +/// unparseable address, a capability-matrix rejection. Tolerance is the right call: one bad tag +/// must not take down the other few thousand. But a skip is not an error and happens before any +/// read, so the driver stays Healthy, errors/5 min stays 0, Last error stays +/// empty, and Last read advances live off the tags that did map. On the Makino in +/// Gitea #540 that combination held while the driver ignored 20 of its 22 tags, and no surface +/// anywhere said otherwise. +/// +/// +/// Deliberately additive to driver state, never a change of it. Latching a driver +/// non-Healthy over a config problem would collide with the Gitea #527 comms-loss detector, +/// which trips on 90 s without a successful read AND non-Healthy. A driver whose tags +/// were all skipped has no successful reads by construction, so it would satisfy both +/// halves and reconnect-loop forever against a device that is perfectly fine. The signal +/// therefore rides alongside the state. +/// +/// +/// The count is not the whole story — the denominator is. "20 skipped" is unreadable +/// without knowing whether that is 20 of 22 or 20 of 20 000, which is why +/// travels with it. +/// +/// +/// Implementations should accumulate into a rather than +/// hand-rolling a counter: the ledger is one implementation of the reset/commit/cap rules +/// instead of one per driver, which is precisely the duplication #541 was filed about. +/// +/// +public interface ISkippedTagReporter +{ + /// + /// Snapshot of what this driver skipped on its most recent tag-table build. + /// + /// + /// Must be a pure in-memory read with no I/O — it is called on the health-publish path, is + /// deliberately NOT routed through the resilience pipeline, and must never block. + /// + /// The current report; never null. An all-zero report means "nothing skipped". + SkippedTagReport GetSkippedTags(); +} + +/// One authored tag a driver could not build into its poll table, and why. +/// The authored raw path, as it appears in the deployment artifact and the AdminUI. +/// +/// Operator-facing explanation, e.g. "invalid Address 'axis:5'". Should name the offending +/// value where there is one — the operator's next move is to go and fix that value. +/// +public sealed record SkippedTag(string RawPath, string Reason); + +/// +/// What a driver skipped when it last built its tag table. +/// +/// How many raw tags the deployment authored for this driver. +/// +/// How many of them were skipped — the TRUE total, which may exceed 's +/// length. A misconfigured deployment can skip thousands of tags, and this record rides a +/// pub/sub message published on every driver state change. +/// +/// +/// The first skips, in encounter order — enough for an +/// operator to recognise the pattern and go fix it. Empty when nothing was skipped. +/// +public sealed record SkippedTagReport(int AuthoredCount, int SkippedCount, IReadOnlyList Sample) +{ + /// A driver that has authored nothing and skipped nothing. + public static readonly SkippedTagReport Empty = new(0, 0, []); + + /// Whether any authored tag was skipped — i.e. whether this is worth an operator's attention. + public bool HasSkips => SkippedCount > 0; + + /// + /// Whether the driver skipped every tag it was given. Worth distinguishing: such a driver polls + /// nothing at all, yet is the MOST likely to look healthy, because it has no failing reads to + /// report — it has no reads. + /// + public bool SkippedEverything => AuthoredCount > 0 && SkippedCount >= AuthoredCount; +} + +/// +/// Accumulator behind . Drivers stage skips while rebuilding their +/// tag table and commit the result in one step, so a health publish landing mid-rebuild reads the +/// previous complete answer rather than a half-built one. +/// +/// +/// +/// The build/swap shape mirrors what the tag tables themselves already do (SqlDriver +/// builds a new dictionary and Volatile.Writes it in). The reason is the same: a rebuild +/// that throws half-way must leave the previous snapshot standing rather than publish a +/// truncated one that would read as "we now skip fewer tags". +/// +/// +/// Reads and writes are lock-guarded. This is a cold path — one build per initialise, one read +/// per health publish — so contention is irrelevant and correctness under the concurrent +/// actor-thread read is not. +/// +/// +public sealed class SkippedTagLedger +{ + /// + /// How many individual skips are carried in . The count is + /// always exact; only the detail list is capped, so a 10 000-tag misconfiguration cannot inflate + /// a telemetry message that is published on every state change. + /// + public const int MaxSample = 20; + + private readonly object _gate = new(); + private SkippedTagReport _visible = SkippedTagReport.Empty; + + private int _stagedAuthored; + private int _stagedSkipped; + private List? _stagedSample; + + /// + /// Starts staging a new tag-table build. The visible snapshot is left alone until + /// , so an abandoned build cannot publish a partial result. + /// + /// How many raw tags this build was handed. + public void BeginRebuild(int authoredCount) + { + lock (_gate) + { + _stagedAuthored = authoredCount; + _stagedSkipped = 0; + _stagedSample = new List(Math.Min(authoredCount, MaxSample)); + } + } + + /// + /// Records one skipped tag against the in-progress build. A call outside a + /// / pair is ignored rather than throwing: this + /// runs inside a driver's initialise, where a bookkeeping exception would fail the very init + /// the ledger exists to describe. + /// + /// The authored raw path that was skipped. + /// Operator-facing reason; should name the offending value. + public void Skip(string rawPath, string reason) + { + lock (_gate) + { + if (_stagedSample is null) return; + _stagedSkipped++; + if (_stagedSample.Count < MaxSample) + _stagedSample.Add(new SkippedTag(rawPath ?? "", reason ?? "")); + } + } + + /// + /// Publishes the staged build as the visible snapshot, atomically. A call with no staged build + /// is a no-op. + /// + public void Commit() + { + lock (_gate) + { + if (_stagedSample is null) return; + _visible = new SkippedTagReport(_stagedAuthored, _stagedSkipped, _stagedSample); + _stagedSample = null; + } + } + + /// The current published snapshot. + /// The last committed report, or before the first commit. + public SkippedTagReport Snapshot() + { + lock (_gate) return _visible; + } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index 2cd918ae..3ee99b70 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -25,8 +25,14 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; /// handles GC-collected tags. /// public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, ISubscribable, - IHostConnectivityProbe, IPerCallHostResolver, IAlarmSource, IDisposable, IAsyncDisposable + IHostConnectivityProbe, IPerCallHostResolver, IAlarmSource, ISkippedTagReporter, IDisposable, IAsyncDisposable { + /// Authored tags this driver could not build into its poll table (Gitea #541). + private readonly SkippedTagLedger _skippedTags = new(); + + /// + public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + private AbCipDriverOptions _options; private readonly string _driverInstanceId; private readonly IAbCipTagFactory _tagFactory; @@ -284,10 +290,12 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, // is mapped by the pure factory; the entry's DeviceName (routing key) + WriteIdempotent flag are // threaded onto the def (they live on the RawTagEntry, not inside the blob). A mapper miss is // skipped (logged), never thrown — a bad tag must not fail the whole driver init. + _skippedTags.BeginRebuild(_options.RawTags.Count); foreach (var entry in _options.RawTags) { if (!AbCipTagDefinitionFactory.FromTagConfig(entry.TagConfig, entry.RawPath, out var mapped)) { + _skippedTags.Skip(entry.RawPath, "TagConfig did not map to a definition"); _logger.LogWarning( "AbCip tag config did not map to a definition; skipping. Driver={DriverInstanceId} RawPath={RawPath}", _driverInstanceId, entry.RawPath); @@ -335,6 +343,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } } } + _skippedTags.Commit(); // Probe loops — one per device when enabled + a ProbeTagPath is configured. if (_options.Probe.Enabled && !string.IsNullOrWhiteSpace(_options.Probe.ProbeTagPath)) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs index 65b2d068..ea28176b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs @@ -11,8 +11,14 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; /// host-resolver capabilities ship in PRs 2 and 3. /// public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscovery, ISubscribable, - IHostConnectivityProbe, IPerCallHostResolver, IDisposable, IAsyncDisposable + IHostConnectivityProbe, IPerCallHostResolver, ISkippedTagReporter, IDisposable, IAsyncDisposable { + /// Authored tags this driver could not build into its poll table (Gitea #541). + private readonly SkippedTagLedger _skippedTags = new(); + + /// + public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + /// Mutable so can adopt a re-parsed config on reinitialize /// (#516). Only ever written on the init path, before any reader/session uses it. private AbLegacyDriverOptions _options; @@ -131,10 +137,12 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover // def (it lives on the RawTagEntry, not inside the blob), and its DeviceName is resolved to // the owning device's host so the existing DeviceHostAddress-keyed routing continues to work. // A mapper miss is skipped (logged), never thrown — a bad tag must not fail the whole init. + _skippedTags.BeginRebuild(_options.RawTags.Count); foreach (var entry in _options.RawTags) { if (!AbLegacyTagDefinitionFactory.FromTagConfig(entry.TagConfig, entry.RawPath, out var def)) { + _skippedTags.Skip(entry.RawPath, "TagConfig did not map to a definition"); _logger.LogWarning( "AbLegacy tag config did not map to a definition; skipping. Driver={DriverInstanceId} RawPath={RawPath}", _driverInstanceId, entry.RawPath); @@ -146,6 +154,7 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover DeviceHostAddress = ResolveDeviceHost(entry.DeviceName), }; } + _skippedTags.Commit(); // Validate tag types against their device's family profile. Long (32-bit integer) // and String (ST-file) are not supported by all PCCC families; reject them early diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriver.cs index 61e505d1..92f0cc14 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriver.cs @@ -35,8 +35,15 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Calculation; /// A historized Bad records BadInternalError. /// /// -public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscribable, IReadable, IDisposable +public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscribable, IReadable, ISkippedTagReporter, IDisposable { + /// Authored tags this driver could not build into its calc table (Gitea #541). A raw tag with + /// no scriptId is not a calc tag and computes nothing. + private readonly SkippedTagLedger _skippedTags = new(); + + /// + public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + // OPC UA status codes surfaced by the calc driver. private const uint StatusGood = 0u; private const uint StatusBadWaitingForInitialData = 0x80320000u; @@ -120,10 +127,12 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba // Retain surviving tags' state; anything not re-authored is dropped when we swap this in below. var retainedState = new Dictionary(StringComparer.Ordinal); + _skippedTags.BeginRebuild(config.RawTags.Count); foreach (var entry in config.RawTags) { if (!CalculationTagDefinition.TryFromTagConfig(entry.TagConfig, entry.RawPath, out var def)) { + _skippedTags.Skip(entry.RawPath, "no scriptId — not a calc tag"); _logger.LogWarning( "Calculation {Driver}: raw tag {RawPath} has no scriptId; skipping (not a calc tag)", _driverInstanceId, entry.RawPath); @@ -151,6 +160,8 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba } } + _skippedTags.Commit(); + // Swap in the retained/new state map — a removed tag's state is dropped so it stops publishing. _stateByRawPath.Clear(); foreach (var (rawPath, state) in retainedState) _stateByRawPath[rawPath] = state; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs index 898d5a6d..0dc16118 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -19,8 +19,16 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS; /// fail fast. /// public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, ISubscribable, - IHostConnectivityProbe, IPerCallHostResolver, IAlarmSource, IDisposable, IAsyncDisposable + IHostConnectivityProbe, IPerCallHostResolver, IAlarmSource, ISkippedTagReporter, IDisposable, IAsyncDisposable { + /// Authored tags this driver could not build into its poll table (Gitea #541). FOCAS has three + /// skip paths — an unmappable TagConfig, an unparseable address, and a capability-matrix rejection — and + /// #540 was caused by the second of them going unreported while the driver read Healthy. + private readonly SkippedTagLedger _skippedTags = new(); + + /// + public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + /// Mutable so can adopt a re-parsed config (#516). Written only /// on the init path, together with , before either is used. private FocasDriverOptions _options; @@ -190,10 +198,12 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, // bad tag must not fail the whole driver init. The ONE hard failure is an unknown device // segment: a tag routed to a device not in the Devices list is a driver-level config error // that fails fast (Driver.FOCAS-003). + _skippedTags.BeginRebuild(_options.RawTags.Count); foreach (var entry in _options.RawTags) { if (!FocasTagDefinitionFactory.FromTagConfig(entry.TagConfig, entry.RawPath, out var def)) { + _skippedTags.Skip(entry.RawPath, "TagConfig did not map to a definition"); _logger.LogWarning( "FOCAS tag config did not map to a definition; skipping. Driver={DriverInstanceId} RawPath={RawPath}", _driverInstanceId, entry.RawPath); @@ -217,6 +227,7 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, var parsed = FocasAddress.TryParse(def.Address); if (parsed is null) { + _skippedTags.Skip(entry.RawPath, $"invalid Address '{def.Address}'"); _logger.LogWarning( "FOCAS tag has invalid Address '{Address}'; skipping. Driver={DriverInstanceId} RawPath={RawPath}", def.Address, _driverInstanceId, entry.RawPath); @@ -224,6 +235,7 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, } if (FocasCapabilityMatrix.Validate(device.Options.Series, parsed) is { } reason) { + _skippedTags.Skip(entry.RawPath, $"'{def.Address}' rejected by capability matrix: {reason}"); _logger.LogWarning( "FOCAS tag '{RawPath}' ({Address}) rejected by capability matrix: {Reason}; skipping. Driver={DriverInstanceId}", entry.RawPath, def.Address, reason, _driverInstanceId); @@ -239,6 +251,10 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, // above; reusing the parsed record avoids per-tick allocs on subscription pollers. _parsedAddressesByTagName[entry.RawPath] = parsed; } + // Publish the skip tally only once the whole table is built. An unknown-device throw above + // leaves the previous snapshot standing rather than a truncated one that would read as + // "we now skip fewer tags". + _skippedTags.Commit(); if (_options.Probe.Enabled) { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs index dd9e7225..b1366ea2 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs @@ -38,8 +38,15 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy; /// /// public sealed class GalaxyDriver - : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IRediscoverable, IHostConnectivityProbe, IAlarmSource, IDisposable, IAsyncDisposable + : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IRediscoverable, IHostConnectivityProbe, IAlarmSource, ISkippedTagReporter, IDisposable, IAsyncDisposable { + /// Authored tags this driver could not dial (Gitea #541) — a TagConfig with no usable + /// attributeRef has no MXAccess address to bind. + private readonly SkippedTagLedger _skippedTags = new(); + + /// + public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + private readonly string _driverInstanceId; private readonly GalaxyDriverOptions _options; private readonly ILogger _logger; @@ -215,7 +222,7 @@ public sealed class GalaxyDriver // Build the v3 RawPath ⇄ attributeRef tables from the authored raw tags. Empty when no tags // are authored (live-browse path), leaving every translation an identity no-op. - (_attributeRefByRawPath, _rawTagByAttributeRef) = BuildRawTagTables(_options.RawTags, _logger); + (_attributeRefByRawPath, _rawTagByAttributeRef) = BuildRawTagTables(_options.RawTags, _logger, _skippedTags); // Forward the aggregator's transitions through IHostConnectivityProbe. _hostStatuses.OnHostStatusChanged += (_, args) => OnHostStatusChanged?.Invoke(this, args); @@ -231,16 +238,25 @@ public sealed class GalaxyDriver /// still dials both. Case-insensitive to match the driver's reference handling. /// private static (Dictionary Forward, Dictionary Reverse) - BuildRawTagTables(IReadOnlyList rawTags, ILogger logger) + BuildRawTagTables(IReadOnlyList rawTags, ILogger logger, SkippedTagLedger skipped) { var forward = new Dictionary(StringComparer.OrdinalIgnoreCase); var reverse = new Dictionary(StringComparer.OrdinalIgnoreCase); + skipped.BeginRebuild(rawTags.Count); foreach (var entry in rawTags) { - if (string.IsNullOrEmpty(entry.RawPath)) continue; + if (string.IsNullOrEmpty(entry.RawPath)) + { + // Counted, though there is no path to name it by: an artifact carrying such an entry + // is malformed, and reporting "N authored, N-1 bound" with nothing accounting for the + // difference is exactly the silence #541 is about. + skipped.Skip("", "entry has no RawPath"); + continue; + } var attributeRef = TryReadAttributeRef(entry.TagConfig); if (string.IsNullOrEmpty(attributeRef)) { + skipped.Skip(entry.RawPath, "TagConfig has no 'attributeRef' — cannot dial MXAccess"); logger.LogWarning( "GalaxyDriver raw tag {RawPath} has no 'attributeRef' in its TagConfig — skipped (cannot dial MXAccess).", entry.RawPath); @@ -249,6 +265,7 @@ public sealed class GalaxyDriver forward[entry.RawPath] = attributeRef; reverse.TryAdd(attributeRef, entry); // first-wins on a shared attributeRef } + skipped.Commit(); return (forward, reverse); } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs index 1a2e8285..bf313b20 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -72,8 +72,15 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; /// /// public sealed class MTConnectDriver - : IDriver, IReadable, IWritable, ISubscribable, ITagDiscovery, IHostConnectivityProbe, IRediscoverable + : IDriver, IReadable, IWritable, ISubscribable, ITagDiscovery, IHostConnectivityProbe, IRediscoverable, + ISkippedTagReporter { + /// Authored tags this driver could not bind to an Agent DataItem (Gitea #541). + private readonly SkippedTagLedger _skippedTags = new(); + + /// + public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + /// /// The Agent could not be reached, answered unusably, or blew the per-call deadline. Distinct /// from the index's BadNoCommunication, which means the Agent answered and said the @@ -3221,10 +3228,13 @@ public sealed class MTConnectDriver definitions[tag.FullName] = tag; } + _skippedTags.BeginRebuild(options.RawTags.Count); + if (options.RawTags.Count == 0) { MTConnectTagDefinition[] only = [.. definitions.Values]; + _skippedTags.Commit(); return new TagBinding( FrozenDictionary.Empty, FrozenDictionary>.Empty, @@ -3242,6 +3252,10 @@ public sealed class MTConnectDriver if (!TryReadRawTag(entry, out var dataItemId, out var authored)) { + _skippedTags.Skip( + entry.RawPath, + "names no readable DataItem id (expected 'fullName', 'dataItemId' or 'address') " + + "or declares an unrecognised driverDataType"); _logger.LogWarning( "MTConnect driver {DriverInstanceId} could not map the raw tag {RawPath} to an Agent DataItem; it names no readable id (expected 'fullName', 'dataItemId' or 'address') or declares an unrecognised driverDataType. The tag is skipped and reports BadNodeIdUnknown; every other tag is unaffected.", _driverInstanceId, entry.RawPath); @@ -3277,6 +3291,7 @@ public sealed class MTConnectDriver MTConnectTagDefinition[] indexTags = [.. definitions.Values]; + _skippedTags.Commit(); return new TagBinding( idByRawPath.ToFrozenDictionary(StringComparer.Ordinal), rawPathsById.ToFrozenDictionary( diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs index 08b845cd..9f1153f6 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs @@ -19,8 +19,14 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus; /// since Modbus has no native push model. /// public sealed class ModbusDriver - : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IHostConnectivityProbe, IPerCallHostResolver, IDisposable, IAsyncDisposable + : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IHostConnectivityProbe, IPerCallHostResolver, ISkippedTagReporter, IDisposable, IAsyncDisposable { + /// Authored tags this driver could not build into its poll table (Gitea #541). + private readonly SkippedTagLedger _skippedTags = new(); + + /// + public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + // ---- instance fields (grouped at top for auditability) ---- /// Mutable so can adopt a re-parsed config on reinitialize @@ -219,15 +225,20 @@ public sealed class ModbusDriver // blob is mapped by the pure factory; the entry's WriteIdempotent flag is threaded onto the // def (it lives on the RawTagEntry, not inside the blob). A mapper miss is skipped (logged), // never thrown — a bad tag must not fail the whole driver init. + _skippedTags.BeginRebuild(_options.RawTags.Count); foreach (var entry in _options.RawTags) { if (ModbusTagDefinitionFactory.FromTagConfig(entry.TagConfig, entry.RawPath, out var def)) _tagsByRawPath[entry.RawPath] = def with { WriteIdempotent = entry.WriteIdempotent }; else + { + _skippedTags.Skip(entry.RawPath, "TagConfig did not map to a definition"); _logger.LogWarning( "Modbus tag config did not map to a definition; skipping. Driver={DriverInstanceId} RawPath={RawPath}", _driverInstanceId, entry.RawPath); + } } + _skippedTags.Commit(); WriteHealth(new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null)); // PR 23: kick off the probe loop once the transport is up. Initial state stays diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs index 7c17e2fc..f1fee36c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs @@ -64,8 +64,14 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt; /// public sealed class MqttDriver : IDriver, ITagDiscovery, ISubscribable, IReadable, IWritable, IHostConnectivityProbe, IRediscoverable, - IAsyncDisposable, IDisposable + ISkippedTagReporter, IAsyncDisposable, IDisposable { + /// Authored tags this driver could not build into its ingest table (Gitea #541). + private readonly SkippedTagLedger _skippedTags = new(); + + /// + public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + // ---- Write status codes (Gitea #508) ---- // // NAMED constants, never inline literals: StatusCodeParityTests checks each against the pinned SDK @@ -1149,6 +1155,17 @@ public sealed class MqttDriver } } + // Gitea #541. Derived from the driver's OWN resolution path rather than from either manager's + // internal bookkeeping, so what is reported skipped is exactly what read/subscribe will fail to + // find — the two cannot drift, and one branch covers both ingest paths. + _skippedTags.BeginRebuild(options.RawTags.Count); + foreach (var entry in options.RawTags) + { + if (!TryResolve(entry.RawPath, out _)) + _skippedTags.Skip(entry.RawPath, "TagConfig did not map to a definition"); + } + _skippedTags.Commit(); + if (mapped != options.RawTags.Count) { _logger?.LogWarning( diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs index 10730d77..28639053 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -29,8 +29,37 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; /// monitored-item handles. That mechanic lands in PR 69. /// /// -public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IHostConnectivityProbe, IAlarmSource, IHistoryProvider, IDisposable, IAsyncDisposable +public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IHostConnectivityProbe, IAlarmSource, IHistoryProvider, ISkippedTagReporter, IDisposable, IAsyncDisposable { + /// Authored tags whose TagConfig carried no usable nodeId, so there is no upstream node + /// to dial (Gitea #541). This driver's skip path was the quietest of the twelve: NamespaceMap + /// drops such an entry with a bare continue and not even a log line. + private readonly SkippedTagLedger _skippedTags = new(); + + /// + public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + + /// + /// Recomputes the skip report from the freshly-built map. Derived by re-asking the map to resolve + /// each authored RawPath rather than by instrumenting NamespaceMap, so what is reported + /// skipped is exactly what read/write/subscribe will fail to resolve — and the shared Contracts + /// type stays free of driver bookkeeping. + /// + /// The map just built from the live session. + private void RecordSkippedTags(NamespaceMap map) + { + var authored = _options.RawTags; + _skippedTags.BeginRebuild(authored.Count); + foreach (var entry in authored) + { + if (string.IsNullOrWhiteSpace(entry.RawPath)) + _skippedTags.Skip("", "entry has no RawPath"); + else if (!map.TryResolve(entry.RawPath, out _)) + _skippedTags.Skip(entry.RawPath, "TagConfig has no usable 'nodeId' — no upstream node to dial"); + } + _skippedTags.Commit(); + } + private readonly ILogger _logger; /// Initializes a new instance of the class. @@ -241,6 +270,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // ingests the authored RawTags so read/write/subscribe/history references (RawPaths) // resolve to their upstream node ids. _namespaceMap = NamespaceMap.FromSession(session, _options.RawTags); + RecordSkippedTags(_namespaceMap); Session = session; _connectedEndpointUrl = connectedUrl; @@ -2035,6 +2065,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // address space re-resolve correctly against this fresh map. v3: re-ingest the // authored RawTags so RawPath resolution survives the reconnect too. _namespaceMap = NamespaceMap.FromSession(newSession, _options.RawTags); + RecordSkippedTags(_namespaceMap); TransitionTo(HostState.Running); _health = new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null); return; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs index 02c3e8d7..89c7a6c8 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs @@ -30,8 +30,14 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.S7; /// /// public sealed class S7Driver - : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IHostConnectivityProbe, IDisposable, IAsyncDisposable + : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IHostConnectivityProbe, ISkippedTagReporter, IDisposable, IAsyncDisposable { + /// Authored tags this driver could not build into its poll table (Gitea #541). + private readonly SkippedTagLedger _skippedTags = new(); + + /// + public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + private readonly string _driverInstanceId; private readonly ILogger _logger; private readonly IS7PlcFactory _plcFactory; @@ -185,15 +191,20 @@ public sealed class S7Driver _tagsByRawPath.Clear(); _parsedByRawPath.Clear(); _resolver.Clear(); // no-op in v3 (the resolver reads the live table by closure); kept for symmetry + _skippedTags.BeginRebuild(_options.RawTags.Count); foreach (var entry in _options.RawTags) { if (S7TagDefinitionFactory.FromTagConfig(entry.TagConfig, entry.RawPath, out var def)) _tagsByRawPath[entry.RawPath] = def with { WriteIdempotent = entry.WriteIdempotent }; else + { + _skippedTags.Skip(entry.RawPath, "TagConfig did not map to a definition"); _logger.LogWarning( "S7 tag config did not map to a definition; skipping. Driver={DriverInstanceId} RawPath={RawPath}", _driverInstanceId, entry.RawPath); + } } + _skippedTags.Commit(); // Phase 4d combined config guard. Timer/Counter addresses and the wide/structured // types (Int64/UInt64/Float64/String/DateTime) are now SUPPORTED, but only in diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs index f837285d..2058d4cb 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs @@ -27,8 +27,41 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; /// manufacture an exception to force backoff. /// public sealed class SqlDriver - : IDriver, ITagDiscovery, IReadable, ISubscribable, IHostConnectivityProbe, IAsyncDisposable + : IDriver, ITagDiscovery, IReadable, ISubscribable, IHostConnectivityProbe, ISkippedTagReporter, IAsyncDisposable { + /// Authored tags this driver could not build into its poll table (Gitea #541). + private readonly SkippedTagLedger _skippedTags = new(); + + /// + /// Blob-level skips from the most recent , retained because Sql drops a + /// tag at TWO points and the report must be the union of both. + /// + /// + /// A tag can fail to map from its blob () OR be rejected against the live + /// catalog () — a renamed column, a dropped view. Both leave the + /// tag out of the polled table publishing BadNodeIdUnknown with the driver Healthy, so both + /// are skips. Reporting only the first would make a Sql driver that lost half its tags to schema + /// drift claim "0 skipped", which is worse than saying nothing at all. + /// + private IReadOnlyList _blobSkips = []; + + /// + public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + + /// + /// Republishes the skip ledger as the union of the blob-level skips and the catalog gate's + /// rejections, against the true authored denominator (RawTags.Count, not the post-mapping + /// table size — a tag that failed to map is still a tag the operator authored). + /// + /// Catalog-gate rejections; empty before the gate has run. + private void PublishSkipLedger(IEnumerable gateRejections) + { + _skippedTags.BeginRebuild(_options.RawTags.Count); + foreach (var skip in _blobSkips) _skippedTags.Skip(skip.RawPath, skip.Reason); + foreach (var skip in gateRejections) _skippedTags.Skip(skip.RawPath, skip.Reason); + _skippedTags.Commit(); + } + /// /// The driver-type string this driver registers under — , the single /// source of truth the deploy pipeline stores in DriverInstance.DriverType and every dispatch @@ -699,6 +732,7 @@ public sealed class SqlDriver private void BuildTagTable() { var table = new Dictionary(_options.RawTags.Count, StringComparer.Ordinal); + var blobSkips = new List(); foreach (var entry in _options.RawTags) { try @@ -706,9 +740,12 @@ public sealed class SqlDriver if (SqlEquipmentTagParser.TryParse(entry.TagConfig, entry.RawPath, out var definition)) table[entry.RawPath] = definition; else + { + blobSkips.Add(new SkippedTag(entry.RawPath, "TagConfig did not map to a definition")); _logger.LogWarning( "Sql tag config did not map to a definition; skipping. Driver={DriverInstanceId} RawPath={RawPath}", _driverInstanceId, entry.RawPath); + } } catch (Exception ex) { @@ -717,12 +754,19 @@ public sealed class SqlDriver // strand health at Initializing across every DriverInstanceActor retry. Skip the offending // tag — the same "one bad tag must not take the whole driver down" rule the malformed-blob // branch above follows — rather than fault the driver over a single entry. + blobSkips.Add(new SkippedTag(entry.RawPath, $"TagConfig threw while parsing: {ex.GetType().Name}")); _logger.LogError( ex, "Sql tag config threw while parsing; skipping. Driver={DriverInstanceId} RawPath={RawPath}", _driverInstanceId, entry.RawPath); } } + // The gate has not run yet, so the report covers blob-level skips only; ApplyCatalogGateAsync + // republishes it with its own rejections folded in. Publishing here rather than waiting means a + // driver whose gate never runs (nothing authored, or the gate threw) still reports what it knows. + _blobSkips = blobSkips; + PublishSkipLedger([]); + Volatile.Write(ref _tagsByRawPath, table); // Nothing is pollable until the catalog gate has validated it. Clearing here (rather than leaving a @@ -775,6 +819,11 @@ public sealed class SqlDriver var result = SqlCatalogGate.Apply(authored.Values, catalog, _dialect); + // Gitea #541: a gate rejection leaves the tag unpolled and publishing BadNodeIdUnknown with the + // driver Healthy — the same invisible outcome as a blob that did not map, so it is the same signal. + PublishSkipLedger(result.Rejected.Select(r => + new SkippedTag(r.RawPath, $"rejected by the catalog gate: {r.Field} — {r.Reason}"))); + foreach (var rejection in result.Rejected) { _logger.LogWarning( diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs index a2aaaf34..210cd29a 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs @@ -11,8 +11,14 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; /// resolver land in PRs 2 and 3. /// public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery, ISubscribable, - IHostConnectivityProbe, IPerCallHostResolver, IRediscoverable, IDisposable, IAsyncDisposable + IHostConnectivityProbe, IPerCallHostResolver, IRediscoverable, ISkippedTagReporter, IDisposable, IAsyncDisposable { + /// Authored tags this driver could not build into its poll table (Gitea #541). + private readonly SkippedTagLedger _skippedTags = new(); + + /// + public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + // Mutable so ReinitializeAsync can apply a new config generation. The constructor seeds // it; InitializeAsync re-parses driverConfigJson over the top of it. private TwinCATDriverOptions _options; @@ -127,6 +133,7 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery // DeviceName is resolved to the target's live host (def.DeviceHostAddress) via ResolveDeviceHost // so the existing device routing (_devices keyed by HostAddress) is unchanged. A mapper miss is // skipped (logged), never thrown — a bad tag must not fail the whole driver init. + _skippedTags.BeginRebuild(_options.RawTags.Count); foreach (var entry in _options.RawTags) { if (TwinCATTagDefinitionFactory.FromTagConfig(entry.TagConfig, entry.RawPath, out var def)) @@ -136,10 +143,14 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery DeviceHostAddress = ResolveDeviceHost(entry.DeviceName), }; else + { + _skippedTags.Skip(entry.RawPath, "TagConfig did not map to a definition"); _logger.LogWarning( "TwinCAT tag config did not map to a definition; skipping. Driver={DriverInstanceId} RawPath={RawPath}", _driverInstanceId, entry.RawPath); + } } + _skippedTags.Commit(); if (_options.Probe.Enabled) { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor index 75fbe44f..0957c267 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor @@ -169,6 +169,7 @@ else Type Status Hosts + Tags Last read Errors/5 min Last error @@ -178,7 +179,7 @@ else @if (g.Drivers.Count == 0) { - No drivers. + No drivers. } else @@ -227,6 +228,30 @@ else } + + @* Authored tags the driver is NOT polling (Gitea #541). This column + exists because a skip is not an error: the Status chip stays + Healthy, Errors/5 min stays 0, Last error stays empty, and Last + read keeps advancing off the tags that DID map. On the Makino in + #540 that read as a perfectly healthy driver ignoring 20 of its 22 + tags. "—" means the driver cannot report skips, which is distinct + from reporting none. *@ + @if (d.SkippedTags is null) + { + — + } + else if (!d.HasSkippedTags) + { + @d.SkippedTags.AuthoredCount @(d.SkippedTags.AuthoredCount == 1 ? "tag" : "tags") + } + else + { + + @d.SkippedTags.SkippedCount / @d.SkippedTags.AuthoredCount skipped + + } + @(d.LastSuccessfulReadUtc?.ToString("HH:mm:ss 'UTC'") ?? "—") @d.ErrorCount5Min @(d.LastError ?? "—") diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/HostsDriverView.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/HostsDriverView.cs index 54717bd1..0cadfb89 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/HostsDriverView.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/HostsDriverView.cs @@ -44,12 +44,41 @@ public sealed record HostsDriverInstanceInfo(string DriverInstanceId, string Clu /// Per-host connectivity for a multi-device driver, ordered by host name; null /// when the driver reports no per-host detail. Lets one unreachable device show even while the driver /// row itself is Healthy. +/// Authored tags the driver could not build into its poll table; null when the +/// driver cannot report them. Lets "ignoring 20 of 22 tags" show even while every other column on the +/// row — state, last read, error count, last error — reads perfectly healthy (Gitea #541). public sealed record HostsDriverRow( string DriverInstanceId, string? Name, string? DriverType, string State, DateTime? LastSuccessfulReadUtc, string? LastError, int ErrorCount5Min, DateTime PublishedUtc, DateTime? RediscoveryNeededUtc = null, string? RediscoveryReason = null, - IReadOnlyList? HostStatuses = null) + IReadOnlyList? HostStatuses = null, + SkippedTagReport? SkippedTags = null) { + /// Whether this driver is ignoring at least one authored tag — the condition the whole + /// column exists to make visible. + public bool HasSkippedTags => SkippedTags is { SkippedCount: > 0 }; + + /// + /// Operator-facing detail for the skip chip: the sample of skipped RawPaths with reasons, plus an + /// explicit note when the sample is shorter than the count. The truncation note is not optional — + /// without it a capped list reads as the complete one, and an operator who fixes those twenty + /// believes they are done. + /// + /// The tooltip text; empty when nothing was skipped. + public string SkippedTagsTitle() + { + if (SkippedTags is not { SkippedCount: > 0 } r) return ""; + + var lead = r.SkippedEverything + ? $"This driver is polling NONE of its {r.AuthoredCount} authored tag(s)." + : $"{r.SkippedCount} of {r.AuthoredCount} authored tag(s) are not being polled."; + var detail = string.Join(" · ", r.Sample.Select(s => $"{s.RawPath}: {s.Reason}")); + var more = r.SkippedCount > r.Sample.Count + ? $" (+{r.SkippedCount - r.Sample.Count} more not shown)" + : ""; + return $"{lead} They report BadNodeIdUnknown and the driver's state does not reflect them. {detail}{more}"; + } + /// Hosts that are not , ordered by name — the ones worth an /// operator's attention. Empty when every host is fine or none are reported. /// counts as degraded on purpose: a probe that has not yet @@ -136,7 +165,8 @@ public static class HostsDriverView s.PublishedUtc, s.RediscoveryNeededUtc, s.RediscoveryReason, - s.HostStatuses); + s.HostStatuses, + s.SkippedTags); }) .OrderBy(d => d.Name ?? d.DriverInstanceId, StringComparer.OrdinalIgnoreCase) .ThenBy(d => d.DriverInstanceId, StringComparer.OrdinalIgnoreCase) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Telemetry/TelemetryProtoMapCentral.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Telemetry/TelemetryProtoMapCentral.cs index 26b03111..b070cc53 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Telemetry/TelemetryProtoMapCentral.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Telemetry/TelemetryProtoMapCentral.cs @@ -7,6 +7,10 @@ using ZB.MOM.WW.OtOpcUa.Commons.Protos.Telemetry.V1; // with the proto DriverHealth this file maps. using HostConnectivityStatus = ZB.MOM.WW.OtOpcUa.Core.Abstractions.HostConnectivityStatus; using HostState = ZB.MOM.WW.OtOpcUa.Core.Abstractions.HostState; +// Same reason: the proto declares SkippedTag / SkippedTags alongside the domain records of near-identical +// name, so the domain side is named explicitly and the proto side reached through the wholesale import. +using SkippedTag = ZB.MOM.WW.OtOpcUa.Core.Abstractions.SkippedTag; +using SkippedTagReport = ZB.MOM.WW.OtOpcUa.Core.Abstractions.SkippedTagReport; namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Telemetry; @@ -125,7 +129,26 @@ public static class TelemetryProtoMapCentral PublishedUtc: Required(msg.PublishedUtc, "DriverHealth", "published_utc"), RediscoveryNeededUtc: msg.RediscoveryNeededUtc?.ToDateTime(), RediscoveryReason: msg.HasRediscoveryReason ? msg.RediscoveryReason : null, - HostStatuses: ToHostStatuses(msg)); + HostStatuses: ToHostStatuses(msg), + SkippedTags: ToSkippedTags(msg)); + } + + /// + /// Projects the skipped-tag report, honouring the field's own presence: null when the node said + /// the driver cannot report skips, a report when it can — including an all-zero one, which is the + /// positive statement "this driver skipped nothing" and must not be flattened back to null. + /// + private static SkippedTagReport? ToSkippedTags(DriverHealth msg) + { + if (msg.SkippedTags is not { } s) return null; + + var sample = new List(s.Sample.Count); + foreach (var entry in s.Sample) + { + sample.Add(new SkippedTag(entry.RawPath, entry.Reason)); + } + + return new SkippedTagReport(s.AuthoredCount, s.SkippedCount, sample); } /// diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Grpc/TelemetryProtoMapNode.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Grpc/TelemetryProtoMapNode.cs index c8480ce4..904d84a7 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Grpc/TelemetryProtoMapNode.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Grpc/TelemetryProtoMapNode.cs @@ -148,6 +148,22 @@ public static class TelemetryProtoMapNode } } + // Gitea #541. The field's own presence carries "driver cannot report skips" vs "reports none", + // so leaving it unset is meaningful — do not replace this with an always-set empty message. + if (e.SkippedTags is { } skipped) + { + var payload = new SkippedTags + { + AuthoredCount = skipped.AuthoredCount, + SkippedCount = skipped.SkippedCount, + }; + foreach (var s in skipped.Sample) + { + payload.Sample.Add(new SkippedTag { RawPath = s.RawPath ?? "", Reason = s.Reason ?? "" }); + } + msg.SkippedTags = payload; + } + return msg; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/AkkaDriverHealthPublisher.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/AkkaDriverHealthPublisher.cs index 854152ba..fb2955a5 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/AkkaDriverHealthPublisher.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/AkkaDriverHealthPublisher.cs @@ -37,7 +37,8 @@ public sealed class AkkaDriverHealthPublisher : IDriverHealthPublisher int errorCount5Min, DateTime? rediscoveryNeededUtc = null, string? rediscoveryReason = null, - IReadOnlyList? hostStatuses = null) + IReadOnlyList? hostStatuses = null, + SkippedTagReport? skippedTags = null) { var msg = new DriverHealthChanged( clusterId, @@ -49,7 +50,8 @@ public sealed class AkkaDriverHealthPublisher : IDriverHealthPublisher DateTime.UtcNow, rediscoveryNeededUtc, rediscoveryReason, - hostStatuses); + hostStatuses, + skippedTags); DistributedPubSub.Get(_system).Mediator.Tell(new Publish(TopicName, msg)); // Phase 5: fan the same snapshot into the node-local live-telemetry hub (no-op until a gRPC // client subscribes). The DPS publish above is unchanged — the hub is a strictly additive tap. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs index a55ac0f3..c96b70c0 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs @@ -1001,6 +1001,27 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers } } + /// What the driver skipped when it last built its tag table, or null when it is not an + /// . Pulled fresh for the same reason as + /// — the driver owns the answer, and a cached copy here would be a + /// second source of truth to keep in sync across every reinitialise. + /// Called UNGUARDED (not through ): the contract is a pure in-memory + /// snapshot with no I/O. The try/catch keeps a misbehaving implementation from taking down the health + /// publish — losing the skip detail is strictly better than losing the whole snapshot. + private SkippedTagReport? CurrentSkippedTags() + { + if (_driver is not ISkippedTagReporter reporter) return null; + try + { + return reporter.GetSkippedTags(); + } + catch (Exception ex) + { + _log.Warning(ex, "DriverInstance {Id}: GetSkippedTags threw during health publish; omitting skip detail", _driverInstanceId); + return null; + } + } + /// Records the driver's rediscovery raise and re-publishes health so the signal reaches the /// AdminUI promptly rather than waiting for the next 30 s heartbeat. /// Advisory only. The served address space is deliberately NOT rebuilt: v3 authors raw tags @@ -1151,6 +1172,7 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers observed = health; var errorCount = ErrorCount5Min(); var hostStatuses = CurrentHostStatuses(); + var skippedTags = CurrentSkippedTags(); // _rediscoveryNeededUtc is PART OF THE FINGERPRINT on purpose. A rediscovery raise on an // otherwise-unchanged Healthy driver leaves (state, lastSuccess, lastError, errorCount) // identical, so without it the dedup below would swallow the very publish that carries the @@ -1162,14 +1184,21 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers // the whole point of this channel — would be deduped away. It is a flattened STRING because a // tuple holding IReadOnlyList compares by reference, which would never match and so would // defeat the dedup in the opposite direction (re-publishing every 30 s heartbeat). + // + // The skip digest is in the fingerprint for the third instance of that same argument, and + // here it is the WHOLE point of the channel: a driver that skipped tags is by construction + // one whose state, error count and last-error are all unremarkable, so every other component + // is identical to a clean driver's and the publish that carries the signal is exactly the one + // dedup would swallow. Flattened to a STRING for the same reason as the host digest — the + // report holds an IReadOnlyList, which a tuple compares by reference. var fingerprint = (health.State, health.LastSuccessfulRead, health.LastError, errorCount, - _rediscoveryNeededUtc, HostStatusDigest(hostStatuses)); + _rediscoveryNeededUtc, HostStatusDigest(hostStatuses), SkippedTagDigest(skippedTags)); if (_lastPublishedFingerprint is { } prev && prev.Equals(fingerprint)) return observed; _lastPublishedFingerprint = fingerprint; _healthPublisher.Publish( _clusterId, _driverInstanceId, health, errorCount, _rediscoveryNeededUtc, _rediscoveryReason, - hostStatuses); + hostStatuses, skippedTags); } catch (Exception ex) { @@ -1191,9 +1220,21 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers .OrderBy(s => s.HostName, StringComparer.Ordinal) .Select(s => $"{s.HostName}={s.State}@{s.LastChangedUtc:O}")); + /// Value digest of a skip report for the publish fingerprint. Null in, null out, so "driver + /// cannot report skips" and "driver reports none" stay distinguishable rather than both collapsing to + /// the same string. + /// The counts alone would be enough to catch a change in HOW MANY tags are skipped, but not a + /// change in WHICH — a redeploy that fixes one tag and breaks another leaves both counts identical, + /// and the operator would keep reading the stale sample. So the sample is in the digest too. + private static string? SkippedTagDigest(SkippedTagReport? report) => + report is null + ? null + : $"{report.AuthoredCount}/{report.SkippedCount}:" + + string.Join('|', report.Sample.Select(s => $"{s.RawPath}={s.Reason}")); + /// Fingerprint of the last call; null until first publish. private (DriverState State, DateTime? LastSuccess, string? LastError, int ErrorCount, - DateTime? RediscoveryNeededUtc, string? HostStatusDigest)? _lastPublishedFingerprint; + DateTime? RediscoveryNeededUtc, string? HostStatusDigest, string? SkippedTagDigest)? _lastPublishedFingerprint; /// protected override void PostStop() diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/InterfaceIndependenceTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/InterfaceIndependenceTests.cs index dcaefcc8..23971b1a 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/InterfaceIndependenceTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/InterfaceIndependenceTests.cs @@ -65,6 +65,7 @@ public sealed class InterfaceIndependenceTests [InlineData(typeof(IHistoryProvider))] [InlineData(typeof(IRediscoverable))] [InlineData(typeof(IHostConnectivityProbe))] + [InlineData(typeof(ISkippedTagReporter))] [InlineData(typeof(IDriverConfigEditor))] [InlineData(typeof(IAddressSpaceBuilder))] public void EveryCapabilityInterface_IsPublic(Type type) diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/SkippedTagLedgerTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/SkippedTagLedgerTests.cs new file mode 100644 index 00000000..eb19b7d9 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/SkippedTagLedgerTests.cs @@ -0,0 +1,210 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests; + +/// +/// Gitea #541. The rules a driver relies on when it accumulates skips into a +/// — one implementation of them, rather than twelve. +/// +public sealed class SkippedTagLedgerTests +{ + [Fact] + public void A_fresh_ledger_reports_nothing() + { + var report = new SkippedTagLedger().Snapshot(); + + report.AuthoredCount.ShouldBe(0); + report.SkippedCount.ShouldBe(0); + report.Sample.ShouldBeEmpty(); + report.HasSkips.ShouldBeFalse(); + } + + [Fact] + public void A_committed_build_reports_its_skips_with_the_authored_denominator() + { + var ledger = new SkippedTagLedger(); + + ledger.BeginRebuild(22); + ledger.Skip("Line1/Makino/Identity/SeriesNumber", "invalid Address"); + ledger.Skip("Line1/Makino/Identity/Version", "invalid Address"); + ledger.Commit(); + + var report = ledger.Snapshot(); + report.AuthoredCount.ShouldBe(22); + report.SkippedCount.ShouldBe(2); + report.Sample.Select(s => s.RawPath) + .ShouldBe(["Line1/Makino/Identity/SeriesNumber", "Line1/Makino/Identity/Version"]); + report.HasSkips.ShouldBeTrue(); + report.SkippedEverything.ShouldBeFalse(); + } + + /// + /// The Makino case from Gitea #540 — the one the whole issue is about. A driver skipping every + /// tag it has is the MOST likely to look healthy, because it has no failing reads to report. + /// + [Fact] + public void Skipping_every_authored_tag_is_distinguishable_from_skipping_some() + { + var ledger = new SkippedTagLedger(); + + ledger.BeginRebuild(3); + ledger.Skip("a", "r"); + ledger.Skip("b", "r"); + ledger.Skip("c", "r"); + ledger.Commit(); + + ledger.Snapshot().SkippedEverything.ShouldBeTrue(); + } + + /// + /// A driver that authored nothing has not "skipped everything" — there was nothing to skip. The + /// distinction matters because the UI paints SkippedEverything as the loudest state, and an + /// empty driver is not an alarm. + /// + [Fact] + public void An_empty_deployment_has_not_skipped_everything() + { + var ledger = new SkippedTagLedger(); + + ledger.BeginRebuild(0); + ledger.Commit(); + + ledger.Snapshot().SkippedEverything.ShouldBeFalse(); + } + + /// + /// The count is exact; only the detail list is capped. A misconfigured deployment can skip + /// thousands of tags, and this report is published on every driver state change. + /// + [Fact] + public void The_sample_is_capped_but_the_count_is_not() + { + var ledger = new SkippedTagLedger(); + + ledger.BeginRebuild(500); + for (var i = 0; i < 500; i++) ledger.Skip($"tag-{i}", "TagConfig did not map to a definition"); + ledger.Commit(); + + var report = ledger.Snapshot(); + report.SkippedCount.ShouldBe(500); + report.Sample.Count.ShouldBe(SkippedTagLedger.MaxSample); + report.Sample[0].RawPath.ShouldBe("tag-0", "the sample keeps the FIRST skips, in encounter order"); + } + + /// + /// The reason the ledger has a commit step at all. A health publish landing part-way through a + /// rebuild must read the previous complete answer, not a half-built one — a truncated snapshot + /// would read as "we now skip fewer tags", which is the opposite of the truth. + /// + [Fact] + public void An_in_progress_rebuild_is_invisible_until_it_commits() + { + var ledger = new SkippedTagLedger(); + ledger.BeginRebuild(10); + ledger.Skip("old", "r"); + ledger.Commit(); + + ledger.BeginRebuild(10); + ledger.Skip("new-1", "r"); + + var midRebuild = ledger.Snapshot(); + midRebuild.SkippedCount.ShouldBe(1); + midRebuild.Sample.Single().RawPath.ShouldBe("old"); + + ledger.Skip("new-2", "r"); + ledger.Commit(); + ledger.Snapshot().Sample.Select(s => s.RawPath).ShouldBe(["new-1", "new-2"]); + } + + /// + /// An abandoned rebuild — a driver init that threw part-way, which several drivers do on an + /// unknown device — leaves the last good snapshot standing rather than publishing a partial one. + /// + [Fact] + public void A_rebuild_that_never_commits_leaves_the_previous_snapshot_standing() + { + var ledger = new SkippedTagLedger(); + ledger.BeginRebuild(5); + ledger.Skip("known-bad", "invalid Address"); + ledger.Commit(); + + ledger.BeginRebuild(5); + ledger.Skip("would-have-been-reported", "invalid Address"); + // …driver throws here; no Commit. + + ledger.Snapshot().Sample.Single().RawPath.ShouldBe("known-bad"); + } + + /// + /// A redeploy that fixes every tag must clear the report. Accumulating across rebuilds would + /// leave a permanent false alarm on a driver that is now correct. + /// + [Fact] + public void A_clean_rebuild_clears_a_previous_report() + { + var ledger = new SkippedTagLedger(); + ledger.BeginRebuild(2); + ledger.Skip("bad", "invalid Address"); + ledger.Commit(); + + ledger.BeginRebuild(2); + ledger.Commit(); + + var report = ledger.Snapshot(); + report.HasSkips.ShouldBeFalse(); + report.SkippedCount.ShouldBe(0); + report.AuthoredCount.ShouldBe(2); + } + + /// + /// A stray Skip outside a rebuild is dropped rather than thrown. This runs inside a + /// driver's initialise: a bookkeeping exception would fail the very init the ledger describes. + /// + [Fact] + public void A_skip_outside_a_rebuild_is_ignored_rather_than_thrown() + { + var ledger = new SkippedTagLedger(); + + Should.NotThrow(() => ledger.Skip("orphan", "r")); + ledger.Snapshot().SkippedCount.ShouldBe(0); + } + + /// + /// Concurrent reads during a build must not tear. The read is on the actor's health-publish path + /// while the build runs on the driver's init path, so this is the real production interleaving. + /// + [Fact] + public async Task Reading_while_a_build_runs_never_observes_a_torn_report() + { + var ledger = new SkippedTagLedger(); + ledger.BeginRebuild(1000); + ledger.Commit(); + + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(2)); + + var writer = Task.Run(() => + { + for (var round = 0; round < 50 && !cts.IsCancellationRequested; round++) + { + ledger.BeginRebuild(1000); + for (var i = 0; i < 100; i++) ledger.Skip($"t{i}", "r"); + ledger.Commit(); + } + }, cts.Token); + + var reader = Task.Run(() => + { + while (!writer.IsCompleted && !cts.IsCancellationRequested) + { + var r = ledger.Snapshot(); + // Every committed snapshot is internally consistent: the sample never exceeds either the + // cap or the true count it is drawn from. + r.Sample.Count.ShouldBeLessThanOrEqualTo(SkippedTagLedger.MaxSample); + r.Sample.Count.ShouldBeLessThanOrEqualTo(r.SkippedCount); + } + }, cts.Token); + + await Task.WhenAll(writer, reader); + } +} diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/SkippedTagReporterFleetTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/SkippedTagReporterFleetTests.cs new file mode 100644 index 00000000..29db14a9 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/SkippedTagReporterFleetTests.cs @@ -0,0 +1,92 @@ +using System.Reflection; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests; + +/// +/// Gitea #541. Pins the fleet-wide invariant: every driver reports what it skipped. +/// +/// +/// +/// The defect this guards was never one driver's bug — it was one missing signal repeated across +/// every driver in the fleet, which is exactly the shape that a per-driver test cannot catch and a +/// thirteenth driver would silently re-introduce. So discovery here is reflective, not a +/// hand-copied list: it scans the driver assemblies deployed to bin for concrete +/// implementations, the same convention +/// DriverTypeNamesGuardTests uses to find factories. +/// +/// +/// The original issue named eight drivers. The real number is twelve — every driver in +/// has an authored-tag skip path, and OpcUaClient's was +/// the quietest of them (a bare continue in NamespaceMap, with not even a log line). +/// A hand-maintained list would have shipped the four the issue missed still invisible. +/// +/// +public sealed class SkippedTagReporterFleetTests +{ + /// Driver-adjacent assemblies that ship no runtime driver (contracts/addressing/CLI helpers, + /// browsers, transports, and the historian client packages). + private static readonly string[] NonDriverAssemblySuffixes = + [".Contracts", ".Addressing", ".Cli", ".Browser", ".Transport", ".Dialect", ".Client"]; + + /// + /// Every concrete implementation deployed to the test output directory. + /// + /// The discovered driver types. + private static IReadOnlyList DiscoverDriverTypes() + { + var found = new List(); + var binDir = Path.GetDirectoryName(typeof(SkippedTagReporterFleetTests).Assembly.Location)!; + + foreach (var dll in Directory.GetFiles(binDir, "ZB.MOM.WW.OtOpcUa.Driver.*.dll")) + { + var name = Path.GetFileNameWithoutExtension(dll); + if (NonDriverAssemblySuffixes.Any(s => name.EndsWith(s, StringComparison.Ordinal))) + continue; + + Assembly asm; + try { asm = Assembly.LoadFrom(dll); } + catch { continue; } + + Type[] types; + try { types = asm.GetTypes(); } + catch (ReflectionTypeLoadException ex) { types = ex.Types.Where(t => t is not null).ToArray()!; } + + found.AddRange(types.Where(t => + t is { IsClass: true, IsAbstract: false } && typeof(IDriver).IsAssignableFrom(t))); + } + + return found; + } + + /// + /// Every driver in the fleet implements . + /// + /// + /// A driver that does not is one whose Healthy is unfalsifiable: it can ignore every tag it + /// was authored and no surface anywhere will say so. + /// + [Fact] + public void Every_driver_reports_the_tags_it_skipped() + { + var drivers = DiscoverDriverTypes(); + + // A zero here means the bin scan broke, not that the fleet has no drivers — fail loudly rather + // than pass a vacuously-empty check, which is how an invariant test quietly stops testing. + drivers.Count.ShouldBeGreaterThanOrEqualTo(DriverTypeNames.All.Count, + "fewer IDriver implementations were discovered than there are registered driver types — the " + + "driver assemblies did not deploy to bin, or a driver moved out of a ZB.MOM.WW.OtOpcUa.Driver.* assembly"); + + var missing = drivers + .Where(t => !typeof(ISkippedTagReporter).IsAssignableFrom(t)) + .Select(t => t.FullName) + .OrderBy(n => n, StringComparer.Ordinal) + .ToList(); + + missing.ShouldBeEmpty( + "these drivers do not report skipped tags, so each can ignore its whole configuration while " + + "reporting Healthy with 0 errors (Gitea #541). Accumulate skips into a SkippedTagLedger during " + + "the tag-table build and expose it via ISkippedTagReporter: " + string.Join(", ", missing)); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasSkippedTagReportingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasSkippedTagReportingTests.cs new file mode 100644 index 00000000..35919234 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasSkippedTagReportingTests.cs @@ -0,0 +1,170 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// Gitea #541, proved against the driver where the defect was actually found. #540 fixed the +/// addressing that produced the 20 unreadable tags on the wonder Makino; this covers the half that +/// made them invisible — the driver reported Healthy with 0 errors and a live-advancing +/// last read while ignoring 20 of its 22 tags, and nothing anywhere said so. +/// FOCAS is the useful driver to prove it on because it has all three skip shapes: an unmappable +/// TagConfig, an unparseable address, and a capability-matrix rejection. +/// +[Trait("Category", "Unit")] +public sealed class FocasSkippedTagReportingTests +{ + private const string Host = "focas://10.0.0.5:8193"; + + private static FocasDriver NewDriver(params FocasTagDefinition[] tags) => + new(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions(Host)], + Probe = new FocasProbeOptions { Enabled = false }, + FixedTree = new FocasFixedTreeOptions { Enabled = false }, + RawTags = FocasRawTags.Entries(tags), + }, "drv-1", new FakeFocasClientFactory()); + + /// The driver advertises the capability at all — the fleet guard asserts this reflectively, + /// but a driver-local pin fails with a message that names FOCAS. + [Fact] + public void The_driver_reports_skipped_tags() + => NewDriver().ShouldBeAssignableTo(); + + /// A driver whose tags all map reports zero skips against the real denominator. + [Fact] + public async Task A_clean_deployment_reports_no_skips() + { + var drv = NewDriver( + new FocasTagDefinition("raw/A", Host, "R100", FocasDataType.Byte), + new FocasTagDefinition("raw/B", Host, "MACRO:500", FocasDataType.Float64)); + + await drv.InitializeAsync("{}", CancellationToken.None); + + var report = drv.GetSkippedTags(); + report.AuthoredCount.ShouldBe(2); + report.SkippedCount.ShouldBe(0); + report.HasSkips.ShouldBeFalse(); + report.SkippedEverything.ShouldBeFalse(); + } + + /// + /// The wonder Makino, reproduced. Twenty tags carrying the exact address shape #540 + /// authored — a browse full reference with the device host in it — plus two good ones. The driver + /// is Healthy with no error either way; the skip report is the only thing that differs. + /// + [Fact] + public async Task The_makino_case_a_healthy_driver_that_is_ignoring_most_of_its_tags() + { + FocasTagDefinition[] broken = + [ + .. Enumerable.Range(0, 20).Select(i => new FocasTagDefinition( + $"raw/Broken{i}", Host, $"10.0.0.5:8193/Identity/Field{i}", FocasDataType.String, Writable: false)), + ]; + var drv = NewDriver([ + .. broken, + new FocasTagDefinition("raw/Good1", Host, "MACRO:500", FocasDataType.Float64), + new FocasTagDefinition("raw/Good2", Host, "MACRO:501", FocasDataType.Float64), + ]); + + await drv.InitializeAsync("{}", CancellationToken.None); + + // Exactly the reading an operator got on /hosts, and why it was believed for weeks. + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + drv.GetHealth().LastError.ShouldBeNull(); + + // …and the fact that contradicts it. + var report = drv.GetSkippedTags(); + report.AuthoredCount.ShouldBe(22); + report.SkippedCount.ShouldBe(20); + report.SkippedEverything.ShouldBeFalse("two tags do poll — this driver is partly working"); + report.Sample.ShouldAllBe(s => s.Reason.Contains("invalid Address")); + report.Sample[0].RawPath.ShouldBe("raw/Broken0"); + } + + /// + /// The sample is capped but the count is not. Twenty-two skips against a cap of twenty: a UI that + /// rendered the sample as the whole story would under-report by two. + /// + [Fact] + public async Task The_count_is_exact_even_when_the_sample_is_capped() + { + var drv = NewDriver([ + .. Enumerable.Range(0, 25).Select(i => new FocasTagDefinition( + $"raw/Broken{i}", Host, "not-an-address", FocasDataType.String, Writable: false)), + ]); + + await drv.InitializeAsync("{}", CancellationToken.None); + + var report = drv.GetSkippedTags(); + report.SkippedCount.ShouldBe(25); + report.Sample.Count.ShouldBe(SkippedTagLedger.MaxSample); + report.SkippedEverything.ShouldBeTrue("this driver polls nothing at all"); + } + + /// + /// Re-initialising replaces the report rather than accumulating onto it. Config edits respawn the + /// driver (#516) and Sql/FOCAS re-parse in place, so a ledger that accumulated would leave a + /// permanent false alarm on a driver whose deployment has since been fixed. + /// + [Fact] + public async Task Re_initialising_replaces_the_report_rather_than_accumulating() + { + var drv = NewDriver( + new FocasTagDefinition("raw/Bad", Host, "not-an-address", FocasDataType.String, Writable: false), + new FocasTagDefinition("raw/Good", Host, "MACRO:500", FocasDataType.Float64)); + + await drv.InitializeAsync("{}", CancellationToken.None); + drv.GetSkippedTags().SkippedCount.ShouldBe(1); + + await drv.InitializeAsync("{}", CancellationToken.None); + + var report = drv.GetSkippedTags(); + report.SkippedCount.ShouldBe(1, "the same one bad tag — not two"); + report.AuthoredCount.ShouldBe(2); + } + + /// + /// A tag routed to an unknown device is a driver-level config error that throws, and an init that + /// throws must leave the previous report standing rather than publish the half-built one it got to. + /// + [Fact] + public async Task An_init_that_throws_leaves_the_previous_report_standing() + { + var good = new FocasDriverOptions + { + Devices = [new FocasDeviceOptions(Host)], + Probe = new FocasProbeOptions { Enabled = false }, + FixedTree = new FocasFixedTreeOptions { Enabled = false }, + RawTags = FocasRawTags.Entries( + [ + new FocasTagDefinition("raw/Bad", Host, "not-an-address", FocasDataType.String, Writable: false), + new FocasTagDefinition("raw/Good", Host, "MACRO:500", FocasDataType.Float64), + ]), + }; + var drv = new FocasDriver(good, "drv-1", new FakeFocasClientFactory()); + await drv.InitializeAsync("{}", CancellationToken.None); + drv.GetSkippedTags().Sample.Single().RawPath.ShouldBe("raw/Bad"); + + // A second driver over an artifact whose tag names a device that is not configured: the loop skips + // the first entry, then throws on the second. + var bad = new FocasDriverOptions + { + Devices = [new FocasDeviceOptions(Host)], + Probe = new FocasProbeOptions { Enabled = false }, + FixedTree = new FocasFixedTreeOptions { Enabled = false }, + RawTags = FocasRawTags.Entries( + [ + new FocasTagDefinition("raw/AlsoBad", Host, "not-an-address", FocasDataType.String, Writable: false), + new FocasTagDefinition("raw/Orphan", "focas://10.9.9.9:8193", "MACRO:500", FocasDataType.Float64), + ]), + }; + var drv2 = new FocasDriver(bad, "drv-2", new FakeFocasClientFactory()); + await Should.ThrowAsync( + () => drv2.InitializeAsync("{}", CancellationToken.None)); + + drv2.GetSkippedTags().SkippedCount.ShouldBe(0, + "an abandoned build must not publish; a partial report would read as 'we skip fewer tags now'"); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs index afeb8ad1..a7243fb0 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs @@ -22,6 +22,53 @@ public sealed class SqlCatalogGateDriverTests private const string DriverInstanceId = "sql-gate"; private const string ConfigJson = """{"provider":"SqlServer"}"""; + /// + /// Gitea #541. Sql drops a tag at TWO points — a blob that does not map, and a rejection against + /// the live catalog — and both leave the tag unpolled and publishing BadNodeIdUnknown with + /// the driver Healthy. The skip report must be the union. + /// This is the case that makes Sql different from the other eleven drivers, and the one worth + /// a live-catalog test rather than a unit one: a gate rejection is precisely what a schema drift + /// (renamed column, dropped view) produces months after a deployment sealed green. Reporting only + /// the blob-level skips would have this driver claim "0 skipped" while polling half its tags — + /// worse than reporting nothing, because the column would be actively reassuring. + /// + [Fact] + public async Task A_catalog_gate_rejection_is_reported_as_a_skipped_tag() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver( + fixture, + KvEntry("Speed", SqlitePollFixture.PresentKey), + KvEntry("Bogus", SqlitePollFixture.PresentKey, valueColumn: "no_such_column")); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + // The reading an operator gets from every other column on /hosts. + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + driver.GetHealth().LastError.ShouldBeNull(); + + // …and the one that contradicts it. + var report = ((ISkippedTagReporter)driver).GetSkippedTags(); + report.AuthoredCount.ShouldBe(2); + report.SkippedCount.ShouldBe(1, "the gate rejected one tag; the blob mapped both"); + report.Sample.Single().RawPath.ShouldBe("Bogus"); + report.Sample.Single().Reason.ShouldContain("catalog gate"); + } + + /// A driver whose tags all map and all pass the gate reports zero skips. + [Fact] + public async Task A_driver_whose_tags_all_pass_the_gate_reports_no_skips() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver(fixture, KvEntry("Speed", SqlitePollFixture.PresentKey)); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + var report = ((ISkippedTagReporter)driver).GetSkippedTags(); + report.AuthoredCount.ShouldBe(1); + report.HasSkips.ShouldBeFalse(); + } + [Fact] public async Task A_tag_naming_a_real_table_and_columns_polls_normally() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hosts/HostsDriverViewTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hosts/HostsDriverViewTests.cs index acc38c31..ea6d5e89 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hosts/HostsDriverViewTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hosts/HostsDriverViewTests.cs @@ -213,4 +213,96 @@ public sealed class HostsDriverViewTests row.HostStatuses.ShouldBeNull(); row.DegradedHosts.ShouldBeEmpty(); } + + // ---- skipped tags (Gitea #541) ---- + + /// + /// The Makino case from Gitea #540, as the row renders it: every driver-level field reads healthy + /// while 20 of 22 tags are not being polled. The skip field is the only thing on the row that + /// contradicts it. + /// + [Fact] + public void A_healthy_row_still_reports_the_tags_its_driver_is_ignoring() + { + var snapshot = Snap("MAIN", "focas-z-34184") with + { + SkippedTags = new SkippedTagReport(22, 20, + [new SkippedTag("Makino/Identity/SeriesNumber", "invalid Address")]), + }; + + var row = HostsDriverView.Build([snapshot], nodes: null, instances: null).Single().Drivers.Single(); + + row.State.ShouldBe("Healthy"); + row.ErrorCount5Min.ShouldBe(0); + row.LastError.ShouldBeNull(); + row.HasSkippedTags.ShouldBeTrue("the row must contradict its own healthy-looking columns"); + row.SkippedTags!.SkippedCount.ShouldBe(20); + } + + /// + /// A truncated sample must SAY it is truncated. Without the note a capped list reads as the whole + /// list, and an operator who fixes the twenty named tags believes the job is done. + /// + [Fact] + public void The_skip_tooltip_admits_when_the_sample_is_shorter_than_the_count() + { + var snapshot = Snap("MAIN", "drv-a") with + { + SkippedTags = new SkippedTagReport(500, 137, [new SkippedTag("tag-0", "TagConfig did not map")]), + }; + + var title = HostsDriverView.Build([snapshot], nodes: null, instances: null) + .Single().Drivers.Single().SkippedTagsTitle(); + + title.ShouldContain("137 of 500"); + title.ShouldContain("+136 more not shown"); + } + + /// A complete sample carries no truncation note. + [Fact] + public void The_skip_tooltip_omits_the_truncation_note_when_the_sample_is_complete() + { + var snapshot = Snap("MAIN", "drv-a") with + { + SkippedTags = new SkippedTagReport(5, 1, [new SkippedTag("tag-0", "TagConfig did not map")]), + }; + + HostsDriverView.Build([snapshot], nodes: null, instances: null) + .Single().Drivers.Single().SkippedTagsTitle() + .ShouldNotContain("more not shown"); + } + + /// + /// A driver polling NONE of its tags gets its own wording — it is the most likely of all to look + /// healthy, because it has no failing reads to report. + /// + [Fact] + public void A_driver_polling_nothing_is_called_out_as_such() + { + var snapshot = Snap("MAIN", "drv-a") with + { + SkippedTags = new SkippedTagReport(3, 3, [new SkippedTag("a", "r")]), + }; + + var row = HostsDriverView.Build([snapshot], nodes: null, instances: null).Single().Drivers.Single(); + + row.SkippedTags!.SkippedEverything.ShouldBeTrue(); + row.SkippedTagsTitle().ShouldContain("NONE"); + } + + /// + /// A driver that cannot report skips keeps null — distinct from one reporting zero. The column + /// renders "—" for the former and "N tags" for the latter, and collapsing them would claim an + /// unreporting driver is positively known to be polling everything. + /// + [Fact] + public void A_driver_without_a_skip_report_keeps_null_and_claims_nothing() + { + var row = HostsDriverView.Build([Snap("MAIN", "drv-a")], nodes: null, instances: null) + .Single().Drivers.Single(); + + row.SkippedTags.ShouldBeNull(); + row.HasSkippedTags.ShouldBeFalse(); + row.SkippedTagsTitle().ShouldBeEmpty(); + } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Telemetry/TelemetryProtoMapCentralTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Telemetry/TelemetryProtoMapCentralTests.cs index 1b4c9c62..a5a5468b 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Telemetry/TelemetryProtoMapCentralTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/Telemetry/TelemetryProtoMapCentralTests.cs @@ -220,6 +220,62 @@ public sealed class TelemetryProtoMapCentralTests TelemetryProtoMapCentral.ToHealth(absent).HostStatuses.ShouldBeNull(); } + /// + /// The skipped-tag report (Gitea #541) survives the wire with the same tri-state discipline: + /// null (the driver cannot report skips) must stay distinguishable from a zero report + /// (it can, and skipped nothing). Unlike the repeated host_statuses field this needs no companion + /// presence flag — a singular message field carries real presence in proto3 — but the distinction it + /// protects is the same one, and collapsing it would render an unreporting driver as positively + /// known-clean. + /// + [Fact] + public void ToHealth_skipped_tags_round_trip_with_the_null_vs_zero_distinction_intact() + { + var populated = new DriverHealth + { + ClusterId = "c1", DriverInstanceId = "d1", State = "Healthy", + PublishedUtc = Timestamp.FromDateTime(SampleUtc), + SkippedTags = new SkippedTags + { + AuthoredCount = 22, + // Deliberately larger than the sample: the cap is on the detail, never on the count. + SkippedCount = 20, + Sample = + { + new SkippedTag { RawPath = "Makino/Identity/SeriesNumber", Reason = "invalid Address" }, + }, + }, + }; + + var mapped = TelemetryProtoMapCentral.ToHealth(populated).SkippedTags; + mapped.ShouldNotBeNull(); + mapped!.AuthoredCount.ShouldBe(22); + mapped.SkippedCount.ShouldBe(20); + mapped.Sample.Count.ShouldBe(1, "a truncated sample must arrive truncated, not padded or dropped"); + mapped.Sample[0].RawPath.ShouldBe("Makino/Identity/SeriesNumber"); + mapped.Sample[0].Reason.ShouldBe("invalid Address"); + + // A driver that reports, and skipped nothing: a zero report, NOT null. + var clean = new DriverHealth + { + ClusterId = "c1", DriverInstanceId = "d1", State = "Healthy", + PublishedUtc = Timestamp.FromDateTime(SampleUtc), + SkippedTags = new SkippedTags { AuthoredCount = 12, SkippedCount = 0 }, + }; + var mappedClean = TelemetryProtoMapCentral.ToHealth(clean).SkippedTags; + mappedClean.ShouldNotBeNull(); + mappedClean!.HasSkips.ShouldBeFalse(); + mappedClean.AuthoredCount.ShouldBe(12); + + // A driver that cannot report at all: null, NOT a zero report. + var absentSkips = new DriverHealth + { + ClusterId = "c1", DriverInstanceId = "d1", State = "Healthy", + PublishedUtc = Timestamp.FromDateTime(SampleUtc), + }; + TelemetryProtoMapCentral.ToHealth(absentSkips).SkippedTags.ShouldBeNull(); + } + /// /// An unparseable host state degrades to rather than throwing. A node /// running a newer build that added an enum member must not be able to kill central's telemetry diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorHostStatusTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorHostStatusTests.cs index e4b77e51..f32d7cca 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorHostStatusTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorHostStatusTests.cs @@ -210,7 +210,8 @@ public sealed class DriverInstanceActorHostStatusTests : RuntimeActorTestBase int errorCount5Min, DateTime? rediscoveryNeededUtc = null, string? rediscoveryReason = null, - IReadOnlyList? hostStatuses = null) + IReadOnlyList? hostStatuses = null, + SkippedTagReport? skippedTags = null) { lock (_published) _published.Add(new HealthPublish(health, hostStatuses)); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorRediscoverySignalTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorRediscoverySignalTests.cs index 0154d205..cf39eb2b 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorRediscoverySignalTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorRediscoverySignalTests.cs @@ -186,7 +186,8 @@ public sealed class DriverInstanceActorRediscoverySignalTests : RuntimeActorTest int errorCount5Min, DateTime? rediscoveryNeededUtc = null, string? rediscoveryReason = null, - IReadOnlyList? hostStatuses = null) + IReadOnlyList? hostStatuses = null, + SkippedTagReport? skippedTags = null) { lock (_published) { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorSkippedTagTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorSkippedTagTests.cs new file mode 100644 index 00000000..8491de2d --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorSkippedTagTests.cs @@ -0,0 +1,269 @@ +using Akka.Actor; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; +using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; + +namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; + +/// +/// Covers the consumer (Gitea #541) — the signal that contradicts a +/// driver reporting Healthy, 0 errors and a live-advancing last read while ignoring most of the +/// tags it was authored. +/// Why it does not go to the DB — the same reason the #521 host statuses do not: a +/// driver-only node has no ConfigDb connection since per-cluster mesh Phase 4, so the data rides the +/// driver-health snapshot and the Phase 5 telemetry stream instead. +/// +[Trait("Category", "Unit")] +public sealed class DriverInstanceActorSkippedTagTests : RuntimeActorTestBase +{ + /// The base case: a reporting driver's skips reach the health publisher. + [Fact] + public void Skipped_tags_reach_the_health_publisher() + { + var driver = new SkipStubDriver(); + driver.SetReport(authored: 22, skipped: ["Makino/Identity/SeriesNumber", "Makino/Identity/Version"]); + var publisher = new RecordingHealthPublisher(); + var actor = SpawnDriverActor(driver, publisher); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + + AwaitAssert( + () => + { + var latest = publisher.Published.LastOrDefault(); + latest.ShouldNotBeNull(); + latest!.SkippedTags.ShouldNotBeNull(); + latest.SkippedTags!.AuthoredCount.ShouldBe(22); + latest.SkippedTags.SkippedCount.ShouldBe(2); + latest.SkippedTags.Sample.Select(s => s.RawPath) + .ShouldBe(["Makino/Identity/SeriesNumber", "Makino/Identity/Version"]); + }, + TimeSpan.FromSeconds(3)); + } + + /// + /// The load-bearing case, and the entire reason this channel exists. The Makino in Gitea #540: + /// a driver ignoring 20 of its 22 tags while every driver-level field reads perfectly healthy. + /// This is the same dedup trap that already bit the rediscovery signal and the host statuses. + /// PublishHealthSnapshot suppresses a publish whose fingerprint repeats, and a skipped tag by + /// construction leaves (state, lastSuccessfulRead, lastError, errorCount) ALL unchanged — a skip is + /// not an error and happens before any read. Unless the skip digest is part of the fingerprint, the + /// dedup swallows exactly the publish that carries the news. + /// Falsifiability: the assertion is that the publish count STRICTLY INCREASES across the + /// change. An "eventually reports skips" assertion would be satisfied by the warm-up publish alone + /// and would prove nothing. Drop SkippedTagDigest from the fingerprint tuple in + /// DriverInstanceActor and this test must go red. + /// + [Fact] + public void A_redeploy_that_starts_skipping_tags_is_not_swallowed_by_the_unchanged_health_dedup() + { + var driver = new SkipStubDriver(); + driver.SetReport(authored: 22, skipped: []); + var publisher = new RecordingHealthPublisher(); + var actor = SpawnDriverActor(driver, publisher); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + AwaitAssert(() => publisher.Published.Count.ShouldBeGreaterThan(0), TimeSpan.FromSeconds(3)); + + // Settle, so the baseline is a quiet actor: from here only the skip report changes. The driver's + // own health stays Healthy throughout — that is the point. + ExpectNoMsg(TimeSpan.FromMilliseconds(200)); + var before = publisher.Published.Count; + + driver.SetReport(authored: 22, skipped: [.. Enumerable.Range(0, 20).Select(i => $"Makino/Tag{i}")]); + + AwaitAssert( + () => publisher.Published.Count.ShouldBeGreaterThan(before), + TimeSpan.FromSeconds(3)); + + var latest = publisher.Published[^1]; + latest.Health.State.ShouldBe(DriverState.Healthy, + "the driver never faulted — a skip is a config problem, not an error, and must not latch state"); + latest.SkippedTags!.SkippedCount.ShouldBe(20); + latest.SkippedTags.AuthoredCount.ShouldBe(22); + } + + /// + /// A redeploy that swaps WHICH tags are broken without changing HOW MANY must still publish. + /// Digesting the counts alone would leave an operator staring at a stale sample naming tags that + /// are now fine, while the ones actually broken went unmentioned. + /// + [Fact] + public void Swapping_which_tags_are_skipped_republishes_even_though_the_count_is_unchanged() + { + var driver = new SkipStubDriver(); + driver.SetReport(authored: 5, skipped: ["old-broken"]); + var publisher = new RecordingHealthPublisher(); + var actor = SpawnDriverActor(driver, publisher); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + AwaitAssert(() => publisher.Published.Count.ShouldBeGreaterThan(0), TimeSpan.FromSeconds(3)); + ExpectNoMsg(TimeSpan.FromMilliseconds(200)); + var before = publisher.Published.Count; + + driver.SetReport(authored: 5, skipped: ["newly-broken"]); + + AwaitAssert(() => publisher.Published.Count.ShouldBeGreaterThan(before), TimeSpan.FromSeconds(3)); + publisher.Published[^1].SkippedTags!.Sample.Single().RawPath.ShouldBe("newly-broken"); + } + + /// + /// The other half of the dedup contract: when nothing changes, the digest must NOT churn. A digest + /// comparing the sample list by reference would differ on every call and re-publish on every 30 s + /// heartbeat forever — turning the dedup off without anyone noticing. + /// + [Fact] + public void An_unchanged_skip_report_does_not_defeat_the_dedup() + { + var driver = new SkipStubDriver(); + driver.SetReport(authored: 22, skipped: ["Makino/Identity/SeriesNumber"]); + var publisher = new RecordingHealthPublisher(); + var actor = SpawnDriverActor(driver, publisher); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + AwaitAssert(() => publisher.Published.Count.ShouldBeGreaterThan(0), TimeSpan.FromSeconds(3)); + ExpectNoMsg(TimeSpan.FromMilliseconds(200)); + var before = publisher.Published.Count; + + // A fresh-but-equal report, rebuilt exactly as a redeploy that changed nothing would. + driver.SetReport(authored: 22, skipped: ["Makino/Identity/SeriesNumber"]); + + // Long enough for many heartbeats to fire and be deduped away. + ExpectNoMsg(TimeSpan.FromMilliseconds(600)); + publisher.Published.Count.ShouldBe(before, + "an equal-but-distinct report must be deduped, not re-published — the digest must compare by VALUE"); + } + + /// + /// A driver that cannot report skips publishes null — NOT an empty report. The two mean different + /// things at the UI ("no skip detail available" vs "this driver skipped nothing"), and collapsing + /// them would render an unreporting driver as one that is positively known to be clean. + /// + [Fact] + public void A_driver_that_cannot_report_publishes_null_rather_than_an_empty_report() + { + var publisher = new RecordingHealthPublisher(); + var actor = SpawnDriverActor(new StubDriver(), publisher); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + AwaitAssert(() => publisher.Published.Count.ShouldBeGreaterThan(0), TimeSpan.FromSeconds(3)); + + publisher.Published.ShouldAllBe(p => p.SkippedTags == null); + } + + /// + /// A reporter that throws must not take the health publish down with it. Losing the skip detail is + /// strictly better than losing the whole health channel. + /// + [Fact] + public void A_throwing_reporter_degrades_to_null_without_killing_the_health_publish() + { + var driver = new SkipStubDriver { ThrowOnGetSkippedTags = true }; + var publisher = new RecordingHealthPublisher(); + var actor = SpawnDriverActor(driver, publisher); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + + AwaitAssert(() => publisher.Published.Count.ShouldBeGreaterThan(0), TimeSpan.FromSeconds(3)); + publisher.Published[^1].Health.State.ShouldBe(DriverState.Healthy); + publisher.Published[^1].SkippedTags.ShouldBeNull(); + } + + /// + /// Spawns the actor with a fast health heartbeat. The heartbeat is the production path by which a + /// changed skip report reaches the publisher — unlike host connectivity, a skip raises no event, + /// because it is discovered during the tag-table build and never afterwards. Driving the tests + /// through it rather than through a synthetic poke means the dedup assertions are made against the + /// mechanism that actually runs in production. + /// + private IActorRef SpawnDriverActor(IDriver driver, IDriverHealthPublisher publisher) + { + var parent = CreateTestProbe(); + parent.IgnoreMessages(_ => true); + return parent.ChildActorOf(DriverInstanceActor.Props( + driver, healthPublisher: publisher, healthPollInterval: TimeSpan.FromMilliseconds(50))); + } + + /// Captures every health publish so a test can assert on the skipped-tag field. + private sealed record HealthPublish(DriverHealth Health, SkippedTagReport? SkippedTags); + + private sealed class RecordingHealthPublisher : IDriverHealthPublisher + { + private readonly List _published = []; + + /// Thread-safe snapshot — Publish runs on the actor thread while the test asserts + /// from its own. + public IReadOnlyList Published + { + get { lock (_published) return _published.ToArray(); } + } + + /// + public void Publish( + string clusterId, + string driverInstanceId, + DriverHealth health, + int errorCount5Min, + DateTime? rediscoveryNeededUtc = null, + string? rediscoveryReason = null, + IReadOnlyList? hostStatuses = null, + SkippedTagReport? skippedTags = null) + { + lock (_published) _published.Add(new HealthPublish(health, skippedTags)); + } + } + + /// + /// A stub driver exposing , backed by a real + /// so the test exercises the production accumulator rather than a + /// parallel fake of it. + /// Its own health is deliberately STABLE (a fixed last-read timestamp). The shared + /// StubDriver returns DateTime.UtcNow from GetHealth(), so its fingerprint + /// differs on every call, the dedup never engages, and every dedup assertion here would pass + /// vacuously whether or not the fix is present. + /// + private sealed class SkipStubDriver : IDriver, ISkippedTagReporter + { + private static readonly DateTime FixedLastRead = new(2026, 8, 5, 12, 0, 0, DateTimeKind.Utc); + + private readonly SkippedTagLedger _ledger = new(); + + /// When set, throws — the misbehaving-reporter case. + public bool ThrowOnGetSkippedTags { get; init; } + + /// + public string DriverInstanceId => "skip-stub-1"; + + /// + public string DriverType => "Stub"; + + /// Rebuilds the ledger as a real driver's tag-table build does. + public void SetReport(int authored, IReadOnlyList skipped) + { + _ledger.BeginRebuild(authored); + foreach (var rawPath in skipped) _ledger.Skip(rawPath, "TagConfig did not map to a definition"); + _ledger.Commit(); + } + + /// + public SkippedTagReport GetSkippedTags() + { + if (ThrowOnGetSkippedTags) throw new InvalidOperationException("reporter is broken"); + return _ledger.Snapshot(); + } + + /// + public Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken) => Task.CompletedTask; + + /// + public Task ReinitializeAsync(string driverConfigJson, CancellationToken cancellationToken) => Task.CompletedTask; + + /// + public Task ShutdownAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + /// + public DriverHealth GetHealth() => new(DriverState.Healthy, FixedLastRead, null); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorSubscriptionReconcileTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorSubscriptionReconcileTests.cs index 568e672e..f63e8765 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorSubscriptionReconcileTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorSubscriptionReconcileTests.cs @@ -123,7 +123,8 @@ public sealed class DriverInstanceActorSubscriptionReconcileTests : RuntimeActor int errorCount5Min, DateTime? rediscoveryNeededUtc = null, string? rediscoveryReason = null, - IReadOnlyList? hostStatuses = null) + IReadOnlyList? hostStatuses = null, + SkippedTagReport? skippedTags = null) => Interlocked.Increment(ref _count); } } From dd900f970ae4bb16f1f3e1f6e583ce1297d0ca79 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 20:16:31 -0400 Subject: [PATCH 2/4] fix(hosts): a driver that never built a tag table reports NO ANSWER, not zero skips Found on the docker-dev live gate, not in tests. Two drivers that faulted during initialise (rtu-gate, opcua1) each rendered "0 tags" on /hosts against two authored tags apiece, because a ledger that had never committed returned SkippedTagReport.Empty. A zero report is a positive claim -- "this driver skipped nothing" -- and making it on behalf of a driver that never got as far as looking is exactly the quiet inaccuracy #541 exists to remove. GetSkippedTags() and SkippedTagLedger.Snapshot() now return null until the first committed build, so the column renders the same em-dash it already used for a driver that cannot report at all. Both are 'no answer'; neither is 'nothing skipped'. Knock-on: a FOCAS driver whose only init threw now reports null rather than a zero report, which is strictly more honest than the assertion the test originally made. --- docs/drivers/README.md | 6 +- .../ISkippedTagReporter.cs | 23 +++++--- .../AbCipDriver.cs | 2 +- .../AbLegacyDriver.cs | 2 +- .../CalculationDriver.cs | 2 +- .../FocasDriver.cs | 2 +- .../GalaxyDriver.cs | 2 +- .../MTConnectDriver.cs | 2 +- .../ModbusDriver.cs | 2 +- .../MqttDriver.cs | 2 +- .../OpcUaClientDriver.cs | 2 +- .../ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs | 2 +- .../ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs | 2 +- .../TwinCATDriver.cs | 2 +- .../Components/Pages/Hosts.razor | 7 ++- .../Hosts/HostsDriverView.cs | 7 ++- .../SkippedTagLedgerTests.cs | 58 ++++++++++++++----- .../FocasSkippedTagReportingTests.cs | 18 +++--- .../DriverInstanceActorSkippedTagTests.cs | 2 +- 19 files changed, 94 insertions(+), 51 deletions(-) diff --git a/docs/drivers/README.md b/docs/drivers/README.md index 0a06ffff..9982505f 100644 --- a/docs/drivers/README.md +++ b/docs/drivers/README.md @@ -43,11 +43,13 @@ Reading the Tags column: | Cell | Meaning | |---|---| -| `—` | the driver cannot report skips. **Not** the same as reporting none. | -| `12 tags` | the driver reports, and skipped nothing. | +| `—` | **no answer.** Either the driver cannot report skips, or it never got as far as building a tag table (it faulted during initialise). Deliberately *not* the same as reporting none. | +| `12 tags` | the driver built a table, and skipped nothing. | | `20 / 22 skipped` (amber) | partly working — some tags poll, some do not. | | `22 / 22 skipped` (red) | polling **nothing**. The state most likely to look healthy, because it has no failing reads to report. | +That first row is a distinction the live rig forced. A pre-fix build rendered `0 tags` for two drivers that had faulted during initialise — each with two authored tags — because a never-built ledger returned a zero report. A zero report is a *positive claim* ("this driver skipped nothing"), and making it on behalf of a driver that never looked is the same quiet inaccuracy the column exists to remove. `GetSkippedTags()` now returns null until the first committed build. + Three properties worth knowing: - **A skip never changes driver state.** Latching non-Healthy for a config problem would satisfy both halves of the [#527](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/527) comms-loss conjunction — *90 s without a successful read AND non-Healthy* — and a driver whose tags were all skipped has no successful reads by construction, so it would reconnect-loop forever against a device that is perfectly fine. The signal is additive to the state, not a change of it. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISkippedTagReporter.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISkippedTagReporter.cs index dec8a980..eead5914 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISkippedTagReporter.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ISkippedTagReporter.cs @@ -44,8 +44,15 @@ public interface ISkippedTagReporter /// Must be a pure in-memory read with no I/O — it is called on the health-publish path, is /// deliberately NOT routed through the resilience pipeline, and must never block. /// - /// The current report; never null. An all-zero report means "nothing skipped". - SkippedTagReport GetSkippedTags(); + /// + /// The report from the driver's most recent tag-table build, or when it has + /// not built one yet — a driver that faulted during initialise has no answer, which is not + /// the same as the answer "nothing skipped". Returning a zero report there would state that a + /// driver with a dozen authored tags has none, which is the class of quiet inaccuracy this whole + /// capability exists to remove. (Found on the docker-dev rig, where two faulted drivers each + /// rendered "0 tags" against two authored ones.) + /// + SkippedTagReport? GetSkippedTags(); } /// One authored tag a driver could not build into its poll table, and why. @@ -71,9 +78,6 @@ public sealed record SkippedTag(string RawPath, string Reason); /// public sealed record SkippedTagReport(int AuthoredCount, int SkippedCount, IReadOnlyList Sample) { - /// A driver that has authored nothing and skipped nothing. - public static readonly SkippedTagReport Empty = new(0, 0, []); - /// Whether any authored tag was skipped — i.e. whether this is worth an operator's attention. public bool HasSkips => SkippedCount > 0; @@ -113,7 +117,9 @@ public sealed class SkippedTagLedger public const int MaxSample = 20; private readonly object _gate = new(); - private SkippedTagReport _visible = SkippedTagReport.Empty; + + /// Null until the first — "no answer yet", never a zero report. + private SkippedTagReport? _visible; private int _stagedAuthored; private int _stagedSkipped; @@ -168,8 +174,9 @@ public sealed class SkippedTagLedger } /// The current published snapshot. - /// The last committed report, or before the first commit. - public SkippedTagReport Snapshot() + /// The last committed report, or before the first commit — a driver + /// that has not built a tag table has no answer, and a zero report would be a false one. + public SkippedTagReport? Snapshot() { lock (_gate) return _visible; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index 3ee99b70..2d6c6d06 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -31,7 +31,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, private readonly SkippedTagLedger _skippedTags = new(); /// - public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + public SkippedTagReport? GetSkippedTags() => _skippedTags.Snapshot(); private AbCipDriverOptions _options; private readonly string _driverInstanceId; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs index ea28176b..3d6aed9b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs @@ -17,7 +17,7 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover private readonly SkippedTagLedger _skippedTags = new(); /// - public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + public SkippedTagReport? GetSkippedTags() => _skippedTags.Snapshot(); /// Mutable so can adopt a re-parsed config on reinitialize /// (#516). Only ever written on the init path, before any reader/session uses it. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriver.cs index 92f0cc14..3c1d9f2f 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriver.cs @@ -42,7 +42,7 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba private readonly SkippedTagLedger _skippedTags = new(); /// - public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + public SkippedTagReport? GetSkippedTags() => _skippedTags.Snapshot(); // OPC UA status codes surfaced by the calc driver. private const uint StatusGood = 0u; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs index 0dc16118..d06529be 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -27,7 +27,7 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, private readonly SkippedTagLedger _skippedTags = new(); /// - public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + public SkippedTagReport? GetSkippedTags() => _skippedTags.Snapshot(); /// Mutable so can adopt a re-parsed config (#516). Written only /// on the init path, together with , before either is used. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs index b1366ea2..153160c6 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs @@ -45,7 +45,7 @@ public sealed class GalaxyDriver private readonly SkippedTagLedger _skippedTags = new(); /// - public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + public SkippedTagReport? GetSkippedTags() => _skippedTags.Snapshot(); private readonly string _driverInstanceId; private readonly GalaxyDriverOptions _options; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs index bf313b20..6302aa9f 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -79,7 +79,7 @@ public sealed class MTConnectDriver private readonly SkippedTagLedger _skippedTags = new(); /// - public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + public SkippedTagReport? GetSkippedTags() => _skippedTags.Snapshot(); /// /// The Agent could not be reached, answered unusably, or blew the per-call deadline. Distinct diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs index 9f1153f6..45d358f5 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs @@ -25,7 +25,7 @@ public sealed class ModbusDriver private readonly SkippedTagLedger _skippedTags = new(); /// - public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + public SkippedTagReport? GetSkippedTags() => _skippedTags.Snapshot(); // ---- instance fields (grouped at top for auditability) ---- diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs index f1fee36c..ab4d3758 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs @@ -70,7 +70,7 @@ public sealed class MqttDriver private readonly SkippedTagLedger _skippedTags = new(); /// - public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + public SkippedTagReport? GetSkippedTags() => _skippedTags.Snapshot(); // ---- Write status codes (Gitea #508) ---- // diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs index 28639053..80ddbd8e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -37,7 +37,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit private readonly SkippedTagLedger _skippedTags = new(); /// - public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + public SkippedTagReport? GetSkippedTags() => _skippedTags.Snapshot(); /// /// Recomputes the skip report from the freshly-built map. Derived by re-asking the map to resolve diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs index 89c7a6c8..04ebf8ae 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs @@ -36,7 +36,7 @@ public sealed class S7Driver private readonly SkippedTagLedger _skippedTags = new(); /// - public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + public SkippedTagReport? GetSkippedTags() => _skippedTags.Snapshot(); private readonly string _driverInstanceId; private readonly ILogger _logger; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs index 2058d4cb..bdaa1702 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs @@ -46,7 +46,7 @@ public sealed class SqlDriver private IReadOnlyList _blobSkips = []; /// - public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + public SkippedTagReport? GetSkippedTags() => _skippedTags.Snapshot(); /// /// Republishes the skip ledger as the union of the blob-level skips and the catalog gate's diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs index 210cd29a..1775959f 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs @@ -17,7 +17,7 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery private readonly SkippedTagLedger _skippedTags = new(); /// - public SkippedTagReport GetSkippedTags() => _skippedTags.Snapshot(); + public SkippedTagReport? GetSkippedTags() => _skippedTags.Snapshot(); // Mutable so ReinitializeAsync can apply a new config generation. The constructor seeds // it; InitializeAsync re-parses driverConfigJson over the top of it. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor index 0957c267..80ca1f26 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor @@ -234,8 +234,11 @@ else Healthy, Errors/5 min stays 0, Last error stays empty, and Last read keeps advancing off the tags that DID map. On the Makino in #540 that read as a perfectly healthy driver ignoring 20 of its 22 - tags. "—" means the driver cannot report skips, which is distinct - from reporting none. *@ + tags. "—" means NO ANSWER — either the driver cannot report skips, + or it faulted before building a tag table — which is distinct from + the positive claim "this driver skipped nothing". A pre-fix build + rendered "0 tags" for two faulted drivers on the docker-dev rig + that had two authored tags apiece. *@ @if (d.SkippedTags is null) { — diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/HostsDriverView.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/HostsDriverView.cs index 0cadfb89..62e752d5 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/HostsDriverView.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hosts/HostsDriverView.cs @@ -44,9 +44,10 @@ public sealed record HostsDriverInstanceInfo(string DriverInstanceId, string Clu /// Per-host connectivity for a multi-device driver, ordered by host name; null /// when the driver reports no per-host detail. Lets one unreachable device show even while the driver /// row itself is Healthy. -/// Authored tags the driver could not build into its poll table; null when the -/// driver cannot report them. Lets "ignoring 20 of 22 tags" show even while every other column on the -/// row — state, last read, error count, last error — reads perfectly healthy (Gitea #541). +/// Authored tags the driver could not build into its poll table; null when there +/// is no answer — the driver cannot report skips, or it faulted before building a tag table. Lets +/// "ignoring 20 of 22 tags" show even while every other column on the row — state, last read, error +/// count, last error — reads perfectly healthy (Gitea #541). public sealed record HostsDriverRow( string DriverInstanceId, string? Name, string? DriverType, string State, DateTime? LastSuccessfulReadUtc, string? LastError, int ErrorCount5Min, DateTime PublishedUtc, diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/SkippedTagLedgerTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/SkippedTagLedgerTests.cs index eb19b7d9..63a20cde 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/SkippedTagLedgerTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/SkippedTagLedgerTests.cs @@ -9,17 +9,45 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests; /// public sealed class SkippedTagLedgerTests { + /// + /// A ledger that has never built reports NO ANSWER, not a clean bill of health. Caught on + /// the docker-dev rig: two drivers that faulted during initialise each rendered "0 tags" on + /// /hosts against two authored tags apiece. A zero report is a positive claim — "this driver + /// skipped nothing" — and making it on behalf of a driver that never got as far as looking is the + /// same species of quiet inaccuracy this capability exists to remove. + /// [Fact] - public void A_fresh_ledger_reports_nothing() - { - var report = new SkippedTagLedger().Snapshot(); + public void A_ledger_that_has_never_built_reports_no_answer_rather_than_a_clean_one() + => new SkippedTagLedger().Snapshot().ShouldBeNull(); - report.AuthoredCount.ShouldBe(0); + /// An empty deployment that DID build reports a zero report — distinct from the above. + [Fact] + public void A_committed_empty_build_reports_zero_rather_than_no_answer() + { + var ledger = new SkippedTagLedger(); + ledger.BeginRebuild(0); + ledger.Commit(); + + var report = ledger.Snapshot(); + report.ShouldNotBeNull(); + report!.AuthoredCount.ShouldBe(0); report.SkippedCount.ShouldBe(0); - report.Sample.ShouldBeEmpty(); report.HasSkips.ShouldBeFalse(); } + /// + /// An in-flight first build is still "no answer" — the staging state must not leak as a report. + /// + [Fact] + public void A_first_build_still_in_progress_reports_no_answer() + { + var ledger = new SkippedTagLedger(); + ledger.BeginRebuild(10); + ledger.Skip("a", "r"); + + ledger.Snapshot().ShouldBeNull(); + } + [Fact] public void A_committed_build_reports_its_skips_with_the_authored_denominator() { @@ -30,7 +58,7 @@ public sealed class SkippedTagLedgerTests ledger.Skip("Line1/Makino/Identity/Version", "invalid Address"); ledger.Commit(); - var report = ledger.Snapshot(); + var report = ledger.Snapshot()!; report.AuthoredCount.ShouldBe(22); report.SkippedCount.ShouldBe(2); report.Sample.Select(s => s.RawPath) @@ -54,7 +82,7 @@ public sealed class SkippedTagLedgerTests ledger.Skip("c", "r"); ledger.Commit(); - ledger.Snapshot().SkippedEverything.ShouldBeTrue(); + ledger.Snapshot()!.SkippedEverything.ShouldBeTrue(); } /// @@ -70,7 +98,7 @@ public sealed class SkippedTagLedgerTests ledger.BeginRebuild(0); ledger.Commit(); - ledger.Snapshot().SkippedEverything.ShouldBeFalse(); + ledger.Snapshot()!.SkippedEverything.ShouldBeFalse(); } /// @@ -86,7 +114,7 @@ public sealed class SkippedTagLedgerTests for (var i = 0; i < 500; i++) ledger.Skip($"tag-{i}", "TagConfig did not map to a definition"); ledger.Commit(); - var report = ledger.Snapshot(); + var report = ledger.Snapshot()!; report.SkippedCount.ShouldBe(500); report.Sample.Count.ShouldBe(SkippedTagLedger.MaxSample); report.Sample[0].RawPath.ShouldBe("tag-0", "the sample keeps the FIRST skips, in encounter order"); @@ -108,13 +136,13 @@ public sealed class SkippedTagLedgerTests ledger.BeginRebuild(10); ledger.Skip("new-1", "r"); - var midRebuild = ledger.Snapshot(); + var midRebuild = ledger.Snapshot()!; midRebuild.SkippedCount.ShouldBe(1); midRebuild.Sample.Single().RawPath.ShouldBe("old"); ledger.Skip("new-2", "r"); ledger.Commit(); - ledger.Snapshot().Sample.Select(s => s.RawPath).ShouldBe(["new-1", "new-2"]); + ledger.Snapshot()!.Sample.Select(s => s.RawPath).ShouldBe(["new-1", "new-2"]); } /// @@ -133,7 +161,7 @@ public sealed class SkippedTagLedgerTests ledger.Skip("would-have-been-reported", "invalid Address"); // …driver throws here; no Commit. - ledger.Snapshot().Sample.Single().RawPath.ShouldBe("known-bad"); + ledger.Snapshot()!.Sample.Single().RawPath.ShouldBe("known-bad"); } /// @@ -151,7 +179,7 @@ public sealed class SkippedTagLedgerTests ledger.BeginRebuild(2); ledger.Commit(); - var report = ledger.Snapshot(); + var report = ledger.Snapshot()!; report.HasSkips.ShouldBeFalse(); report.SkippedCount.ShouldBe(0); report.AuthoredCount.ShouldBe(2); @@ -167,7 +195,7 @@ public sealed class SkippedTagLedgerTests var ledger = new SkippedTagLedger(); Should.NotThrow(() => ledger.Skip("orphan", "r")); - ledger.Snapshot().SkippedCount.ShouldBe(0); + ledger.Snapshot().ShouldBeNull("a stray Skip must not conjure a report out of nothing"); } /// @@ -197,7 +225,7 @@ public sealed class SkippedTagLedgerTests { while (!writer.IsCompleted && !cts.IsCancellationRequested) { - var r = ledger.Snapshot(); + var r = ledger.Snapshot()!; // Every committed snapshot is internally consistent: the sample never exceeds either the // cap or the true count it is drawn from. r.Sample.Count.ShouldBeLessThanOrEqualTo(SkippedTagLedger.MaxSample); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasSkippedTagReportingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasSkippedTagReportingTests.cs index 35919234..75474ae9 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasSkippedTagReportingTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasSkippedTagReportingTests.cs @@ -42,7 +42,7 @@ public sealed class FocasSkippedTagReportingTests await drv.InitializeAsync("{}", CancellationToken.None); - var report = drv.GetSkippedTags(); + var report = drv.GetSkippedTags()!; report.AuthoredCount.ShouldBe(2); report.SkippedCount.ShouldBe(0); report.HasSkips.ShouldBeFalse(); @@ -75,7 +75,7 @@ public sealed class FocasSkippedTagReportingTests drv.GetHealth().LastError.ShouldBeNull(); // …and the fact that contradicts it. - var report = drv.GetSkippedTags(); + var report = drv.GetSkippedTags()!; report.AuthoredCount.ShouldBe(22); report.SkippedCount.ShouldBe(20); report.SkippedEverything.ShouldBeFalse("two tags do poll — this driver is partly working"); @@ -97,7 +97,7 @@ public sealed class FocasSkippedTagReportingTests await drv.InitializeAsync("{}", CancellationToken.None); - var report = drv.GetSkippedTags(); + var report = drv.GetSkippedTags()!; report.SkippedCount.ShouldBe(25); report.Sample.Count.ShouldBe(SkippedTagLedger.MaxSample); report.SkippedEverything.ShouldBeTrue("this driver polls nothing at all"); @@ -116,11 +116,11 @@ public sealed class FocasSkippedTagReportingTests new FocasTagDefinition("raw/Good", Host, "MACRO:500", FocasDataType.Float64)); await drv.InitializeAsync("{}", CancellationToken.None); - drv.GetSkippedTags().SkippedCount.ShouldBe(1); + drv.GetSkippedTags()!.SkippedCount.ShouldBe(1); await drv.InitializeAsync("{}", CancellationToken.None); - var report = drv.GetSkippedTags(); + var report = drv.GetSkippedTags()!; report.SkippedCount.ShouldBe(1, "the same one bad tag — not two"); report.AuthoredCount.ShouldBe(2); } @@ -145,7 +145,7 @@ public sealed class FocasSkippedTagReportingTests }; var drv = new FocasDriver(good, "drv-1", new FakeFocasClientFactory()); await drv.InitializeAsync("{}", CancellationToken.None); - drv.GetSkippedTags().Sample.Single().RawPath.ShouldBe("raw/Bad"); + drv.GetSkippedTags()!.Sample.Single().RawPath.ShouldBe("raw/Bad"); // A second driver over an artifact whose tag names a device that is not configured: the loop skips // the first entry, then throws on the second. @@ -164,7 +164,9 @@ public sealed class FocasSkippedTagReportingTests await Should.ThrowAsync( () => drv2.InitializeAsync("{}", CancellationToken.None)); - drv2.GetSkippedTags().SkippedCount.ShouldBe(0, - "an abandoned build must not publish; a partial report would read as 'we skip fewer tags now'"); + // NO ANSWER, not "zero skipped": this driver's only build threw, so it never reached a verdict. + // A zero report here would be the same false reassurance the docker-dev rig showed for two + // faulted drivers, and a PARTIAL one would read as "we skip fewer tags now". + drv2.GetSkippedTags().ShouldBeNull(); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorSkippedTagTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorSkippedTagTests.cs index 8491de2d..090aa352 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorSkippedTagTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorSkippedTagTests.cs @@ -248,7 +248,7 @@ public sealed class DriverInstanceActorSkippedTagTests : RuntimeActorTestBase } /// - public SkippedTagReport GetSkippedTags() + public SkippedTagReport? GetSkippedTags() { if (ThrowOnGetSkippedTags) throw new InvalidOperationException("reporter is broken"); return _ledger.Snapshot(); From b25279dbbd238da3da8c6e05db6c68c3f99d9e0f Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 20:26:00 -0400 Subject: [PATCH 3/4] docs: live-gate record for #541 on docker-dev (PASS; found the 0-tags defect) --- ...-08-05-skipped-tag-visibility-live-gate.md | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 docs/plans/2026-08-05-skipped-tag-visibility-live-gate.md diff --git a/docs/plans/2026-08-05-skipped-tag-visibility-live-gate.md b/docs/plans/2026-08-05-skipped-tag-visibility-live-gate.md new file mode 100644 index 00000000..8410f7e3 --- /dev/null +++ b/docs/plans/2026-08-05-skipped-tag-visibility-live-gate.md @@ -0,0 +1,94 @@ +# Live gate — skipped-tag visibility (Gitea #541) + +**Date:** 2026-08-05 +**Rig:** local `docker-dev/` (OrbStack), MAIN pair `central-1` / `central-2`, AdminUI `http://localhost:9200` +**Build under test:** `fix/541-skipped-tag-visibility` +**Result: PASS**, and the gate found one defect that unit tests did not (§4). + +## What needed proving + +The unit suites pin the ledger semantics, the publish fingerprint, the proto round-trip and the +view-model. None of them prove the thing an operator actually depends on: that the **Tags** column on +`/hosts` renders the right cell, in the right colour, with actionable detail, against a driver that is +genuinely skipping tags on a real deployment. + +## Method + +The rig already carried eleven driver instances. Three of the thirteen tags on the `pymodbus` Modbus +driver had their `TagConfig.region` rewritten to `NoSuchRegion` — the realistic shape of a bad blob — +then MAIN was redeployed via `POST /api/deployments`. Backup of the original thirteen blobs taken +first; all three restored and redeployed at the end (§5). + +## Results + +Driver logs confirmed the skips reached the driver: + +``` +[WRN] Modbus tag config did not map to a definition; skipping. Driver=MAIN-modbus RawPath=pymodbus/plc/Probe3 +[WRN] Modbus tag config did not map to a definition; skipping. Driver=MAIN-modbus RawPath=pymodbus/plc/Probe4 +[WRN] Modbus tag config did not map to a definition; skipping. Driver=MAIN-modbus RawPath=pymodbus/plc/Probe5 +``` + +`/hosts` then rendered — note every other column on the `pymodbus` row reads perfectly healthy, which +is the entire point: + +| Driver | Status | Errors/5 min | Last error | Last read | **Tags** | Chip | +|---|---|---|---|---|---|---| +| `pymodbus` | HEALTHY | 0 | — | advancing | **3 / 13 SKIPPED** | `chip-warn` (amber) | +| `sql-oracle-fixture` | HEALTHY | 0 | — | advancing | **1 / 1 SKIPPED** | `chip-bad` (red) | +| `sql-postgres-fixture` | HEALTHY | 0 | — | advancing | **1 / 1 SKIPPED** | `chip-bad` (red) | +| `gate-modbus-renamed` | HEALTHY | 0 | — | advancing | **1 / 1 SKIPPED** | `chip-bad` (red) | +| `abcip-browse` | HEALTHY | 0 | — | advancing | 5 tags | none | +| `adsfa` (Calculation) | HEALTHY | 0 | — | — | 0 tags | none | +| `opcua1` | FAULTED | 1 | (config error) | — | — | none | +| `rtu-gate` | FAULTED | 1 | Connection refused | — | — | none | + +Tooltip on the amber chip, verbatim: + +> 3 of 13 authored tag(s) are not being polled. They report BadNodeIdUnknown and the driver's state +> does not reflect them. pymodbus/plc/Probe3: TagConfig did not map to a definition · +> pymodbus/plc/Probe4: … · pymodbus/plc/Probe5: … + +and on a red one: + +> This driver is polling NONE of its 1 authored tag(s). … + +**Three of the four skipping drivers were not planted.** `sql-oracle-fixture`, `sql-postgres-fixture` +and `gate-modbus-renamed` were already misconfigured on this rig and had been silently skipping their +tags for an unknown length of time, each reporting Healthy with zero errors. The column surfaced them +on its first run — an unplanned but fairly direct demonstration of the defect being real rather than +theoretical. + +Restoring the three blobs and redeploying returned `pymodbus` to `13 tags` with no chip, which also +exercises the clear-on-clean-rebuild path live. + +## 4. Defect the gate found — `0 tags` on a driver that never built a table + +The first pass rendered **`0 tags`** for `opcua1` and `rtu-gate`. Both are FAULTED, both have **two +authored tags** in the ConfigDb, and neither ever completed `InitializeAsync` — so their ledgers had +never committed and `Snapshot()` returned `SkippedTagReport.Empty`. + +A zero report is a **positive claim** — *"this driver skipped nothing"* — and making it on behalf of a +driver that never got as far as looking is precisely the quiet inaccuracy #541 exists to remove. Not +caught by unit tests because every one of them drove a driver through a completed build first. + +Fixed by making `ISkippedTagReporter.GetSkippedTags()` and `SkippedTagLedger.Snapshot()` return +**null until the first committed build**, so the column shows the same em-dash it already used for a +driver that cannot report at all. Both are *no answer*; neither is *nothing skipped*. Pinned by three +new `SkippedTagLedgerTests` cases plus a corrected FOCAS assertion. Re-gated on a rebuilt image: both +faulted drivers now render `—`, while `adsfa` — which genuinely has zero authored tags and **did** +build — correctly still renders `0 tags`. That pair is the discriminating case. + +## 5. Rig left as found + +The three `pymodbus` blobs were restored from the pre-change backup and MAIN redeployed; `/hosts` +shows `13 tags`. The three pre-existing skipping drivers were left exactly as they were — they are the +rig's own configuration, not this gate's. + +## Not proven here + +- The `+N more not shown` truncation note (needs a deployment skipping more than + `SkippedTagLedger.MaxSample` = 20 tags; unit-covered only). +- The central-side proto path — the rig's central pair is fused admin+driver, so the health snapshot + never crossed the Phase 5 gRPC telemetry stream. `TelemetryProtoMapCentral`/`Node` are unit-covered + round-trip only. From 6e9e4fa5ab3252617a55145d26cfaa46a7e593ba Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 02:12:44 -0400 Subject: [PATCH 4/4] =?UTF-8?q?docs:=20live-gate=20round=20two=20=E2=80=94?= =?UTF-8?q?=20truncation=20note=20+=20central=20proto=20path=20both=20prov?= =?UTF-8?q?en=20(#541)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 25 unmappable tags on SITE-A's sa-modbus2 closed both gaps in one setup: chip 25/26 SKIPPED, sample capped at exactly MaxSample=20, tooltip '(+5 more not shown)'. Trimming to 20 skipped gave 20/21 with NO truncation note, pinning the off-by-one boundary. SITE-A is the discriminator for the proto path. Phase 6 partitioned it into its own Akka mesh -- central's MEMBERS panel lists only central-1/central-2, and DistributedPubSub does not federate across meshes. The single site-a reference in central's log is the ClusterClient receptionist, a point-to-point command path, not gossip. So the report's only route to the page is TelemetryProtoMapNode -> gRPC -> TelemetryProtoMapCentral, and central's view tracking a change made solely on the site node is conclusive. Also corrects round one's claim that the fused central pair meant telemetry never crossed gRPC -- wrong about this rig. All six ClusterNode rows carry GrpcPort 4056 and Telemetry__Mode=Grpc; the two 'Telemetry stream to node' lines that suggested a two-node dial set were startup failure warnings. Rig restored: test tags deleted, SITE-A redeployed, sa-modbus2 back to 1 tag. --- ...-08-05-skipped-tag-visibility-live-gate.md | 52 ++++++++++++++++--- 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/docs/plans/2026-08-05-skipped-tag-visibility-live-gate.md b/docs/plans/2026-08-05-skipped-tag-visibility-live-gate.md index 8410f7e3..6080625e 100644 --- a/docs/plans/2026-08-05-skipped-tag-visibility-live-gate.md +++ b/docs/plans/2026-08-05-skipped-tag-visibility-live-gate.md @@ -3,7 +3,8 @@ **Date:** 2026-08-05 **Rig:** local `docker-dev/` (OrbStack), MAIN pair `central-1` / `central-2`, AdminUI `http://localhost:9200` **Build under test:** `fix/541-skipped-tag-visibility` -**Result: PASS**, and the gate found one defect that unit tests did not (§4). +**Result: PASS.** The gate found one defect unit tests did not (§4). Round two on 2026-08-06 closed +the two items round one left unproven and corrected a wrong claim about the rig's topology (§6). ## What needed proving @@ -85,10 +86,47 @@ The three `pymodbus` blobs were restored from the pre-change backup and MAIN red shows `13 tags`. The three pre-existing skipping drivers were left exactly as they were — they are the rig's own configuration, not this gate's. -## Not proven here +## 6. Round two (2026-08-06) — truncation note + the central proto path -- The `+N more not shown` truncation note (needs a deployment skipping more than - `SkippedTagLedger.MaxSample` = 20 tags; unit-covered only). -- The central-side proto path — the rig's central pair is fused admin+driver, so the health snapshot - never crossed the Phase 5 gRPC telemetry stream. `TelemetryProtoMapCentral`/`Node` are unit-covered - round-trip only. +Both items round one left unproven were closed by one setup, chosen so that a single observation +settles both. **One correction first:** round one recorded "the rig's central pair is fused +admin+driver, so the snapshot never crossed the Phase 5 gRPC telemetry stream." That was wrong about +the rig. All six `ClusterNode` rows carry `GrpcPort 4056`, every node runs `Telemetry__Mode=Grpc`, and +central dials all six — the two `Telemetry stream to node …` lines seen in round one were startup +*failure warnings*, not the dial set. + +**Setup.** 25 tags with `region: NoSuchRegion` added to `sa-modbus2` on **SITE-A**, joining its one +good tag (26 total), then SITE-A deployed. SITE-A is the discriminator: Phase 6 partitioned it into +its own Akka mesh, so central's MEMBERS panel lists only `central-1:4053` and `central-2:4053`. +DistributedPubSub fans out within one mesh's mediator and does not federate, and the single +`akka.tcp://otopcua@site-a-1` reference in central's log is the ClusterClient **receptionist** — a +point-to-point command path, not gossip. So a SITE-A driver's health can reach central's `/hosts` by +exactly one route: `TelemetryProtoMapNode` → gRPC `TelemetryStreamService/Subscribe` → +`TelemetryProtoMapCentral`. + +**Result — 25 skipped of 26 authored:** + +| Fact | Observed | +|---|---| +| Chip | `25 / 26 SKIPPED`, `chip-warn` | +| Driver status | HEALTHY, 0 errors (as ever) | +| Sample entries in tooltip | **20** (= `SkippedTagLedger.MaxSample`) | +| Truncation note | **`(+5 more not shown)`** | +| Skips logged on `site-a-1` | 25 | + +That proves the truncation note, and simultaneously proves the proto path: the counts *and* all 20 +sample entries with their RawPath + reason survived the wire. + +**Positive control, doubling as the cap boundary.** Five of the 25 broken tags were deleted and SITE-A +redeployed. Central's page tracked the change made only on the site node — `20 / 21 SKIPPED`, sample +still 20, and **no truncation note**, correct at exactly `MaxSample` (the off-by-one that would have +read "+0 more not shown"). A live-updating view of a node with no gossip path is conclusive. + +**Rig restored:** all 25 test tags deleted, SITE-A redeployed, `sa-modbus2` back to `1 tag`; the +deployment revision hash returned to the pre-test `6178a8f3…`. + +## Still not proven live + +Nothing outstanding from the original two. Remaining unit-only coverage is the ledger's concurrency +guard (a torn read under a real init/publish interleaving) — inherently hard to force on a rig, and +the lock makes it a code-inspection matter rather than an observational one.