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

Revisit user logging configuration patterns with EF Core #14136

Closed
divega opened this issue Dec 10, 2018 · 2 comments
Closed

Revisit user logging configuration patterns with EF Core #14136

divega opened this issue Dec 10, 2018 · 2 comments
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed.

Comments

@divega
Copy link
Contributor

divega commented Dec 10, 2018

(Creating a new issue after reading after realizing that #8350 isn't very relevant):

The patterns EF Core customers would normally use to configure logging were made obsolete (again) in 2.2.

From @pakrym and @ajcvickers comments at dotnet/EntityFramework.Docs#1164 (comment):

... we moved logging to use DI in 2.2 and added a centralized way to do filtering and configuration. So obsolete members mostly were:

  1. Extension methods for ILoggerFactory (they moved to DI aware ILoggingBuilder)
  2. Logger specific configuration and filtering constructors - console logger had the ability to pass both IConfiguration and filter Func (all these features moved to ILoggingBuilder too)
  3. Some old LoggerSettings classes that were converted into Options objects.

@divega I think we'll want to add some sugar here. It's super sucky that the code requires using D.I. even when the application is not using D.I. at all and the developer may not have a good understanding of how D.I. works. Makes me wonder if we should get away from exposing this at all in EF and instead have our own logging configuration API that is more aligned to how logging is used in EF.

@mqudsi
Copy link

mqudsi commented Feb 18, 2019

I'm aware of the new methods in v3 for creating a new LoggingFactory without DI (cf @pakrym's comment at dotnet/extensions#615 (comment)), but I think there's still a part of this story that is missing, which is logging in libraries without an explicit ILogger instance.

Most 3rd party logging libraries that are being phased out for ILogger and co allowed for a model where a library could include the core logging class and issue log calls as they wish, without being aware of any details (or even the mere existence) of a consumer for those logging events. If an application consuming such a library installed the frontend to the logging library, it can turn these calls from writing to the ether (or /dev/null, if you prefer) to actual log calls that are filtered at the level the calling application specifies, streamed to the providers the calling application chooses. But they can also be consumed by an application that does not use logging at all, without needing to add a dependency on the logging library and then go out of its way to explicitly disable it.

You've now made it possible to create a new log terminator without DI for use in applications which is very useful for application, but libraries should not be creating their own logger factor - they just need to be able to create their own Logger<T> that will write to a preconfigured LoggerFactory, if one exists - whereas the constructor for Microsoft.Extensions.Logging.Logger<T> still requires a ILoggerFactory, which a library should not be creating but also does not have unless it's passed in from upstream.

IMHO there needs to be a "default" logger factory that is used when no explicit LoggerFactory<T> is provided when creating a new Logger<T>, any logs generated by that should be made available to all LoggerFactory instances running in the same memory space. A Logger<T> created with an explicit LoggerFactory instance would continue to operate as it currently does. This makes it a no-brainer for libraries to include logging in their development lifecycle, without worrying about introducing friction or hindering adoption for downstream users (that may be from a different organization altogether).

This would allow libraries that want to optionally expose logging to be used easily by clients that may not want logging at all (or really, may not realize that they need logging). The current ILogger model requires that as soon as one dependency tries to optionally log something, all downstream clients will need to add logging in order to use it.

@ajcvickers
Copy link
Member

@mqudsi I would suggest https://github.com/aspnet/Extensions as a better place for these kind of comments, since this is where the logging code is maintained.

@ajcvickers ajcvickers added the closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. label Mar 10, 2022
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed.
Projects
None yet
Development

No branches or pull requests

3 participants