Open vanbukin opened 2 weeks ago
@vanbukin Could you, please, try to use the following property?
<PropertyGroup>
<DisableMicrosoftExtensionsLoggingSourceGenerator>false</DisableMicrosoftExtensionsLoggingSourceGenerator>
</PropertyGroup>
Setting it to false
should disable replacing of M.E.Logging
with M.E.Telemetry.Abstractions
.
@iliar-turdushev
I checked and it doesn't work.
I also made a dotnet build -bl
and looked at the MSBuild logs using https://msbuildlog.com and I found the following there:
Property reassignment: $(DisableMicrosoftExtensionsLoggingSourceGenerator)="true" (previous value: "false") at C:\Users\vanbukin\.nuget\packages\microsoft.extensions.telemetry.abstractions\8.9.1\buildTransitive\net6.0\Microsoft.Extensions.Telemetry.Abstractions.props (4,5)
This happens because the NuGet package contains a buildTransitive
with a Microsoft.Extensions.Telemetry.Abstractions.props
file that explicitly overrides the value of DisableMicrosoftExtensionsLoggingSourceGenerator
to true
.
This happens because the NuGet package contains a buildTransitive with a Microsoft.Extensions.Telemetry.Abstractions.props file that explicitly overrides the value of DisableMicrosoftExtensionsLoggingSourceGenerator to true.
That sounds like a bug, unless I'm missing something.
@RussKie @dariusclay How do you think if we should remove the props
file, and users will be able to decide whether to enable/disable M.E.Telemetry.Abstractions
logging?
I'm less familiar with the Resilience package. Does it depend on anything in particular from the Telemetry.Abstractions package? I'll need to confirm.
The thing is, if redaction or enrichment is required anywhere then ExtendedLogger should be used.
The other comment here, is that the dotnet/extensions logging generator SHOULD be compatible with the one provided by dotnet/runtime, and thus should not break users when this one is used. If breaks users then it's a bug and we can work to fix the problem instead.
@iliar-turdushev
I checked and it doesn't work. I also made a
dotnet build -bl
and looked at the MSBuild logs using msbuildlog.com and I found the following there:Property reassignment: $(DisableMicrosoftExtensionsLoggingSourceGenerator)="true" (previous value: "false") at C:\Users\vanbukin\.nuget\packages\microsoft.extensions.telemetry.abstractions\8.9.1\buildTransitive\net6.0\Microsoft.Extensions.Telemetry.Abstractions.props (4,5)
This happens because the NuGet package contains a
buildTransitive
with aMicrosoft.Extensions.Telemetry.Abstractions.props
file that explicitly overrides the value ofDisableMicrosoftExtensionsLoggingSourceGenerator
totrue
.
It means you set it too early. Try setting it in a targets file.
It would be great if you could provide a simple standalone repro so we could investigate it further. Without a repro we'd be speculating a lot.
I made a repository that reproduces the problem.
Initially, the repository is in a state where everything is working.
To reproduce the problem, uncomment the installation of Microsoft.Extensions.Http.Resilience
in ReproExtensions5476.csproj
There are also added Directory.Build.props
and Directory.Build.targets
. You are free to experiment with them.
@RussKie
Try setting it in a targets file.
Uncomment the installation of Microsoft.Extensions.Http.Resilience
:
0>Program.cs(135,91): Error LOGGEN036 : The type "ReproExtensions5476.WeatherForecast" doesn't implement ToString(), IConvertible, or IFormattable (did you forget to apply [LogProperties] or [TagProvider] to "first"?) (https://aka.ms/dotnet-extensions-warnings/LOGGEN036)
Next, I try to fix these errors by setting the DisableMicrosoftExtensionsLoggingSourceGenerator
property to false
.
1) Directory.Build.targets
with the following contents:
<Project>
<PropertyGroup>
<DisableMicrosoftExtensionsLoggingSourceGenerator>false</DisableMicrosoftExtensionsLoggingSourceGenerator>
</PropertyGroup>
</Project>
leads me to the following errors:
0>Program.cs(135,91): Error LOGGEN036 : The type "ReproExtensions5476.WeatherForecast" doesn't implement ToString(), IConvertible, or IFormattable (did you forget to apply [LogProperties] or [TagProvider] to "first"?) (https://aka.ms/dotnet-extensions-warnings/LOGGEN036)
0>LoggerMessage.g.cs(13,36): Error CS0757 : A partial method may not have multiple implementing declarations
0>LoggerMessage.g.cs(25,36): Error CS0757 : A partial method may not have multiple implementing declarations
2) Try with Directory.Build.props
:
<Project>
<PropertyGroup>
...
<DisableMicrosoftExtensionsLoggingSourceGenerator>false</DisableMicrosoftExtensionsLoggingSourceGenerator>
</PropertyGroup>
</Project>
and we get the following error:
0>Program.cs(135,91): Error LOGGEN036 : The type "ReproExtensions5476.WeatherForecast" doesn't implement ToString(), IConvertible, or IFormattable (did you forget to apply [LogProperties] or [TagProvider] to "first"?) (https://aka.ms/dotnet-extensions-warnings/LOGGEN036)
3) ReproExtensions5476.csproj
:
<Project Sdk="Microsoft.NET.Sdk.Web">
<PropertyGroup>
...
<DisableMicrosoftExtensionsLoggingSourceGenerator>false</DisableMicrosoftExtensionsLoggingSourceGenerator>
</PropertyGroup>
</Project>
and we get:
0>Program.cs(135,91): Error LOGGEN036 : The type "ReproExtensions5476.WeatherForecast" doesn't implement ToString(), IConvertible, or IFormattable (did you forget to apply [LogProperties] or [TagProvider] to "first"?) (https://aka.ms/dotnet-extensions-warnings/LOGGEN036)
0>LoggerMessage.g.cs(13,36): Error CS0757 : A partial method may not have multiple implementing declarations
0>LoggerMessage.g.cs(25,36): Error CS0757 : A partial method may not have multiple implementing declarations
4) Now let's try to combine my workaround with the proposed solution and edit the Directory.Build.targets
as follows:
<Project>
<Target Name="_Microsoft_Extensions_Telemetry_AbstractionsRemoveAnalyzers" BeforeTargets="ResolveReferences">
<PropertyGroup>
<DisableMicrosoftExtensionsLoggingSourceGenerator>false</DisableMicrosoftExtensionsLoggingSourceGenerator>
</PropertyGroup>
</Target>
</Project>
no luck:
0>Program.cs(135,91): Error LOGGEN036 : The type "ReproExtensions5476.WeatherForecast" doesn't implement ToString(), IConvertible, or IFormattable (did you forget to apply [LogProperties] or [TagProvider] to "first"?) (https://aka.ms/dotnet-extensions-warnings/LOGGEN036)
0>LoggerMessage.g.cs(13,36): Error CS0757 : A partial method may not have multiple implementing declarations
0>LoggerMessage.g.cs(25,36): Error CS0757 : A partial method may not have multiple implementing declarations
@vanbukin Thank you for sharing the repro steps. I played with the project and wasn't able to fix the issue too. As @dariusclay mentioned, our source generator should be compatible with the one provided by Microsoft.Extensions.Logging.Abstractions and shouldn't break customers, e.g. triggering warnings or errors. What if we update our source generator not to trigger LOGGEN036 : The type "WeatherForecast" doesn't implement ToString(), IConvertible...
warning/error?
@geeknoid any thoughts on this?
@iliar-turdushev If the logging behavior does not change, then this also looks like an acceptable option.
Background and motivation
As a developer, I want to add different policies to my http clients that ensure resilience: retry, bulkhead, timeout, etc. I am connecting the nuget package
Microsoft.Extensions.Http.Resilience
, which contains all the necessary classes for this. My project includestreat warnings as errors
, as well as all analyzers. My logging is breaking down.This happens because the
Microsoft.Extensions.Http.Resilience
package depends on theMicrosoft.Extensions.Resilience
package, which in turn depends onMicrosoft.Extensions.Telemetry.Abstractions
, which contains custom code generation for logging and its own set of analyzers, which also override code generation for logs fromMicrosoft.Extensions.Logging
. As a developer, I want to retries for HTTP clients, please do not touch my logs.I have the following code in my project that works with the
Microsoft.Extensions.Logging
code generator, which understands the MessageTemplates formatthen my logs are transferred to
Serilog
, which also understands the MessageTemplates format and when writing to sink via the formatter, it decomposes the transferred object into separate properties.And I want it to stay as it is, but instead I get a warning
Due to
treat warnings as errors
, this becomes a compilation error. I could suppress it or implement what is required from me, but the problem is not in the warning as such, but in the fact that instead of the code generator fromMicrosoft.Extensions.Logging
, I now use the code generator fromMicrosoft.Extensions.Telemetry.Abstractions
.Therefore, I suggest adding an MSBuild property that allows to disable this logic globally (via Directory.Build.props at the level .sln) or locally (at the level of a single project, in its .csproj file).
API Proposal
MSBuild property that disables the replacement of the log generator from
Microsoft.Extensions.Logging
with the log generator fromMicrosoft.Extensions.Telemetry.Abstractions
API Usage
Set the required value to
true
in the csproj file or in theDirectory.Build.props
Workaround
Directory.Build.targets
Risks
When referencing any NuGet package that has a dependency on
Microsoft.Extensions.Telemetry.Abstractions
, another code generator for logs begins to be used.