[API Proposal]: ProviderAliasAttribute: Better move to Microsoft.Extensions.Logging.Abstractions

[SetTrend]

Background and motivation

The ProviderAliasAttribute class should rather be defined in the Microsoft.Extensions.Logging.Abstractions package than in the Microsoft.Extensions.Logging package.

Defining a new logging provider doesn't need anything beyond the Microsoft.Extensions.Logging.Abstractions package — except for the ProviderAliasAttribute.

Being required to include the Microsoft.Extensions.Logging package instead of the Microsoft.Extensions.Logging.Abstractions package adds an unnecessary attack surface and dependencies.

The namespace could remain, so this won't break any existing code. I propose just to move the ProviderAliasAttribute class definition to the Microsoft.Extensions.Logging.Abstractions package.

Risks

None.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

[huoyaoyuan]

The namespace could remain, so this won't break any existing code.

Assembly is also a part of type identity. This is not the full reason for compatibility.

We need TypeForwardedToAttribute to reflect the assembly change. This is achievable so existing code wouldn't be broken.

[KalleOlaviNiemitalo]

If you are implementing a logger provider and don't want a dependency on Microsoft.Extensions.Logging, you can define a type named Microsoft.Extensions.Logging.ProviderAliasAttribute in your own assembly and it will be recognized by this:

runtime/src/libraries/Common/src/Extensions/ProviderAliasUtilities/ProviderAliasUtilities.cs

Lines 21 to 22 in 5535e31

	CustomAttributeData attributeData = attributes[i];
	if (attributeData.AttributeType.FullName == AliasAttributeTypeFullName &&

To prevent name ambiguity in projects that reference both your logger provider package and Microsoft.Extensions.Logging, you should then define the type as internal.

[SetTrend]

@KalleOlaviNiemitalo: Interesting thought!

Now I unterstand why the name is referenced there as a string rather than using nameof() or typeof().

Was the orignal intention for this implementation to be able to define the ProviderAliasAttribute anywhere? What would the design princible behind this be called? Isn't this way of implementing rather sort of a quirk?

[KalleOlaviNiemitalo]

ProviderAliasAttribute was originally added in aspnet/Logging#626. I guess it was placed in Microsoft.Extensions.Logging because it was not needed by Microsoft.Extensions.Logging.Abstractions itself and the logger provider projects (e.g. Microsoft.Extensions.Logging.Debug) already referenced Microsoft.Extensions.Logging. Those references were added in aspnet/Logging#576 in order to call LoggerFactory.AddProvider(string, ILoggerProvider) rather than ILoggerFactory.AddProvider(ILoggerProvider). The LoggerFactory.AddProvider(string, ILoggerProvider) method was removed in the aforementioned aspnet/Logging#626, and that might have been an opportunity to clean up the dependencies, but perhaps it was not noticed then. Later, ProviderAliasUtilities was added in aspnet/Logging#706, where aspnet/Logging#706 (comment) mentions that the lookup by name was for the sake of logger provider packages that could not depend on such a new version of Microsoft.Extensions.Logging.

However, if your logger provider package calls LoggingBuilderConfigurationExtensions.AddConfiguration(this ILoggingBuilder builder), like Microsoft.Extensions.Logging.Console does, then that requires the Microsoft.Extensions.Logging.Configuration package, which depends on Microsoft.Extensions.Logging. So in that case, you cannot avoid the dependency.

[tarekgh]

@SetTrend If you're interested in avoiding the workaround suggested by @KalleOlaviNiemitalo, you’re welcome to submit a PR that moves the type from the logging library to the abstractions, along with adding a type forward in the logging library. We’ll also need to file a breaking change document, since there’s a corner case where it could break if someone uses an older version of the logging library with a newer version of the abstractions.

CC @ericstj

[SetTrend]

Sure, if that's alright with everyone? Then I'd transfer the attribute class.

Where exactly would you want me to add the TypeForwardedToAttribute? I suppose you have some sort of convention for that?

I wouldn't be able to write the Breaking Change document, though. Would you want to deal with that issue?

[KalleOlaviNiemitalo]

I expect the TypeForwardedToAttribute would go in src/libraries/Microsoft.Extensions.Logging/src/Properties/TypeForwards.cs and src/libraries/Microsoft.Extensions.Logging/ref/Microsoft.Extensions.Logging.Forwards.cs.

[tarekgh]

Where exactly would you want me to add the TypeForwardedToAttribute? I suppose you have some sort of convention for that?

@KalleOlaviNiemitalo already pointed out where you can add the type forward. Thanks @KalleOlaviNiemitalo

I wouldn't be able to write the Breaking Change document, though. Would you want to deal with that issue?

Yes, I can help in that.

[SetTrend]

I now created two WIP pull requests:

• One with the ProviderAliasAttribute moved to Microsoft.Extensions.Logging.Abstractions
• and another one with the same changes plus additionally having the reference to the ProviderAliasAttribute changed from an unsafe plain string to a type safe typeof(ProviderAliasAttribute).FullName reference.

Please advise which of the two versions you'd prefer to merge.