serilog-aspnetcore icon indicating copy to clipboard operation
serilog-aspnetcore copied to clipboard

Convert this package into a pure metapackage, so that `UseSerilogRequestLogging()` can be made available without any other dependencies

Open mike-schenk opened this issue 2 years ago • 7 comments

https://github.com/serilog/serilog-aspnetcore/blob/76f4a513e0538418b829ff4ef266444351b726fa/src/Serilog.AspNetCore/Serilog.AspNetCore.csproj#L31

There is no code in the project that depends on the Serilog.Sinks.File package.

This causes the resulting Serilog.AspNetCore NuGet package to carry along an unnecessary dependency.

mike-schenk avatar Sep 24 '21 22:09 mike-schenk

Thanks for the note, Mike.

This package was originally a metapackage with no substantial code of its own; we might consider a refactor now that there's more implementation here, but there's a usability trade-off: even the primary overloads of UseSerilog() and ReadFrom.Configuration() come from other packages and aren't directly relied upon by the code in this package.

I'd be inclined to leave this for now, but extracting RequestLoggingMiddleware into a package of its own (e.g. Serilog.AspNetCore.Http) would also seem to be a reasonable path to consuming the functionality pulled in here in a more selective manner 🤔

nblumhardt avatar Sep 30 '21 04:09 nblumhardt

Relates to #127

sungam3r avatar Oct 04 '21 21:10 sungam3r

This is starting to cause problems - the unnecessary dependencies are becoming outdated which can lead to version downgrade errors when building (especially when targeting linux, it seems), and the workaround is to add direct dependencies to the later versions of these implicitly-included packages from our projects. Feels very wrong, as our projects don't use these packages at all! 😄

GraemeF avatar Dec 14 '21 08:12 GraemeF

https://github.com/serilog/serilog-aspnetcore/pull/127#issuecomment-527261578

sungam3r avatar Dec 14 '21 13:12 sungam3r

@GraemeF thanks for dropping by!

Still open to extracting the middleware/other implementation code from this package, to allow consumption of the bundled parts. Is anyone keen to analyze what would actually get moved/propose some package names for it to move into?

nblumhardt avatar Dec 16 '21 23:12 nblumhardt

In #127 I proposed to remove 5 packages. Just remove, without making any new meta-packages. Any who needs specific sinks may just drop in sink package into project, that's all.

sungam3r avatar Dec 17 '21 20:12 sungam3r

I'm not keen on dropping the dependencies here, not least because of the downstream breakage, but also because this creates one more barrier to adopting Serilog: the default .NET web SDK includes the console logger provider for MEL, there's no extra step to install it; the default .NET web SDK enables JSON config support ... etc. etc.

Apart from the logging middleware in this package, everything else is already in fine-grained packages. If we extract the logging middleware to its own package, referenced from this one, then it's easy to just install Hosting, Configuration, the middleware package, and whatever other sinks you require. Let's push this one forward :-)

nblumhardt avatar Jan 19 '22 04:01 nblumhardt