serilog-contrib / serilog-sinks-grafana-loki

A Serilog sink sending log events to Grafana Loki
MIT License
201 stars 30 forks source link

System.Text.Json 7.0 really necessary? (Raises Warnings) #160

Closed TsengSR closed 1 year ago

TsengSR commented 1 year ago

Which version of Serilog.Sinks.Grafana.Loki are you using?

v8.1.0

Which version of .NET are you using?

net5.0

Describe the bug

Does the Sink really needs a dependency on System.Text.Json 7.0.0? For libraries it's generally favored to take the lowest versions which support the required features.

Referencing Serilog.Sinks.Grafana.Loki in a net5.0 (and probably also in net6.0 gives some nasty and annoying warnings

Warning     System.Text.Encodings.Web 7.0.0 doesn't support net5.0 and has not been tested with it. Consider upgrading your TargetFramework to net6.0 or later. You may also set <SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings> in the project file to ignore this warning and attempt to run in this unsupported configuration at your own risk.  SomeProject C:\Users\<Username>\.nuget\packages\system.text.encodings.web\7.0.0\buildTransitive\netcoreapp2.0\System.Text.Encodings.Web.targets 4   

even though it also targets netstandard2.0.

To Reproduce

Add Serilog.Sinks.Grafana.Loki to an empty net5.0 project.

Expected behavior

Adding the Sink should not raise warnings.

Log/SelfLog output or exception with stacktrace

No response

Application or code sample, which could be used to reproduce a bug

No response

Additional context

No response

I have read the documentation

mishamyte commented 1 year ago

Hi @TsengSR,

Thanks for your opinion. Sounds reasonable, will think about it.

Also plan to drop "standard" from v9+ and set the explicit targets

mishamyte commented 1 year ago

Extracted to more global #171

github-actions[bot] commented 1 year ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue.