fix(ab-transport): final review I2 — wire DeviceSessionPool.Shared to the host logger

ConfigureSharedLogger had no production caller, so the pool's only
diagnostics (first-wins options conflicts, scheduler/close failures)
went to NullLogger. DriverFactoryBootstrap now configures it from the
driver registry factory and from the AbCip/AbLegacy probe factories
(admin-only nodes reach the pool only through Test Connect). Wiring tests
reset the pool to a null logger first, so removing the call goes red
(verified).

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-07 21:36:37 -04:00
parent b75e013d0a
commit 2cea1a0687
5 changed files with 139 additions and 2 deletions
@@ -49,6 +49,9 @@ public sealed class DeviceSessionPool
Shared._logger = loggerFactory.CreateLogger<DeviceSessionPool>();
}
/// <summary>Wiring-test hook: the logger this pool currently writes to (final review I2).</summary>
internal ILogger Logger => _logger;
/// <summary>Test hook: keys with a live entry.</summary>
internal int EntryCount { get { lock (_lock) return _entries.Count; } }
@@ -18,5 +18,6 @@
<InternalsVisibleTo Include="ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests"/>
<InternalsVisibleTo Include="ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests"/>
<InternalsVisibleTo Include="ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests"/>
<InternalsVisibleTo Include="ZB.MOM.WW.OtOpcUa.Host.Tests"/>
</ItemGroup>
</Project>
@@ -80,6 +80,9 @@ public static class DriverFactoryBootstrap
// whole mechanism exists to prevent, and it would fail as a plant outage rather than as a
// startup error.
var primaryRoleSource = sp.GetRequiredService<IDriverPrimaryRoleSource>();
// Final review I2: the native AB transport's process-wide pool logs options conflicts and close
// failures; without this it writes to NullLogger in production.
ConfigureAbTransportLogging(sp);
Register(registry, loggerFactory, secretResolver, primaryRoleSource, ResolveAbCipDefaultTransport(sp),
scriptRoot);
return registry;
@@ -148,8 +151,16 @@ public static class DriverFactoryBootstrap
// driver would run. The generic factory overload keeps AbCipProbe as the implementation type, which
// TryAddEnumerable de-dups on.
services.TryAddEnumerable(ServiceDescriptor.Singleton<IDriverProbe, AbCipProbe>(
sp => new AbCipProbe(tagFactory: null, defaultTransport: ResolveAbCipDefaultTransport(sp))));
services.TryAddEnumerable(ServiceDescriptor.Singleton<IDriverProbe, AbLegacyProbe>());
sp =>
{
ConfigureAbTransportLogging(sp); // admin-only nodes reach the pool through the probes alone
return new AbCipProbe(tagFactory: null, defaultTransport: ResolveAbCipDefaultTransport(sp));
}));
services.TryAddEnumerable(ServiceDescriptor.Singleton<IDriverProbe, AbLegacyProbe>(sp =>
{
ConfigureAbTransportLogging(sp);
return new AbLegacyProbe();
}));
services.TryAddEnumerable(ServiceDescriptor.Singleton<IDriverProbe, S7Probe>());
services.TryAddEnumerable(ServiceDescriptor.Singleton<IDriverProbe, TwinCATProbe>());
services.TryAddEnumerable(ServiceDescriptor.Singleton<IDriverProbe, FocasProbe>());
@@ -163,6 +174,19 @@ public static class DriverFactoryBootstrap
return services;
}
/// <summary>
/// Gives the native AB transport's process-wide <c>DeviceSessionPool.Shared</c> the host logger factory
/// (final review I2). Called from every DI factory that can put the pool to use on this node (the driver
/// registry, and the AbCip/AbLegacy probes for admin-only nodes); idempotent. A container without an
/// <see cref="ILoggerFactory"/> leaves the pool on its null logger.
/// </summary>
/// <param name="sp">The service provider.</param>
internal static void ConfigureAbTransportLogging(IServiceProvider sp)
{
if (sp.GetService<ILoggerFactory>() is { } loggerFactory)
Driver.Ab.Transport.DeviceSessionPool.ConfigureSharedLogger(loggerFactory);
}
/// <summary>
/// The node-level AbCip default transport (<c>Drivers:AbCip:DefaultTransport</c>, bound + validated in
/// Program.cs). A container without the options bound (test graphs) gets the compiled default.
@@ -20,6 +20,7 @@ namespace ZB.MOM.WW.OtOpcUa.Host.Tests.Configuration;
/// node (Libplctag) against the compiled default (Native), so a wiring that ignores the node value goes red.
/// </summary>
[Trait("Category", "Unit")]
[Collection(AbTransportSharedPoolCollection.Name)] // resolves the registry, which configures the shared pool
public sealed class AbCipDriverDefaultsOptionsTests
{
private const string NoKey = """{"devices":[{"hostAddress":"ab://10.0.0.1/1,0"}],"probe":{"enabled":false}}""";
@@ -0,0 +1,108 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
using ZB.MOM.WW.OtOpcUa.Core.Hosting;
using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport;
using ZB.MOM.WW.OtOpcUa.Host.Drivers;
using ZB.MOM.WW.Secrets.Abstractions;
namespace ZB.MOM.WW.OtOpcUa.Host.Tests.Configuration;
/// <summary>The shared pool is process-wide, so the tests that touch it must not run in parallel.</summary>
[CollectionDefinition(Name, DisableParallelization = true)]
public sealed class AbTransportSharedPoolCollection
{
public const string Name = "AbTransportSharedPool";
}
/// <summary>
/// Final review I2: <c>DeviceSessionPool.Shared</c> must log through the HOST logger factory, on driver
/// nodes (via the driver registry) and on admin-only nodes (via the AbCip / AbLegacy Test Connect probes).
/// Each test first resets the pool to a null logger and asserts that baseline, so deleting the wiring
/// call turns it red (the logger would stay the reset one).
/// </summary>
[Trait("Category", "Unit")]
[Collection(AbTransportSharedPoolCollection.Name)]
public sealed class AbTransportSharedLoggerWiringTests
{
private static RecordingLoggerFactory Reset()
{
DeviceSessionPool.ConfigureSharedLogger(NullLoggerFactory.Instance);
var factory = new RecordingLoggerFactory();
AssertPoolLogsTo(factory, expected: false); // baseline: the wiring under test has not run yet
return factory;
}
// Logger<T> wraps the factory's logger, so identity cannot be compared: emit through the pool's logger
// and check whether it lands in the recording factory under the pool's category.
private static void AssertPoolLogsTo(RecordingLoggerFactory factory, bool expected)
{
var marker = Guid.NewGuid().ToString("N");
DeviceSessionPool.Shared.Logger.LogWarning("wiring-probe {Marker}", marker);
factory.Messages(typeof(DeviceSessionPool).FullName!).Any(m => m.Contains(marker)).ShouldBe(expected);
}
[Fact]
public void Resolving_the_driver_registry_wires_the_pool_to_the_host_logger_factory()
{
var factory = Reset();
var services = new ServiceCollection();
services.AddSingleton<ILoggerFactory>(factory);
services.AddSingleton<ISecretResolver>(new StubSecretResolver());
services.AddOtOpcUaDriverFactories();
using var sp = services.BuildServiceProvider();
sp.GetRequiredService<DriverFactoryRegistry>();
AssertPoolLogsTo(factory, expected: true);
}
[Theory]
[InlineData(DriverTypeNames.AbCip)]
[InlineData(DriverTypeNames.AbLegacy)]
public void An_admin_only_node_wires_the_pool_through_the_AB_probes(string driverType)
{
var factory = Reset();
var services = new ServiceCollection();
services.AddSingleton<ILoggerFactory>(factory);
services.AddOtOpcUaDriverProbes(); // the admin-only path: no driver registry
using var sp = services.BuildServiceProvider();
sp.GetServices<IDriverProbe>().Single(p => p.DriverType == driverType);
AssertPoolLogsTo(factory, expected: true);
}
private sealed class RecordingLoggerFactory : ILoggerFactory
{
private readonly List<(string Category, string Message)> _messages = [];
public IReadOnlyList<string> Messages(string category)
{
lock (_messages) return [.. _messages.Where(m => m.Category == category).Select(m => m.Message)];
}
public ILogger CreateLogger(string categoryName) => new Recording(this, categoryName);
public void AddProvider(ILoggerProvider provider) { }
public void Dispose() { }
private sealed class Recording(RecordingLoggerFactory owner, string category) : ILogger
{
public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
public bool IsEnabled(LogLevel logLevel) => true;
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception,
Func<TState, Exception?, string> formatter)
{
lock (owner._messages) owner._messages.Add((category, formatter(state, exception)));
}
}
}
private sealed class StubSecretResolver : ISecretResolver
{
public Task<string?> GetAsync(SecretName name, CancellationToken ct) => Task.FromResult<string?>(null);
}
}