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

CreateBootstrapLogger no longer available in .NET Standard 2.1 since 8.0.0 #82

Open
botder opened this issue Nov 29, 2023 · 8 comments
Open

Comments

@botder
Copy link

botder commented Nov 29, 2023

Hello there,

in the 8.0.0 release, the target framework netstandard2.1 was removed (commit f22e3fd), and in turn, support for a reloadable bootstrap logger was removed for any .NET Standard 2.1 project using this library. See also the code snippet below:

<PropertyGroup Condition=" '$(TargetFramework)' == 'net462' or '$(TargetFramework)' == 'netstandard2.0' ">
<DefineConstants>$(DefineConstants);NO_RELOADABLE_LOGGER</DefineConstants>
</PropertyGroup>

@bartelink
Copy link
Member

Do you have a netstandard2.1 lib that needs to use this functionality?

Can you retarget it to net6.0 (or later) ?

In other words, can you share some more context as to why this is a must-have for you please?

Ultimately if the diff to re-add it is reasonable, and there is a solid case for why a significant cohort of people would not be able to do something desirable, reinstatement is not necessarily off the table - the general principles here are outlined in serilog/serilog#1970 but they are guidelines/principles, not absolutes

@nblumhardt
Copy link
Member

Just to add to @bartelink's reply; this package and the other .Extensions-related ones use a different versioning policy to the rest of the Serilog packages, following their Microsoft.* dependencies exactly (or so the hope goes):

https://github.com/serilog/serilog-extensions-hosting#versioning

This came out of many and varied confusing issues supporting anything different, so while I think serilog/serilog#1970 is the way forward for the rest of serilog/, the Serilog.Extensions.Logging, Serilog.Extensions.Hosting, Serilog.AspNetCore, and Serilog.Settings.Configuration packages will ideally stay as they are (until .NET 9 and whatever TFMs the matching .NET packages target).

Unfortunately this does lead to issues like this one, sorry this was disruptive!

@botder
Copy link
Author

botder commented Nov 30, 2023

Do you have a netstandard2.1 lib that needs to use this functionality?
Can you retarget it to net6.0 (or later) ?
In other words, can you share some more context as to why this is a must-have for you please?

Well yeah, it's in the title and description: I am using CreateBootstrapLogger to set up a basic logger for a generic host in a shared C# library (-> .NET Standard). I am not sure if I actually need this functionality (to automatically reload the logger after runtime/host configuration) at all, but it is the only library in my project, that bumps the target framework from .NET Standard 2.0 to 2.1 (you disable reloading for .NET Standard 2.0 in this project). I mean, if I can avoid it, I try not to target serveral frameworks if I can choose .NET Standard in my project - single compilation & less headaches.

@bartelink
Copy link
Member

Thanks for the update.

I think Nick's policy makes sense so if you can work around it somehow, that's for the best.

The thinking behind my questioning was to explore:

  • it's clear what has changed (and that, all things being equal, you'd like it changed back), but I wanted a why
  • was also wondering why you are targeting NS2.1; is it historic, or do you have a mainline scenario that still exists that makes it an advantage to do so. In general, for libs I maintain, I tend to use either netstandard2.0 or net6.0 as a baseline, and avoid getting into netstandard2.1, netcoreapp3.1, net5.0. I was looking for a "well, I have a lib that people use both in the ASP.NET host but also in and , and there is common log wiring across all three of those scenarios"

And I can definitely appreciate not having to add a TFM to multi-target, serilog/serilog#1970 and this lib are all seeking to limit that, both from a compile time/storage space perspective, but equally from the point of view of limiting how much egregious stuff you need to wade through to troubleshoot scenarios

Assuming you can find a reasonable workaround, can the issue be closed on the basis Nick outlined?

@botder
Copy link
Author

botder commented Nov 30, 2023

Are you reading my message? I stated precisely why I am using .NET Standard 2.1 instead of 2.0 there.

I am using CreateBootstrapLogger, which creates a ReloadableLogger:

public static ReloadableLogger CreateBootstrapLogger(this LoggerConfiguration loggerConfiguration)
{
return new ReloadableLogger(loggerConfiguration.CreateLogger());
}

The ReloadableLogger class is only available, when NO_RELOADABLE_LOGGER is false:

NO_RELOADABLE_LOGGER is non-false for .NET Standard 2.0, but that was not the case for .NET Standard 2.1.

<PropertyGroup Condition=" '$(TargetFramework)' == 'net462' or '$(TargetFramework)' == 'netstandard2.0' ">
<DefineConstants>$(DefineConstants);NO_RELOADABLE_LOGGER</DefineConstants>
</PropertyGroup>


Also, .NET Standard 2.1 (the one you dropped) is supported by .NET (Core) 3.0, 3.1, 5.0, 6.0, 7.0, 8.0 and it worked fine in 7.0.0. Is there any reason to explicitly target any of those frameworks directly in this project? And if that reloadable logger works in .NET Standard 2.0 for you, you could also just target .NET Standard 2.0 alone.

@bartelink
Copy link
Member

bartelink commented Nov 30, 2023

Are you reading my message?

I absolutely understand the mechanics of it. And I appreciate that your code used to work as it was and does not now. And that it will not unless your code changes or the Serilog supported TFMs in V8 change.

There is a primary maintainer of this package.

He's doing a decent job by some standards: https://www.nuget.org/stats/packages/Serilog.Extensions.Hosting?groupby=Version

Nick's response is the overriding concern here. Please consider the reasons he gave for leaving it as it is in formulating a response that's useful for him in deciding whether there is a real unavoidable thing that's been missed which would justify changing the policy here. Again: I am not the maintainer of this package and there's no point beating points into my stupid head or 'winning' a debate with me.

@botder
Copy link
Author

botder commented Nov 30, 2023

Microsoft.Extensions.Hosting 8.0.0 targets netstandard2.1, netstandard2.0 and some others. The Serilog version (3.1.1) this project uses, still supports .NET Standard 2.1. There is no reason to drop .NET Standard 2.1 right now.

https://github.com/dotnet/runtime/blob/ff6b26f0474698ee2fafd3639069c5ef0cc2d9e5/src/libraries/Microsoft.Extensions.Hosting/src/Microsoft.Extensions.Hosting.csproj#L4

If you don't re-add support for .NET Standard 2.1, then please go ahead and remove this comment:

<!-- These must match the Dependencies tab in https://www.nuget.org/packages/microsoft.extensions.hosting at
the target version. -->
<TargetFrameworks>net462;netstandard2.0;net6.0;net7.0;net8.0</TargetFrameworks>

@nblumhardt
Copy link
Member

Ah, looks like my mistake, here! Seems as though netstandard2.1 is targeted directly by the current crop of *.Extensions projects (wonder if that changed, temporarily, during the previews?). We should add this back across affected packages.

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

No branches or pull requests

3 participants