feat(drivers): every driver reports the authored tags it skipped (Gitea #541)
A driver builds its RawPath -> definition table at init and tolerates a
per-tag miss: an unmappable TagConfig, an unparseable address, a
capability-matrix rejection. That tolerance is right -- one bad tag must not
take down the other few thousand. What was missing is any statement of the
fact afterwards. A skip is not an error and happens before any read, so the
driver stays Healthy, errors/5 min stays 0, Last error stays empty, and Last
read advances live off the tags that did map. On the wonder Makino in #540
that reading held while focas-z-34184 ignored 20 of its 22 tags.
New optional capability ISkippedTagReporter, backed by a shared
SkippedTagLedger so the reset/commit/cap rules are one implementation rather
than one per driver. The report -- authored count, exact skipped count, and a
capped sample of RawPath + reason -- rides DriverHealthChanged to a new Tags
column on /hosts, over the Phase 5 telemetry stream. No table: a driver-only
node has no ConfigDb to write to, the lesson from #521.
The issue named eight drivers. It is twelve -- every driver in
DriverTypeNames.All has an authored-tag skip path. Calculation (no scriptId),
MTConnect (no readable DataItem id), Galaxy (no attributeRef) and OpcUaClient
(no nodeId) were unlisted, and OpcUaClient's was the quietest of the twelve:
NamespaceMap drops such an entry with a bare `continue` and not even a log
line. SkippedTagReporterFleetTests pins the invariant reflectively over every
IDriver in src/Drivers/, so a thirteenth cannot re-introduce the silence.
Sql drops a tag at TWO points and its report is the union of both: a blob
that does not map, AND an identifier the 8.1 catalog gate rejects against the
live catalog. The second is what schema drift produces -- a renamed column, a
dropped view -- months after a deployment sealed green, and reporting only
the first would have that driver claim "0 skipped" while polling half its
tags, which is worse than reporting nothing because the column would then be
actively reassuring.
Deliberately additive to driver state, never a change of it. Latching
non-Healthy for a config problem would satisfy both halves of the #527
comms-loss conjunction -- 90 s without a successful read AND non-Healthy --
and a driver whose tags were ALL skipped has no successful reads by
construction, so it would reconnect-loop forever against a healthy device.
Three details that are load-bearing rather than incidental:
- The skip digest is part of the publish fingerprint. A skipped tag leaves
every other fingerprint component identical to a clean driver's, so without
it PublishHealthSnapshot's dedup swallows exactly the publish carrying the
signal -- the trap that already bit the rediscovery and host-status
channels. The digest covers the sample, not just the counts: a redeploy
that fixes one tag and breaks another leaves both counts unchanged.
- The count is exact; only the sample is capped, and the tooltip says so when
it is showing fewer than it counted. A truncated list that looked complete
would have an operator fix twenty tags and believe the job done.
- The ledger stages and commits, so an init that throws part-way leaves the
previous report standing. A partial snapshot would read as "we now skip
fewer tags", the opposite of the truth.
The proto field needs no companion presence flag, unlike host_statuses: a
singular message field carries real presence in proto3, and a has_ flag would
collide with protoc's generated HasSkippedTags.
Verified red-first three ways: dropping SkippedTagDigest from the fingerprint
fails the two dedup tests, removing the seam from one driver fails the fleet
guard, and dropping the catalog-gate leg fails the Sql union test. Full
solution builds with zero new warnings against 4e22f024.
This commit is contained in:
Joseph Doherty
@@ -169,6 +169,7 @@ else
|
||||
<th>Type</th>
|
||||
<th>Status</th>
|
||||
<th>Hosts</th>
|
||||
<th>Tags</th>
|
||||
<th>Last read</th>
|
||||
<th>Errors/5 min</th>
|
||||
<th>Last error</th>
|
||||
@@ -178,7 +179,7 @@ else
|
||||
@if (g.Drivers.Count == 0)
|
||||
{
|
||||
<tr>
|
||||
<td colspan="7"><span class="text-muted">No drivers.</span></td>
|
||||
<td colspan="8"><span class="text-muted">No drivers.</span></td>
|
||||
</tr>
|
||||
}
|
||||
else
|
||||
@@ -227,6 +228,30 @@ else
|
||||
</span>
|
||||
}
|
||||
</td>
|
||||
<td>
|
||||
@* 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)
|
||||
{
|
||||
<span class="text-muted">—</span>
|
||||
}
|
||||
else if (!d.HasSkippedTags)
|
||||
{
|
||||
<span class="text-muted small">@d.SkippedTags.AuthoredCount @(d.SkippedTags.AuthoredCount == 1 ? "tag" : "tags")</span>
|
||||
}
|
||||
else
|
||||
{
|
||||
<span class="chip @(d.SkippedTags.SkippedEverything ? "chip-bad" : "chip-warn")"
|
||||
title="@d.SkippedTagsTitle()">
|
||||
@d.SkippedTags.SkippedCount / @d.SkippedTags.AuthoredCount skipped
|
||||
</span>
|
||||
}
|
||||
</td>
|
||||
<td>@(d.LastSuccessfulReadUtc?.ToString("HH:mm:ss 'UTC'") ?? "—")</td>
|
||||
<td class="numeric">@d.ErrorCount5Min</td>
|
||||
<td><span class="text-muted small">@(d.LastError ?? "—")</span></td>
|
||||
|
||||
@@ -44,12 +44,41 @@ public sealed record HostsDriverInstanceInfo(string DriverInstanceId, string Clu
|
||||
/// <param name="HostStatuses">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.</param>
|
||||
/// <param name="SkippedTags">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).</param>
|
||||
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<HostConnectivityStatus>? HostStatuses = null)
|
||||
IReadOnlyList<HostConnectivityStatus>? HostStatuses = null,
|
||||
SkippedTagReport? SkippedTags = null)
|
||||
{
|
||||
/// <summary>Whether this driver is ignoring at least one authored tag — the condition the whole
|
||||
/// column exists to make visible.</summary>
|
||||
public bool HasSkippedTags => SkippedTags is { SkippedCount: > 0 };
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
/// <returns>The tooltip text; empty when nothing was skipped.</returns>
|
||||
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}";
|
||||
}
|
||||
|
||||
/// <summary>Hosts that are not <see cref="HostState.Running"/>, ordered by name — the ones worth an
|
||||
/// operator's attention. Empty when every host is fine or none are reported.
|
||||
/// <para><see cref="HostState.Unknown"/> 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)
|
||||
|
||||
@@ -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));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
private static SkippedTagReport? ToSkippedTags(DriverHealth msg)
|
||||
{
|
||||
if (msg.SkippedTags is not { } s) return null;
|
||||
|
||||
var sample = new List<SkippedTag>(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);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -37,7 +37,8 @@ public sealed class AkkaDriverHealthPublisher : IDriverHealthPublisher
|
||||
int errorCount5Min,
|
||||
DateTime? rediscoveryNeededUtc = null,
|
||||
string? rediscoveryReason = null,
|
||||
IReadOnlyList<HostConnectivityStatus>? hostStatuses = null)
|
||||
IReadOnlyList<HostConnectivityStatus>? 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.
|
||||
|
||||
@@ -1001,6 +1001,27 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>What the driver skipped when it last built its tag table, or null when it is not an
|
||||
/// <see cref="ISkippedTagReporter"/>. Pulled fresh for the same reason as
|
||||
/// <see cref="CurrentHostStatuses"/> — the driver owns the answer, and a cached copy here would be a
|
||||
/// second source of truth to keep in sync across every reinitialise.
|
||||
/// <para>Called UNGUARDED (not through <see cref="_invoker"/>): 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.</para></summary>
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>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.
|
||||
/// <para><b>Advisory only.</b> 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}"));
|
||||
|
||||
/// <summary>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.
|
||||
/// <para>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.</para></summary>
|
||||
private static string? SkippedTagDigest(SkippedTagReport? report) =>
|
||||
report is null
|
||||
? null
|
||||
: $"{report.AuthoredCount}/{report.SkippedCount}:" +
|
||||
string.Join('|', report.Sample.Select(s => $"{s.RawPath}={s.Reason}"));
|
||||
|
||||
/// <summary>Fingerprint of the last <see cref="PublishHealthSnapshot"/> call; null until first publish.</summary>
|
||||
private (DriverState State, DateTime? LastSuccess, string? LastError, int ErrorCount,
|
||||
DateTime? RediscoveryNeededUtc, string? HostStatusDigest)? _lastPublishedFingerprint;
|
||||
DateTime? RediscoveryNeededUtc, string? HostStatusDigest, string? SkippedTagDigest)? _lastPublishedFingerprint;
|
||||
|
||||
/// <inheritdoc />
|
||||
protected override void PostStop()
|
||||
|
||||
Reference in New Issue
Block a user