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