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); } }