From 55eac5f68bf5432481d891892a70d5d6b6d44ac9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 14:24:25 -0400 Subject: [PATCH 1/3] feat(history): full Part 13 aggregate set; unsupported-by-backend aggregates answer BadAggregateNotSupported (#285) Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- .../IHistoryProvider.cs | 44 +++++++++++++++++ .../GatewayHistorianDataSource.cs | 4 +- .../Mapping/AggregateModeMapper.cs | 7 ++- .../OpcUaClientDriver.cs | 20 ++++++++ .../OtOpcUaNodeManager.cs | 28 +++++++++++ .../Mapping/AggregateModeMapperTests.cs | 20 ++++++-- .../OpcUaClientHistoryTests.cs | 19 ++++++-- .../NodeManagerHistoryReadTests.cs | 48 +++++++++++++++++++ 8 files changed, 180 insertions(+), 10 deletions(-) diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IHistoryProvider.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IHistoryProvider.cs index 684b8b7c..5b8be190 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IHistoryProvider.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IHistoryProvider.cs @@ -132,6 +132,14 @@ public sealed record HistoryReadResult( byte[]? ContinuationPoint); /// Aggregate function for processed history reads. Mirrors OPC UA Part 13 standard aggregates. +/// +/// The first five members are the original set every historian backend serves. The remainder (#285) are +/// the further standard Part 13 aggregates a server-side HistoryRead can forward to a backend that +/// supports them (the OPC UA Client driver forwards all of them upstream). A backend that cannot serve a +/// member must throw — the server maps that to +/// BadAggregateNotSupported rather than a generic failure. Append only: the numeric values +/// are not persisted, but every switch over this enum must keep a total default. +/// public enum HistoryAggregateType { Average, @@ -139,6 +147,42 @@ public enum HistoryAggregateType Maximum, Total, Count, + Interpolative, + TimeAverage, + Start, + End, + Delta, + Range, + MinimumActualTime, + MaximumActualTime, + StartBound, + EndBound, + DeltaBounds, + DurationGood, + DurationBad, + PercentGood, + PercentBad, + WorstQuality, + NumberOfTransitions, + AnnotationCount, + StandardDeviationPopulation, + VariancePopulation, +} + +/// +/// Thrown by a history backend when it cannot serve the requested . +/// The Server HistoryRead path translates it to BadAggregateNotSupported for that node only. +/// +public sealed class HistoryAggregateNotSupportedException : NotSupportedException +{ + /// The aggregate the backend could not serve. + public HistoryAggregateType Aggregate { get; } + + /// Creates the exception for . + /// The unsupported aggregate. + public HistoryAggregateNotSupportedException(HistoryAggregateType aggregate) + : base($"The historian backend does not support the {aggregate} aggregate.") + => Aggregate = aggregate; } /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayHistorianDataSource.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayHistorianDataSource.cs index 033f5403..556bbde2 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayHistorianDataSource.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayHistorianDataSource.cs @@ -79,11 +79,13 @@ public sealed class GatewayHistorianDataSource : IHistorianDataSource, IAsyncDis string fullReference, DateTime startUtc, DateTime endUtc, TimeSpan interval, HistoryAggregateType aggregate, CancellationToken cancellationToken) { + // Mapped OUTSIDE the failure-recording try: an aggregate the gateway has no mode for is a client + // request the server answers BadAggregateNotSupported, not a backend fault to count (#285). + var mode = AggregateModeMapper.ToRetrievalMode(aggregate); try { // Total/Count are now native gateway retrieval modes — no client-side scaling // (unlike the Wonderware path that derived Total as Average × interval-seconds). - var mode = AggregateModeMapper.ToRetrievalMode(aggregate); var buckets = new List(); await foreach (var bucket in _client .ReadAggregateAsync(fullReference, startUtc, endUtc, mode, interval, cancellationToken) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Mapping/AggregateModeMapper.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Mapping/AggregateModeMapper.cs index f3d8b3a0..09eb3921 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Mapping/AggregateModeMapper.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Mapping/AggregateModeMapper.cs @@ -25,7 +25,8 @@ internal static class AggregateModeMapper /// Maps an aggregate function to the gateway retrieval mode. /// The driver-agnostic aggregate function. /// The matching gateway . - /// A future, unmapped enum member (fails the matrix guard). + /// A defined member the gateway has no retrieval mode for. + /// An undefined enum value. public static RetrievalMode ToRetrievalMode(HistoryAggregateType aggregate) => aggregate switch { HistoryAggregateType.Average => RetrievalMode.TimeWeightedAverage, @@ -33,6 +34,10 @@ internal static class AggregateModeMapper HistoryAggregateType.Maximum => RetrievalMode.MaximumWithTime, HistoryAggregateType.Total => RetrievalMode.Integral, HistoryAggregateType.Count => RetrievalMode.Counter, + // Part 13 aggregates the gateway has no retrieval mode for (#285) — an UNDEFINED enum value is a + // programming error, but a defined-yet-unserved member is a normal client request the server + // answers with BadAggregateNotSupported. + _ when Enum.IsDefined(aggregate) => throw new HistoryAggregateNotSupportedException(aggregate), _ => throw new ArgumentOutOfRangeException( nameof(aggregate), aggregate, "Unmapped HistoryAggregateType — add a RetrievalMode mapping."), }; 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 967111cf..cc9a3f70 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -1779,6 +1779,26 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit HistoryAggregateType.Maximum => ObjectIds.AggregateFunction_Maximum, HistoryAggregateType.Total => ObjectIds.AggregateFunction_Total, HistoryAggregateType.Count => ObjectIds.AggregateFunction_Count, + HistoryAggregateType.Interpolative => ObjectIds.AggregateFunction_Interpolative, + HistoryAggregateType.TimeAverage => ObjectIds.AggregateFunction_TimeAverage, + HistoryAggregateType.Start => ObjectIds.AggregateFunction_Start, + HistoryAggregateType.End => ObjectIds.AggregateFunction_End, + HistoryAggregateType.Delta => ObjectIds.AggregateFunction_Delta, + HistoryAggregateType.Range => ObjectIds.AggregateFunction_Range, + HistoryAggregateType.MinimumActualTime => ObjectIds.AggregateFunction_MinimumActualTime, + HistoryAggregateType.MaximumActualTime => ObjectIds.AggregateFunction_MaximumActualTime, + HistoryAggregateType.StartBound => ObjectIds.AggregateFunction_StartBound, + HistoryAggregateType.EndBound => ObjectIds.AggregateFunction_EndBound, + HistoryAggregateType.DeltaBounds => ObjectIds.AggregateFunction_DeltaBounds, + HistoryAggregateType.DurationGood => ObjectIds.AggregateFunction_DurationGood, + HistoryAggregateType.DurationBad => ObjectIds.AggregateFunction_DurationBad, + HistoryAggregateType.PercentGood => ObjectIds.AggregateFunction_PercentGood, + HistoryAggregateType.PercentBad => ObjectIds.AggregateFunction_PercentBad, + HistoryAggregateType.WorstQuality => ObjectIds.AggregateFunction_WorstQuality, + HistoryAggregateType.NumberOfTransitions => ObjectIds.AggregateFunction_NumberOfTransitions, + HistoryAggregateType.AnnotationCount => ObjectIds.AggregateFunction_AnnotationCount, + HistoryAggregateType.StandardDeviationPopulation => ObjectIds.AggregateFunction_StandardDeviationPopulation, + HistoryAggregateType.VariancePopulation => ObjectIds.AggregateFunction_VariancePopulation, _ => throw new ArgumentOutOfRangeException(nameof(aggregate), aggregate, null), }; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 91cca9b0..22db9ecd 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -5169,6 +5169,14 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 errors[handle.Index] = StatusCodes.BadTimeout; results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.BadTimeout }; } + catch (HistoryAggregateNotSupportedException ex) + { + // The backend cannot serve this Part 13 aggregate (#285) — the spec status for THIS node, + // not a logged server fault. + _logger.LogDebug(ex, "OtOpcUaNodeManager: backend does not support aggregate for node {NodeId}", handle.NodeId); + errors[handle.Index] = StatusCodes.BadAggregateNotSupported; + results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.BadAggregateNotSupported }; + } catch (Exception ex) { // One node's backend failure (throw / timeout) must not throw out of the batch — surface a @@ -5424,6 +5432,26 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 if (aggregateNodeId == ObjectIds.AggregateFunction_Maximum) return HistoryAggregateType.Maximum; if (aggregateNodeId == ObjectIds.AggregateFunction_Total) return HistoryAggregateType.Total; if (aggregateNodeId == ObjectIds.AggregateFunction_Count) return HistoryAggregateType.Count; + if (aggregateNodeId == ObjectIds.AggregateFunction_Interpolative) return HistoryAggregateType.Interpolative; + if (aggregateNodeId == ObjectIds.AggregateFunction_TimeAverage) return HistoryAggregateType.TimeAverage; + if (aggregateNodeId == ObjectIds.AggregateFunction_Start) return HistoryAggregateType.Start; + if (aggregateNodeId == ObjectIds.AggregateFunction_End) return HistoryAggregateType.End; + if (aggregateNodeId == ObjectIds.AggregateFunction_Delta) return HistoryAggregateType.Delta; + if (aggregateNodeId == ObjectIds.AggregateFunction_Range) return HistoryAggregateType.Range; + if (aggregateNodeId == ObjectIds.AggregateFunction_MinimumActualTime) return HistoryAggregateType.MinimumActualTime; + if (aggregateNodeId == ObjectIds.AggregateFunction_MaximumActualTime) return HistoryAggregateType.MaximumActualTime; + if (aggregateNodeId == ObjectIds.AggregateFunction_StartBound) return HistoryAggregateType.StartBound; + if (aggregateNodeId == ObjectIds.AggregateFunction_EndBound) return HistoryAggregateType.EndBound; + if (aggregateNodeId == ObjectIds.AggregateFunction_DeltaBounds) return HistoryAggregateType.DeltaBounds; + if (aggregateNodeId == ObjectIds.AggregateFunction_DurationGood) return HistoryAggregateType.DurationGood; + if (aggregateNodeId == ObjectIds.AggregateFunction_DurationBad) return HistoryAggregateType.DurationBad; + if (aggregateNodeId == ObjectIds.AggregateFunction_PercentGood) return HistoryAggregateType.PercentGood; + if (aggregateNodeId == ObjectIds.AggregateFunction_PercentBad) return HistoryAggregateType.PercentBad; + if (aggregateNodeId == ObjectIds.AggregateFunction_WorstQuality) return HistoryAggregateType.WorstQuality; + if (aggregateNodeId == ObjectIds.AggregateFunction_NumberOfTransitions) return HistoryAggregateType.NumberOfTransitions; + if (aggregateNodeId == ObjectIds.AggregateFunction_AnnotationCount) return HistoryAggregateType.AnnotationCount; + if (aggregateNodeId == ObjectIds.AggregateFunction_StandardDeviationPopulation) return HistoryAggregateType.StandardDeviationPopulation; + if (aggregateNodeId == ObjectIds.AggregateFunction_VariancePopulation) return HistoryAggregateType.VariancePopulation; return null; } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/AggregateModeMapperTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/AggregateModeMapperTests.cs index 871b513f..a608b500 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/AggregateModeMapperTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/AggregateModeMapperTests.cs @@ -16,10 +16,24 @@ public sealed class AggregateModeMapperTests public void Maps_each_aggregate(HistoryAggregateType a, RetrievalMode expected) => Assert.Equal(expected, AggregateModeMapper.ToRetrievalMode(a)); - [Fact] // matrix guard: a new HistoryAggregateType member must fail here - public void Every_aggregate_member_is_mapped() + [Fact] // matrix guard: every member is either mapped or explicitly reported as not supported + public void Every_aggregate_member_is_mapped_or_reports_not_supported() { + var original = new[] + { + HistoryAggregateType.Average, HistoryAggregateType.Minimum, HistoryAggregateType.Maximum, + HistoryAggregateType.Total, HistoryAggregateType.Count, + }; foreach (var a in Enum.GetValues()) - _ = AggregateModeMapper.ToRetrievalMode(a); // must not throw for any defined member + { + if (original.Contains(a)) + _ = AggregateModeMapper.ToRetrievalMode(a); + else + Assert.Throws(() => AggregateModeMapper.ToRetrievalMode(a)); + } } + + [Fact] + public void Undefined_value_is_a_programming_error_not_a_not_supported() + => Assert.Throws(() => AggregateModeMapper.ToRetrievalMode((HistoryAggregateType)9999)); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientHistoryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientHistoryTests.cs index a515dac5..7fe81eca 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientHistoryTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientHistoryTests.cs @@ -8,14 +8,23 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; [Trait("Category", "Unit")] public sealed class OpcUaClientHistoryTests { + /// Every member (#285: the full Part 13 set, not just five). + public static IEnumerable AllAggregates() => + Enum.GetValues().Select(a => new object[] { a }); + + /// Every member maps to a DISTINCT aggregate NodeId (a copy/paste mapping would collide). + [Fact] + public void MapAggregateToNodeId_is_injective_over_the_whole_enum() + { + var ids = Enum.GetValues().Select(OpcUaClientDriver.MapAggregateToNodeId).ToList(); + ids.Distinct().Count().ShouldBe(ids.Count); + Enum.GetValues().Length.ShouldBeGreaterThan(5); + } + /// Verifies MapAggregateToNodeId returns standard Part 13 aggregate for every enum. /// The history aggregate type to test. [Theory] - [InlineData(HistoryAggregateType.Average)] - [InlineData(HistoryAggregateType.Minimum)] - [InlineData(HistoryAggregateType.Maximum)] - [InlineData(HistoryAggregateType.Total)] - [InlineData(HistoryAggregateType.Count)] + [MemberData(nameof(AllAggregates))] public void MapAggregateToNodeId_returns_standard_Part13_aggregate_for_every_enum(HistoryAggregateType agg) { var nodeId = OpcUaClientDriver.MapAggregateToNodeId(agg); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadTests.cs index 12b4b9d8..ac267f35 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadTests.cs @@ -268,6 +268,52 @@ public sealed class NodeManagerHistoryReadTests : IDisposable await host.DisposeAsync(); } + /// #285: a Part 13 aggregate beyond the original five (Interpolative) is forwarded to the + /// backend as its own member. + [Fact] + public async Task Processed_extended_aggregate_dispatches_to_the_source() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + var fake = new RecordingHistorianDataSource { ProcessedResult = Empty() }; + nm.HistorianDataSource = fake; + nm.EnsureVariable("eq-1/interp", parentFolderNodeId: null, displayName: "I", dataType: "Float", + writable: false, historianTagname: "WW.Interp", realm: AddressSpaceRealm.Uns); + var nodeId = nm.TryGetVariable("eq-1/interp")!.NodeId; + + InvokeHistoryRead(server, nm, new ReadProcessedDetails + { + StartTime = DateTime.UtcNow.AddHours(-1), EndTime = DateTime.UtcNow, ProcessingInterval = 10_000.0, + AggregateType = new NodeIdCollection { ObjectIds.AggregateFunction_Interpolative }, + }, nodeId); + + fake.LastCall.ShouldBe("Processed"); + fake.LastAggregate.ShouldBe(HistoryAggregateType.Interpolative); + await host.DisposeAsync(); + } + + /// #285: a backend that cannot serve a mapped aggregate answers BadAggregateNotSupported + /// (not the generic BadHistoryOperationUnsupported a plain throw produces). + [Fact] + public async Task Processed_backend_unsupported_aggregate_yields_BadAggregateNotSupported() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + nm.HistorianDataSource = new RecordingHistorianDataSource { AggregateNotSupported = true }; + nm.EnsureVariable("eq-1/ns", parentFolderNodeId: null, displayName: "N", dataType: "Float", + writable: false, historianTagname: "WW.Ns", realm: AddressSpaceRealm.Uns); + var nodeId = nm.TryGetVariable("eq-1/ns")!.NodeId; + + var (_, errors) = InvokeHistoryRead(server, nm, new ReadProcessedDetails + { + StartTime = DateTime.UtcNow.AddHours(-1), EndTime = DateTime.UtcNow, ProcessingInterval = 10_000.0, + AggregateType = new NodeIdCollection { ObjectIds.AggregateFunction_PercentGood }, + }, nodeId); + + errors[0].StatusCode.Code.ShouldBe(StatusCodes.BadAggregateNotSupported); + await host.DisposeAsync(); + } + /// AtTime read: the requested timestamps reach ReadAtTimeAsync in order. [Fact] public async Task AtTime_dispatches_requested_timestamps() @@ -430,6 +476,7 @@ public sealed class NodeManagerHistoryReadTests : IDisposable private sealed class RecordingHistorianDataSource : IHistorianDataSource { public bool ThrowOnRead { get; init; } + public bool AggregateNotSupported { get; init; } public HistorianRead RawResult { get; set; } = new(Array.Empty(), null); public HistorianRead ProcessedResult { get; set; } = new(Array.Empty(), null); public HistorianRead AtTimeResult { get; set; } = new(Array.Empty(), null); @@ -461,6 +508,7 @@ public sealed class NodeManagerHistoryReadTests : IDisposable HistoryAggregateType aggregate, CancellationToken cancellationToken) { if (ThrowOnRead) throw new InvalidOperationException("backend boom"); + if (AggregateNotSupported) throw new HistoryAggregateNotSupportedException(aggregate); LastCall = "Processed"; LastTagname = fullReference; LastStart = startUtc; From ba62022e88d003d2fbf2b2576d7360ccb13ceda8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 14:40:20 -0400 Subject: [PATCH 2/3] feat(opcuaclient): subscription tuning, OperationLimits chunking, diagnostics counters, revocation options, DiscoveryUrl (#273 #275 #276 #277 #278) Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- docs/Historian.md | 11 + docs/drivers/OpcUaClient.md | 63 ++- .../DriverHealth.cs | 8 +- .../OpcUaClientDriverOptions.cs | 80 +++ .../OpcUaClientDiagnostics.cs | 102 ++++ .../OpcUaClientDriver.cs | 484 ++++++++++++++---- .../OpcUaClientOperationLimits.cs | 94 ++++ .../Drivers/Forms/OpcUaClientDriverForm.razor | 91 ++++ .../OpcUaClientDiagnosticsTests.cs | 111 ++++ .../OpcUaClientOperationLimitsTests.cs | 163 ++++++ .../OpcUaClientTuningAndDiscoveryTests.cs | 157 ++++++ ...aClientDriverPageFormSerializationTests.cs | 44 ++ 12 files changed, 1308 insertions(+), 100 deletions(-) create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDiagnostics.cs create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientOperationLimits.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientDiagnosticsTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientOperationLimitsTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientTuningAndDiscoveryTests.cs diff --git a/docs/Historian.md b/docs/Historian.md index 1307f197..ae1568b3 100644 --- a/docs/Historian.md +++ b/docs/Historian.md @@ -431,6 +431,17 @@ RetrievalMode.Integral`) — no client-side scaling. (This replaces the retired likewise a native gateway mode. Bucket status codes and timestamps are preserved unchanged; empty / null buckets surface as `BadNoData`. +### Aggregate set and `BadAggregateNotSupported` (#285) + +`HistoryAggregateType` carries the wider Part 13 set (Interpolative, TimeAverage, Start, End, Delta, Range, +Min/MaxActualTime, Start/EndBound, DeltaBounds, DurationGood/Bad, PercentGood/Bad, WorstQuality, +NumberOfTransitions, AnnotationCount, StandardDeviationPopulation, VariancePopulation); `OtOpcUaNodeManager.MapAggregate` +maps each standard aggregate NodeId. The **HistorianGateway** backend only has retrieval modes for the original +five (Average, Minimum, Maximum, Total, Count): for any other member `AggregateModeMapper` throws +`HistoryAggregateNotSupportedException`, which the HistoryRead path turns into `BadAggregateNotSupported` for that +node only (not a generic failure, and not counted as a backend fault). The OPC UA Client driver forwards every +member upstream. A NodeId outside the mapped set (e.g. `StandardDeviationSample`) is still `BadAggregateNotSupported`. + ### Known limitations - **Processed and AtTime are single-shot** (no continuation points). Unlike Raw, neither diff --git a/docs/drivers/OpcUaClient.md b/docs/drivers/OpcUaClient.md index 50bcb24d..c90672ab 100644 --- a/docs/drivers/OpcUaClient.md +++ b/docs/drivers/OpcUaClient.md @@ -99,8 +99,11 @@ rather than surfacing as a runtime surprise. ### History passthrough `IHistoryProvider` forwards `HistoryRead` to the upstream server's own historian. -Raw, processed (Average / Minimum / Maximum / Total / Count aggregates mapped to -OPC UA Part 13 standard aggregate NodeIds), and at-time reads are supported; each +Raw, processed (the full `HistoryAggregateType` set — Average / Minimum / Maximum / Total / Count plus +Interpolative, TimeAverage, Start, End, Delta, Range, Min/Max ActualTime, Start/EndBound, DeltaBounds, +DurationGood/Bad, PercentGood/Bad, WorstQuality, NumberOfTransitions, AnnotationCount and the +population StandardDeviation/Variance — each mapped to its OPC UA Part 13 standard aggregate NodeId; #285), +and at-time reads are supported; each returned `DataValue` keeps its upstream `StatusCode` and timestamps verbatim. Event-history (`ReadEventsAsync`) is left at the interface default — the interface doesn't yet carry the EventFilter surface needed to forward it. @@ -109,7 +112,61 @@ interface doesn't yet carry the EventFilter surface needed to forward it. `AutoAcceptCertificates` accepts any self-signed / untrusted server certificate. It is **dev-only** — leave it `false` in production so a MITM against the -opc.tcp channel fails closed. +opc.tcp channel fails closed. A **revoked** certificate (`BadCertificateRevoked` / +`BadCertificateIssuerRevoked`) is never auto-accepted (#277). + +`CertificateValidation` (defaults change nothing): + +| Key | Default | Meaning | +|---|---|---| +| `rejectUnknownRevocationStatus` | `false` | Reject a server certificate whose revocation status cannot be determined (no CRL for its issuer, or a stale one). Leave `false` for brownfield sites without CRL infrastructure. | +| `issuerStorePath` | `null` (= `%LocalAppData%/OtOpcUa/pki/issuers`) | Directory used as the trusted-issuer store. The SDK reads issuer CRLs (`*.crl`) from its `crl/` sub-folder, so this is how an operator points the driver at a managed CRL set. | + +### Subscription tuning (#273) + +The per-subscription parameters that used to be hardcoded are driver options; the defaults are the old values. + +| Key | Default | Notes | +|---|---|---| +| `subscriptionPublishingInterval` | `null` | `null` = the interval the runtime requests per subscribe (floored at 50 ms). When set it overrides that value (floor still applies); monitored-item sampling follows it. | +| `subscriptionKeepAliveCount` | `10` | `>= 1`. | +| `subscriptionLifetimeCount` | `1000` | Must be `>= 3 x` keep-alive count (Part 4 §5.13.2). | +| `subscriptionMaxNotificationsPerPublish` | `0` | `0` = server decides. | +| `subscriptionPriority` | `0` | `0`-`255`. | + +Out-of-range values fail `InitializeAsync` (`ValidateTuning`). The alarm subscription uses the same counts. +*Per-tag* tuning (deadband / sampling interval / queue size, #274) is not implemented — it needs an +`ISubscribable` signature change on Core. + +### Server OperationLimits (#275) + +After connect the driver reads `Server/ServerCapabilities/OperationLimits` +(`MaxNodesPerRead`, `MaxNodesPerWrite`, `MaxNodesPerBrowse`, `MaxMonitoredItemsPerCall`, +`MaxNodesPerHistoryReadData/Events`; `0` or unreadable = no limit) and splits requests that exceed them: + +- `ReadAsync`, `WriteAsync` and the discovery attribute-enrichment read go out in `MaxNodesPerRead` / + `MaxNodesPerWrite`-sized chunks, results re-stitched positionally. A failed Read chunk marks only its own tags + `BadCommunicationError`; a Write that times out marks that chunk and every later one `BadTimeout`, while + earlier chunks keep their real results (those writes happened). +- `SubscribeAsync` creates monitored items in `MaxMonitoredItemsPerCall` batches. +- Browse and HistoryRead already carry one node per call, so a limit `>= 1` is always met and nothing is split. + +`honorServerOperationLimits` (default `true`) switches it off. The limits are re-read after a successful reconnect. + +### Discovery URL (#278) + +`discoveryUrl` (default `null`, e.g. `opc.tcp://lds:4840`): at `InitializeAsync` the driver runs `FindServers` on it +and `GetEndpoints` on every returned server, keeps the endpoints matching `securityPolicy` + `securityMode`, and +tries them **before** `endpointUrls`/`endpointUrl` (case-insensitive de-dup). A discovery failure is logged and +non-fatal — the static list is used. The discovery channel itself is always unsecured (spec). + +### Diagnostics counters (#276) + +`GetHealth().Diagnostics` (a `name -> double` map, not part of the health-change fingerprint) carries: +`PublishResponseCount`, `NotificationCount`, `DataChangeCount`, `BadStatusValueCount`, `PublishErrorCount`, +`KeepAliveMissCount`, `ReconnectAttemptCount`, `ReconnectSuccessCount`, `ReconnectExhaustedCount`, +`LastReconnectUtcTicks`, `ReadCallCount`, `ReadFailureCount`, `WriteCallCount`, `WriteFailureCount`, +`ChunkedRequestCount`. All counters are `Interlocked`; they are cumulative for the driver instance's lifetime. ## Testing diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverHealth.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverHealth.cs index f76d163f..09f06269 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverHealth.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverHealth.cs @@ -15,10 +15,16 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; /// carry a non-null message. Callers must not key behaviour on the LastError-null ↔ Healthy /// pairing. /// +/// +/// Optional driver-specific counters/gauges (name → value) for the diagnostics surface (#276). +/// Null for every driver that publishes none. Deliberately NOT part of any health-change +/// fingerprint: counters move on every poll, so keying a dedup on them would defeat it. +/// public sealed record DriverHealth( DriverState State, DateTime? LastSuccessfulRead, - string? LastError); + string? LastError, + IReadOnlyDictionary? Diagnostics = null); /// Driver-instance lifecycle state. public enum DriverState diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs index 47c27525..249eb775 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs @@ -189,6 +189,86 @@ public sealed record OpcUaClientDriverOptions /// [Range(1, 60)] public int ProbeTimeoutSeconds { get; init; } = 15; + + // ---- #273 per-subscription tuning (defaults are the values that used to be hardcoded) ---- + + /// + /// Fixed publishing interval for every data-change subscription this driver creates. null + /// (default) keeps the historical behaviour: the interval the runtime asks for per + /// SubscribeAsync call, floored at 50 ms. When set it overrides the per-call value (the + /// 50 ms floor still applies); monitored-item sampling intervals follow it. + /// + public TimeSpan? SubscriptionPublishingInterval { get; init; } + + /// + /// Publish cycles with no data before the server sends a keep-alive (default 10). Must be >= 1. + /// + [Range(1, int.MaxValue)] + public uint SubscriptionKeepAliveCount { get; init; } = 10; + + /// + /// Publish cycles with no Publish request before the server drops the subscription (default 1000). + /// Must be at least 3x (OPC UA Part 4 §5.13.2). + /// + [Range(3, int.MaxValue)] + public uint SubscriptionLifetimeCount { get; init; } = 1000; + + /// Max notifications per Publish response; 0 (default) = server decides. + public uint SubscriptionMaxNotificationsPerPublish { get; init; } + + /// Relative subscription priority 0-255 (default 0). + [Range(0, 255)] + public byte SubscriptionPriority { get; init; } + + // ---- #275 ---- + + /// + /// When true (default) the driver reads the remote server's + /// ServerCapabilities/OperationLimits after connect and splits Read / Write / CreateMonitoredItems + /// requests that exceed them (a declared limit of 0 = unlimited). Set false to send every request + /// whole and let the server answer BadTooManyOperations itself. + /// + public bool HonorServerOperationLimits { get; init; } = true; + + // ---- #277 ---- + + /// Certificate-revocation / issuer-store settings. Defaults preserve the SDK behaviour. + public OpcUaCertificateValidationOptions CertificateValidation { get; init; } = new(); + + // ---- #278 ---- + + /// + /// Optional discovery endpoint (a Local Discovery Server or the server itself), e.g. + /// opc.tcp://lds.internal:4840. When set, InitializeAsync calls FindServers and + /// GetEndpoints on it and tries the discovered URLs that match + + /// before the statically configured / + /// . A discovery failure is non-fatal: the static list is used. Discovery + /// itself always runs over an unsecured channel, as the spec requires. null (default) = off. + /// + public string? DiscoveryUrl { get; init; } +} + +/// +/// Server-certificate revocation handling for the OPC UA Client driver (#277). The SDK reads CRLs from +/// the crl sub-folder of the trusted-peer / trusted-issuer stores; these settings only decide how +/// a certificate whose revocation status cannot be determined is treated and where the issuer store lives. +/// +public sealed record OpcUaCertificateValidationOptions +{ + /// + /// When true, a server certificate whose revocation status is unknown (no CRL for its issuer, or a + /// stale one) is rejected. Default false keeps brownfield deployments without CRL infrastructure + /// connecting. A certificate that IS revoked is always rejected — even with + /// set. + /// + public bool RejectUnknownRevocationStatus { get; init; } + + /// + /// Directory used as the trusted-issuer certificate store. Its crl/ sub-folder is where the SDK + /// looks for issuer CRLs (.crl files), so this is the knob for pointing the driver at an + /// operator-managed CRL set. null/empty (default) = %LocalAppData%/OtOpcUa/pki/issuers. + /// + public string? IssuerStorePath { get; init; } } /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDiagnostics.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDiagnostics.cs new file mode 100644 index 00000000..220ec92f --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDiagnostics.cs @@ -0,0 +1,102 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; + +/// +/// Thread-safe counters for the OPC UA Client driver (#276), surfaced through +/// DriverHealth.Diagnostics. Writers are the SDK's notification / keep-alive threads and +/// the request paths, so every field is touched only through . +/// +internal sealed class OpcUaClientDiagnostics +{ + private long _publishResponses; + private long _notifications; + private long _dataChanges; + private long _badStatusValues; + private long _publishErrors; + private long _keepAliveMisses; + private long _reconnectAttempts; + private long _reconnectsSucceeded; + private long _reconnectsExhausted; + private long _lastReconnectUtcTicks; + private long _readCalls; + private long _readFailures; + private long _writeCalls; + private long _writeFailures; + private long _chunkedRequests; + + /// A NotificationMessage arrived (one per Publish response that carried data). + public void RecordPublishResponse() => Interlocked.Increment(ref _publishResponses); + + /// A Publish failed (missing publish requests, subscription errors). + public void RecordPublishError() => Interlocked.Increment(ref _publishErrors); + + /// One monitored-item data change was delivered. + /// Whether the value's status code was Bad. + public void RecordDataChange(bool bad) + { + Interlocked.Increment(ref _dataChanges); + if (bad) Interlocked.Increment(ref _badStatusValues); + } + + /// Count raw notification items carried by a message. + /// Number of items in the message. + public void RecordNotifications(int count) => Interlocked.Add(ref _notifications, count); + + /// A keep-alive came back Bad. + public void RecordKeepAliveMiss() => Interlocked.Increment(ref _keepAliveMisses); + + /// The reconnect handler was armed. + public void RecordReconnectAttempt() => Interlocked.Increment(ref _reconnectAttempts); + + /// A reconnect landed a new session. + public void RecordReconnectSucceeded() + { + Interlocked.Increment(ref _reconnectsSucceeded); + Interlocked.Exchange(ref _lastReconnectUtcTicks, DateTime.UtcNow.Ticks); + } + + /// A reconnect handler exhausted its retry window. + public void RecordReconnectExhausted() => Interlocked.Increment(ref _reconnectsExhausted); + + /// A Read request (all chunks) finished. + /// Whether any chunk failed on the wire. + public void RecordRead(bool failed) + { + Interlocked.Increment(ref _readCalls); + if (failed) Interlocked.Increment(ref _readFailures); + } + + /// A Write request (all chunks) finished. + /// Whether any chunk failed on the wire. + public void RecordWrite(bool failed) + { + Interlocked.Increment(ref _writeCalls); + if (failed) Interlocked.Increment(ref _writeFailures); + } + + /// A logical request was split into more than one wire call to honour a server OperationLimit. + public void RecordChunkedRequest() => Interlocked.Increment(ref _chunkedRequests); + + /// + /// Point-in-time copy for DriverHealth.Diagnostics. Keys are stable and + /// PascalCase; LastReconnectUtcTicks is 0 until a reconnect succeeds. + /// + /// A fresh dictionary the caller may keep. + public IReadOnlyDictionary Snapshot() => new Dictionary + { + ["PublishResponseCount"] = Interlocked.Read(ref _publishResponses), + ["NotificationCount"] = Interlocked.Read(ref _notifications), + ["DataChangeCount"] = Interlocked.Read(ref _dataChanges), + ["BadStatusValueCount"] = Interlocked.Read(ref _badStatusValues), + ["PublishErrorCount"] = Interlocked.Read(ref _publishErrors), + ["KeepAliveMissCount"] = Interlocked.Read(ref _keepAliveMisses), + ["ReconnectAttemptCount"] = Interlocked.Read(ref _reconnectAttempts), + ["ReconnectSuccessCount"] = Interlocked.Read(ref _reconnectsSucceeded), + ["ReconnectExhaustedCount"] = Interlocked.Read(ref _reconnectsExhausted), + ["LastReconnectUtcTicks"] = Interlocked.Read(ref _lastReconnectUtcTicks), + ["ReadCallCount"] = Interlocked.Read(ref _readCalls), + ["ReadFailureCount"] = Interlocked.Read(ref _readFailures), + ["WriteCallCount"] = Interlocked.Read(ref _writeCalls), + ["WriteFailureCount"] = Interlocked.Read(ref _writeFailures), + ["ChunkedRequestCount"] = Interlocked.Read(ref _chunkedRequests), + }; +} 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 cc9a3f70..559d9177 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -144,7 +144,24 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit /// The namespace map to install, or null to clear it. internal void SetNamespaceMapForTest(NamespaceMap? map) => _namespaceMap = map; + /// Test-only seam installing the OperationLimits that connect would have read (#275). + /// The limits to apply to subsequent Read / Write / subscribe calls. + internal void SetOperationLimitsForTest(OpcUaClientOperationLimits limits) => _limits = limits; + + /// Test-only view of the diagnostics counters (#276). + internal OpcUaClientDiagnostics DiagnosticsForTest => _diag; + private DriverHealth _health = new(DriverState.Unknown, null, null); + + /// Counters surfaced through (#276). + private readonly OpcUaClientDiagnostics _diag = new(); + + /// Server OperationLimits read at connect (#275); None when unread / disabled. + /// Replaced wholesale (never mutated), so readers just snapshot the reference. + private OpcUaClientOperationLimits _limits = OpcUaClientOperationLimits.None; + + private NotificationEventHandler? _notificationHandler; + private PublishErrorEventHandler? _publishErrorHandler; private bool _disposed; /// URL of the endpoint the driver actually connected to. Exposed via . private string? _connectedEndpointUrl; @@ -208,9 +225,31 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // §8 "Namespace Assignment" — a misconfigured remote fails draft validation here, // not as a runtime surprise. ValidateNamespaceKind(_options); + ValidateTuning(_options); var appConfig = await BuildApplicationConfigurationAsync(cancellationToken).ConfigureAwait(false); - var candidates = ResolveEndpointCandidates(_options); + + // #278: when a DiscoveryUrl is configured, FindServers/GetEndpoints on it first and try the + // discovered URLs ahead of the static list. Non-fatal — an LDS outage must not block init. + IReadOnlyList discovered = []; + if (!string.IsNullOrWhiteSpace(_options.DiscoveryUrl)) + { + try + { + discovered = await DiscoverEndpointsAsync( + appConfig, _options.DiscoveryUrl!, _options.SecurityPolicy, _options.SecurityMode, + cancellationToken).ConfigureAwait(false); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { throw; } + catch (Exception ex) + { + _logger.LogWarning(ex, + "OpcUaClientDriver '{DriverInstanceId}': discovery against '{DiscoveryUrl}' failed; " + + "falling back to the configured endpoint list.", _driverInstanceId, _options.DiscoveryUrl); + } + } + + var candidates = ResolveEndpointCandidates(_options, discovered); // G-2b: resolve any secret:-prefixed Password / UserCertificatePassword through the // shared secret store ONCE, here in the async connect flow, just before the credentials @@ -262,6 +301,10 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // via TransferSubscriptions so local MonitoredItem handles stay valid. _keepAliveHandler = OnKeepAlive; session.KeepAlive += _keepAliveHandler; + AttachDiagnostics(session); + _limits = _options.HonorServerOperationLimits + ? await OpcUaClientOperationLimits.ReadAsync(session, cancellationToken).ConfigureAwait(false) + : OpcUaClientOperationLimits.None; // Build the bidirectional namespace map from the freshly negotiated session's // NamespaceUris (driver-specs.md §8 "Namespace Remapping"). Stored NodeIds carry @@ -327,7 +370,11 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit TrustedIssuerCertificates = new CertificateTrustList { StoreType = CertificateStoreType.Directory, - StorePath = Path.Combine(pkiRoot, "issuers"), + // #277: the issuer store's crl/ sub-folder is where the SDK reads issuer CRLs, so an + // operator-supplied path is the CRL knob. Unset = the historical default. + StorePath = string.IsNullOrWhiteSpace(_options.CertificateValidation.IssuerStorePath) + ? Path.Combine(pkiRoot, "issuers") + : _options.CertificateValidation.IssuerStorePath, }, RejectedCertificateStore = new CertificateTrustList { @@ -335,6 +382,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit StorePath = Path.Combine(pkiRoot, "rejected"), }, AutoAcceptUntrustedCertificates = _options.AutoAcceptCertificates, + RejectUnknownRevocationStatus = _options.CertificateValidation.RejectUnknownRevocationStatus, }, TransportQuotas = new TransportQuotas { OperationTimeout = (int)_options.Timeout.TotalMilliseconds }, ClientConfiguration = new ClientConfiguration @@ -360,8 +408,22 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // Accept the full set of certificate-validation error codes: a real dev cert can // fail with BadCertificateChainIncomplete, BadCertificateTimeInvalid, or // BadCertificateHostNameInvalid, not only BadCertificateUntrusted. Only accepting - // the latter would silently fail for those certs. - CertificateValidationEventHandler handler = (_, e) => e.Accept = true; + // the latter would silently fail for those certs. A REVOKED certificate is the one + // exception (#277): auto-accept is for dev self-signed certs, never for a cert its issuer + // has explicitly withdrawn. + CertificateValidationEventHandler handler = (_, e) => + { + if (IsRevocationFailure(e.Error.StatusCode)) + { + _logger.LogWarning( + "OpcUaClientDriver '{DriverInstanceId}': REVOKED server certificate rejected " + + "(subject={Subject}, status=0x{Status:X8}) — AutoAcceptCertificates does not apply to revoked certs.", + _driverInstanceId, e.Certificate?.Subject, e.Error.StatusCode.Code); + e.Accept = false; + return; + } + e.Accept = true; + }; config.CertificateValidator.CertificateValidation += handler; // Store refs so ShutdownAsync + Dispose can detach the delegate and avoid // leaking a closure on a potentially process-shared validator. @@ -377,6 +439,12 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit return config; } + /// True for the two hard "this certificate (or its issuer) is revoked" results (#277). + /// The SDK's certificate-validation status. + /// true when the status is Revoked / IssuerRevoked. + internal static bool IsRevocationFailure(StatusCode status) => + status.Code == StatusCodes.BadCertificateRevoked || status.Code == StatusCodes.BadCertificateIssuerRevoked; + /// /// Resolve the ordered failover candidate list. EndpointUrls wins when /// non-empty; otherwise fall back to EndpointUrl as a single-URL shortcut so @@ -384,10 +452,136 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit /// /// Driver options containing endpoint configuration. /// The ordered list of endpoint URLs to try. - internal static IReadOnlyList ResolveEndpointCandidates(OpcUaClientDriverOptions opts) + internal static IReadOnlyList ResolveEndpointCandidates(OpcUaClientDriverOptions opts) => + ResolveEndpointCandidates(opts, []); + + /// + /// Resolve the ordered failover list with optional discovery results (#278). Discovered URLs are + /// prepended so a discovery sweep gets first attempt; static entries follow, de-duplicated + /// case-insensitively so one server listed in both is not attempted twice in a row. With no + /// discovered URLs the result is exactly the pre-discovery behaviour. + /// + /// Driver options containing endpoint configuration. + /// URLs resolved via (may be empty). + /// The ordered list of endpoint URLs to try. + internal static IReadOnlyList ResolveEndpointCandidates( + OpcUaClientDriverOptions opts, IReadOnlyList discovered) { - if (opts.EndpointUrls is { Count: > 0 }) return opts.EndpointUrls; - return [opts.EndpointUrl]; + IReadOnlyList staticList = opts.EndpointUrls is { Count: > 0 } ? opts.EndpointUrls : [opts.EndpointUrl]; + if (discovered.Count == 0) return staticList; + + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); + var merged = new List(discovered.Count + staticList.Count); + foreach (var u in discovered) + if (!string.IsNullOrWhiteSpace(u) && seen.Add(u)) merged.Add(u); + foreach (var u in staticList) + if (!string.IsNullOrWhiteSpace(u) && seen.Add(u)) merged.Add(u); + return merged; + } + + /// + /// FindServers on (an LDS, or the server itself), then + /// GetEndpoints on each returned server's discovery URL, keeping the endpoints whose security + /// policy + mode match. One unreachable server in the list is skipped, not fatal. The discovery + /// channel is unsecured by design (OPC UA Part 4 §5.4) regardless of the session's security mode. + /// + /// Application configuration (client identity for the discovery channel). + /// The LDS / server discovery URL. + /// Required security policy. + /// Required security mode. + /// Cancellation token. + /// Matching endpoint URLs in discovery order, de-duplicated. + internal static async Task> DiscoverEndpointsAsync( + ApplicationConfiguration appConfig, string discoveryUrl, + OpcUaSecurityPolicy policy, OpcUaSecurityMode mode, CancellationToken ct) + { + var wantedPolicyUri = MapSecurityPolicy(policy); + var wantedMode = ToMessageSecurityMode(mode); + + ApplicationDescriptionCollection servers; + using (var lds = await DiscoveryClient.CreateAsync( + appConfig, new Uri(discoveryUrl), Opc.Ua.DiagnosticsMasks.None, ct).ConfigureAwait(false)) + { + servers = await lds.FindServersAsync(null, ct).ConfigureAwait(false); + } + + var urls = new List(); + foreach (var server in servers) + { + if (server.DiscoveryUrls is null) continue; + foreach (var serverDiscoveryUrl in server.DiscoveryUrls) + { + if (string.IsNullOrWhiteSpace(serverDiscoveryUrl)) continue; + EndpointDescriptionCollection endpoints; + try + { + using var ep = await DiscoveryClient.CreateAsync( + appConfig, new Uri(serverDiscoveryUrl), Opc.Ua.DiagnosticsMasks.None, ct).ConfigureAwait(false); + endpoints = await ep.GetEndpointsAsync(null, ct).ConfigureAwait(false); + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } + catch { continue; } + + urls.AddRange(FilterEndpointUrls(endpoints, wantedPolicyUri, wantedMode)); + } + } + return urls.Distinct(StringComparer.OrdinalIgnoreCase).ToList(); + } + + /// The endpoint URLs in matching the wanted policy URI + mode. + /// Endpoints advertised by a server. + /// Required SecurityPolicyUri. + /// Required message security mode. + /// Matching EndpointUrls, in advertised order. + internal static IEnumerable FilterEndpointUrls( + IEnumerable endpoints, string wantedPolicyUri, MessageSecurityMode wantedMode) => + endpoints + .Where(e => e.SecurityPolicyUri == wantedPolicyUri && e.SecurityMode == wantedMode + && !string.IsNullOrWhiteSpace(e.EndpointUrl)) + .Select(e => e.EndpointUrl); + + private static MessageSecurityMode ToMessageSecurityMode(OpcUaSecurityMode mode) => mode switch + { + OpcUaSecurityMode.None => MessageSecurityMode.None, + OpcUaSecurityMode.Sign => MessageSecurityMode.Sign, + OpcUaSecurityMode.SignAndEncrypt => MessageSecurityMode.SignAndEncrypt, + _ => throw new ArgumentOutOfRangeException(nameof(mode)), + }; + + /// + /// Range checks for the subscription-tuning and discovery options (#273/#278). Throws at + /// so a bad config fails like the namespace-kind check does rather + /// than surfacing later as a server-side BadInvalidArgument on every subscribe. + /// + /// The options to validate. + internal static void ValidateTuning(OpcUaClientDriverOptions opts) + { + if (opts.SubscriptionKeepAliveCount < 1) + throw new InvalidOperationException("OpcUaClient SubscriptionKeepAliveCount must be >= 1."); + if (opts.SubscriptionLifetimeCount < 3L * opts.SubscriptionKeepAliveCount) + throw new InvalidOperationException( + "OpcUaClient SubscriptionLifetimeCount must be at least 3x SubscriptionKeepAliveCount " + + $"(got lifetime={opts.SubscriptionLifetimeCount}, keep-alive={opts.SubscriptionKeepAliveCount})."); + if (opts.SubscriptionPublishingInterval is { } pi && pi <= TimeSpan.Zero) + throw new InvalidOperationException("OpcUaClient SubscriptionPublishingInterval must be positive when set."); + if (!string.IsNullOrWhiteSpace(opts.DiscoveryUrl) + && !Uri.TryCreate(opts.DiscoveryUrl, UriKind.Absolute, out _)) + throw new InvalidOperationException( + $"OpcUaClient DiscoveryUrl '{opts.DiscoveryUrl}' is not an absolute URI."); + } + + /// + /// Publishing interval (ms) a new subscription gets: the configured override if set, else the + /// interval the runtime requested, floored at 50 ms either way (servers negotiate up anyway, and + /// sub-50 ms just wastes negotiation). + /// + /// Driver options. + /// The per-call interval from SubscribeAsync. + /// The publishing interval in milliseconds. + internal static int ResolvePublishingIntervalMs(OpcUaClientDriverOptions opts, TimeSpan requested) + { + var effective = opts.SubscriptionPublishingInterval ?? requested; + return effective < TimeSpan.FromMilliseconds(50) ? 50 : (int)effective.TotalMilliseconds; } /// @@ -506,13 +700,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit var all = await client.GetEndpointsAsync(null, ct).ConfigureAwait(false); var wantedPolicyUri = MapSecurityPolicy(policy); - var wantedMode = mode switch - { - OpcUaSecurityMode.None => MessageSecurityMode.None, - OpcUaSecurityMode.Sign => MessageSecurityMode.Sign, - OpcUaSecurityMode.SignAndEncrypt => MessageSecurityMode.SignAndEncrypt, - _ => throw new ArgumentOutOfRangeException(nameof(mode)), - }; + var wantedMode = ToMessageSecurityMode(mode); var match = all.FirstOrDefault(e => e.SecurityPolicyUri == wantedPolicyUri && e.SecurityMode == wantedMode); @@ -639,8 +827,10 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit try { sessionToClose.KeepAlive -= _keepAliveHandler; } catch { } } _keepAliveHandler = null; + if (sessionToClose is not null) DetachDiagnostics(sessionToClose); Session = null; } + _limits = OpcUaClientOperationLimits.None; try { if (sessionToClose is Session s) await s.CloseAsync(cancellationToken).ConfigureAwait(false); } catch { /* best-effort */ } @@ -662,7 +852,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit } /// - public DriverHealth GetHealth() => _health; + public DriverHealth GetHealth() => _health with { Diagnostics = _diag.Snapshot() }; // ---- IReadable ---- @@ -712,44 +902,59 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit if (toSend.Count == 0) return results; - try + // #275: one wire Read per OperationLimits.MaxNodesPerRead-sized chunk. A chunk that fails + // only fails ITS tags — the rest of the batch keeps its values. + var chunks = OpcUaClientOperationLimits.Chunk(toSend.Count, _limits.MaxNodesPerRead).ToList(); + if (chunks.Count > 1) _diag.RecordChunkedRequest(); + Exception? firstFailure = null; + foreach (var (start, count) in chunks) { - var resp = await session.ReadAsync( - requestHeader: null, - maxAge: 0, - timestampsToReturn: TimestampsToReturn.Both, - nodesToRead: toSend, - ct: cancellationToken).ConfigureAwait(false); + var slice = new ReadValueIdCollection(count); + for (var w = start; w < start + count; w++) slice.Add(toSend[w]); + try + { + var resp = await session.ReadAsync( + requestHeader: null, + maxAge: 0, + timestampsToReturn: TimestampsToReturn.Both, + nodesToRead: slice, + ct: cancellationToken).ConfigureAwait(false); - var values = resp.Results; - for (var w = 0; w < values.Count; w++) - { - var r = indexMap[w]; - var dv = values[w]; - // Preserve the upstream StatusCode verbatim — including Bad codes per - // §8's cascading-quality rule. Also preserve SourceTimestamp so downstream - // clients can detect stale upstream data. - results[r] = new DataValueSnapshot( - Value: dv.Value, - StatusCode: dv.StatusCode.Code, - SourceTimestampUtc: dv.SourceTimestamp == DateTime.MinValue ? null : dv.SourceTimestamp, - ServerTimestampUtc: dv.ServerTimestamp == DateTime.MinValue ? now : dv.ServerTimestamp); + var values = resp.Results; + for (var w = 0; w < values.Count && w < count; w++) + { + var r = indexMap[start + w]; + var dv = values[w]; + // Preserve the upstream StatusCode verbatim — including Bad codes per + // §8's cascading-quality rule. Also preserve SourceTimestamp so downstream + // clients can detect stale upstream data. + results[r] = new DataValueSnapshot( + Value: dv.Value, + StatusCode: dv.StatusCode.Code, + SourceTimestampUtc: dv.SourceTimestamp == DateTime.MinValue ? null : dv.SourceTimestamp, + ServerTimestampUtc: dv.ServerTimestamp == DateTime.MinValue ? now : dv.ServerTimestamp); + } } - _health = new DriverHealth(DriverState.Healthy, now, null); - } - catch (Exception ex) - { - // Transport / timeout / session-dropped — fan out the same fault across every - // tag in this batch. Per-tag StatusCode stays BadCommunicationError (not - // BadInternalError) so operators distinguish "upstream unreachable" from - // "driver bug". - for (var w = 0; w < indexMap.Count; w++) + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { - var r = indexMap[w]; - results[r] = new DataValueSnapshot(null, StatusBadCommunicationError, null, now); + throw; + } + catch (Exception ex) + { + // Transport / timeout / session-dropped — fan out the same fault across every + // tag in this chunk. Per-tag StatusCode stays BadCommunicationError (not + // BadInternalError) so operators distinguish "upstream unreachable" from + // "driver bug". + for (var w = start; w < start + count; w++) + results[indexMap[w]] = new DataValueSnapshot(null, StatusBadCommunicationError, null, now); + firstFailure ??= ex; } - _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message); } + + _diag.RecordRead(failed: firstFailure is not null); + _health = firstFailure is null + ? new DriverHealth(DriverState.Healthy, now, null) + : new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, firstFailure.Message); } finally { _gate.Release(); } return results; @@ -799,40 +1004,55 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit if (toSend.Count == 0) return results; - try + // #275: one wire Write per OperationLimits.MaxNodesPerWrite-sized chunk. Chunks are sent in + // order and each fails independently: a later chunk that times out leaves earlier chunks' + // real results intact (those writes DID happen). + var chunks = OpcUaClientOperationLimits.Chunk(toSend.Count, _limits.MaxNodesPerWrite).ToList(); + if (chunks.Count > 1) _diag.RecordChunkedRequest(); + var anyFailed = false; + foreach (var (start, count) in chunks) { - var resp = await session.WriteAsync( - requestHeader: null, - nodesToWrite: toSend, - ct: cancellationToken).ConfigureAwait(false); - - var codes = resp.Results; - for (var w = 0; w < codes.Count; w++) + var slice = new WriteValueCollection(count); + for (var w = start; w < start + count; w++) slice.Add(toSend[w]); + try { - var r = indexMap[w]; - // Pass upstream WriteResult StatusCode through verbatim. Success codes - // include Good (0) and any warning-level Good* variants; anything with - // the severity bits set is a Bad. - results[r] = new WriteResult(codes[w].Code); + var resp = await session.WriteAsync( + requestHeader: null, + nodesToWrite: slice, + ct: cancellationToken).ConfigureAwait(false); + + var codes = resp.Results; + for (var w = 0; w < codes.Count && w < count; w++) + { + // Pass upstream WriteResult StatusCode through verbatim. Success codes + // include Good (0) and any warning-level Good* variants; anything with + // the severity bits set is a Bad. + results[indexMap[start + w]] = new WriteResult(codes[w].Code); + } + } + catch (OperationCanceledException) + { + // Timeout / cancellation after the wire request may have been dispatched. + // Writes are non-idempotent — BadTimeout ("outcome unknown, do not blindly + // retry") is more honest than BadCommunicationError ("definitely did not + // happen"). Downstream callers that need retry semantics check for BadTimeout + // and can decide whether to re-issue. Every not-yet-answered chunk (this one and + // the ones after it) gets the same code. + const uint StatusBadTimeout = 0x800A0000u; + anyFailed = true; + for (var w = start; w < toSend.Count; w++) + results[indexMap[w]] = new WriteResult(StatusBadTimeout); + break; + } + catch (Exception) + { + // Pre-wire transport failure — the write definitely did not reach the server. + anyFailed = true; + for (var w = start; w < start + count; w++) + results[indexMap[w]] = new WriteResult(StatusBadCommunicationError); } } - catch (OperationCanceledException) - { - // Timeout / cancellation after the wire request may have been dispatched. - // Writes are non-idempotent — BadTimeout ("outcome unknown, do not blindly - // retry") is more honest than BadCommunicationError ("definitely did not - // happen"). Downstream callers that need retry semantics check for BadTimeout - // and can decide whether to re-issue. - const uint StatusBadTimeout = 0x800A0000u; - for (var w = 0; w < indexMap.Count; w++) - results[indexMap[w]] = new WriteResult(StatusBadTimeout); - } - catch (Exception) - { - // Pre-wire transport failure — the write definitely did not reach the server. - for (var w = 0; w < indexMap.Count; w++) - results[indexMap[w]] = new WriteResult(StatusBadCommunicationError); - } + _diag.RecordWrite(anyFailed); } finally { _gate.Release(); } return results; @@ -1099,16 +1319,27 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit nodesToRead.Add(new ReadValueId { NodeId = pv.NodeId, AttributeId = Attributes.Historizing }); } - DataValueCollection values; + // #275: split the attribute reads to the server's MaxNodesPerRead (each variable contributes 4). + var values = new DataValueCollection(nodesToRead.Count); try { - var resp = await session.ReadAsync( - requestHeader: null, - maxAge: 0, - timestampsToReturn: TimestampsToReturn.Neither, - nodesToRead: nodesToRead, - ct: ct).ConfigureAwait(false); - values = resp.Results; + foreach (var (start, count) in OpcUaClientOperationLimits.Chunk(nodesToRead.Count, _limits.MaxNodesPerRead)) + { + var slice = new ReadValueIdCollection(count); + for (var w = start; w < start + count; w++) slice.Add(nodesToRead[w]); + var resp = await session.ReadAsync( + requestHeader: null, + maxAge: 0, + timestampsToReturn: TimestampsToReturn.Neither, + nodesToRead: slice, + ct: ct).ConfigureAwait(false); + values.AddRange(resp.Results); + } + if (values.Count < nodesToRead.Count) throw new InvalidOperationException("Short enrichment read."); + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + throw; } catch { @@ -1230,20 +1461,18 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // Floor the publishing interval at 50ms — OPC UA servers routinely negotiate // minimum-supported intervals up anyway, but sending sub-50ms wastes negotiation - // bandwidth on every subscription create. - var intervalMs = publishingInterval < TimeSpan.FromMilliseconds(50) - ? 50 - : (int)publishingInterval.TotalMilliseconds; + // bandwidth on every subscription create. #273: the options may pin the interval. + var intervalMs = ResolvePublishingIntervalMs(_options, publishingInterval); var subscription = new Subscription(telemetry: null!, new SubscriptionOptions { DisplayName = $"opcua-sub-{id}", PublishingInterval = intervalMs, - KeepAliveCount = 10, - LifetimeCount = 1000, - MaxNotificationsPerPublish = 0, + KeepAliveCount = _options.SubscriptionKeepAliveCount, + LifetimeCount = _options.SubscriptionLifetimeCount, + MaxNotificationsPerPublish = _options.SubscriptionMaxNotificationsPerPublish, PublishingEnabled = true, - Priority = 0, + Priority = _options.SubscriptionPriority, TimestampsToReturn = TimestampsToReturn.Both, }); @@ -1263,6 +1492,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // captures `handle`, so we must hold the exact delegate instance returned by // `+=` to be able to remove it. var itemHandlers = new List(); + var itemsAdded = 0; foreach (var fullRef in fullReferences) { if (!TryResolveTagNode(session, fullRef, out var nodeId)) continue; @@ -1287,6 +1517,15 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit item.Notification += notifHandler; itemHandlers.Add(new MonitoredItemNotificationHandle(item, notifHandler)); subscription.AddItem(item); + itemsAdded++; + + // #275: flush a CreateMonitoredItems call whenever the server's per-call cap is reached. + // CreateItemsAsync only sends the items not yet created, so calling it per batch is safe. + if (_limits.MaxMonitoredItemsPerCall > 0 && itemsAdded % _limits.MaxMonitoredItemsPerCall == 0) + { + _diag.RecordChunkedRequest(); + await subscription.CreateItemsAsync(cancellationToken).ConfigureAwait(false); + } } await subscription.CreateItemsAsync(cancellationToken).ConfigureAwait(false); @@ -1336,6 +1575,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit var dv = mn.Value; if (dv is null) return; var fullRef = (item.Handle as string) ?? item.DisplayName ?? string.Empty; + _diag.RecordDataChange(bad: StatusCode.IsBad(dv.StatusCode)); var snapshot = new DataValueSnapshot( Value: dv.Value, StatusCode: dv.StatusCode.Code, @@ -1405,9 +1645,9 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit { DisplayName = $"opcua-alarm-sub-{id}", PublishingInterval = 500, // 500ms — alarms don't need fast polling; the server pushes - KeepAliveCount = 10, - LifetimeCount = 1000, - MaxNotificationsPerPublish = 0, + KeepAliveCount = _options.SubscriptionKeepAliveCount, + LifetimeCount = _options.SubscriptionLifetimeCount, + MaxNotificationsPerPublish = _options.SubscriptionMaxNotificationsPerPublish, PublishingEnabled = true, Priority = 0, TimestampsToReturn = TimestampsToReturn.Both, @@ -1988,6 +2228,49 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit return [new HostConnectivityStatus(HostName, _hostState, _hostStateChangedUtc)]; } + /// Hook the SDK's publish-side events into the diagnostics counters (#276). + /// The session to observe. + private void AttachDiagnostics(ISession session) + { + _notificationHandler ??= (_, e) => + { + _diag.RecordPublishResponse(); + var msg = e.NotificationMessage; + if (msg?.NotificationData is { } data) + foreach (var nd in data) + _diag.RecordNotifications(CountNotifications(nd)); + }; + _publishErrorHandler ??= (_, _) => _diag.RecordPublishError(); + session.Notification -= _notificationHandler; + session.Notification += _notificationHandler; + session.PublishError -= _publishErrorHandler; + session.PublishError += _publishErrorHandler; + } + + private void DetachDiagnostics(ISession session) + { + try + { + if (_notificationHandler is not null) session.Notification -= _notificationHandler; + if (_publishErrorHandler is not null) session.PublishError -= _publishErrorHandler; + } + catch { /* best-effort — SDK may have already cleared its invocation lists on session loss */ } + } + + private static int CountNotifications(ExtensionObject nd) => nd.Body switch + { + DataChangeNotification dc => dc.MonitoredItems?.Count ?? 0, + EventNotificationList ev => ev.Events?.Count ?? 0, + StatusChangeNotification => 1, + _ => 1, + }; + + private async Task RefreshOperationLimitsAsync(ISession session) + { + try { _limits = await OpcUaClientOperationLimits.ReadAsync(session, CancellationToken.None).ConfigureAwait(false); } + catch { /* best-effort: keep the previous limits */ } + } + /// /// Session keep-alive handler. On a healthy ping, bumps HostState back to Running /// (typical bounce after a transient network blip). On a bad ping, starts the SDK's @@ -2003,6 +2286,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit } TransitionTo(HostState.Stopped); + _diag.RecordKeepAliveMiss(); // Kick off the SDK's reconnect loop exactly once per drop. Keep-alive callbacks fire // from the SDK keep-alive timer thread and the SDK can fire this handler repeatedly @@ -2022,6 +2306,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // BeginReconnect is started outside the lock — it does no _reconnectHandler mutation // and we don't want to hold _probeLock across an SDK call. + _diag.RecordReconnectAttempt(); handler.BeginReconnect( sender, (int)_options.ReconnectPeriod.TotalMilliseconds, @@ -2062,6 +2347,10 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit { newSession.KeepAlive += _keepAliveHandler; } + // Detach-then-attach: the SDK may hand back the SAME session object, and -= on a handler that + // is not attached is a no-op, so this never double-counts. + if (oldSession is not null) DetachDiagnostics(oldSession); + if (newSession is not null) AttachDiagnostics(newSession); Session = newSession; @@ -2084,6 +2373,8 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit RecordSkippedTags(_namespaceMap); TransitionTo(HostState.Running); _health = new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null); + _diag.RecordReconnectSucceeded(); + if (_options.HonorServerOperationLimits) _ = RefreshOperationLimitsAsync(newSession); return; } @@ -2096,6 +2387,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // the last-known session for an always-on gateway rather than abandoning recovery // entirely. TransitionTo(HostState.Faulted); + _diag.RecordReconnectExhausted(); _health = new DriverHealth( DriverState.Faulted, _health.LastSuccessfulRead, "OPC UA session reconnect exhausted its retry window without recovering. " + diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientOperationLimits.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientOperationLimits.cs new file mode 100644 index 00000000..7f38ac5c --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientOperationLimits.cs @@ -0,0 +1,94 @@ +using Opc.Ua; +using Opc.Ua.Client; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; + +/// +/// The remote server's Server/ServerCapabilities/OperationLimits that bound how many +/// items one service call may carry (#275). Every field is 0 = "no limit declared" (the +/// OPC UA convention), so is the safe value when the server publishes nothing +/// or the read fails. +/// +/// Items per Read. +/// Items per Write. +/// Nodes per Browse. +/// Items per CreateMonitoredItems / Modify / SetMonitoringMode. +/// Nodes per HistoryRead (raw / processed / at-time). +/// Nodes per HistoryRead (events). +internal sealed record OpcUaClientOperationLimits( + uint MaxNodesPerRead, + uint MaxNodesPerWrite, + uint MaxNodesPerBrowse, + uint MaxMonitoredItemsPerCall, + uint MaxNodesPerHistoryReadData, + uint MaxNodesPerHistoryReadEvents) +{ + /// No limits declared — every call goes out in one request. + public static OpcUaClientOperationLimits None { get; } = new(0, 0, 0, 0, 0, 0); + + /// + /// Split items into consecutive (Start, Count) chunks of at most + /// each. limit == 0 (no limit) or a total that already fits yields a + /// single chunk; total == 0 yields none. + /// + /// Number of items to send. + /// The server's per-call cap; 0 = unlimited. + /// The chunk ranges, in order, covering [0, total) exactly once. + public static IEnumerable<(int Start, int Count)> Chunk(int total, uint limit) + { + if (total <= 0) yield break; + if (limit == 0 || limit >= (uint)total) + { + yield return (0, total); + yield break; + } + var size = (int)Math.Min(limit, int.MaxValue); + for (var start = 0; start < total; start += size) + yield return (start, Math.Min(size, total - start)); + } + + /// The standard variables read to populate the limits, in order. + internal static readonly NodeId[] LimitNodeIds = + [ + VariableIds.Server_ServerCapabilities_OperationLimits_MaxNodesPerRead, + VariableIds.Server_ServerCapabilities_OperationLimits_MaxNodesPerWrite, + VariableIds.Server_ServerCapabilities_OperationLimits_MaxNodesPerBrowse, + VariableIds.Server_ServerCapabilities_OperationLimits_MaxMonitoredItemsPerCall, + VariableIds.Server_ServerCapabilities_OperationLimits_MaxNodesPerHistoryReadData, + VariableIds.Server_ServerCapabilities_OperationLimits_MaxNodesPerHistoryReadEvents, + ]; + + /// + /// Build limits from the values read for . A Bad or non-uint value + /// (server doesn't publish that limit) is treated as 0 = unlimited. + /// + /// One value per entry, in order. + /// The parsed limits. + public static OpcUaClientOperationLimits FromValues(IReadOnlyList values) + { + uint At(int i) => + i < values.Count && values[i] is { } dv && StatusCode.IsGood(dv.StatusCode) && dv.Value is uint u ? u : 0; + return new OpcUaClientOperationLimits(At(0), At(1), At(2), At(3), At(4), At(5)); + } + + /// + /// Read the server's declared limits. Best-effort: any failure (older server without the + /// capability object, transport error) yields so connect never fails over this. + /// + /// The freshly connected session. + /// Cancellation token. + /// The declared limits, or . + public static async Task ReadAsync(ISession session, CancellationToken ct) + { + try + { + var nodes = new ReadValueIdCollection(LimitNodeIds.Length); + foreach (var id in LimitNodeIds) + nodes.Add(new ReadValueId { NodeId = id, AttributeId = Attributes.Value }); + var resp = await session.ReadAsync(null, 0, TimestampsToReturn.Neither, nodes, ct).ConfigureAwait(false); + return FromValues(resp.Results); + } + catch (OperationCanceledException) { throw; } + catch { return None; } + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor index 1dace2bb..ba66a78e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor @@ -185,6 +185,63 @@ +@* Subscriptions, limits, revocation, discovery (#273/#275/#277/#278) *@ +
+
Subscriptions, limits & discovery
+
+
+
+ + +
Blank = use the interval the runtime requests per subscription.
+
+
+ + +
Default 10.
+
+
+ + +
Default 1000; at least 3x keep-alive count.
+
+
+ + +
0 = server decides.
+
+
+ + +
+
+
+ + +
+
Split oversized Read / Write / monitored-item calls.
+
+
+
+ + +
+
Revoked certificates are always rejected.
+
+
+ + +
CRLs are read from its crl/ sub-folder. Blank = default PKI store.
+
+
+ + +
Optional. Discovered endpoints are tried before the failover list.
+
+
+
+
+ @* Diagnostics *@
Diagnostics
@@ -318,6 +375,18 @@ public int ProbeTimeoutSeconds { get; set; } = 15; + // #273 / #275 / #277 / #278 — without these the typed form silently drops the keys on save (#576). + public int? SubscriptionPublishingIntervalMs { get; set; } + // int, not uint/byte: InputNumber 500s the page (#505). + public int SubscriptionKeepAliveCount { get; set; } = 10; + public int SubscriptionLifetimeCount { get; set; } = 1000; + public int SubscriptionMaxNotificationsPerPublish { get; set; } + public int SubscriptionPriority { get; set; } + public bool HonorServerOperationLimits { get; set; } = true; + public bool RejectUnknownRevocationStatus { get; set; } + public string? IssuerStorePath { get; set; } + public string? DiscoveryUrl { get; set; } + internal IReadOnlyDictionary _unsMappingTable = new Dictionary(); public static OpcUaClientFormModel FromRecord(OpcUaClientDriverOptions r) => new() @@ -343,6 +412,15 @@ UserCertificatePassword = r.UserCertificatePassword, TargetNamespaceKind = r.TargetNamespaceKind, ProbeTimeoutSeconds = r.ProbeTimeoutSeconds, + SubscriptionPublishingIntervalMs = r.SubscriptionPublishingInterval is { } spi ? (int)spi.TotalMilliseconds : null, + SubscriptionKeepAliveCount = (int)Math.Min(r.SubscriptionKeepAliveCount, int.MaxValue), + SubscriptionLifetimeCount = (int)Math.Min(r.SubscriptionLifetimeCount, int.MaxValue), + SubscriptionMaxNotificationsPerPublish = (int)Math.Min(r.SubscriptionMaxNotificationsPerPublish, int.MaxValue), + SubscriptionPriority = r.SubscriptionPriority, + HonorServerOperationLimits = r.HonorServerOperationLimits, + RejectUnknownRevocationStatus = r.CertificateValidation.RejectUnknownRevocationStatus, + IssuerStorePath = r.CertificateValidation.IssuerStorePath, + DiscoveryUrl = r.DiscoveryUrl, _unsMappingTable = r.UnsMappingTable, }; @@ -371,6 +449,19 @@ TargetNamespaceKind = TargetNamespaceKind, UnsMappingTable = _unsMappingTable, ProbeTimeoutSeconds = ProbeTimeoutSeconds, + SubscriptionPublishingInterval = SubscriptionPublishingIntervalMs is { } ms and > 0 + ? TimeSpan.FromMilliseconds(ms) : null, + SubscriptionKeepAliveCount = (uint)Math.Max(SubscriptionKeepAliveCount, 0), + SubscriptionLifetimeCount = (uint)Math.Max(SubscriptionLifetimeCount, 0), + SubscriptionMaxNotificationsPerPublish = (uint)Math.Max(SubscriptionMaxNotificationsPerPublish, 0), + SubscriptionPriority = (byte)Math.Clamp(SubscriptionPriority, 0, 255), + HonorServerOperationLimits = HonorServerOperationLimits, + CertificateValidation = new OpcUaCertificateValidationOptions + { + RejectUnknownRevocationStatus = RejectUnknownRevocationStatus, + IssuerStorePath = string.IsNullOrWhiteSpace(IssuerStorePath) ? null : IssuerStorePath, + }, + DiscoveryUrl = string.IsNullOrWhiteSpace(DiscoveryUrl) ? null : DiscoveryUrl, }; } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientDiagnosticsTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientDiagnosticsTests.cs new file mode 100644 index 00000000..9fa0353c --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientDiagnosticsTests.cs @@ -0,0 +1,111 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; + +/// #276 — counters are thread-safe and surface through DriverHealth.Diagnostics. +[Trait("Category", "Unit")] +public sealed class OpcUaClientDiagnosticsTests +{ + [Fact] + public void Fresh_snapshot_is_all_zero_with_the_full_key_set() + { + var snap = new OpcUaClientDiagnostics().Snapshot(); + + snap.Keys.ShouldBe( + [ + "PublishResponseCount", "NotificationCount", "DataChangeCount", "BadStatusValueCount", + "PublishErrorCount", "KeepAliveMissCount", "ReconnectAttemptCount", "ReconnectSuccessCount", + "ReconnectExhaustedCount", "LastReconnectUtcTicks", "ReadCallCount", "ReadFailureCount", + "WriteCallCount", "WriteFailureCount", "ChunkedRequestCount", + ], ignoreOrder: true); + snap.Values.ShouldAllBe(v => v == 0); + } + + [Fact] + public void Each_recorder_moves_exactly_its_own_counter() + { + var d = new OpcUaClientDiagnostics(); + d.RecordPublishResponse(); + d.RecordNotifications(3); + d.RecordDataChange(bad: false); + d.RecordDataChange(bad: true); + d.RecordPublishError(); + d.RecordKeepAliveMiss(); + d.RecordReconnectAttempt(); + d.RecordReconnectSucceeded(); + d.RecordReconnectExhausted(); + d.RecordRead(failed: true); + d.RecordWrite(failed: false); + d.RecordChunkedRequest(); + + var s = d.Snapshot(); + s["PublishResponseCount"].ShouldBe(1); + s["NotificationCount"].ShouldBe(3); + s["DataChangeCount"].ShouldBe(2); + s["BadStatusValueCount"].ShouldBe(1); + s["PublishErrorCount"].ShouldBe(1); + s["KeepAliveMissCount"].ShouldBe(1); + s["ReconnectAttemptCount"].ShouldBe(1); + s["ReconnectSuccessCount"].ShouldBe(1); + s["ReconnectExhaustedCount"].ShouldBe(1); + s["LastReconnectUtcTicks"].ShouldBeGreaterThan(0); + s["ReadCallCount"].ShouldBe(1); + s["ReadFailureCount"].ShouldBe(1); + s["WriteCallCount"].ShouldBe(1); + s["WriteFailureCount"].ShouldBe(0); + s["ChunkedRequestCount"].ShouldBe(1); + } + + [Fact] + public void Concurrent_increments_do_not_lose_updates() + { + var d = new OpcUaClientDiagnostics(); + const int threads = 8, each = 5_000; + + Parallel.For(0, threads, _ => + { + for (var i = 0; i < each; i++) { d.RecordDataChange(bad: i % 2 == 0); d.RecordNotifications(2); } + }); + + var s = d.Snapshot(); + s["DataChangeCount"].ShouldBe(threads * each); + s["BadStatusValueCount"].ShouldBe(threads * each / 2); + s["NotificationCount"].ShouldBe(threads * each * 2); + } + + [Fact] + public void Snapshot_is_a_point_in_time_copy() + { + var d = new OpcUaClientDiagnostics(); + var before = d.Snapshot(); + d.RecordKeepAliveMiss(); + + before["KeepAliveMissCount"].ShouldBe(0); + d.Snapshot()["KeepAliveMissCount"].ShouldBe(1); + } + + [Fact] + public void GetHealth_carries_the_diagnostics_and_keeps_state_fields() + { + using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-diag"); + drv.DiagnosticsForTest.RecordKeepAliveMiss(); + drv.DiagnosticsForTest.RecordKeepAliveMiss(); + + var h = drv.GetHealth(); + + h.State.ShouldBe(DriverState.Unknown); + h.Diagnostics.ShouldNotBeNull(); + h.Diagnostics!["KeepAliveMissCount"].ShouldBe(2); + } + + [Fact] + public void DriverHealth_without_diagnostics_stays_back_compatible() + { + var h = new DriverHealth(DriverState.Healthy, null, null); + + h.Diagnostics.ShouldBeNull(); + h.ShouldBe(new DriverHealth(DriverState.Healthy, null, null)); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientOperationLimitsTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientOperationLimitsTests.cs new file mode 100644 index 00000000..a97c586f --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientOperationLimitsTests.cs @@ -0,0 +1,163 @@ +using Moq; +using Opc.Ua; +using Opc.Ua.Client; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; + +/// #275 — the driver splits Read / Write calls to the server's declared OperationLimits. +[Trait("Category", "Unit")] +public sealed class OpcUaClientOperationLimitsTests +{ + [Theory] + [InlineData(0, 0u, new int[0])] // nothing to send => no call + [InlineData(5, 0u, new[] { 5 })] // 0 = unlimited + [InlineData(5, 5u, new[] { 5 })] // exactly fits + [InlineData(5, 100u, new[] { 5 })] // limit larger than the batch + [InlineData(10, 4u, new[] { 4, 4, 2 })] // remainder chunk + [InlineData(9, 3u, new[] { 3, 3, 3 })] + [InlineData(3, 1u, new[] { 1, 1, 1 })] + public void Chunk_splits_to_the_limit(int total, uint limit, int[] expectedSizes) + { + var chunks = OpcUaClientOperationLimits.Chunk(total, limit).ToList(); + + chunks.Select(c => c.Count).ToArray().ShouldBe(expectedSizes); + // Contiguous cover of [0, total): every item exactly once, in order. + var next = 0; + foreach (var (start, count) in chunks) { start.ShouldBe(next); next += count; } + next.ShouldBe(total); + } + + [Fact] + public void FromValues_reads_good_uints_and_treats_bad_or_missing_as_unlimited() + { + var values = new List + { + new(new Variant(100u)), + new(new Variant(50u)), + new(StatusCodes.BadNodeIdUnknown), // server doesn't publish MaxNodesPerBrowse + new(new Variant(7u)), + }; + + var limits = OpcUaClientOperationLimits.FromValues(values); + + limits.MaxNodesPerRead.ShouldBe(100u); + limits.MaxNodesPerWrite.ShouldBe(50u); + limits.MaxNodesPerBrowse.ShouldBe(0u); + limits.MaxMonitoredItemsPerCall.ShouldBe(7u); + limits.MaxNodesPerHistoryReadData.ShouldBe(0u); // short list => 0 + limits.MaxNodesPerHistoryReadEvents.ShouldBe(0u); + } + + private static (OpcUaClientDriver Drv, Mock Session, List ReadSizes, List WriteSizes) Rig( + uint readLimit, uint writeLimit, int tagCount) + { + var tags = Enumerable.Range(0, tagCount) + .Select(i => new RawTagEntry($"raw/t{i}", $$"""{"nodeId":"ns=2;s=T{{i}}"}""", WriteIdempotent: false)) + .ToList(); + var options = new OpcUaClientDriverOptions { RawTags = tags }; + var drv = new OpcUaClientDriver(options, "opcua-limits"); + drv.SetNamespaceMapForTest(ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.NamespaceMap.FromTable(new NamespaceTable(), tags)); + drv.SetOperationLimitsForTest(OpcUaClientOperationLimits.None with + { + MaxNodesPerRead = readLimit, MaxNodesPerWrite = writeLimit, + }); + + var readSizes = new List(); + var writeSizes = new List(); + var mock = new Mock(MockBehavior.Loose); + mock.SetupGet(s => s.MessageContext) + .Returns(new ServiceMessageContext(SdkTelemetry.Context)); + mock.SetupGet(s => s.NamespaceUris).Returns(new NamespaceTable()); + mock.Setup(s => s.ReadAsync(It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())) + .ReturnsAsync((RequestHeader _, double _, TimestampsToReturn _, ReadValueIdCollection nodes, CancellationToken _) => + { + readSizes.Add(nodes.Count); + // Echo each node's identifier number back as the value so ordering across chunks is checkable. + var results = new DataValueCollection(); + foreach (var n in nodes) results.Add(new DataValue(new Variant((string)n.NodeId.Identifier))); + return new ReadResponse { ResponseHeader = new ResponseHeader(), Results = results }; + }); + mock.Setup(s => s.WriteAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((RequestHeader _, WriteValueCollection nodes, CancellationToken _) => + { + writeSizes.Add(nodes.Count); + var codes = new StatusCodeCollection(); + foreach (var _ in nodes) codes.Add(StatusCodes.Good); + return new WriteResponse { ResponseHeader = new ResponseHeader(), Results = codes }; + }); + drv.SetSessionForTest(mock.Object); + return (drv, mock, readSizes, writeSizes); + } + + [Fact] + public async Task ReadAsync_splits_to_MaxNodesPerRead_and_keeps_result_order() + { + var (drv, _, readSizes, _) = Rig(readLimit: 4, writeLimit: 0, tagCount: 10); + using var _ = drv; + + var refs = Enumerable.Range(0, 10).Select(i => $"raw/t{i}").ToList(); + var res = await drv.ReadAsync(refs, TestContext.Current.CancellationToken); + + readSizes.ShouldBe([4, 4, 2]); + res.Count.ShouldBe(10); + for (var i = 0; i < 10; i++) res[i].Value.ShouldBe($"T{i}"); // positional mapping survives chunking + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + drv.GetHealth().Diagnostics!["ChunkedRequestCount"].ShouldBe(1); + } + + [Fact] + public async Task ReadAsync_with_no_declared_limit_sends_one_request() + { + var (drv, _, readSizes, _) = Rig(readLimit: 0, writeLimit: 0, tagCount: 10); + using var _ = drv; + + await drv.ReadAsync(Enumerable.Range(0, 10).Select(i => $"raw/t{i}").ToList(), TestContext.Current.CancellationToken); + + readSizes.ShouldBe([10]); + } + + [Fact] + public async Task ReadAsync_a_failing_chunk_only_fails_its_own_tags() + { + var (drv, session, _, _) = Rig(readLimit: 2, writeLimit: 0, tagCount: 4); + using var _ = drv; + var call = 0; + session.Setup(s => s.ReadAsync(It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())) + .Returns((RequestHeader _, double _, TimestampsToReturn _, ReadValueIdCollection nodes, CancellationToken _) => + { + if (Interlocked.Increment(ref call) == 2) throw new ServiceResultException(StatusCodes.BadConnectionClosed); + var results = new DataValueCollection(); + foreach (var n in nodes) results.Add(new DataValue(new Variant((string)n.NodeId.Identifier))); + return Task.FromResult(new ReadResponse { ResponseHeader = new ResponseHeader(), Results = results }); + }); + + var res = await drv.ReadAsync(["raw/t0", "raw/t1", "raw/t2", "raw/t3"], TestContext.Current.CancellationToken); + + res[0].Value.ShouldBe("T0"); + res[1].Value.ShouldBe("T1"); + res[2].StatusCode.ShouldBe(0x80050000u); // BadCommunicationError, chunk 2 only + res[3].StatusCode.ShouldBe(0x80050000u); + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + drv.GetHealth().Diagnostics!["ReadFailureCount"].ShouldBe(1); + } + + [Fact] + public async Task WriteAsync_splits_to_MaxNodesPerWrite() + { + var (drv, _, _, writeSizes) = Rig(readLimit: 0, writeLimit: 3, tagCount: 7); + using var _ = drv; + + var res = await drv.WriteAsync( + Enumerable.Range(0, 7).Select(i => new Core.Abstractions.WriteRequest($"raw/t{i}", i)).ToList(), + TestContext.Current.CancellationToken); + + writeSizes.ShouldBe([3, 3, 1]); + res.Count.ShouldBe(7); + res.ShouldAllBe(r => r.StatusCode == 0u); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientTuningAndDiscoveryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientTuningAndDiscoveryTests.cs new file mode 100644 index 00000000..910ced5a --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientTuningAndDiscoveryTests.cs @@ -0,0 +1,157 @@ +using Opc.Ua; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; + +/// #273 subscription tuning, #277 revocation options, #278 discovery URL. +[Trait("Category", "Unit")] +public sealed class OpcUaClientTuningAndDiscoveryTests +{ + // ---- #273 ---- + + [Fact] + public void Defaults_equal_the_values_that_used_to_be_hardcoded() + { + var o = new OpcUaClientDriverOptions(); + + o.SubscriptionKeepAliveCount.ShouldBe(10u); + o.SubscriptionLifetimeCount.ShouldBe(1000u); + o.SubscriptionMaxNotificationsPerPublish.ShouldBe(0u); + o.SubscriptionPriority.ShouldBe((byte)0); + o.SubscriptionPublishingInterval.ShouldBeNull(); + o.HonorServerOperationLimits.ShouldBeTrue(); + o.DiscoveryUrl.ShouldBeNull(); + o.CertificateValidation.RejectUnknownRevocationStatus.ShouldBeFalse(); + o.CertificateValidation.IssuerStorePath.ShouldBeNull(); + Should.NotThrow(() => OpcUaClientDriver.ValidateTuning(o)); + } + + [Fact] + public void Factory_json_binds_the_new_keys_and_old_json_keeps_defaults() + { + var o = OpcUaClientDriverFactoryExtensions.ParseOptions("d", """ + {"subscriptionPublishingInterval":"00:00:02","subscriptionKeepAliveCount":20, + "subscriptionLifetimeCount":600,"subscriptionMaxNotificationsPerPublish":500, + "subscriptionPriority":9,"honorServerOperationLimits":false, + "discoveryUrl":"opc.tcp://lds:4840", + "certificateValidation":{"rejectUnknownRevocationStatus":true,"issuerStorePath":"/pki/issuers"}} + """); + + o.SubscriptionPublishingInterval.ShouldBe(TimeSpan.FromSeconds(2)); + o.SubscriptionKeepAliveCount.ShouldBe(20u); + o.SubscriptionLifetimeCount.ShouldBe(600u); + o.SubscriptionMaxNotificationsPerPublish.ShouldBe(500u); + o.SubscriptionPriority.ShouldBe((byte)9); + o.HonorServerOperationLimits.ShouldBeFalse(); + o.DiscoveryUrl.ShouldBe("opc.tcp://lds:4840"); + o.CertificateValidation.RejectUnknownRevocationStatus.ShouldBeTrue(); + o.CertificateValidation.IssuerStorePath.ShouldBe("/pki/issuers"); + + var legacy = OpcUaClientDriverFactoryExtensions.ParseOptions("d", """{"endpointUrl":"opc.tcp://x:1"}"""); + legacy.SubscriptionKeepAliveCount.ShouldBe(10u); + legacy.SubscriptionLifetimeCount.ShouldBe(1000u); + } + + [Theory] + [InlineData(0u, 1000u)] // keep-alive must be >= 1 + [InlineData(10u, 29u)] // lifetime < 3x keep-alive + [InlineData(500u, 1000u)] + public void ValidateTuning_rejects_out_of_range_counts(uint keepAlive, uint lifetime) + { + var o = new OpcUaClientDriverOptions { SubscriptionKeepAliveCount = keepAlive, SubscriptionLifetimeCount = lifetime }; + + Should.Throw(() => OpcUaClientDriver.ValidateTuning(o)); + } + + [Fact] + public void ValidateTuning_accepts_the_3x_boundary_and_rejects_bad_interval_or_discovery_url() + { + Should.NotThrow(() => OpcUaClientDriver.ValidateTuning( + new OpcUaClientDriverOptions { SubscriptionKeepAliveCount = 10, SubscriptionLifetimeCount = 30 })); + Should.Throw(() => OpcUaClientDriver.ValidateTuning( + new OpcUaClientDriverOptions { SubscriptionPublishingInterval = TimeSpan.Zero })); + Should.Throw(() => OpcUaClientDriver.ValidateTuning( + new OpcUaClientDriverOptions { DiscoveryUrl = "not a uri" })); + } + + [Theory] + [InlineData(null, 1000, 1000)] // no override: the per-call value + [InlineData(null, 10, 50)] // floor still applies to the per-call value + [InlineData(2000, 250, 2000)] // override wins over the per-call value + [InlineData(5, 250, 50)] // ... and is floored too + public void ResolvePublishingIntervalMs_prefers_the_override_and_floors_at_50(int? overrideMs, int requestedMs, int expected) + { + var o = new OpcUaClientDriverOptions + { + SubscriptionPublishingInterval = overrideMs is { } ms ? TimeSpan.FromMilliseconds(ms) : null, + }; + + OpcUaClientDriver.ResolvePublishingIntervalMs(o, TimeSpan.FromMilliseconds(requestedMs)).ShouldBe(expected); + } + + // ---- #277 ---- + + [Fact] + public void Revocation_failures_are_exactly_Revoked_and_IssuerRevoked() + { + OpcUaClientDriver.IsRevocationFailure(StatusCodes.BadCertificateRevoked).ShouldBeTrue(); + OpcUaClientDriver.IsRevocationFailure(StatusCodes.BadCertificateIssuerRevoked).ShouldBeTrue(); + // "unknown" is governed by RejectUnknownRevocationStatus, not auto-accept's hard exception; + // untrusted is what auto-accept exists to wave through. + OpcUaClientDriver.IsRevocationFailure(StatusCodes.BadCertificateRevocationUnknown).ShouldBeFalse(); + OpcUaClientDriver.IsRevocationFailure(StatusCodes.BadCertificateUntrusted).ShouldBeFalse(); + OpcUaClientDriver.IsRevocationFailure(StatusCodes.Good).ShouldBeFalse(); + } + + // ---- #278 ---- + + private static EndpointDescription Ep(string url, string policyUri, MessageSecurityMode mode) => + new() { EndpointUrl = url, SecurityPolicyUri = policyUri, SecurityMode = mode }; + + [Fact] + public void ResolveEndpointCandidates_without_discovery_is_unchanged() + { + var o = new OpcUaClientDriverOptions { EndpointUrls = ["opc.tcp://a:1", "opc.tcp://b:2"] }; + + OpcUaClientDriver.ResolveEndpointCandidates(o, []).ShouldBe(["opc.tcp://a:1", "opc.tcp://b:2"]); + OpcUaClientDriver.ResolveEndpointCandidates(o).ShouldBe(["opc.tcp://a:1", "opc.tcp://b:2"]); + } + + [Fact] + public void ResolveEndpointCandidates_prepends_discovered_urls_and_dedups_case_insensitively() + { + var o = new OpcUaClientDriverOptions { EndpointUrls = ["opc.tcp://Static:1", "opc.tcp://found:2"] }; + + var merged = OpcUaClientDriver.ResolveEndpointCandidates(o, ["opc.tcp://found:2", "opc.tcp://new:3"]); + + merged.ShouldBe(["opc.tcp://found:2", "opc.tcp://new:3", "opc.tcp://Static:1"]); + } + + [Fact] + public void ResolveEndpointCandidates_uses_the_single_EndpointUrl_after_discovered_when_no_list() + { + var o = new OpcUaClientDriverOptions { EndpointUrl = "opc.tcp://only:9" }; + + OpcUaClientDriver.ResolveEndpointCandidates(o, ["opc.tcp://d:1"]).ShouldBe(["opc.tcp://d:1", "opc.tcp://only:9"]); + } + + [Fact] + public void FilterEndpointUrls_keeps_only_the_wanted_policy_and_mode() + { + const string b256 = "http://opcfoundation.org/UA/SecurityPolicy#Basic256Sha256"; + const string none = "http://opcfoundation.org/UA/SecurityPolicy#None"; + var endpoints = new[] + { + Ep("opc.tcp://h:1/none", none, MessageSecurityMode.None), + Ep("opc.tcp://h:1/sign", b256, MessageSecurityMode.Sign), + Ep("opc.tcp://h:1/enc", b256, MessageSecurityMode.SignAndEncrypt), + Ep("", b256, MessageSecurityMode.SignAndEncrypt), + }; + + OpcUaClientDriver.FilterEndpointUrls(endpoints, b256, MessageSecurityMode.SignAndEncrypt) + .ShouldBe(["opc.tcp://h:1/enc"]); + OpcUaClientDriver.FilterEndpointUrls(endpoints, none, MessageSecurityMode.None) + .ShouldBe(["opc.tcp://h:1/none"]); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/OpcUaClientDriverPageFormSerializationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/OpcUaClientDriverPageFormSerializationTests.cs index 174dce73..66002093 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/OpcUaClientDriverPageFormSerializationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/OpcUaClientDriverPageFormSerializationTests.cs @@ -156,6 +156,50 @@ public sealed class OpcUaClientDriverPageFormSerializationTests result.ProbeTimeoutSeconds.ShouldBe(25); } + /// + /// #273/#275/#277/#278: the typed form rewrites the whole record on save, so a key it does not model + /// is silently dropped (#576). Every new option must survive FromRecord -> ToRecord. + /// + [Fact] + public void FormModel_RoundTrip_PreservesTuningLimitsRevocationAndDiscovery() + { + var original = new OpcUaClientDriverOptions + { + SubscriptionPublishingInterval = TimeSpan.FromMilliseconds(2500), + SubscriptionKeepAliveCount = 20, + SubscriptionLifetimeCount = 600, + SubscriptionMaxNotificationsPerPublish = 500, + SubscriptionPriority = 9, + HonorServerOperationLimits = false, + CertificateValidation = new OpcUaCertificateValidationOptions + { + RejectUnknownRevocationStatus = true, + IssuerStorePath = "/pki/issuers", + }, + DiscoveryUrl = "opc.tcp://lds:4840", + }; + + var back = OpcUaClientDriverForm.OpcUaClientFormModel.FromRecord(original).ToRecord([]); + + back.SubscriptionPublishingInterval.ShouldBe(TimeSpan.FromMilliseconds(2500)); + back.SubscriptionKeepAliveCount.ShouldBe(20u); + back.SubscriptionLifetimeCount.ShouldBe(600u); + back.SubscriptionMaxNotificationsPerPublish.ShouldBe(500u); + back.SubscriptionPriority.ShouldBe((byte)9); + back.HonorServerOperationLimits.ShouldBeFalse(); + back.CertificateValidation.RejectUnknownRevocationStatus.ShouldBeTrue(); + back.CertificateValidation.IssuerStorePath.ShouldBe("/pki/issuers"); + back.DiscoveryUrl.ShouldBe("opc.tcp://lds:4840"); + + // Defaults round-trip as defaults (blank override stays null, not 0 ms). + var def = OpcUaClientDriverForm.OpcUaClientFormModel.FromRecord(new OpcUaClientDriverOptions()).ToRecord([]); + def.SubscriptionPublishingInterval.ShouldBeNull(); + def.SubscriptionKeepAliveCount.ShouldBe(10u); + def.SubscriptionLifetimeCount.ShouldBe(1000u); + def.HonorServerOperationLimits.ShouldBeTrue(); + def.DiscoveryUrl.ShouldBeNull(); + } + [Fact] public void EndpointUrlRow_FromUrl_ToUrl_Trims() { From 1219b7b13ec9773eac053c8afc75186640a5da54 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 14:55:53 -0400 Subject: [PATCH 3/3] fix(opcuaclient): review fixes - drop driver-side OperationLimits chunking (SDK batches), diagnostics as OTel counter, safe discovery selection, options validation at load (#273 #275 #276 #277 #278) Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- docs/drivers/OpcUaClient.md | 56 ++-- .../Observability/OtOpcUaTelemetry.cs | 10 + .../DriverHealth.cs | 8 +- .../OpcUaClientDriverOptions.cs | 25 +- .../OpcUaClientDiagnostics.cs | 118 ++----- .../OpcUaClientDriver.cs | 300 ++++++++---------- .../OpcUaClientDriverFactoryExtensions.cs | 12 +- .../OpcUaClientOperationLimits.cs | 94 ------ .../Drivers/Forms/OpcUaClientDriverForm.razor | 20 +- .../OpcUaClientDiagnosticsTests.cs | 172 +++++----- .../OpcUaClientOperationLimitsTests.cs | 163 ---------- .../OpcUaClientTuningAndDiscoveryTests.cs | 75 ++++- ...aClientDriverPageFormSerializationTests.cs | 5 +- 13 files changed, 406 insertions(+), 652 deletions(-) delete mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientOperationLimits.cs delete mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientOperationLimitsTests.cs diff --git a/docs/drivers/OpcUaClient.md b/docs/drivers/OpcUaClient.md index c90672ab..f8a1d6de 100644 --- a/docs/drivers/OpcUaClient.md +++ b/docs/drivers/OpcUaClient.md @@ -112,14 +112,15 @@ interface doesn't yet carry the EventFilter surface needed to forward it. `AutoAcceptCertificates` accepts any self-signed / untrusted server certificate. It is **dev-only** — leave it `false` in production so a MITM against the -opc.tcp channel fails closed. A **revoked** certificate (`BadCertificateRevoked` / -`BadCertificateIssuerRevoked`) is never auto-accepted (#277). +opc.tcp channel fails closed. The SDK rejects a **revoked** certificate +(`BadCertificateRevoked` / `BadCertificateIssuerRevoked`) before the auto-accept handler runs, and the handler +also refuses them explicitly — defence in depth (#277), not a behaviour change. `CertificateValidation` (defaults change nothing): | Key | Default | Meaning | |---|---|---| -| `rejectUnknownRevocationStatus` | `false` | Reject a server certificate whose revocation status cannot be determined (no CRL for its issuer, or a stale one). Leave `false` for brownfield sites without CRL infrastructure. | +| `rejectUnknownRevocationStatus` | `false` | Reject a server certificate whose revocation status cannot be determined (no CRL for its issuer, or a stale one). Leave `false` for brownfield sites without CRL infrastructure. **Only effective with `autoAcceptCertificates` off** — the auto-accept handler accepts every suppressible error, including revocation-unknown. | | `issuerStorePath` | `null` (= `%LocalAppData%/OtOpcUa/pki/issuers`) | Directory used as the trusted-issuer store. The SDK reads issuer CRLs (`*.crl`) from its `crl/` sub-folder, so this is how an operator points the driver at a managed CRL set. | ### Subscription tuning (#273) @@ -128,45 +129,50 @@ The per-subscription parameters that used to be hardcoded are driver options; th | Key | Default | Notes | |---|---|---| -| `subscriptionPublishingInterval` | `null` | `null` = the interval the runtime requests per subscribe (floored at 50 ms). When set it overrides that value (floor still applies); monitored-item sampling follows it. | +| `subscriptionPublishingInterval` | `null` | A .NET `TimeSpan` string (`"00:00:02"`), not a number. `null` = the interval the runtime requests per subscribe (floored at 50 ms). When set it overrides that value (floor still applies); monitored-item sampling follows it. | | `subscriptionKeepAliveCount` | `10` | `>= 1`. | | `subscriptionLifetimeCount` | `1000` | Must be `>= 3 x` keep-alive count (Part 4 §5.13.2). | | `subscriptionMaxNotificationsPerPublish` | `0` | `0` = server decides. | | `subscriptionPriority` | `0` | `0`-`255`. | -Out-of-range values fail `InitializeAsync` (`ValidateTuning`). The alarm subscription uses the same counts. +Out-of-range values (and a non-absolute `discoveryUrl`) are rejected when the options are loaded — driver creation / `InitializeAsync` fails once with a clear message rather than the driver faulting and retrying forever. The alarm subscription uses the same counts. *Per-tag* tuning (deadband / sampling interval / queue size, #274) is not implemented — it needs an `ISubscribable` signature change on Core. ### Server OperationLimits (#275) -After connect the driver reads `Server/ServerCapabilities/OperationLimits` -(`MaxNodesPerRead`, `MaxNodesPerWrite`, `MaxNodesPerBrowse`, `MaxMonitoredItemsPerCall`, -`MaxNodesPerHistoryReadData/Events`; `0` or unreadable = no limit) and splits requests that exceed them: - -- `ReadAsync`, `WriteAsync` and the discovery attribute-enrichment read go out in `MaxNodesPerRead` / - `MaxNodesPerWrite`-sized chunks, results re-stitched positionally. A failed Read chunk marks only its own tags - `BadCommunicationError`; a Write that times out marks that chunk and every later one `BadTimeout`, while - earlier chunks keep their real results (those writes happened). -- `SubscribeAsync` creates monitored items in `MaxMonitoredItemsPerCall` batches. -- Browse and HistoryRead already carry one node per call, so a limit `>= 1` is always met and nothing is split. - -`honorServerOperationLimits` (default `true`) switches it off. The limits are re-read after a successful reconnect. +Nothing driver-side: the SDK session already honours the remote server's +`ServerCapabilities/OperationLimits`. `Opc.Ua.Client.Session` derives from `Opc.Ua.SessionClientBatched`, +fetches the limits on open (`FetchOperationLimitsAsync`) and splits Read / Write / CreateMonitoredItems / Browse / +HistoryRead requests that exceed them (a limit of `0` = unlimited). The driver issues one logical call per +operation and a unit test pins the `Session : SessionClientBatched` relationship so an SDK change that dropped +it is caught. (A driver-side splitter was tried and removed: it duplicated the SDK and, on a stalled link, held +the driver gate for chunks x timeout.) ### Discovery URL (#278) `discoveryUrl` (default `null`, e.g. `opc.tcp://lds:4840`): at `InitializeAsync` the driver runs `FindServers` on it -and `GetEndpoints` on every returned server, keeps the endpoints matching `securityPolicy` + `securityMode`, and -tries them **before** `endpointUrls`/`endpointUrl` (case-insensitive de-dup). A discovery failure is logged and -non-fatal — the static list is used. The discovery channel itself is always unsecured (spec). +and `GetEndpoints` on every returned server, keeping the endpoints matching `securityPolicy` + `securityMode`. +Discovered URLs are tried **after** `endpointUrls`/`endpointUrl` (case-insensitive de-dup) — discovery can add +fallbacks but never outranks or replaces the configured server. Because an LDS commonly registers several servers +(identical line PLCs, a gateway plus a historian) and the first to accept a connection would be bound silently: + +- `discoveryServerUri` set: only endpoints of the server with that ApplicationUri are used. +- unset and exactly one distinct server discovered: that server is used. +- unset and more than one: the connect attempt **fails** with an error naming the discovered ApplicationUris and + telling the operator to set `discoveryServerUri`. + +A discovery *network* failure (LDS down) is logged and non-fatal — the static list is used (note it is retried on +every `InitializeAsync`, including comms-loss reconnects). The discovery channel itself is always unsecured (spec). ### Diagnostics counters (#276) -`GetHealth().Diagnostics` (a `name -> double` map, not part of the health-change fingerprint) carries: -`PublishResponseCount`, `NotificationCount`, `DataChangeCount`, `BadStatusValueCount`, `PublishErrorCount`, -`KeepAliveMissCount`, `ReconnectAttemptCount`, `ReconnectSuccessCount`, `ReconnectExhaustedCount`, -`LastReconnectUtcTicks`, `ReadCallCount`, `ReadFailureCount`, `WriteCallCount`, `WriteFailureCount`, -`ChunkedRequestCount`. All counters are `Interlocked`; they are cumulative for the driver instance's lifetime. +Session/publish events are published on the shared OTel meter as the counter +`otopcua.driver.opcuaclient.events`, tagged `driver_instance` (the driver instance id) and `event` = +`publish_response`, `notification` (items), `data_change`, `bad_status_value`, `publish_error`, +`keepalive_miss`, `reconnect_attempt`, `reconnect_success`, `reconnect_exhausted`. A climbing `keepalive_miss` / +`reconnect_attempt` rate is the signature of a flapping upstream. They are not part of `DriverHealth` or the +`/hosts` panel. ## Testing diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Observability/OtOpcUaTelemetry.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Observability/OtOpcUaTelemetry.cs index 6dc7c9c3..bade4e94 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Observability/OtOpcUaTelemetry.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Observability/OtOpcUaTelemetry.cs @@ -76,6 +76,16 @@ public static class OtOpcUaTelemetry ///
public const string PollOverrunDriverInstanceTag = "driver_instance"; + /// + /// Session/publish-level events of an OPC UA Client driver instance (#276): the operator's view of a + /// flapping upstream. Tagged (driver_instance) + + /// event = publish_response|notification|data_change|bad_status_value|publish_error| + /// keepalive_miss|reconnect_attempt|reconnect_success|reconnect_exhausted. + /// + public static readonly Counter OpcUaClientEvents = + Meter.CreateCounter("otopcua.driver.opcuaclient.events", unit: "{event}", + description: "OPC UA Client driver session/publish events (event=publish_response|notification|data_change|bad_status_value|publish_error|keepalive_miss|reconnect_attempt|reconnect_success|reconnect_exhausted)."); + // ---------------- VirtualTag / ScriptedAlarm engines ---------------- public static readonly Counter VirtualTagEval = diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverHealth.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverHealth.cs index 09f06269..f76d163f 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverHealth.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverHealth.cs @@ -15,16 +15,10 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; /// carry a non-null message. Callers must not key behaviour on the LastError-null ↔ Healthy /// pairing. /// -/// -/// Optional driver-specific counters/gauges (name → value) for the diagnostics surface (#276). -/// Null for every driver that publishes none. Deliberately NOT part of any health-change -/// fingerprint: counters move on every poll, so keying a dedup on them would defeat it. -/// public sealed record DriverHealth( DriverState State, DateTime? LastSuccessfulRead, - string? LastError, - IReadOnlyDictionary? Diagnostics = null); + string? LastError); /// Driver-instance lifecycle state. public enum DriverState diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs index 249eb775..80f56c11 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs @@ -203,33 +203,20 @@ public sealed record OpcUaClientDriverOptions /// /// Publish cycles with no data before the server sends a keep-alive (default 10). Must be >= 1. /// - [Range(1, int.MaxValue)] public uint SubscriptionKeepAliveCount { get; init; } = 10; /// /// Publish cycles with no Publish request before the server drops the subscription (default 1000). /// Must be at least 3x (OPC UA Part 4 §5.13.2). /// - [Range(3, int.MaxValue)] public uint SubscriptionLifetimeCount { get; init; } = 1000; /// Max notifications per Publish response; 0 (default) = server decides. public uint SubscriptionMaxNotificationsPerPublish { get; init; } /// Relative subscription priority 0-255 (default 0). - [Range(0, 255)] public byte SubscriptionPriority { get; init; } - // ---- #275 ---- - - /// - /// When true (default) the driver reads the remote server's - /// ServerCapabilities/OperationLimits after connect and splits Read / Write / CreateMonitoredItems - /// requests that exceed them (a declared limit of 0 = unlimited). Set false to send every request - /// whole and let the server answer BadTooManyOperations itself. - /// - public bool HonorServerOperationLimits { get; init; } = true; - // ---- #277 ---- /// Certificate-revocation / issuer-store settings. Defaults preserve the SDK behaviour. @@ -241,11 +228,19 @@ public sealed record OpcUaClientDriverOptions /// Optional discovery endpoint (a Local Discovery Server or the server itself), e.g. /// opc.tcp://lds.internal:4840. When set, InitializeAsync calls FindServers and /// GetEndpoints on it and tries the discovered URLs that match + - /// before the statically configured / - /// . A discovery failure is non-fatal: the static list is used. Discovery + /// after the statically configured / + /// (pin the server with ). A discovery failure is non-fatal: the static list is used. Discovery /// itself always runs over an unsecured channel, as the spec requires. null (default) = off. /// public string? DiscoveryUrl { get; init; } + + /// + /// ApplicationUri of the one server discovery may resolve to. An LDS usually registers several servers; + /// with this set only that server's endpoints are used. When unset and discovery finds more than one + /// server, connect fails with an error listing their ApplicationUris. Discovered endpoints are always + /// tried AFTER the configured . + /// + public string? DiscoveryServerUri { get; init; } } /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDiagnostics.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDiagnostics.cs index 220ec92f..2d379a63 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDiagnostics.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDiagnostics.cs @@ -1,102 +1,48 @@ +using ZB.MOM.WW.OtOpcUa.Commons.Observability; + namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; /// -/// Thread-safe counters for the OPC UA Client driver (#276), surfaced through -/// DriverHealth.Diagnostics. Writers are the SDK's notification / keep-alive threads and -/// the request paths, so every field is touched only through . +/// Publishes an OPC UA Client driver instance's session/publish events (#276) on the shared +/// otopcua.driver.opcuaclient.events OTel counter, tagged with driver_instance and +/// event. The counter is thread-safe, so the SDK's notification / keep-alive threads record +/// directly with no locking here. /// internal sealed class OpcUaClientDiagnostics { - private long _publishResponses; - private long _notifications; - private long _dataChanges; - private long _badStatusValues; - private long _publishErrors; - private long _keepAliveMisses; - private long _reconnectAttempts; - private long _reconnectsSucceeded; - private long _reconnectsExhausted; - private long _lastReconnectUtcTicks; - private long _readCalls; - private long _readFailures; - private long _writeCalls; - private long _writeFailures; - private long _chunkedRequests; - /// A NotificationMessage arrived (one per Publish response that carried data). - public void RecordPublishResponse() => Interlocked.Increment(ref _publishResponses); - - /// A Publish failed (missing publish requests, subscription errors). - public void RecordPublishError() => Interlocked.Increment(ref _publishErrors); - + public const string PublishResponse = "publish_response"; + /// Items carried by notification messages. + public const string Notification = "notification"; /// One monitored-item data change was delivered. - /// Whether the value's status code was Bad. - public void RecordDataChange(bool bad) - { - Interlocked.Increment(ref _dataChanges); - if (bad) Interlocked.Increment(ref _badStatusValues); - } - - /// Count raw notification items carried by a message. - /// Number of items in the message. - public void RecordNotifications(int count) => Interlocked.Add(ref _notifications, count); - + public const string DataChange = "data_change"; + /// A delivered data change carried a Bad status. + public const string BadStatusValue = "bad_status_value"; + /// A Publish failed (missing publish requests, subscription errors). + public const string PublishError = "publish_error"; /// A keep-alive came back Bad. - public void RecordKeepAliveMiss() => Interlocked.Increment(ref _keepAliveMisses); - + public const string KeepAliveMiss = "keepalive_miss"; /// The reconnect handler was armed. - public void RecordReconnectAttempt() => Interlocked.Increment(ref _reconnectAttempts); - + public const string ReconnectAttempt = "reconnect_attempt"; /// A reconnect landed a new session. - public void RecordReconnectSucceeded() - { - Interlocked.Increment(ref _reconnectsSucceeded); - Interlocked.Exchange(ref _lastReconnectUtcTicks, DateTime.UtcNow.Ticks); - } - + public const string ReconnectSuccess = "reconnect_success"; /// A reconnect handler exhausted its retry window. - public void RecordReconnectExhausted() => Interlocked.Increment(ref _reconnectsExhausted); + public const string ReconnectExhausted = "reconnect_exhausted"; - /// A Read request (all chunks) finished. - /// Whether any chunk failed on the wire. - public void RecordRead(bool failed) + private readonly string _driverInstanceId; + + /// Creates the recorder for one driver instance. + /// Value of the driver_instance tag. + public OpcUaClientDiagnostics(string driverInstanceId) => _driverInstanceId = driverInstanceId; + + /// Add to the counter for . + /// One of the event constants. + /// Amount to add (default 1). + public void Record(string evt, long count = 1) { - Interlocked.Increment(ref _readCalls); - if (failed) Interlocked.Increment(ref _readFailures); + if (count <= 0) return; + OtOpcUaTelemetry.OpcUaClientEvents.Add(count, + new KeyValuePair(OtOpcUaTelemetry.PollOverrunDriverInstanceTag, _driverInstanceId), + new KeyValuePair("event", evt)); } - - /// A Write request (all chunks) finished. - /// Whether any chunk failed on the wire. - public void RecordWrite(bool failed) - { - Interlocked.Increment(ref _writeCalls); - if (failed) Interlocked.Increment(ref _writeFailures); - } - - /// A logical request was split into more than one wire call to honour a server OperationLimit. - public void RecordChunkedRequest() => Interlocked.Increment(ref _chunkedRequests); - - /// - /// Point-in-time copy for DriverHealth.Diagnostics. Keys are stable and - /// PascalCase; LastReconnectUtcTicks is 0 until a reconnect succeeds. - /// - /// A fresh dictionary the caller may keep. - public IReadOnlyDictionary Snapshot() => new Dictionary - { - ["PublishResponseCount"] = Interlocked.Read(ref _publishResponses), - ["NotificationCount"] = Interlocked.Read(ref _notifications), - ["DataChangeCount"] = Interlocked.Read(ref _dataChanges), - ["BadStatusValueCount"] = Interlocked.Read(ref _badStatusValues), - ["PublishErrorCount"] = Interlocked.Read(ref _publishErrors), - ["KeepAliveMissCount"] = Interlocked.Read(ref _keepAliveMisses), - ["ReconnectAttemptCount"] = Interlocked.Read(ref _reconnectAttempts), - ["ReconnectSuccessCount"] = Interlocked.Read(ref _reconnectsSucceeded), - ["ReconnectExhaustedCount"] = Interlocked.Read(ref _reconnectsExhausted), - ["LastReconnectUtcTicks"] = Interlocked.Read(ref _lastReconnectUtcTicks), - ["ReadCallCount"] = Interlocked.Read(ref _readCalls), - ["ReadFailureCount"] = Interlocked.Read(ref _readFailures), - ["WriteCallCount"] = Interlocked.Read(ref _writeCalls), - ["WriteFailureCount"] = Interlocked.Read(ref _writeFailures), - ["ChunkedRequestCount"] = Interlocked.Read(ref _chunkedRequests), - }; } 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 87c17144..03c7e758 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -79,6 +79,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit { _options = options; _driverInstanceId = driverInstanceId; + _diag = new OpcUaClientDiagnostics(driverInstanceId); _logger = logger ?? NullLogger.Instance; _secretResolver = secretResolver ?? NullSecretResolver.Instance; } @@ -144,21 +145,13 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit /// The namespace map to install, or null to clear it. internal void SetNamespaceMapForTest(NamespaceMap? map) => _namespaceMap = map; - /// Test-only seam installing the OperationLimits that connect would have read (#275). - /// The limits to apply to subsequent Read / Write / subscribe calls. - internal void SetOperationLimitsForTest(OpcUaClientOperationLimits limits) => _limits = limits; - /// Test-only view of the diagnostics counters (#276). internal OpcUaClientDiagnostics DiagnosticsForTest => _diag; private DriverHealth _health = new(DriverState.Unknown, null, null); - /// Counters surfaced through (#276). - private readonly OpcUaClientDiagnostics _diag = new(); - - /// Server OperationLimits read at connect (#275); None when unread / disabled. - /// Replaced wholesale (never mutated), so readers just snapshot the reference. - private OpcUaClientOperationLimits _limits = OpcUaClientOperationLimits.None; + /// Per-instance event counters published as OTel metrics (#276). + private readonly OpcUaClientDiagnostics _diag; private NotificationEventHandler? _notificationHandler; private PublishErrorEventHandler? _publishErrorHandler; @@ -238,9 +231,10 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit IReadOnlyList discovered = []; if (!string.IsNullOrWhiteSpace(_options.DiscoveryUrl)) { + IReadOnlyList found = []; try { - discovered = await DiscoverEndpointsAsync( + found = await DiscoverEndpointsAsync( appConfig, _options.DiscoveryUrl!, _options.SecurityPolicy, _options.SecurityMode, cancellationToken).ConfigureAwait(false); } @@ -249,8 +243,12 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit { _logger.LogWarning(ex, "OpcUaClientDriver '{DriverInstanceId}': discovery against '{DiscoveryUrl}' failed; " + - "falling back to the configured endpoint list.", _driverInstanceId, _options.DiscoveryUrl); + "using the configured endpoint list only.", _driverInstanceId, _options.DiscoveryUrl); } + + // Selection is OUTSIDE the try on purpose: an ambiguous LDS must fail the connect with a + // clear message, not be swallowed into "discovery unavailable". + discovered = SelectDiscoveredUrls(found, _options.DiscoveryServerUri); } var candidates = ResolveEndpointCandidates(_options, discovered); @@ -306,9 +304,6 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit _keepAliveHandler = OnKeepAlive; session.KeepAlive += _keepAliveHandler; AttachDiagnostics(session); - _limits = _options.HonorServerOperationLimits - ? await OpcUaClientOperationLimits.ReadAsync(session, cancellationToken).ConfigureAwait(false) - : OpcUaClientOperationLimits.None; // Build the bidirectional namespace map from the freshly negotiated session's // NamespaceUris (driver-specs.md §8 "Namespace Remapping"). Stored NodeIds carry @@ -460,10 +455,10 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit ResolveEndpointCandidates(opts, []); /// - /// Resolve the ordered failover list with optional discovery results (#278). Discovered URLs are - /// prepended so a discovery sweep gets first attempt; static entries follow, de-duplicated - /// case-insensitively so one server listed in both is not attempted twice in a row. With no - /// discovered URLs the result is exactly the pre-discovery behaviour. + /// Resolve the ordered failover list with optional discovery results (#278). The configured + /// endpoints always come FIRST — a discovered endpoint can only ever be a fallback, never outrank or + /// replace the server the operator named. Discovered URLs follow, de-duplicated case-insensitively. + /// With no discovered URLs the result is exactly the pre-discovery behaviour. /// /// Driver options containing endpoint configuration. /// URLs resolved via (may be empty). @@ -476,10 +471,10 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit var seen = new HashSet(StringComparer.OrdinalIgnoreCase); var merged = new List(discovered.Count + staticList.Count); - foreach (var u in discovered) - if (!string.IsNullOrWhiteSpace(u) && seen.Add(u)) merged.Add(u); foreach (var u in staticList) if (!string.IsNullOrWhiteSpace(u) && seen.Add(u)) merged.Add(u); + foreach (var u in discovered) + if (!string.IsNullOrWhiteSpace(u) && seen.Add(u)) merged.Add(u); return merged; } @@ -495,7 +490,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit /// Required security mode. /// Cancellation token. /// Matching endpoint URLs in discovery order, de-duplicated. - internal static async Task> DiscoverEndpointsAsync( + internal static async Task> DiscoverEndpointsAsync( ApplicationConfiguration appConfig, string discoveryUrl, OpcUaSecurityPolicy policy, OpcUaSecurityMode mode, CancellationToken ct) { @@ -509,7 +504,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit servers = await lds.FindServersAsync(null, ct).ConfigureAwait(false); } - var urls = new List(); + var found = new List(); foreach (var server in servers) { if (server.DiscoveryUrls is null) continue; @@ -526,10 +521,47 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } catch { continue; } - urls.AddRange(FilterEndpointUrls(endpoints, wantedPolicyUri, wantedMode)); + found.AddRange(FilterEndpointUrls(endpoints, wantedPolicyUri, wantedMode) + .Select(u => new DiscoveredEndpoint(server.ApplicationUri ?? string.Empty, u))); } } - return urls.Distinct(StringComparer.OrdinalIgnoreCase).ToList(); + return found; + } + + /// One endpoint URL found by discovery, with the ApplicationUri of the server advertising it. + /// The advertising server's ApplicationUri (empty when it published none). + /// The endpoint URL. + internal readonly record struct DiscoveredEndpoint(string ApplicationUri, string Url); + + /// + /// Decide which discovered endpoints may be used (#278). An LDS can register many servers (identical + /// line PLCs, a gateway plus a historian), and the first one that accepts a connection would be bound + /// silently — mis-attributing every value and write. So: with set, only + /// that ApplicationUri's endpoints; unset and exactly one distinct server, that server; unset and + /// several, throw naming them so the operator pins one. + /// + /// Matching endpoints from every server the LDS listed. + /// Optional DiscoveryServerUri filter. + /// The usable URLs, de-duplicated, in discovery order (possibly empty). + /// Several servers discovered and no filter configured. + internal static IReadOnlyList SelectDiscoveredUrls( + IReadOnlyList found, string? serverUri) + { + IEnumerable chosen = found; + if (!string.IsNullOrWhiteSpace(serverUri)) + { + chosen = found.Where(e => string.Equals(e.ApplicationUri, serverUri, StringComparison.Ordinal)); + } + else + { + var servers = found.Select(e => e.ApplicationUri).Distinct(StringComparer.Ordinal).ToList(); + if (servers.Count > 1) + throw new InvalidOperationException( + "OpcUaClient discovery found endpoints for more than one server " + + $"({string.Join(", ", servers.Select(u => string.IsNullOrEmpty(u) ? "" : u))}). " + + "Set DiscoveryServerUri to the ApplicationUri of the server this driver should use."); + } + return chosen.Select(e => e.Url).Distinct(StringComparer.OrdinalIgnoreCase).ToList(); } /// The endpoint URLs in matching the wanted policy URI + mode. @@ -816,7 +848,6 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit if (sessionToClose is not null) DetachDiagnostics(sessionToClose); Session = null; } - _limits = OpcUaClientOperationLimits.None; try { if (sessionToClose is Session s) await s.CloseAsync(cancellationToken).ConfigureAwait(false); } catch { /* best-effort */ } @@ -867,7 +898,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit } /// - public DriverHealth GetHealth() => _health with { Diagnostics = _diag.Snapshot() }; + public DriverHealth GetHealth() => _health; // ---- IReadable ---- @@ -917,59 +948,44 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit if (toSend.Count == 0) return results; - // #275: one wire Read per OperationLimits.MaxNodesPerRead-sized chunk. A chunk that fails - // only fails ITS tags — the rest of the batch keeps its values. - var chunks = OpcUaClientOperationLimits.Chunk(toSend.Count, _limits.MaxNodesPerRead).ToList(); - if (chunks.Count > 1) _diag.RecordChunkedRequest(); - Exception? firstFailure = null; - foreach (var (start, count) in chunks) + try { - var slice = new ReadValueIdCollection(count); - for (var w = start; w < start + count; w++) slice.Add(toSend[w]); - try - { - var resp = await session.ReadAsync( - requestHeader: null, - maxAge: 0, - timestampsToReturn: TimestampsToReturn.Both, - nodesToRead: slice, - ct: cancellationToken).ConfigureAwait(false); + var resp = await session.ReadAsync( + requestHeader: null, + maxAge: 0, + timestampsToReturn: TimestampsToReturn.Both, + nodesToRead: toSend, + ct: cancellationToken).ConfigureAwait(false); - var values = resp.Results; - for (var w = 0; w < values.Count && w < count; w++) - { - var r = indexMap[start + w]; - var dv = values[w]; - // Preserve the upstream StatusCode verbatim — including Bad codes per - // §8's cascading-quality rule. Also preserve SourceTimestamp so downstream - // clients can detect stale upstream data. - results[r] = new DataValueSnapshot( - Value: dv.Value, - StatusCode: dv.StatusCode.Code, - SourceTimestampUtc: dv.SourceTimestamp == DateTime.MinValue ? null : dv.SourceTimestamp, - ServerTimestampUtc: dv.ServerTimestamp == DateTime.MinValue ? now : dv.ServerTimestamp); - } - } - catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + var values = resp.Results; + for (var w = 0; w < values.Count; w++) { - throw; - } - catch (Exception ex) - { - // Transport / timeout / session-dropped — fan out the same fault across every - // tag in this chunk. Per-tag StatusCode stays BadCommunicationError (not - // BadInternalError) so operators distinguish "upstream unreachable" from - // "driver bug". - for (var w = start; w < start + count; w++) - results[indexMap[w]] = new DataValueSnapshot(null, StatusBadCommunicationError, null, now); - firstFailure ??= ex; + var r = indexMap[w]; + var dv = values[w]; + // Preserve the upstream StatusCode verbatim — including Bad codes per + // §8's cascading-quality rule. Also preserve SourceTimestamp so downstream + // clients can detect stale upstream data. + results[r] = new DataValueSnapshot( + Value: dv.Value, + StatusCode: dv.StatusCode.Code, + SourceTimestampUtc: dv.SourceTimestamp == DateTime.MinValue ? null : dv.SourceTimestamp, + ServerTimestampUtc: dv.ServerTimestamp == DateTime.MinValue ? now : dv.ServerTimestamp); } + _health = new DriverHealth(DriverState.Healthy, now, null); + } + catch (Exception ex) + { + // Transport / timeout / session-dropped — fan out the same fault across every + // tag in this batch. Per-tag StatusCode stays BadCommunicationError (not + // BadInternalError) so operators distinguish "upstream unreachable" from + // "driver bug". + for (var w = 0; w < indexMap.Count; w++) + { + var r = indexMap[w]; + results[r] = new DataValueSnapshot(null, StatusBadCommunicationError, null, now); + } + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message); } - - _diag.RecordRead(failed: firstFailure is not null); - _health = firstFailure is null - ? new DriverHealth(DriverState.Healthy, now, null) - : new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, firstFailure.Message); } finally { _gate.Release(); } return results; @@ -1019,55 +1035,40 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit if (toSend.Count == 0) return results; - // #275: one wire Write per OperationLimits.MaxNodesPerWrite-sized chunk. Chunks are sent in - // order and each fails independently: a later chunk that times out leaves earlier chunks' - // real results intact (those writes DID happen). - var chunks = OpcUaClientOperationLimits.Chunk(toSend.Count, _limits.MaxNodesPerWrite).ToList(); - if (chunks.Count > 1) _diag.RecordChunkedRequest(); - var anyFailed = false; - foreach (var (start, count) in chunks) + try { - var slice = new WriteValueCollection(count); - for (var w = start; w < start + count; w++) slice.Add(toSend[w]); - try - { - var resp = await session.WriteAsync( - requestHeader: null, - nodesToWrite: slice, - ct: cancellationToken).ConfigureAwait(false); + var resp = await session.WriteAsync( + requestHeader: null, + nodesToWrite: toSend, + ct: cancellationToken).ConfigureAwait(false); - var codes = resp.Results; - for (var w = 0; w < codes.Count && w < count; w++) - { - // Pass upstream WriteResult StatusCode through verbatim. Success codes - // include Good (0) and any warning-level Good* variants; anything with - // the severity bits set is a Bad. - results[indexMap[start + w]] = new WriteResult(codes[w].Code); - } - } - catch (OperationCanceledException) + var codes = resp.Results; + for (var w = 0; w < codes.Count; w++) { - // Timeout / cancellation after the wire request may have been dispatched. - // Writes are non-idempotent — BadTimeout ("outcome unknown, do not blindly - // retry") is more honest than BadCommunicationError ("definitely did not - // happen"). Downstream callers that need retry semantics check for BadTimeout - // and can decide whether to re-issue. Every not-yet-answered chunk (this one and - // the ones after it) gets the same code. - const uint StatusBadTimeout = 0x800A0000u; - anyFailed = true; - for (var w = start; w < toSend.Count; w++) - results[indexMap[w]] = new WriteResult(StatusBadTimeout); - break; - } - catch (Exception) - { - // Pre-wire transport failure — the write definitely did not reach the server. - anyFailed = true; - for (var w = start; w < start + count; w++) - results[indexMap[w]] = new WriteResult(StatusBadCommunicationError); + var r = indexMap[w]; + // Pass upstream WriteResult StatusCode through verbatim. Success codes + // include Good (0) and any warning-level Good* variants; anything with + // the severity bits set is a Bad. + results[r] = new WriteResult(codes[w].Code); } } - _diag.RecordWrite(anyFailed); + catch (OperationCanceledException) + { + // Timeout / cancellation after the wire request may have been dispatched. + // Writes are non-idempotent — BadTimeout ("outcome unknown, do not blindly + // retry") is more honest than BadCommunicationError ("definitely did not + // happen"). Downstream callers that need retry semantics check for BadTimeout + // and can decide whether to re-issue. + const uint StatusBadTimeout = 0x800A0000u; + for (var w = 0; w < indexMap.Count; w++) + results[indexMap[w]] = new WriteResult(StatusBadTimeout); + } + catch (Exception) + { + // Pre-wire transport failure — the write definitely did not reach the server. + for (var w = 0; w < indexMap.Count; w++) + results[indexMap[w]] = new WriteResult(StatusBadCommunicationError); + } } finally { _gate.Release(); } return results; @@ -1334,27 +1335,16 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit nodesToRead.Add(new ReadValueId { NodeId = pv.NodeId, AttributeId = Attributes.Historizing }); } - // #275: split the attribute reads to the server's MaxNodesPerRead (each variable contributes 4). - var values = new DataValueCollection(nodesToRead.Count); + DataValueCollection values; try { - foreach (var (start, count) in OpcUaClientOperationLimits.Chunk(nodesToRead.Count, _limits.MaxNodesPerRead)) - { - var slice = new ReadValueIdCollection(count); - for (var w = start; w < start + count; w++) slice.Add(nodesToRead[w]); - var resp = await session.ReadAsync( - requestHeader: null, - maxAge: 0, - timestampsToReturn: TimestampsToReturn.Neither, - nodesToRead: slice, - ct: ct).ConfigureAwait(false); - values.AddRange(resp.Results); - } - if (values.Count < nodesToRead.Count) throw new InvalidOperationException("Short enrichment read."); - } - catch (OperationCanceledException) when (ct.IsCancellationRequested) - { - throw; + var resp = await session.ReadAsync( + requestHeader: null, + maxAge: 0, + timestampsToReturn: TimestampsToReturn.Neither, + nodesToRead: nodesToRead, + ct: ct).ConfigureAwait(false); + values = resp.Results; } catch { @@ -1507,7 +1497,6 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // captures `handle`, so we must hold the exact delegate instance returned by // `+=` to be able to remove it. var itemHandlers = new List(); - var itemsAdded = 0; foreach (var fullRef in fullReferences) { if (!TryResolveTagNode(session, fullRef, out var nodeId)) continue; @@ -1532,15 +1521,6 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit item.Notification += notifHandler; itemHandlers.Add(new MonitoredItemNotificationHandle(item, notifHandler)); subscription.AddItem(item); - itemsAdded++; - - // #275: flush a CreateMonitoredItems call whenever the server's per-call cap is reached. - // CreateItemsAsync only sends the items not yet created, so calling it per batch is safe. - if (_limits.MaxMonitoredItemsPerCall > 0 && itemsAdded % _limits.MaxMonitoredItemsPerCall == 0) - { - _diag.RecordChunkedRequest(); - await subscription.CreateItemsAsync(cancellationToken).ConfigureAwait(false); - } } await subscription.CreateItemsAsync(cancellationToken).ConfigureAwait(false); @@ -1590,7 +1570,8 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit var dv = mn.Value; if (dv is null) return; var fullRef = (item.Handle as string) ?? item.DisplayName ?? string.Empty; - _diag.RecordDataChange(bad: StatusCode.IsBad(dv.StatusCode)); + _diag.Record(OpcUaClientDiagnostics.DataChange); + if (StatusCode.IsBad(dv.StatusCode)) _diag.Record(OpcUaClientDiagnostics.BadStatusValue); var snapshot = new DataValueSnapshot( Value: dv.Value, StatusCode: dv.StatusCode.Code, @@ -2249,13 +2230,13 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit { _notificationHandler ??= (_, e) => { - _diag.RecordPublishResponse(); + _diag.Record(OpcUaClientDiagnostics.PublishResponse); var msg = e.NotificationMessage; if (msg?.NotificationData is { } data) foreach (var nd in data) - _diag.RecordNotifications(CountNotifications(nd)); + _diag.Record(OpcUaClientDiagnostics.Notification, CountNotifications(nd)); }; - _publishErrorHandler ??= (_, _) => _diag.RecordPublishError(); + _publishErrorHandler ??= (_, _) => _diag.Record(OpcUaClientDiagnostics.PublishError); session.Notification -= _notificationHandler; session.Notification += _notificationHandler; session.PublishError -= _publishErrorHandler; @@ -2280,12 +2261,6 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit _ => 1, }; - private async Task RefreshOperationLimitsAsync(ISession session) - { - try { _limits = await OpcUaClientOperationLimits.ReadAsync(session, CancellationToken.None).ConfigureAwait(false); } - catch { /* best-effort: keep the previous limits */ } - } - /// /// Session keep-alive handler. On a healthy ping, bumps HostState back to Running /// (typical bounce after a transient network blip). On a bad ping, starts the SDK's @@ -2301,7 +2276,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit } TransitionTo(HostState.Stopped); - _diag.RecordKeepAliveMiss(); + _diag.Record(OpcUaClientDiagnostics.KeepAliveMiss); // Kick off the SDK's reconnect loop exactly once per drop. Keep-alive callbacks fire // from the SDK keep-alive timer thread and the SDK can fire this handler repeatedly @@ -2321,7 +2296,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // BeginReconnect is started outside the lock — it does no _reconnectHandler mutation // and we don't want to hold _probeLock across an SDK call. - _diag.RecordReconnectAttempt(); + _diag.Record(OpcUaClientDiagnostics.ReconnectAttempt); handler.BeginReconnect( sender, (int)_options.ReconnectPeriod.TotalMilliseconds, @@ -2388,8 +2363,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit RecordSkippedTags(_namespaceMap); TransitionTo(HostState.Running); _health = new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null); - _diag.RecordReconnectSucceeded(); - if (_options.HonorServerOperationLimits) _ = RefreshOperationLimitsAsync(newSession); + _diag.Record(OpcUaClientDiagnostics.ReconnectSuccess); return; } @@ -2402,7 +2376,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // the last-known session for an always-on gateway rather than abandoning recovery // entirely. TransitionTo(HostState.Faulted); - _diag.RecordReconnectExhausted(); + _diag.Record(OpcUaClientDiagnostics.ReconnectExhausted); _health = new DriverHealth( DriverState.Faulted, _health.LastSuccessfulRead, "OPC UA session reconnect exhausted its retry window without recovering. " + diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverFactoryExtensions.cs index dfd53972..146ec164 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverFactoryExtensions.cs @@ -86,8 +86,18 @@ public static class OpcUaClientDriverFactoryExtensions ArgumentException.ThrowIfNullOrWhiteSpace(driverInstanceId); ArgumentException.ThrowIfNullOrWhiteSpace(driverConfigJson); - return JsonSerializer.Deserialize(driverConfigJson, JsonOptions) + var options = JsonSerializer.Deserialize(driverConfigJson, JsonOptions) ?? throw new InvalidOperationException( $"OpcUaClient driver config for '{driverInstanceId}' deserialised to null"); + + // #273/#278: nothing runs DataAnnotations on driver options, so range-check here — at options load — + // and fail with a clear message instead of a driver that faults and retries forever. + try { OpcUaClientDriver.ValidateTuning(options); } + catch (InvalidOperationException ex) + { + throw new InvalidOperationException( + $"OpcUaClient driver config for '{driverInstanceId}' is invalid: {ex.Message}", ex); + } + return options; } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientOperationLimits.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientOperationLimits.cs deleted file mode 100644 index 7f38ac5c..00000000 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientOperationLimits.cs +++ /dev/null @@ -1,94 +0,0 @@ -using Opc.Ua; -using Opc.Ua.Client; - -namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; - -/// -/// The remote server's Server/ServerCapabilities/OperationLimits that bound how many -/// items one service call may carry (#275). Every field is 0 = "no limit declared" (the -/// OPC UA convention), so is the safe value when the server publishes nothing -/// or the read fails. -/// -/// Items per Read. -/// Items per Write. -/// Nodes per Browse. -/// Items per CreateMonitoredItems / Modify / SetMonitoringMode. -/// Nodes per HistoryRead (raw / processed / at-time). -/// Nodes per HistoryRead (events). -internal sealed record OpcUaClientOperationLimits( - uint MaxNodesPerRead, - uint MaxNodesPerWrite, - uint MaxNodesPerBrowse, - uint MaxMonitoredItemsPerCall, - uint MaxNodesPerHistoryReadData, - uint MaxNodesPerHistoryReadEvents) -{ - /// No limits declared — every call goes out in one request. - public static OpcUaClientOperationLimits None { get; } = new(0, 0, 0, 0, 0, 0); - - /// - /// Split items into consecutive (Start, Count) chunks of at most - /// each. limit == 0 (no limit) or a total that already fits yields a - /// single chunk; total == 0 yields none. - /// - /// Number of items to send. - /// The server's per-call cap; 0 = unlimited. - /// The chunk ranges, in order, covering [0, total) exactly once. - public static IEnumerable<(int Start, int Count)> Chunk(int total, uint limit) - { - if (total <= 0) yield break; - if (limit == 0 || limit >= (uint)total) - { - yield return (0, total); - yield break; - } - var size = (int)Math.Min(limit, int.MaxValue); - for (var start = 0; start < total; start += size) - yield return (start, Math.Min(size, total - start)); - } - - /// The standard variables read to populate the limits, in order. - internal static readonly NodeId[] LimitNodeIds = - [ - VariableIds.Server_ServerCapabilities_OperationLimits_MaxNodesPerRead, - VariableIds.Server_ServerCapabilities_OperationLimits_MaxNodesPerWrite, - VariableIds.Server_ServerCapabilities_OperationLimits_MaxNodesPerBrowse, - VariableIds.Server_ServerCapabilities_OperationLimits_MaxMonitoredItemsPerCall, - VariableIds.Server_ServerCapabilities_OperationLimits_MaxNodesPerHistoryReadData, - VariableIds.Server_ServerCapabilities_OperationLimits_MaxNodesPerHistoryReadEvents, - ]; - - /// - /// Build limits from the values read for . A Bad or non-uint value - /// (server doesn't publish that limit) is treated as 0 = unlimited. - /// - /// One value per entry, in order. - /// The parsed limits. - public static OpcUaClientOperationLimits FromValues(IReadOnlyList values) - { - uint At(int i) => - i < values.Count && values[i] is { } dv && StatusCode.IsGood(dv.StatusCode) && dv.Value is uint u ? u : 0; - return new OpcUaClientOperationLimits(At(0), At(1), At(2), At(3), At(4), At(5)); - } - - /// - /// Read the server's declared limits. Best-effort: any failure (older server without the - /// capability object, transport error) yields so connect never fails over this. - /// - /// The freshly connected session. - /// Cancellation token. - /// The declared limits, or . - public static async Task ReadAsync(ISession session, CancellationToken ct) - { - try - { - var nodes = new ReadValueIdCollection(LimitNodeIds.Length); - foreach (var id in LimitNodeIds) - nodes.Add(new ReadValueId { NodeId = id, AttributeId = Attributes.Value }); - var resp = await session.ReadAsync(null, 0, TimestampsToReturn.Neither, nodes, ct).ConfigureAwait(false); - return FromValues(resp.Results); - } - catch (OperationCanceledException) { throw; } - catch { return None; } - } -} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor index ba66a78e..bf04f380 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor @@ -214,13 +214,6 @@ -
-
- - -
-
Split oversized Read / Write / monitored-item calls.
-
@@ -236,7 +229,12 @@
-
Optional. Discovered endpoints are tried before the failover list.
+
Optional. Discovered endpoints are tried after the failover list.
+
+
+ + +
Required when the discovery server lists more than one server.
@@ -382,10 +380,10 @@ public int SubscriptionLifetimeCount { get; set; } = 1000; public int SubscriptionMaxNotificationsPerPublish { get; set; } public int SubscriptionPriority { get; set; } - public bool HonorServerOperationLimits { get; set; } = true; public bool RejectUnknownRevocationStatus { get; set; } public string? IssuerStorePath { get; set; } public string? DiscoveryUrl { get; set; } + public string? DiscoveryServerUri { get; set; } internal IReadOnlyDictionary _unsMappingTable = new Dictionary(); @@ -417,10 +415,10 @@ SubscriptionLifetimeCount = (int)Math.Min(r.SubscriptionLifetimeCount, int.MaxValue), SubscriptionMaxNotificationsPerPublish = (int)Math.Min(r.SubscriptionMaxNotificationsPerPublish, int.MaxValue), SubscriptionPriority = r.SubscriptionPriority, - HonorServerOperationLimits = r.HonorServerOperationLimits, RejectUnknownRevocationStatus = r.CertificateValidation.RejectUnknownRevocationStatus, IssuerStorePath = r.CertificateValidation.IssuerStorePath, DiscoveryUrl = r.DiscoveryUrl, + DiscoveryServerUri = r.DiscoveryServerUri, _unsMappingTable = r.UnsMappingTable, }; @@ -455,13 +453,13 @@ SubscriptionLifetimeCount = (uint)Math.Max(SubscriptionLifetimeCount, 0), SubscriptionMaxNotificationsPerPublish = (uint)Math.Max(SubscriptionMaxNotificationsPerPublish, 0), SubscriptionPriority = (byte)Math.Clamp(SubscriptionPriority, 0, 255), - HonorServerOperationLimits = HonorServerOperationLimits, CertificateValidation = new OpcUaCertificateValidationOptions { RejectUnknownRevocationStatus = RejectUnknownRevocationStatus, IssuerStorePath = string.IsNullOrWhiteSpace(IssuerStorePath) ? null : IssuerStorePath, }, DiscoveryUrl = string.IsNullOrWhiteSpace(DiscoveryUrl) ? null : DiscoveryUrl, + DiscoveryServerUri = string.IsNullOrWhiteSpace(DiscoveryServerUri) ? null : DiscoveryServerUri, }; } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientDiagnosticsTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientDiagnosticsTests.cs index 9fa0353c..acb3497b 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientDiagnosticsTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientDiagnosticsTests.cs @@ -1,111 +1,131 @@ +using System.Diagnostics.Metrics; +using Opc.Ua; +using Opc.Ua.Client; using Shouldly; using Xunit; -using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; -/// #276 — counters are thread-safe and surface through DriverHealth.Diagnostics. +/// +/// #276 — session/publish events are published on the otopcua.driver.opcuaclient.events OTel counter. +/// The listener filters on a UNIQUE driver_instance tag: the Meter is process-static, so a +/// name-only filter would also collect parallel tests' measurements. +/// [Trait("Category", "Unit")] public sealed class OpcUaClientDiagnosticsTests { - [Fact] - public void Fresh_snapshot_is_all_zero_with_the_full_key_set() + private sealed class Capture : IDisposable { - var snap = new OpcUaClientDiagnostics().Snapshot(); + private readonly MeterListener _listener = new(); + private readonly object _gate = new(); + private readonly Dictionary _byEvent = new(); - snap.Keys.ShouldBe( - [ - "PublishResponseCount", "NotificationCount", "DataChangeCount", "BadStatusValueCount", - "PublishErrorCount", "KeepAliveMissCount", "ReconnectAttemptCount", "ReconnectSuccessCount", - "ReconnectExhaustedCount", "LastReconnectUtcTicks", "ReadCallCount", "ReadFailureCount", - "WriteCallCount", "WriteFailureCount", "ChunkedRequestCount", - ], ignoreOrder: true); - snap.Values.ShouldAllBe(v => v == 0); + public Capture(string instance) + { + _listener.InstrumentPublished = (inst, l) => + { + if (inst.Name == "otopcua.driver.opcuaclient.events") l.EnableMeasurementEvents(inst); + }; + _listener.SetMeasurementEventCallback((_, value, tags, _) => + { + string? driver = null, evt = null; + foreach (var t in tags) + { + if (t.Key == "driver_instance") driver = t.Value as string; + else if (t.Key == "event") evt = t.Value as string; + } + if (driver != instance || evt is null) return; + lock (_gate) _byEvent[evt] = _byEvent.GetValueOrDefault(evt) + value; + }); + _listener.Start(); + } + + public long this[string evt] { get { lock (_gate) return _byEvent.GetValueOrDefault(evt); } } + public void Dispose() => _listener.Dispose(); } [Fact] - public void Each_recorder_moves_exactly_its_own_counter() + public void Each_event_lands_on_its_own_tagged_series() { - var d = new OpcUaClientDiagnostics(); - d.RecordPublishResponse(); - d.RecordNotifications(3); - d.RecordDataChange(bad: false); - d.RecordDataChange(bad: true); - d.RecordPublishError(); - d.RecordKeepAliveMiss(); - d.RecordReconnectAttempt(); - d.RecordReconnectSucceeded(); - d.RecordReconnectExhausted(); - d.RecordRead(failed: true); - d.RecordWrite(failed: false); - d.RecordChunkedRequest(); + var id = $"diag-{Guid.NewGuid():N}"; + using var cap = new Capture(id); + var d = new OpcUaClientDiagnostics(id); - var s = d.Snapshot(); - s["PublishResponseCount"].ShouldBe(1); - s["NotificationCount"].ShouldBe(3); - s["DataChangeCount"].ShouldBe(2); - s["BadStatusValueCount"].ShouldBe(1); - s["PublishErrorCount"].ShouldBe(1); - s["KeepAliveMissCount"].ShouldBe(1); - s["ReconnectAttemptCount"].ShouldBe(1); - s["ReconnectSuccessCount"].ShouldBe(1); - s["ReconnectExhaustedCount"].ShouldBe(1); - s["LastReconnectUtcTicks"].ShouldBeGreaterThan(0); - s["ReadCallCount"].ShouldBe(1); - s["ReadFailureCount"].ShouldBe(1); - s["WriteCallCount"].ShouldBe(1); - s["WriteFailureCount"].ShouldBe(0); - s["ChunkedRequestCount"].ShouldBe(1); + d.Record(OpcUaClientDiagnostics.PublishResponse); + d.Record(OpcUaClientDiagnostics.Notification, 3); + d.Record(OpcUaClientDiagnostics.DataChange); + d.Record(OpcUaClientDiagnostics.BadStatusValue); + d.Record(OpcUaClientDiagnostics.PublishError); + d.Record(OpcUaClientDiagnostics.KeepAliveMiss); + d.Record(OpcUaClientDiagnostics.ReconnectAttempt); + d.Record(OpcUaClientDiagnostics.ReconnectSuccess); + d.Record(OpcUaClientDiagnostics.ReconnectExhausted); + + cap[OpcUaClientDiagnostics.PublishResponse].ShouldBe(1); + cap[OpcUaClientDiagnostics.Notification].ShouldBe(3); + cap[OpcUaClientDiagnostics.DataChange].ShouldBe(1); + cap[OpcUaClientDiagnostics.BadStatusValue].ShouldBe(1); + cap[OpcUaClientDiagnostics.PublishError].ShouldBe(1); + cap[OpcUaClientDiagnostics.KeepAliveMiss].ShouldBe(1); + cap[OpcUaClientDiagnostics.ReconnectAttempt].ShouldBe(1); + cap[OpcUaClientDiagnostics.ReconnectSuccess].ShouldBe(1); + cap[OpcUaClientDiagnostics.ReconnectExhausted].ShouldBe(1); } [Fact] - public void Concurrent_increments_do_not_lose_updates() + public void Another_instances_events_are_not_attributed_to_this_one() { - var d = new OpcUaClientDiagnostics(); - const int threads = 8, each = 5_000; + var mine = $"diag-{Guid.NewGuid():N}"; + using var cap = new Capture(mine); + + // Positive control: my own event IS seen; the other instance's identical event is not. + new OpcUaClientDiagnostics("someone-else").Record(OpcUaClientDiagnostics.KeepAliveMiss); + new OpcUaClientDiagnostics(mine).Record(OpcUaClientDiagnostics.PublishError); + + cap[OpcUaClientDiagnostics.PublishError].ShouldBe(1); + cap[OpcUaClientDiagnostics.KeepAliveMiss].ShouldBe(0); + } + + [Fact] + public void Concurrent_records_do_not_lose_updates() + { + var id = $"diag-{Guid.NewGuid():N}"; + using var cap = new Capture(id); + var d = new OpcUaClientDiagnostics(id); + const int threads = 8, each = 2_000; Parallel.For(0, threads, _ => { - for (var i = 0; i < each; i++) { d.RecordDataChange(bad: i % 2 == 0); d.RecordNotifications(2); } + for (var i = 0; i < each; i++) d.Record(OpcUaClientDiagnostics.DataChange); }); - var s = d.Snapshot(); - s["DataChangeCount"].ShouldBe(threads * each); - s["BadStatusValueCount"].ShouldBe(threads * each / 2); - s["NotificationCount"].ShouldBe(threads * each * 2); + cap[OpcUaClientDiagnostics.DataChange].ShouldBe(threads * each); } [Fact] - public void Snapshot_is_a_point_in_time_copy() + public void Zero_or_negative_counts_are_ignored() { - var d = new OpcUaClientDiagnostics(); - var before = d.Snapshot(); - d.RecordKeepAliveMiss(); + var id = $"diag-{Guid.NewGuid():N}"; + using var cap = new Capture(id); + var d = new OpcUaClientDiagnostics(id); - before["KeepAliveMissCount"].ShouldBe(0); - d.Snapshot()["KeepAliveMissCount"].ShouldBe(1); + d.Record(OpcUaClientDiagnostics.Notification, 0); + d.Record(OpcUaClientDiagnostics.Notification, -2); + d.Record(OpcUaClientDiagnostics.DataChange); // positive control + + cap[OpcUaClientDiagnostics.Notification].ShouldBe(0); + cap[OpcUaClientDiagnostics.DataChange].ShouldBe(1); } + /// + /// #275: the driver does not split Read/Write/CreateMonitoredItems itself because the SDK session already + /// does — Session derives from SessionClientBatched (which batches by the server's + /// OperationLimits, fetched on open). If an SDK bump ever drops that, this goes red and the driver needs + /// its own chunking again. + /// [Fact] - public void GetHealth_carries_the_diagnostics_and_keeps_state_fields() + public void The_sdk_session_is_the_batching_session_type() { - using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-diag"); - drv.DiagnosticsForTest.RecordKeepAliveMiss(); - drv.DiagnosticsForTest.RecordKeepAliveMiss(); - - var h = drv.GetHealth(); - - h.State.ShouldBe(DriverState.Unknown); - h.Diagnostics.ShouldNotBeNull(); - h.Diagnostics!["KeepAliveMissCount"].ShouldBe(2); - } - - [Fact] - public void DriverHealth_without_diagnostics_stays_back_compatible() - { - var h = new DriverHealth(DriverState.Healthy, null, null); - - h.Diagnostics.ShouldBeNull(); - h.ShouldBe(new DriverHealth(DriverState.Healthy, null, null)); + typeof(SessionClientBatched).IsAssignableFrom(typeof(Session)).ShouldBeTrue(); } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientOperationLimitsTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientOperationLimitsTests.cs deleted file mode 100644 index a97c586f..00000000 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientOperationLimitsTests.cs +++ /dev/null @@ -1,163 +0,0 @@ -using Moq; -using Opc.Ua; -using Opc.Ua.Client; -using Shouldly; -using Xunit; -using ZB.MOM.WW.OtOpcUa.Core.Abstractions; - -namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; - -/// #275 — the driver splits Read / Write calls to the server's declared OperationLimits. -[Trait("Category", "Unit")] -public sealed class OpcUaClientOperationLimitsTests -{ - [Theory] - [InlineData(0, 0u, new int[0])] // nothing to send => no call - [InlineData(5, 0u, new[] { 5 })] // 0 = unlimited - [InlineData(5, 5u, new[] { 5 })] // exactly fits - [InlineData(5, 100u, new[] { 5 })] // limit larger than the batch - [InlineData(10, 4u, new[] { 4, 4, 2 })] // remainder chunk - [InlineData(9, 3u, new[] { 3, 3, 3 })] - [InlineData(3, 1u, new[] { 1, 1, 1 })] - public void Chunk_splits_to_the_limit(int total, uint limit, int[] expectedSizes) - { - var chunks = OpcUaClientOperationLimits.Chunk(total, limit).ToList(); - - chunks.Select(c => c.Count).ToArray().ShouldBe(expectedSizes); - // Contiguous cover of [0, total): every item exactly once, in order. - var next = 0; - foreach (var (start, count) in chunks) { start.ShouldBe(next); next += count; } - next.ShouldBe(total); - } - - [Fact] - public void FromValues_reads_good_uints_and_treats_bad_or_missing_as_unlimited() - { - var values = new List - { - new(new Variant(100u)), - new(new Variant(50u)), - new(StatusCodes.BadNodeIdUnknown), // server doesn't publish MaxNodesPerBrowse - new(new Variant(7u)), - }; - - var limits = OpcUaClientOperationLimits.FromValues(values); - - limits.MaxNodesPerRead.ShouldBe(100u); - limits.MaxNodesPerWrite.ShouldBe(50u); - limits.MaxNodesPerBrowse.ShouldBe(0u); - limits.MaxMonitoredItemsPerCall.ShouldBe(7u); - limits.MaxNodesPerHistoryReadData.ShouldBe(0u); // short list => 0 - limits.MaxNodesPerHistoryReadEvents.ShouldBe(0u); - } - - private static (OpcUaClientDriver Drv, Mock Session, List ReadSizes, List WriteSizes) Rig( - uint readLimit, uint writeLimit, int tagCount) - { - var tags = Enumerable.Range(0, tagCount) - .Select(i => new RawTagEntry($"raw/t{i}", $$"""{"nodeId":"ns=2;s=T{{i}}"}""", WriteIdempotent: false)) - .ToList(); - var options = new OpcUaClientDriverOptions { RawTags = tags }; - var drv = new OpcUaClientDriver(options, "opcua-limits"); - drv.SetNamespaceMapForTest(ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.NamespaceMap.FromTable(new NamespaceTable(), tags)); - drv.SetOperationLimitsForTest(OpcUaClientOperationLimits.None with - { - MaxNodesPerRead = readLimit, MaxNodesPerWrite = writeLimit, - }); - - var readSizes = new List(); - var writeSizes = new List(); - var mock = new Mock(MockBehavior.Loose); - mock.SetupGet(s => s.MessageContext) - .Returns(new ServiceMessageContext(SdkTelemetry.Context)); - mock.SetupGet(s => s.NamespaceUris).Returns(new NamespaceTable()); - mock.Setup(s => s.ReadAsync(It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny())) - .ReturnsAsync((RequestHeader _, double _, TimestampsToReturn _, ReadValueIdCollection nodes, CancellationToken _) => - { - readSizes.Add(nodes.Count); - // Echo each node's identifier number back as the value so ordering across chunks is checkable. - var results = new DataValueCollection(); - foreach (var n in nodes) results.Add(new DataValue(new Variant((string)n.NodeId.Identifier))); - return new ReadResponse { ResponseHeader = new ResponseHeader(), Results = results }; - }); - mock.Setup(s => s.WriteAsync(It.IsAny(), It.IsAny(), It.IsAny())) - .ReturnsAsync((RequestHeader _, WriteValueCollection nodes, CancellationToken _) => - { - writeSizes.Add(nodes.Count); - var codes = new StatusCodeCollection(); - foreach (var _ in nodes) codes.Add(StatusCodes.Good); - return new WriteResponse { ResponseHeader = new ResponseHeader(), Results = codes }; - }); - drv.SetSessionForTest(mock.Object); - return (drv, mock, readSizes, writeSizes); - } - - [Fact] - public async Task ReadAsync_splits_to_MaxNodesPerRead_and_keeps_result_order() - { - var (drv, _, readSizes, _) = Rig(readLimit: 4, writeLimit: 0, tagCount: 10); - using var _ = drv; - - var refs = Enumerable.Range(0, 10).Select(i => $"raw/t{i}").ToList(); - var res = await drv.ReadAsync(refs, TestContext.Current.CancellationToken); - - readSizes.ShouldBe([4, 4, 2]); - res.Count.ShouldBe(10); - for (var i = 0; i < 10; i++) res[i].Value.ShouldBe($"T{i}"); // positional mapping survives chunking - drv.GetHealth().State.ShouldBe(DriverState.Healthy); - drv.GetHealth().Diagnostics!["ChunkedRequestCount"].ShouldBe(1); - } - - [Fact] - public async Task ReadAsync_with_no_declared_limit_sends_one_request() - { - var (drv, _, readSizes, _) = Rig(readLimit: 0, writeLimit: 0, tagCount: 10); - using var _ = drv; - - await drv.ReadAsync(Enumerable.Range(0, 10).Select(i => $"raw/t{i}").ToList(), TestContext.Current.CancellationToken); - - readSizes.ShouldBe([10]); - } - - [Fact] - public async Task ReadAsync_a_failing_chunk_only_fails_its_own_tags() - { - var (drv, session, _, _) = Rig(readLimit: 2, writeLimit: 0, tagCount: 4); - using var _ = drv; - var call = 0; - session.Setup(s => s.ReadAsync(It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny())) - .Returns((RequestHeader _, double _, TimestampsToReturn _, ReadValueIdCollection nodes, CancellationToken _) => - { - if (Interlocked.Increment(ref call) == 2) throw new ServiceResultException(StatusCodes.BadConnectionClosed); - var results = new DataValueCollection(); - foreach (var n in nodes) results.Add(new DataValue(new Variant((string)n.NodeId.Identifier))); - return Task.FromResult(new ReadResponse { ResponseHeader = new ResponseHeader(), Results = results }); - }); - - var res = await drv.ReadAsync(["raw/t0", "raw/t1", "raw/t2", "raw/t3"], TestContext.Current.CancellationToken); - - res[0].Value.ShouldBe("T0"); - res[1].Value.ShouldBe("T1"); - res[2].StatusCode.ShouldBe(0x80050000u); // BadCommunicationError, chunk 2 only - res[3].StatusCode.ShouldBe(0x80050000u); - drv.GetHealth().State.ShouldBe(DriverState.Degraded); - drv.GetHealth().Diagnostics!["ReadFailureCount"].ShouldBe(1); - } - - [Fact] - public async Task WriteAsync_splits_to_MaxNodesPerWrite() - { - var (drv, _, _, writeSizes) = Rig(readLimit: 0, writeLimit: 3, tagCount: 7); - using var _ = drv; - - var res = await drv.WriteAsync( - Enumerable.Range(0, 7).Select(i => new Core.Abstractions.WriteRequest($"raw/t{i}", i)).ToList(), - TestContext.Current.CancellationToken); - - writeSizes.ShouldBe([3, 3, 1]); - res.Count.ShouldBe(7); - res.ShouldAllBe(r => r.StatusCode == 0u); - } -} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientTuningAndDiscoveryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientTuningAndDiscoveryTests.cs index 910ced5a..d4410754 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientTuningAndDiscoveryTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientTuningAndDiscoveryTests.cs @@ -20,7 +20,6 @@ public sealed class OpcUaClientTuningAndDiscoveryTests o.SubscriptionMaxNotificationsPerPublish.ShouldBe(0u); o.SubscriptionPriority.ShouldBe((byte)0); o.SubscriptionPublishingInterval.ShouldBeNull(); - o.HonorServerOperationLimits.ShouldBeTrue(); o.DiscoveryUrl.ShouldBeNull(); o.CertificateValidation.RejectUnknownRevocationStatus.ShouldBeFalse(); o.CertificateValidation.IssuerStorePath.ShouldBeNull(); @@ -33,8 +32,7 @@ public sealed class OpcUaClientTuningAndDiscoveryTests var o = OpcUaClientDriverFactoryExtensions.ParseOptions("d", """ {"subscriptionPublishingInterval":"00:00:02","subscriptionKeepAliveCount":20, "subscriptionLifetimeCount":600,"subscriptionMaxNotificationsPerPublish":500, - "subscriptionPriority":9,"honorServerOperationLimits":false, - "discoveryUrl":"opc.tcp://lds:4840", + "subscriptionPriority":9, "discoveryUrl":"opc.tcp://lds:4840","discoveryServerUri":"urn:plant:l1", "certificateValidation":{"rejectUnknownRevocationStatus":true,"issuerStorePath":"/pki/issuers"}} """); @@ -43,8 +41,8 @@ public sealed class OpcUaClientTuningAndDiscoveryTests o.SubscriptionLifetimeCount.ShouldBe(600u); o.SubscriptionMaxNotificationsPerPublish.ShouldBe(500u); o.SubscriptionPriority.ShouldBe((byte)9); - o.HonorServerOperationLimits.ShouldBeFalse(); o.DiscoveryUrl.ShouldBe("opc.tcp://lds:4840"); + o.DiscoveryServerUri.ShouldBe("urn:plant:l1"); o.CertificateValidation.RejectUnknownRevocationStatus.ShouldBeTrue(); o.CertificateValidation.IssuerStorePath.ShouldBe("/pki/issuers"); @@ -119,21 +117,82 @@ public sealed class OpcUaClientTuningAndDiscoveryTests } [Fact] - public void ResolveEndpointCandidates_prepends_discovered_urls_and_dedups_case_insensitively() + public void ResolveEndpointCandidates_puts_configured_endpoints_before_discovered_and_dedups() { var o = new OpcUaClientDriverOptions { EndpointUrls = ["opc.tcp://Static:1", "opc.tcp://found:2"] }; var merged = OpcUaClientDriver.ResolveEndpointCandidates(o, ["opc.tcp://found:2", "opc.tcp://new:3"]); - merged.ShouldBe(["opc.tcp://found:2", "opc.tcp://new:3", "opc.tcp://Static:1"]); + merged.ShouldBe(["opc.tcp://Static:1", "opc.tcp://found:2", "opc.tcp://new:3"]); } [Fact] - public void ResolveEndpointCandidates_uses_the_single_EndpointUrl_after_discovered_when_no_list() + public void ResolveEndpointCandidates_uses_the_single_EndpointUrl_before_discovered_when_no_list() { var o = new OpcUaClientDriverOptions { EndpointUrl = "opc.tcp://only:9" }; - OpcUaClientDriver.ResolveEndpointCandidates(o, ["opc.tcp://d:1"]).ShouldBe(["opc.tcp://d:1", "opc.tcp://only:9"]); + OpcUaClientDriver.ResolveEndpointCandidates(o, ["opc.tcp://d:1"]).ShouldBe(["opc.tcp://only:9", "opc.tcp://d:1"]); + } + + private static OpcUaClientDriver.DiscoveredEndpoint De(string appUri, string url) => new(appUri, url); + + [Fact] + public void SelectDiscoveredUrls_with_one_distinct_server_uses_it_and_dedups() + { + var urls = OpcUaClientDriver.SelectDiscoveredUrls( + [De("urn:a", "opc.tcp://a:1"), De("urn:a", "opc.tcp://A:1"), De("urn:a", "opc.tcp://a:2")], null); + + urls.ShouldBe(["opc.tcp://a:1", "opc.tcp://a:2"]); + } + + [Fact] + public void SelectDiscoveredUrls_with_several_servers_and_no_filter_fails_naming_them() + { + var ex = Should.Throw(() => OpcUaClientDriver.SelectDiscoveredUrls( + [De("urn:line1", "opc.tcp://l1:1"), De("urn:line2", "opc.tcp://l2:1")], null)); + + ex.Message.ShouldContain("urn:line1"); + ex.Message.ShouldContain("urn:line2"); + ex.Message.ShouldContain("DiscoveryServerUri"); + } + + [Fact] + public void SelectDiscoveredUrls_filter_picks_only_that_server_even_when_several_are_listed() + { + var urls = OpcUaClientDriver.SelectDiscoveredUrls( + [De("urn:line1", "opc.tcp://l1:1"), De("urn:line2", "opc.tcp://l2:1"), De("urn:line2", "opc.tcp://l2:2")], + "urn:line2"); + + urls.ShouldBe(["opc.tcp://l2:1", "opc.tcp://l2:2"]); + } + + [Fact] + public void SelectDiscoveredUrls_filter_matching_nothing_yields_empty_not_another_server() + { + OpcUaClientDriver.SelectDiscoveredUrls([De("urn:line1", "opc.tcp://l1:1")], "urn:other").ShouldBeEmpty(); + OpcUaClientDriver.SelectDiscoveredUrls([], null).ShouldBeEmpty(); + } + + // ---- #273: validation at options load ---- + + [Theory] + [InlineData("""{"subscriptionKeepAliveCount":0}""")] + [InlineData("""{"subscriptionKeepAliveCount":10,"subscriptionLifetimeCount":29}""")] + [InlineData("""{"discoveryUrl":"not a uri"}""")] + public void ParseOptions_rejects_out_of_range_tuning_with_a_clear_message(string json) + { + var ex = Should.Throw( + () => OpcUaClientDriverFactoryExtensions.ParseOptions("drv-bad", json)); + + ex.Message.ShouldContain("drv-bad"); + ex.Message.ShouldContain("invalid"); + } + + [Fact] + public void Factory_CreateInstance_fails_fast_on_a_bad_tuning_config() + { + Should.Throw(() => OpcUaClientDriverFactoryExtensions.CreateInstance( + "drv-bad", """{"subscriptionLifetimeCount":5}""")); } [Fact] diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/OpcUaClientDriverPageFormSerializationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/OpcUaClientDriverPageFormSerializationTests.cs index 66002093..28dc2a66 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/OpcUaClientDriverPageFormSerializationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/OpcUaClientDriverPageFormSerializationTests.cs @@ -170,13 +170,13 @@ public sealed class OpcUaClientDriverPageFormSerializationTests SubscriptionLifetimeCount = 600, SubscriptionMaxNotificationsPerPublish = 500, SubscriptionPriority = 9, - HonorServerOperationLimits = false, CertificateValidation = new OpcUaCertificateValidationOptions { RejectUnknownRevocationStatus = true, IssuerStorePath = "/pki/issuers", }, DiscoveryUrl = "opc.tcp://lds:4840", + DiscoveryServerUri = "urn:plant:line1", }; var back = OpcUaClientDriverForm.OpcUaClientFormModel.FromRecord(original).ToRecord([]); @@ -186,7 +186,7 @@ public sealed class OpcUaClientDriverPageFormSerializationTests back.SubscriptionLifetimeCount.ShouldBe(600u); back.SubscriptionMaxNotificationsPerPublish.ShouldBe(500u); back.SubscriptionPriority.ShouldBe((byte)9); - back.HonorServerOperationLimits.ShouldBeFalse(); + back.DiscoveryServerUri.ShouldBe("urn:plant:line1"); back.CertificateValidation.RejectUnknownRevocationStatus.ShouldBeTrue(); back.CertificateValidation.IssuerStorePath.ShouldBe("/pki/issuers"); back.DiscoveryUrl.ShouldBe("opc.tcp://lds:4840"); @@ -196,7 +196,6 @@ public sealed class OpcUaClientDriverPageFormSerializationTests def.SubscriptionPublishingInterval.ShouldBeNull(); def.SubscriptionKeepAliveCount.ShouldBe(10u); def.SubscriptionLifetimeCount.ShouldBe(1000u); - def.HonorServerOperationLimits.ShouldBeTrue(); def.DiscoveryUrl.ShouldBeNull(); }