Fix crash when logging providers are cleared after dynamic logging setup#1675
Fix crash when logging providers are cleared after dynamic logging setup#1675
Conversation
…AddDynamicSerilog Register the concrete provider type as a standalone singleton so that IDynamicLoggerProvider remains resolvable even when ClearProviders() is called after the dynamic logging setup. Previously, IDynamicLoggerProvider was resolved via a factory that searched through ILoggerProvider services using .Single(), which threw when ClearProviders() removed them. Agent-Logs-Url: https://github.com/SteeltoeOSS/Steeltoe/sessions/9b9035c5-5b65-49bc-b0dd-a2e9cdd0fd14 Co-authored-by: bart-vmware <104792814+bart-vmware@users.noreply.github.com>
| builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton<ILoggerProvider, DynamicConsoleLoggerProvider>()); | ||
| builder.Services.AddSingleton(provider => provider.GetServices<ILoggerProvider>().OfType<IDynamicLoggerProvider>().Single()); | ||
| builder.Services.TryAddSingleton<DynamicConsoleLoggerProvider>(); | ||
| builder.Services.AddSingleton<ILoggerProvider>(serviceProvider => serviceProvider.GetRequiredService<DynamicConsoleLoggerProvider>()); |
There was a problem hiding this comment.
@copilot Beware that IDynamicLoggerProvider is a public API extensibility point. This means the feature also needs to work with third-party implementations. Add a test with a fake implementation (as a private nested type) to verify this is still possible.
There was a problem hiding this comment.
Added CanUseThirdPartyDynamicLoggerProvider in DynamicConsole.Test/HostBuilderTest.cs (commit 7b47671). It registers the existing OtherDynamicLoggerProvider (private nested type) as IDynamicLoggerProvider before calling AddDynamicConsole(), then asserts that GetService<IDynamicLoggerProvider>() resolves to the fake implementation — not DynamicConsoleLoggerProvider. The test passes with both the old and new code, confirming the extensibility point is preserved.
There was a problem hiding this comment.
@copilot My concern is that injecting IEnumerable<ILoggerProvider> returns a DynamicConsoleLoggerProvider instead of the OtherDynamicLoggerProvider. Update the test to verify this.
There was a problem hiding this comment.
Updated in commit 9480d4f. The test now registers OtherDynamicLoggerProvider as both ILoggerProvider and IDynamicLoggerProvider (the same pattern Steeltoe uses for its own providers), then asserts:
IDynamicLoggerProviderresolves toOtherDynamicLoggerProviderIEnumerable<ILoggerProvider>contains exactly oneOtherDynamicLoggerProviderIEnumerable<ILoggerProvider>does NOT containDynamicConsoleLoggerProvider
Also fixed OtherDynamicLoggerProvider.CreateLogger to return NullLogger.Instance instead of throwing, since the logging infrastructure calls it when the provider is registered as ILoggerProvider.
|
Summary - All Code Coverage (ubuntu-latest)
|
Agent-Logs-Url: https://github.com/SteeltoeOSS/Steeltoe/sessions/daa346d0-4279-4e7d-b1b1-e906e2e04020 Co-authored-by: bart-vmware <104792814+bart-vmware@users.noreply.github.com>
…gerProvider behavior Agent-Logs-Url: https://github.com/SteeltoeOSS/Steeltoe/sessions/0abdafc7-d2c6-4eea-bb74-cf1283bd7ed7 Co-authored-by: bart-vmware <104792814+bart-vmware@users.noreply.github.com>
ClearProviders()is called afterAddDynamicConsole()/AddDynamicSerilog()DoesNotCrashWhenLoggingProvidersAreClearedfor DynamicConsoleDoesNotCrashWhenLoggingProvidersAreClearedfor DynamicSerilogCanUseThirdPartyDynamicLoggerProviderto verifyIDynamicLoggerProviderextensibility: registers third-party provider as bothILoggerProviderandIDynamicLoggerProvider, asserts it is resolvable through both, and assertsDynamicConsoleLoggerProvideris NOT inIEnumerable<ILoggerProvider>