refactor(opcua): node manager logs through injected ILogger, drop CS0618 suppressions (#621)

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe

src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs

@@ -141,7 +141,7 @@ public sealed class OtOpcUaServerHostedService : IHostedService, IAsyncDisposabl
     /// <returns>A task that represents the asynchronous operation.</returns>
     public async Task StartAsync(CancellationToken cancellationToken)
     {
-        _server = new OtOpcUaSdkServer();
+        _server = new OtOpcUaSdkServer(_loggerFactory.CreateLogger<OtOpcUaNodeManager>());
         _appHost = new OpcUaApplicationHost(
             _options,
             _loggerFactory.CreateLogger<OpcUaApplicationHost>(),

src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs

@@ -1,4 +1,6 @@
 using System.Collections.Concurrent;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
 using Opc.Ua;
 using Opc.Ua.Server;
 using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
@@ -92,13 +94,18 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
     /// <see cref="RebuildAddressSpace"/>.</summary>
     private readonly ConcurrentDictionary<RealmKey, string> _eventNotifierSources = new();
     private FolderState? _root;
+    private readonly ILogger<OtOpcUaNodeManager> _logger;
 
     /// <summary>Initializes a new instance of the <see cref="OtOpcUaNodeManager"/> class with the OPC UA server and configuration.</summary>
     /// <param name="server">The OPC UA server instance.</param>
     /// <param name="configuration">The application configuration.</param>
-    public OtOpcUaNodeManager(IServerInternal server, ApplicationConfiguration configuration)
+    /// <param name="logger">Logger for the manager's swallowed-fault and skipped-wiring diagnostics;
+    /// <c>null</c> (tests) falls back to <see cref="NullLogger{T}.Instance"/>.</param>
+    public OtOpcUaNodeManager(
+        IServerInternal server, ApplicationConfiguration configuration, ILogger<OtOpcUaNodeManager>? logger = null)
         : base(server, configuration, RegisteredNamespaceUris)
     {
+        _logger = logger ?? NullLogger<OtOpcUaNodeManager>.Instance;
         // SystemContext is initialised by the base ctor; both namespace URIs are now registered and their
         // indexes captured (in the order passed) in NamespaceIndexes.
     }
@@ -153,11 +160,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
         // Unreachable by construction — every call site passes a node this manager minted with a string
         // `s=` identifier. If it ever fires, the derived key resolves to nothing and the caller sees an
         // unexplained miss, so name the seam rather than collapsing quietly.
-#pragma warning disable CS0618 // Utils.LogWarning is [Obsolete] in favour of an ITelemetryContext this manager doesn't carry.
-        Utils.LogWarning(
-            "OtOpcUaNodeManager.MapKey: non-string NodeId identifier reached the map-key seam (node {0}, identifier type {1}) — the derived key will not resolve",
+        _logger.LogWarning(
+            "OtOpcUaNodeManager.MapKey: non-string NodeId identifier reached the map-key seam (node {NodeId}, identifier type {IdentifierType}) — the derived key will not resolve",
             nodeId, nodeId.Identifier?.GetType().Name ?? "null");
-#pragma warning restore CS0618
         return new RealmKey(RealmOf(nodeId), nodeId.Identifier?.ToString() ?? string.Empty);
     }
 
@@ -742,14 +747,11 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
         if (skipped > 0)
         {
             // Debug, not Warning: the caller's index racing a surgical remove is expected, and a driver
-            // whose nodes were all just removed would otherwise log once per ref. This manager carries no
-            // ILogger, so log via the SDK's static trace (see ServeNodeAsync).
-#pragma warning disable CS0618 // Type or member is obsolete
-            Utils.LogDebug(
-                "OtOpcUaNodeManager: WriteStatuses skipped {0} of {1} item(s) — node not materialised.",
+            // whose nodes were all just removed would otherwise log once per ref.
+            _logger.LogDebug(
+                "OtOpcUaNodeManager: WriteStatuses skipped {Skipped} of {Total} item(s) — node not materialised.",
                 skipped,
                 statuses.Count);
-#pragma warning restore CS0618
         }
 
         if (failed > 0)
@@ -1055,17 +1057,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
         {
             // A failed event report must NOT break the state projection or the calling actor: the node's
             // state has already been applied + ClearChangeMasks'd, so attribute subscribers still see the
-            // change; only the event delivery is lost. This CustomNodeManager2 carries no ILogger, so log
-            // through the SDK's static trace (Utils.LogError) instead of swallowing silently — a recurring
-            // failure here is then visible in the server log rather than invisible. T19's live Client.CLI
-            // run is the integration proof that the happy path delivers.
-            // Utils.LogError routes to the SDK's trace sink. It's [Obsolete] in 1.5.378 in favour of an
-            // ITelemetryContext/ILogger this CustomNodeManager2 doesn't have wired — suppress the
-            // deprecation here (wiring the telemetry logger through is a separate follow-up); the point is
-            // that a recurring failure is visible in the server trace rather than silently swallowed.
-#pragma warning disable CS0618 // Type or member is obsolete
-            Utils.LogError(ex, "OtOpcUaNodeManager: failed to report Part 9 condition event for {0}", alarm.NodeId);
-#pragma warning restore CS0618
+            // change; only the event delivery is lost. Log through the injected ILogger instead of
+            // swallowing silently — a recurring failure here is then visible in the server log rather than
+            // invisible. T19's live Client.CLI run is the integration proof that the happy path delivers.
+            _logger.LogError(ex, "OtOpcUaNodeManager: failed to report Part 9 condition event for {NodeId}", alarm.NodeId);
         }
     }
 
@@ -1479,15 +1474,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
             // not fault on the notifier wiring.
             if (!_alarmConditions.TryGetValue(alarmKey, out var alarm))
             {
-                // This CustomNodeManager2 carries no ILogger; log through the SDK's static trace (see the
-                // ReportEvent catch below for the same pattern). Utils.LogInfo is [Obsolete] in 1.5.378 in
-                // favour of an ITelemetryContext this manager doesn't wire — suppress the deprecation.
-#pragma warning disable CS0618 // Type or member is obsolete
                 // Interpolate the key's PARTS, not the RealmKey struct — a struct dump would render a shape
                 // no log-grep pattern in this file matches.
-                Utils.LogInfo("OtOpcUaNodeManager.WireAlarmNotifiers: condition {0} (realm {1}) not materialised — skipping notifier wiring",
+                _logger.LogInformation("OtOpcUaNodeManager.WireAlarmNotifiers: condition {ConditionId} (realm {Realm}) not materialised — skipping notifier wiring",
                     alarmKey.Id, alarmKey.Realm);
-#pragma warning restore CS0618
                 return;
             }
 
@@ -1520,10 +1510,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
                 if (!_folders.TryGetValue(MapKey(notifierFolderRealm, folderNodeId), out var folder))
                 {
                     // Referencing-equipment folder not (yet) materialised — skip this one (logged, never thrown).
-#pragma warning disable CS0618 // Type or member is obsolete
-                    Utils.LogInfo("OtOpcUaNodeManager.WireAlarmNotifiers: notifier folder {0} for condition {1} (realm {2}) not present — skipped",
+                    _logger.LogInformation("OtOpcUaNodeManager.WireAlarmNotifiers: notifier folder {FolderId} for condition {ConditionId} (realm {Realm}) not present — skipped",
                         folderNodeId, alarmKey.Id, alarmKey.Realm);
-#pragma warning restore CS0618
                     continue;
                 }
 
@@ -2212,9 +2200,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
             }
             catch (Exception ex)
             {
-#pragma warning disable CS0618 // Utils.LogError is [Obsolete] in favour of an ITelemetryContext this manager doesn't carry.
-                Utils.LogError(ex, "OtOpcUaNodeManager: failed to build AuditWriteUpdateEvent for {0}", nodeId);
-#pragma warning restore CS0618
+                _logger.LogError(ex, "OtOpcUaNodeManager: failed to build AuditWriteUpdateEvent for {NodeId}", nodeId);
                 auditEvent = null;
             }
         }
@@ -2297,9 +2283,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
         {
             // Auditing disabled / no monitored items / server shutting down ⇒ ReportEvent may no-op or throw;
             // either way the revert already stands. Surface a recurring failure in the SDK trace, don't rethrow.
-#pragma warning disable CS0618 // Utils.LogError is [Obsolete] in favour of an ITelemetryContext this manager doesn't carry.
-            Utils.LogError(ex, "OtOpcUaNodeManager: failed to report AuditWriteUpdateEvent for {0}", auditEvent.SourceNode?.Value);
-#pragma warning restore CS0618
+            _logger.LogError(ex, "OtOpcUaNodeManager: failed to report AuditWriteUpdateEvent for {NodeId}", auditEvent.SourceNode?.Value);
         }
     }
 
@@ -2660,9 +2644,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
         {
             // Model-change reporting disabled / no monitored items / server shutting down ⇒ ReportEvent may
             // no-op or throw; either way the in-place update already stands. Log to the SDK trace, don't rethrow.
-#pragma warning disable CS0618 // Utils.LogError is [Obsolete] in favour of an ITelemetryContext this manager doesn't carry.
-            Utils.LogError(ex, "OtOpcUaNodeManager: failed to report GeneralModelChangeEvent for {0}", variable.NodeId);
-#pragma warning restore CS0618
+            _logger.LogError(ex, "OtOpcUaNodeManager: failed to report GeneralModelChangeEvent for {NodeId}", variable.NodeId);
         }
     }
 
@@ -2704,9 +2686,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
         {
             // Model-change reporting disabled / no monitored items / server shutting down ⇒ ReportEvent may
             // no-op or throw; either way the node add already stands. Log to the SDK trace, don't rethrow.
-#pragma warning disable CS0618 // Utils.LogError is [Obsolete] in favour of an ITelemetryContext this manager doesn't carry.
-            Utils.LogError(ex, "OtOpcUaNodeManager: failed to report GeneralModelChangeEvent(NodeAdded) for {0}", affectedNodeId);
-#pragma warning restore CS0618
+            _logger.LogError(ex, "OtOpcUaNodeManager: failed to report GeneralModelChangeEvent(NodeAdded) for {NodeId}", affectedNodeId);
         }
     }
 
@@ -2730,11 +2710,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
             // concurrent rebuild has torn one side down; never throw out of a deploy-time reference wire.
             if (source is null || target is null)
             {
-#pragma warning disable CS0618 // Utils.LogWarning is [Obsolete] in favour of an ITelemetryContext this manager doesn't carry.
-                Utils.LogWarning(
-                    "OtOpcUaNodeManager: AddReference({0}) skipped — {1} node missing (source '{2}' present={3}, target '{4}' present={5}).",
+                _logger.LogWarning(
+                    "OtOpcUaNodeManager: AddReference({RefType}) skipped — {Side} node missing (source '{SourceKey}' present={SourcePresent}, target '{TargetKey}' present={TargetPresent}).",
                     refType, source is null ? "source" : "target", sourceKey, source is not null, targetKey, target is not null);
-#pragma warning restore CS0618
                 return;
             }
 
@@ -2888,9 +2866,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
         }
         catch (Exception ex)
         {
-#pragma warning disable CS0618 // Utils.LogError is [Obsolete] in favour of an ITelemetryContext this manager doesn't carry.
-            Utils.LogError(ex, "OtOpcUaNodeManager: failed to report GeneralModelChangeEvent for {0}", affectedNodeId);
-#pragma warning restore CS0618
+            _logger.LogError(ex, "OtOpcUaNodeManager: failed to report GeneralModelChangeEvent for {NodeId}", affectedNodeId);
         }
     }
 
@@ -4719,10 +4695,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
         catch (Exception ex)
         {
             // One node's backend failure must not throw out of the batch — surface Bad for THIS node
-            // only. This manager carries no ILogger, so log via the SDK's static trace (see ServeNodeAsync).
-#pragma warning disable CS0618 // Type or member is obsolete
-            Utils.LogError(ex, "OtOpcUaNodeManager: HistoryReadEvents failed for node {0}", handle.NodeId);
-#pragma warning restore CS0618
+            // only. Logged via the injected ILogger.
+            _logger.LogError(ex, "OtOpcUaNodeManager: HistoryReadEvents failed for node {NodeId}", handle.NodeId);
             errors[handle.Index] = StatusCodes.BadHistoryOperationUnsupported;
             results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.BadHistoryOperationUnsupported };
         }
@@ -4994,13 +4968,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
         catch (Exception ex)
         {
             // One node's backend failure (throw / timeout) must not throw out of the batch — surface a
-            // Bad status for THIS node only. This CustomNodeManager2 carries no ILogger (see
-            // ReportConditionEvent), so log through the SDK's static trace rather than swallowing silently.
-            // Utils.LogError is [Obsolete] in 1.5.378 (favours an ITelemetryContext this manager doesn't
-            // wire) — suppress the deprecation, matching the existing pattern.
-#pragma warning disable CS0618 // Type or member is obsolete
-            Utils.LogError(ex, "OtOpcUaNodeManager: HistoryRead failed for node {0}", handle.NodeId);
-#pragma warning restore CS0618
+            // Bad status for THIS node only. Logged through the injected ILogger
+            // rather than swallowed silently.
+            _logger.LogError(ex, "OtOpcUaNodeManager: HistoryRead failed for node {NodeId}", handle.NodeId);
             errors[handle.Index] = StatusCodes.BadHistoryOperationUnsupported;
         }
     }
@@ -5127,12 +5097,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
                 // larger ServerHistorian:MaxTieClusterOverfetch (or NumValuesPerNode).
                 if (cluster.Count > MaxTieClusterOverfetch)
                 {
-#pragma warning disable CS0618 // Type or member is obsolete
-                    Utils.LogError(
-                        "OtOpcUaNodeManager: HistoryReadRaw tie cluster at {0:O} for tag '{1}' has {2} samples, " +
-                        "exceeding MaxTieClusterOverfetch={3}; cannot page within it. Increase MaxTieClusterOverfetch.",
+                    _logger.LogError(
+                        "OtOpcUaNodeManager: HistoryReadRaw tie cluster at {StartUtc:O} for tag '{Tagname}' has {Count} samples, " +
+                        "exceeding MaxTieClusterOverfetch={Max}; cannot page within it. Increase MaxTieClusterOverfetch.",
                         startUtc, tagname, cluster.Count, MaxTieClusterOverfetch);
-#pragma warning restore CS0618
                     errors[handle.Index] = StatusCodes.BadHistoryOperationUnsupported;
                     results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.BadHistoryOperationUnsupported };
                     return;
@@ -5201,9 +5169,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
         catch (Exception ex)
         {
             // One node's backend failure must not throw out of the batch — Bad for THIS node only.
-#pragma warning disable CS0618 // Type or member is obsolete
-            Utils.LogError(ex, "OtOpcUaNodeManager: HistoryReadRaw (paged) failed for node {0}", handle.NodeId);
-#pragma warning restore CS0618
+            _logger.LogError(ex, "OtOpcUaNodeManager: HistoryReadRaw (paged) failed for node {NodeId}", handle.NodeId);
             errors[handle.Index] = StatusCodes.BadHistoryOperationUnsupported;
         }
     }

src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaSdkServer.cs

@@ -1,3 +1,4 @@
+using Microsoft.Extensions.Logging;
 using Opc.Ua;
 using Opc.Ua.Server;
 using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
@@ -15,8 +16,14 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer;
 /// </summary>
 public sealed class OtOpcUaSdkServer : StandardServer
 {
+    private readonly ILogger<OtOpcUaNodeManager>? _nodeManagerLogger;
     private OtOpcUaNodeManager? _otOpcUaNodeManager;
 
+    /// <summary>Initializes a new instance of the <see cref="OtOpcUaSdkServer"/> class.</summary>
+    /// <param name="nodeManagerLogger">Logger handed to the created <see cref="OtOpcUaNodeManager"/>;
+    /// <c>null</c> leaves it on a no-op logger.</param>
+    public OtOpcUaSdkServer(ILogger<OtOpcUaNodeManager>? nodeManagerLogger = null) => _nodeManagerLogger = nodeManagerLogger;
+
     /// <summary>The custom node manager once <c>StartAsync</c> has called
     /// <see cref="CreateMasterNodeManager"/>. Null until the SDK has bootstrapped.</summary>
     public OtOpcUaNodeManager? NodeManager => _otOpcUaNodeManager;
@@ -163,7 +170,7 @@ public sealed class OtOpcUaSdkServer : StandardServer
     protected override MasterNodeManager CreateMasterNodeManager(
         IServerInternal server, ApplicationConfiguration configuration)
     {
-        _otOpcUaNodeManager = new OtOpcUaNodeManager(server, configuration);
+        _otOpcUaNodeManager = new OtOpcUaNodeManager(server, configuration, _nodeManagerLogger);
         return new MasterNodeManager(server, configuration, dynamicNamespaceUri: null, _otOpcUaNodeManager);
     }
 }

tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ServerEventRig.cs

@@ -229,9 +229,7 @@ internal sealed class ServerEventRig : IAsyncDisposable
 
         if (encrypted)
         {
-#pragma warning disable CS0618 // the telemetry-context overload is not needed for a test client
-            var app = new ApplicationInstance { ApplicationName = appConfig.ApplicationName, ApplicationType = ApplicationType.Client };
-#pragma warning restore CS0618
+            var app = new ApplicationInstance(DefaultTelemetry.Create(_ => { })) { ApplicationName = appConfig.ApplicationName, ApplicationType = ApplicationType.Client };
             app.ApplicationConfiguration = appConfig;
             await app.CheckApplicationInstanceCertificatesAsync(silent: true, lifeTimeInMonths: null, ct);
         }