Skip to content

Fix crash when logging providers are cleared after dynamic logging setup #1675

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 4 commits into from
Closed

Fix crash when logging providers are cleared after dynamic logging setup #1675

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a2b3763
Initial plan
Copilot Apr 16, 2026
f2d8635
Fix crash when logging providers are cleared after AddDynamicConsole/…
Copilot Apr 16, 2026
7b47671
Add test verifying third-party IDynamicLoggerProvider extensibility
Copilot Apr 16, 2026
9480d4f
Update CanUseThirdPartyDynamicLoggerProvider test to also assert ILog…
Copilot Apr 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/Logging/src/DynamicConsole/LoggingBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ public static ILoggingBuilder AddDynamicConsole(this ILoggingBuilder builder)
UpdateConsoleLoggerProviderRegistration(builder.Services);

builder.AddFilter<DynamicConsoleLoggerProvider>(null, LogLevel.Trace);
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>());

@bart-vmware bart-vmware Apr 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copilot AI Apr 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@bart-vmware bart-vmware Apr 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot My concern is that injecting IEnumerable<ILoggerProvider> returns a DynamicConsoleLoggerProvider instead of the OtherDynamicLoggerProvider. Update the test to verify this.

@bart-vmware bart-vmware Apr 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Take a look at my previous remark.

Copilot AI Apr 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  • IDynamicLoggerProvider resolves to OtherDynamicLoggerProvider
  • IEnumerable<ILoggerProvider> contains exactly one OtherDynamicLoggerProvider
  • IEnumerable<ILoggerProvider> does NOT contain DynamicConsoleLoggerProvider

Also fixed OtherDynamicLoggerProvider.CreateLogger to return NullLogger.Instance instead of throwing, since the logging infrastructure calls it when the provider is registered as ILoggerProvider.

builder.Services.AddSingleton<IDynamicLoggerProvider>(serviceProvider => serviceProvider.GetRequiredService<DynamicConsoleLoggerProvider>());

DisableConsoleColorsOnCloudPlatform(builder);
}
Expand Down
9 changes: 4 additions & 5 deletions src/Logging/src/DynamicSerilog/SerilogLoggingBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Serilog;
using Steeltoe.Common;

namespace Steeltoe.Logging.DynamicSerilog;

Expand Down Expand Up @@ -94,17 +93,17 @@ public static ILoggingBuilder AddDynamicSerilog(this ILoggingBuilder builder, Lo

ConfigureSerilogOptions(builder, serilogConfiguration);

builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton<ILoggerProvider, DynamicSerilogLoggerProvider>());
builder.Services.AddSingleton(provider => provider.GetServices<ILoggerProvider>().OfType<IDynamicLoggerProvider>().Single());
builder.Services.TryAddSingleton<DynamicSerilogLoggerProvider>();
builder.Services.AddSingleton<ILoggerProvider>(serviceProvider => serviceProvider.GetRequiredService<DynamicSerilogLoggerProvider>());
builder.Services.AddSingleton<IDynamicLoggerProvider>(serviceProvider => serviceProvider.GetRequiredService<DynamicSerilogLoggerProvider>());
}

return builder;
}

private static bool IsSerilogDynamicLoggerProviderAlreadyRegistered(ILoggingBuilder builder)
{
return builder.Services.Any(descriptor =>
descriptor.SafeGetImplementationType() == typeof(DynamicSerilogLoggerProvider) && descriptor.ServiceType == typeof(ILoggerProvider));
return builder.Services.Any(descriptor => descriptor.ServiceType == typeof(DynamicSerilogLoggerProvider));
}

private static void AssertNoDynamicLoggerProviderRegistered(ILoggingBuilder builder)
Expand Down
30 changes: 29 additions & 1 deletion src/Logging/test/DynamicConsole.Test/HostBuilderTest.cs
View file
Open in desktop
Comment thread
bart-vmware marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Logging.Console;
using Microsoft.Extensions.Options;
using Steeltoe.Common.Logging;
Expand Down Expand Up @@ -53,6 +54,21 @@ public async Task DoesNotRegisterWhenOtherDynamicProviderIsAlreadyRegistered()
host.Services.GetServices<ILoggerProvider>().Should().NotContain(provider => provider is DynamicConsoleLoggerProvider);
}

[Fact]
public async Task CanUseThirdPartyDynamicLoggerProvider()
{
WebApplicationBuilder builder = TestWebApplicationBuilderFactory.Create();
builder.Services.AddSingleton<OtherDynamicLoggerProvider>();
builder.Services.AddSingleton<ILoggerProvider>(serviceProvider => serviceProvider.GetRequiredService<OtherDynamicLoggerProvider>());
builder.Services.AddSingleton<IDynamicLoggerProvider>(serviceProvider => serviceProvider.GetRequiredService<OtherDynamicLoggerProvider>());
builder.Logging.AddDynamicConsole();
await using WebApplication host = builder.Build();

host.Services.GetService<IDynamicLoggerProvider>().Should().BeOfType<OtherDynamicLoggerProvider>();
host.Services.GetServices<ILoggerProvider>().OfType<OtherDynamicLoggerProvider>().Should().ContainSingle();
host.Services.GetServices<ILoggerProvider>().Should().NotContain(provider => provider is DynamicConsoleLoggerProvider);
}

[Fact]
public async Task CanChangeMinimumLogLevels()
{
Expand Down Expand Up @@ -203,11 +219,23 @@ public async Task DisposeTwiceSucceeds()
action.Should().NotThrow();
}

[Fact]
public async Task DoesNotCrashWhenLoggingProvidersAreCleared()
{
WebApplicationBuilder builder = TestWebApplicationBuilderFactory.Create();
builder.Logging.AddDynamicConsole();
builder.Services.AddLogging(loggingBuilder => loggingBuilder.ClearProviders());
await using WebApplication host = builder.Build();

host.Services.GetService<IDynamicLoggerProvider>().Should().BeOfType<DynamicConsoleLoggerProvider>();
host.Services.GetServices<ILoggerProvider>().OfType<DynamicConsoleLoggerProvider>().Should().BeEmpty();
}

private sealed class OtherDynamicLoggerProvider : IDynamicLoggerProvider
{
public ILogger CreateLogger(string categoryName)
{
throw new NotSupportedException();
return NullLogger.Instance;
}

public ICollection<DynamicLoggerState> GetLogLevels()
Expand Down
12 changes: 12 additions & 0 deletions src/Logging/test/DynamicSerilog.Test/HostBuilderTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,18 @@ public async Task DisposeTwiceSucceeds()
action.Should().NotThrow();
}

[Fact]
public async Task DoesNotCrashWhenLoggingProvidersAreCleared()
{
WebApplicationBuilder builder = TestWebApplicationBuilderFactory.Create();
builder.Logging.AddDynamicSerilog();
builder.Services.AddLogging(loggingBuilder => loggingBuilder.ClearProviders());
await using WebApplication host = builder.Build();

host.Services.GetService<IDynamicLoggerProvider>().Should().BeOfType<DynamicSerilogLoggerProvider>();
host.Services.GetServices<ILoggerProvider>().OfType<DynamicSerilogLoggerProvider>().Should().BeEmpty();
}

public void Dispose()
{
_consoleOutput.Dispose();
Expand Down