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

Use LoggerFactory instead of LoggerProvider #235

Closed
304NotModified opened this issue Dec 6, 2017 · 13 comments
Closed

Use LoggerFactory instead of LoggerProvider #235

304NotModified opened this issue Dec 6, 2017 · 13 comments
Labels
ASP.NET Core ASP.NET Core - all versions
Milestone

Comments

@304NotModified
Copy link
Member

304NotModified commented Dec 6, 2017

current pro and cons:

pro

  • faster (less overhead)
  • MEL-Filtering ignored (less confusion for some)

con:

@304NotModified
Copy link
Member Author

also related: NLog/NLog.Extensions.Logging#202

@alexandrejobin
Copy link

pros

  • you only have one configuration file to configure everything instead of two
  • a lot less confusion

cons

  • you have to choose between Microsoft Logger framework and NLog because you can't use both but since that nlog already have targets for Console et Debug, you don't really lose any feature

My opinion is that if you want to register a new provider in the LoggerProvider, you must fully follow the way it work. It must be driven only by the Logger configuration.

Since that nlog is not a real provider for the Logger but a full logging framework, I think it make sens that nlog should take full control of the logging system. Why choose AddConsole and AddDebug when nlog already have targets for them?

@304NotModified
Copy link
Member Author

Thanks! I think you're right. It would be nice if we could support both (referring to you have to choose between Microsoft Logger framework and NLog because you can't use both ).

This is a breaking change, thus I think it should be V5 if applied

@alexandrejobin
Copy link

When you call UseNLog, maybe we can pass a parameter to tell if you want to use it as a full framework or a provider for Logger?

@304NotModified
Copy link
Member Author

that's an option, but maybe also confusing if we have 2 modes.

@alexandrejobin
Copy link

So what are you suggesting? You've said that you wanted to support both :)

@snakefoot
Copy link
Contributor

snakefoot commented Jul 25, 2018

The NLogLoggerFactory was created with NLog/NLog.Extensions.Logging#135 and works just fine. Though not sure how to ensure NLogLoggerFactory is used at startup :)

See also aspnet/Logging#420

@nblumhardt
Copy link

We've had some experience taking the ILoggerFactory route with Serilog this last year or so. Your pros/cons are pretty close to the mark.

The user experience has worked out well in most respects, there's minimal semi-compatible gunk to work around, and no perf/complexity penalty to suffer choosing Serilog (via the factory) vs. the compromises we'd be stuck with if we'd stayed on the provider track. I'd definitely take this route again, if we were to face the decision a second time.

There are a few places where there are gaps, but hopefully these will close over time if plugging in at the factory level becomes more commonplace. If you need any details of how things have come out for Serilog please drop us a line! :-)

@snakefoot
Copy link
Contributor

Just some random noise. It is only serilog-aspnetcore that performs the LoggerFactory replace.

While serilog-extensions-logging is playing nice with Microsoft Extension Logging-framework.

Looks like serilog-extensions-logging is more widely used than serilog-aspnetcore:

https://www.nuget.org/packages/Serilog.Extensions.Logging/
https://www.nuget.org/packages/serilog.aspnetcore/

@nblumhardt
Copy link

@snakefoot Serilog.AspNetCore has a dependency on Serilog.Extensions.Logging, so the latter will always be ahead in download counts :-) It's also much older, so there's a lot of outdated documentation/blog posts/tutorials out there that still point to the older package. Cheers!

@snakefoot
Copy link
Contributor

snakefoot commented Dec 3, 2018

If wanting Serilog to live along with Azure Application Logging, then one will have to step around Serilog.AspNetCore. Because of the LoggerFactory-swap-trick:

serilog/serilog-aspnetcore#60
serilog/serilog-aspnetcore#52
SteeltoeOSS/Management#21

@snakefoot
Copy link
Contributor

snakefoot commented Feb 25, 2019

Serilog is now doing investigation on implementing their own custom version of LoggerFactory with full support of external LoggerProviders. Special headaches might arrive in supporting the same new features that arrives with the official LoggerFactory for new platform versions, while still trying to support older platform versions (Ex. IExternalScopeProvider)

serilog/serilog-extensions-logging#132

@snakefoot
Copy link
Contributor

Resolved by #657 that adds the option ReplaceLoggerFactory (And RemoveLoggerFactoryFilter that will become enabled by default with ver. 5)

@snakefoot snakefoot modified the milestones: 5.0, 4.12 Mar 30, 2021
@snakefoot snakefoot added ASP.NET Core ASP.NET Core - all versions and removed ASP.NET Core 2 labels Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASP.NET Core ASP.NET Core - all versions
Projects
None yet
Development

No branches or pull requests

4 participants