diff --git a/docs/requirements/Component-DataConnectionLayer.md b/docs/requirements/Component-DataConnectionLayer.md index 93d00b0c..d9c68d08 100644 --- a/docs/requirements/Component-DataConnectionLayer.md +++ b/docs/requirements/Component-DataConnectionLayer.md @@ -414,7 +414,9 @@ Note: Pre-deployment validation at central does **not** verify that tag paths re The DCL reports the following metrics to the Health Monitoring component via the existing periodic heartbeat: - **Connection status**: `connected`, `disconnected`, or `reconnecting` per data connection. -- **Tag resolution counts**: Per connection, the number of total subscribed tags vs. successfully resolved tags. This gives operators visibility into misconfigured templates without needing to open the debug view for individual instances. Both numbers are **derived at report time from the connection actor's authoritative per-tag state**, never accumulated in counters: the total is the number of distinct tag paths at least one instance currently subscribes to (the per-tag counted set that also drives value fan-out and the last-subscriber release decision), and the resolved count is the number of tags for which the adapter currently holds a subscription handle. A tag counts toward the total from the moment an instance registers it, whatever its resolution outcome — resolved, awaiting a resolution retry, or failed at connection level — so resolved can never exceed total. Deriving rather than accumulating is deliberate: increment/decrement counters drifted whenever an unsubscribe landed inside a reconnect window (the reconnect clears the per-tag maps the decrements keyed off), leaking a phantom tag into the reported total on every churn cycle. +- **Tag resolution counts**: Per connection, the number of total subscribed tags vs. successfully resolved tags. This gives operators visibility into misconfigured templates without needing to open the debug view for individual instances. Both numbers are **derived from the connection actor's authoritative per-tag state**, never accumulated in counters: the total is the number of distinct tag paths at least one instance currently subscribes to (the per-tag counted set that also drives value fan-out and the last-subscriber release decision), and the resolved count is the number of tags for which the adapter currently holds a subscription handle. A tag counts toward the total from the moment an instance registers it, whatever its resolution outcome — resolved, awaiting a resolution retry, or failed at connection level — so resolved can never exceed total. Deriving rather than accumulating is deliberate: increment/decrement counters drifted whenever an unsubscribe landed inside a reconnect window (the reconnect clears the per-tag maps the decrements keyed off), leaking a phantom tag into the reported total on every churn cycle. + + The pair is **pushed to the health collector, not pulled from it**, so it is a cached value that is only as fresh as its last push — and the collector holds it until the next one. Every site that mutates either underlying set must therefore push: entering the Connected state (a legitimate 0/0 — no instance has subscribed yet), the first-subscribe apply, the batch apply (reconnect re-subscribe and resolution probe), and unsubscribe. Missing the first-subscribe push is what made a *perfectly healthy* connection report 0/0 indefinitely — the connect-time 0/0 was never superseded, because a connection whose tags all resolve on the first attempt never reaches the batch path and has its resolution-retry timer cancelled outright for having nothing unresolved. That is the reverse of the usual staleness risk, and it blinded the diagnostic exactly when it should have read clean (deferred-work register row 55; the actor's own `GetHealthReport` reply, which really is derived on demand, was correct throughout). - **Tag quality counters** are pushed on a genuine quality **transition** only, coalesced onto a `QualityFlushInterval` (1s) single-shot timer. Counter arithmetic still runs per message; only the collector push is deferred, and a value whose quality is unchanged moves no counter at all. Health reports poll at 30s, so the coalescing loses nothing. Three paths flush **synchronously** because their correctness depends on it: the bad-quality push on disconnect, unsubscribe, and the reconnect counter reset. ## Dependencies diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Actors/DataConnectionActor.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Actors/DataConnectionActor.cs index 9cd2c598..e69880d2 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Actors/DataConnectionActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Actors/DataConnectionActor.cs @@ -1275,6 +1275,20 @@ public class DataConnectionActor : UntypedActor, IWithStash, IWithTimers // Fire-and-forget release of every handle this request created redundantly. _ = UnsubscribeIdsAsync(_adapter, redundantIds); + // Publish the resolution counts this request just changed. Without this push a + // connection whose tags ALL resolve on the first attempt reported 0/0 forever: + // the only other pushes are BecomeConnected's (which fires before any instance + // has subscribed, so its 0/0 is legitimate but is never superseded), + // HandleBatchSubscribeCompleted's (reconnect re-subscribe / resolution probe) and + // HandleUnsubscribe's (teardown) — and ScheduleTagResolutionRetry below cancels + // the retry timer outright when nothing is unresolved, so no later tick pushed + // either. The blind spot was therefore worst on a perfectly healthy connection, + // which is precisely when the "device still booting / tag path unresolved" + // diagnostic is supposed to read clean rather than empty. Mirrors the push site + // in HandleBatchSubscribeCompleted, and stays on the actor thread where both + // derived counts are stable. + _healthCollector.UpdateTagResolution(_connectionName, TotalSubscribedTagCount, ResolvedTagCount); + // Now that every tag is registered in // _subscriptionsByInstance, deliver the values captured by the initial read. // Re-entering via Self reuses HandleTagValueReceived's generation guard, fan-out diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorTests.cs index 6bd888e5..5ecf6797 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorTests.cs @@ -1770,4 +1770,61 @@ public class DataConnectionActorTests : TestKit // Belt-and-braces: after the fresh value, no further (stale) update may arrive. ExpectNoMsg(TimeSpan.FromMilliseconds(500)); } + + // ── Deferred register row 55: first-subscribe resolution counts never reached the collector ── + + [Fact] + public async Task FirstSubscribe_AllTagsResolve_PushesResolutionCountsToHealthCollector() + { + // Regression test for deferred-work register row 55, reproduced live on the docker + // rig: site-a's health report showed + // tagResolutionCounts = { "OPC PLC Simulator": { totalSubscribed: 0, resolved: 0 } } + // while dataConnectionTagQuality on the SAME report (same collector, same active + // node) said good: 2. + // + // Mechanism: the ONLY health push on the happy path was BecomeConnected's, which + // fires BEFORE any instance has subscribed and is therefore a legitimate 0/0. + // HandleSubscribeCompleted — the path that registers tags on a first, healthy + // subscribe — mutated _instancesByTag and _subscriptionIds but pushed nothing, so + // that 0/0 was never superseded. The two sites that DO push are + // HandleBatchSubscribeCompleted (reconnect re-subscribe / resolution probe) and + // HandleUnsubscribe (teardown); neither runs on a connection whose tags all + // resolve first time, and ScheduleTagResolutionRetry cancels the retry timer + // outright when _unresolvedTags is empty. Result: a fully healthy connection + // reported 0/0 forever, blinding the "device still booting / tag path unresolved" + // diagnostic exactly when it is supposed to read clean. + // + // The actor's OWN derived counts were correct the whole time — that asymmetry is + // the signature of the bug, so this test pins both halves. + _mockAdapter.ConnectAsync(Arg.Any>(), Arg.Any()) + .Returns(Task.CompletedTask); + _mockAdapter.Status.Returns(ConnectionHealth.Connected); + _mockAdapter.SubscribeAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(ci => Task.FromResult("sub-" + ci.ArgAt(0))); + _mockAdapter.ReadAsync(Arg.Any(), Arg.Any()) + .Returns(new ReadResult(false, null, null)); + + var actor = CreateConnectionActor("row55-first-subscribe"); + await Task.Delay(300); + + // BecomeConnected has now pushed its legitimate 0/0 (no tags subscribed yet). + _mockHealthCollector.Received().UpdateTagResolution("row55-first-subscribe", 0, 0); + + // One instance subscribes to two tags; both resolve on the first attempt, so the + // batch/probe path never runs — exactly the rig's rig-probe-1 shape. + actor.Tell(new SubscribeTagsRequest( + "c1", "inst1", "row55-first-subscribe", + ["sensor/temp", "sensor/pressure"], DateTimeOffset.UtcNow)); + ExpectMsg(TimeSpan.FromSeconds(5)); + + // The actor's own derived counts are correct — this half passed pre-fix too. + actor.Tell(new DataConnectionActor.GetHealthReport()); + var report = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal(2, report.TotalSubscribedTags); + Assert.Equal(2, report.ResolvedTags); + + // The collector must agree. Pre-fix this is the failing assertion: the only call + // ever recorded was the (0, 0) from BecomeConnected. + _mockHealthCollector.Received().UpdateTagResolution("row55-first-subscribe", 2, 2); + } }