Allow replacing IExternalScopeProvider via DI

[shirhatti]

Copied from #52153 created by @Sundow:

Background and Motivation

Currently Microsoft.Extensions.Logging.LoggerFactory class always instantiates IExternalScopeProvider with a default implementation. #50590 proposes to enable dependency injection so it is possible to use a custom implementation. This requires to add a constructor with one more parameter of type IExternalScopeProvider to be used by DI engine.

Proposed API

public partial class LoggerFactory : Microsoft.Extensions.Logging.ILoggerFactory
{
    public LoggerFactory(System.Collections.Generic.IEnumerable<Microsoft.Extensions.Logging.ILoggerProvider> providers, Microsoft.Extensions.Options.IOptionsMonitor<Microsoft.Extensions.Logging.LoggerFilterOptions> filterOption, Microsoft.Extensions.Options.IOptions<Microsoft.Extensions.Logging.LoggerFactoryOptions> options = null, Microsoft.Extensions.Logging.IExternalScopeProvider scopeProvider = null) { }
}

Usage Examples

Will be implicitly used by DI engine.

Risks

Low, as no existing interfaces are changed and only a trivial field assignment is required for a new one.

Other notes

The actual implementation of the proposal already exists in #52132 and can be looked at there, but requires API change approval.

---

Original description by @shirhatti:

Original Description

Today the default LoggerFactory implementation instantiates it's own IExternalScopeProvider. We should resolve it from DI instead allowing folks to replace the default IExternalScopeProvider.

[ghost]

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Today the default LoggerFactory implementation instantiates it's own IExternalScopeProvider. We should resolve it from DI instead allowing folks to replace the default IExternalScopeProvider.

Author:	shirhatti
Assignees:	-
Labels:	area-Extensions-Logging, enhancement
Milestone:	-

[davidfowl]

It might make sense to link the previous issues

[Sundow]

Interested to work on it. Can you assign?

Would it be a reasonable approach if I try to get IExternalScopeProvider from DI and if not successful fall back to instantiating IExternalScopeProvider explicitly as it is now?

[KalleOlaviNiemitalo]

I thought LoggingServiceCollectionExtensions.AddLogging would be able to register LoggerExternalScopeProvider like it registers the other services:

runtime/src/libraries/Microsoft.Extensions.Logging/src/LoggingServiceCollectionExtensions.cs

Lines 41 to 42 in 236cb21

	services.TryAdd(ServiceDescriptor.Singleton<ILoggerFactory, LoggerFactory>());
	services.TryAdd(ServiceDescriptor.Singleton(typeof(ILogger<>), typeof(Logger<>)));

but apparently #37092 changed things so that LoggerFactory instantiates LoggerFactoryScopeProvider instead, and LoggerExternalScopeProvider is no longer used by anything other than tests. I think @Sundow's approach is OK then. And if an app provides its own IExternalScopeProvider, then LoggerFactoryOptions.ActivityTrackingOptions will have no effect by default.

[maryamariyan]

The ask in this issue is to allow replacing the default IExternalScopeProvider.

The default one is set in

runtime/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactory.cs

Line 191 in eeb185f

_scopeProvider = new LoggerFactoryScopeProvider(_factoryOptions.ActivityTrackingOptions);

Do we need new API proposed in order to allow injecting IExternalScopeProvider here? (if yes, an API suggestion available here: #52132)

API Suggestion:

public partial class LoggerFactory : Microsoft.Extensions.Logging.ILoggerFactory
{
    public LoggerFactory(System.Collections.Generic.IEnumerable<Microsoft.Extensions.Logging.ILoggerProvider> providers, Microsoft.Extensions.Options.IOptionsMonitor<Microsoft.Extensions.Logging.LoggerFilterOptions> filterOption, Microsoft.Extensions.Options.IOptions<Microsoft.Extensions.Logging.LoggerFactoryOptions> options = null, Microsoft.Extensions.Logging.IExternalScopeProvider scopeProvider = null) { }
}

cc: @tarekgh

[maryamariyan]

cc: @Sundow (the linked PR author) who prototyped this proposal. Are you interested in preparing a proposal following instructions in api-review-process.md?

Would be good to add usage examples for the proposed API to show how the proposal addresses the issue.

[Sundow]

cc: @maryamariyan Yes, I'll try. Thanks for your comments, it's really helpful.

[bartonjs]

Video

The existing longest constructor needs to have its default removed to follow the guidance for how to add new defaulted parameters safely.

namespace Microsoft.Extensions.Logging
{
    public partial class LoggerFactory : Microsoft.Extensions.Logging.ILoggerFactory
    {
         public LoggerFactory(
             IEnumerable<ILoggerProvider> providers,
             IOptionsMonitor<LoggerFilterOptions> filterOption,
-            IOptions<LoggerFactoryOptions> options = null)
+            IOptions<LoggerFactoryOptions> options) { }

+        public LoggerFactory(
+            IEnumerable<ILoggerProvider> providers,
+            IOptionsMonitor<LoggerFilterOptions> filterOption,
+            LoggerFactoryOptions> options = null,
+            IExternalScopeProvider scopeProvider = null) { }

        }
    }
}

[danmoseley]

@maryamariyan should this be marked up for grabs?

[maryamariyan]

marked up for grabs

[heathbm]

I have some time and would like to contribute to this.

[eerhardt]

Resolved by #67520