Skip to content

Making sure the basic logging infrastructure is always wired up#7659

Merged
danielmarbach merged 11 commits into
masterfrom
logging
Mar 17, 2026
Merged

Making sure the basic logging infrastructure is always wired up#7659
danielmarbach merged 11 commits into
masterfrom
logging

Conversation

@danielmarbach
Copy link
Copy Markdown
Contributor

@danielmarbach danielmarbach commented Mar 16, 2026

Follow through on #7658

This pull request introduces a significant refactor of the logging infrastructure in the acceptance testing and core hosting code. The main changes are a migration from custom logger factories to Microsoft.Extensions.Logging providers, removal of legacy logging classes, and updates to test and endpoint setup logic to align with the new logging approach. Additionally, some test code has been modernized for clarity and conciseness.

Logging infrastructure refactor

  • Replaced the custom ContextAppenderMicrosoftLoggerFactory with a new ContextAppenderLoggerProvider implementing ILoggerProvider and ISupportExternalScope, and updated registration logic to use Microsoft.Extensions.Logging APIs. (src/NServiceBus.AcceptanceTesting/ContextAppenderLoggerProvider.cs) [1] [2] [3]
  • Removed legacy logging classes: ContextAppenderNServiceBusLogger and ContextAppenderNServiceBusLoggerFactory, as well as the associated factory registration in Scenario. (src/NServiceBus.AcceptanceTesting/ContextAppenderNServiceBusLogger.cs, src/NServiceBus.AcceptanceTesting/ContextAppenderNServiceBusLoggerFactory.cs, src/NServiceBus.AcceptanceTesting/Scenario.cs) [1] [2] [3]

Core hosting and endpoint setup

  • Updated EndpointCreator.Configure() to register logging as a global service concern using Microsoft.Extensions.Logging, and modernized service registration patterns. (src/NServiceBus.Core/EndpointCreator.cs) [1] [2] [3]
  • Modified EndpointPreparation to resolve and register the Microsoft logger factory as the slot factory for logging, replacing the previous bridge logic. (src/NServiceBus.Core/Hosting/EndpointPreparation.cs) [1] [2]

Test code improvements

  • Updated tests to use the new logging provider and simplified assembly scanner disabling. Also, refactored test classes to use modern C# syntax for constructors and collection initializations. (src/NServiceBus.Core.Tests/Config/When_configuring_transport_twice.cs, src/NServiceBus.Core.Tests/Config/When_starting_an_endpoint_with_already_used_configuration.cs, src/NServiceBus.Core.Tests/Config/When_starting_endpoint_without_configuring_serializer.cs, src/NServiceBus.Core.Tests/Config/When_using_initialization.cs) [1] [2] [3] [4] [5] [6] [7] [8]

Miscellaneous

  • Improved thread safety for service collection access in KeyedServiceCollectionAdapter by adding a scoped lock. (src/NServiceBus.Core/Hosting/KeyedServices/KeyedServiceCollectionAdapter.cs)
  • Removed two tests related to deferred log flushing and duplication from EndpointLoggingScopeTests, as they are no longer relevant to the new logging infrastructure. (src/NServiceBus.Core.Tests/Logging/EndpointLoggingScopeTests.cs)

@andreasohlund andreasohlund marked this pull request as ready for review March 17, 2026 05:30
@andreasohlund andreasohlund self-requested a review March 17, 2026 05:30
Comment thread src/NServiceBus.Core/Installation/Installer.cs Outdated
@andreasohlund andreasohlund marked this pull request as draft March 17, 2026 05:57
@andreasohlund
Copy link
Copy Markdown
Member

https://github.com/Particular/NServiceBus/blob/5787dbd2ecbe2e361c21e618af5b55ef024c9619/src/NServiceBus.Core.Tests/Config/When_starting_an_endpoint_with_already_used_configuration.cs now fails due to transport being reused and not that the endpoint config is being reused. Don't we already have a test for this?

Comment thread src/NServiceBus.AcceptanceTesting/ContextAppenderLoggerProvider.cs

public static class Scenario
{
public static Func<ScenarioContext, ILoggerFactory> GetLoggerFactory { get; set; } = _ => new ContextAppenderNServiceBusLoggerFactory();
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.

We are not giving any backward compatibility guarantees for acceptance testing, so this is OK to remove

https://github.com/search?q=org%3AParticular%20GetLoggerFactory&type=code

TestExecutionContext.CurrentContext.AddRunDescriptor(runDescriptor);
ScenarioContext.Current = scenarioContext;

LogManager.UseFactory(Scenario.GetLoggerFactory(scenarioContext));
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.

This is no longer required because the Core logging bridge takes care of intercepting things properly

get;
get
{
using var _ = gate.EnterScope();
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.

Making sure the access to Inner is gated too

@danielmarbach danielmarbach added this to the 10.2.0 milestone Mar 17, 2026
@danielmarbach danielmarbach changed the title Logging Making sure the basic logging infrastructure is always wired up Mar 17, 2026
@danielmarbach danielmarbach marked this pull request as ready for review March 17, 2026 11:05
@danielmarbach
Copy link
Copy Markdown
Contributor Author

FYI @bording

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants