Skip to content
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.

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

Replace platform's Logging source generator #4238

Merged
merged 1 commit into from
Aug 4, 2023
Merged

Conversation

joperezr
Copy link
Member

@joperezr joperezr commented Aug 2, 2023

cc: @geeknoid @RussKie @ViktorHofer @tarekgh

This PR will make sure that when Microsoft.Extensions.Telemetry.Abstractions package gets referenced, the import of the Microsoft.Extensions.Logging.Abstractions package is disabled as the new referenced package's source generator should take precedence.

For additional context, the package Microsoft.Extensions.Logging.Abstractions has a built-in targets file which will check if the property DisableMicrosoftExtensionsLoggingSourceGenerator is set to true, and if so, it will disable the automatic reference of the logging generator included in that package. The changes in this PR are just setting that value when the extensions Telemetry package gets referenced, in order to replace the logging generator with the extensions version.

Microsoft Reviewers: Open in CodeFlow

@ghost ghost assigned joperezr Aug 2, 2023
@joperezr
Copy link
Member Author

joperezr commented Aug 2, 2023

@ViktorHofer this currently doesn't work when targeting netcoreapp3.1, because currently the logging.abstractions package has a different buildTransitive .targets file for that TFM which just shows the not supported warning instead of having the mechanism that the rest of the TFMs have for disabling the source generator:

image

You would need a workaround similar than the one I'm introducing here which allows for extra content on that generated file in order for this to work.

Copy link
Member

@geeknoid geeknoid left a comment

Choose a reason for hiding this comment

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

Terrific!

@ViktorHofer
Copy link
Member

@ViktorHofer this currently doesn't work when targeting netcoreapp3.1, because currently the logging.abstractions package has a different buildTransitive .targets file for that TFM which just shows the not supported warning instead of having the mechanism that the rest of the TFMs have for disabling the source generator

This is by design as packages produced by dotnet/runtime don't support netcoreapp3.1. The runtime has been out-of-support since December 2022. We communicate this via the Warning task that we emit. cc @ericstj

@joperezr joperezr merged commit 60800f8 into main Aug 4, 2023
6 checks passed
@joperezr joperezr deleted the ReplaceLoggingGenerator branch August 4, 2023 16:13
@ghost ghost added this to the 8.0 RC1 milestone Aug 4, 2023
@xakep139
Copy link
Contributor

xakep139 commented Aug 8, 2023

@joperezr does this PR resolves #4207?
If so, could you please link these two and close the issue? Thanks!

@joperezr
Copy link
Member Author

joperezr commented Aug 8, 2023

Yes, it does. 🙂

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide ability to replace the default logging code-gen
4 participants