Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Unintuitive: DebugLoggerFactoryExtensions.AddDebug defaults to LogLevel.Information #275

Closed
ashimoon opened this issue Oct 25, 2015 · 11 comments

Comments

@ashimoon
Copy link

This was a bit of a head scratcher, I added a Debug logger to my beta8 project using the AddDebug extension method here:

https://github.com/aspnet/Logging/blob/dev/src/Microsoft.Extensions.Logging.Debug/DebugLoggerFactoryExtensions.cs#L18

However, my log statements weren't being shown in the Debug output window. When I dug into the source I saw that the default log level for AddDebug is... LogLevel.Information.

Is this intentional? Shouldn't the default be LogLevel.Debug?

@Eilon
Copy link
Member

Eilon commented Oct 28, 2015

The logger providers are named based on where they write information to (e.g. console, Windows Event Log, etc.). The Debug logger provider writes information to the Debug output window. It is unrelated to the level of detail that is flowed to it, which is separately configurable.

@ashimoon
Copy link
Author

@Eilon Thanks for the response, I think I get it - providers have configurable logging levels independent of logger logging levels, right?

However in the case of the Debug output window, what are your thoughts on the default logging level for that provider? I'm trying to approach this from the viewpoint of a newcomer to ASP.NET 5 - they want logging, they add a Debug logger, and then add a bunch of LogDebug statements and see nothing.

This is what happened to me anyways, and luckily I'm the type of guy that will dig into the source code to see what's happening and I figured it out pretty quickly, but I'm not sure it's going to be the most friendly experience for most users. Maybe I'm overthinking it, what do you think?

Can you give me a bit of context on how LogLevel.Information was chosen as the default log level for the Debug provider maybe? I bet there's a piece I'm missing here?

@Eilon
Copy link
Member

Eilon commented Oct 28, 2015

Yeah I do agree it can be a bit confusing - and you're not the first one to feel some confusion around it 😄

The reason for Information being the default is that as a whole we decided that the frameworks (ASP.NET + EF) should emit about ~10 log messages at the Information level or higher on a "typical" request (started, some DB queries, routing, MVC, view rendering, etc.).

If the default was Debug or Verbose then a typical request would have hundreds of log messages flowing through, and that would probably make the log output unusable in most cases.

@ashimoon
Copy link
Author

@Eilon Okay that makes sense. I would agree with that. Also once a new user encounters this issue one time they will probably understand it pretty well going forward.

I'm still wondering if you have any thoughts on how it can be easier for newcomers? I'm thinking specifically about this piece of code that gets scaffolded automatically in a new ASP.NET 5 project:

loggerFactory.MinimumLevel = LogLevel.Information;
loggerFactory.AddConsole();
loggerFactory.AddDebug();

I ended up changing it to:

loggerFactory.MinimumLevel = LogLevel.Debug;
loggerFactory.AddConsole(LogLevel.Debug);
loggerFactory.AddDebug(LogLevel.Debug);

When I look at the updated block of code, I see immediately that there are provider vs. logger log levels. Before, it wasn't entirely clear what loggerFactory.MinimumLevel was for.

I'd be really curious to hear your thoughts on that since I would bet a ton of people will be adopting ASP.NET 5 for the first time since it is awesome sauce, and they will use the default template as a starting point for learning probably!

Is there a good way you can think of to make it obvious to new users that they can do something like this?

@Eilon
Copy link
Member

Eilon commented Oct 28, 2015

Well, for one, we're considering removing the MinimumLevel option to begin with - that should help reduce some confusion.

Beyond that, perhaps by changing the templates to say something like:

loggerFactory.AddConsole(minLevel: LogLevel.Information);
loggerFactory.AddDebug(minLevel: LogLevel.Information);

Would make it a bit clearer what that option is saying.

@ashimoon
Copy link
Author

ashimoon commented Nov 4, 2015

Interesting, looks like you guys are adding some Configuration integration for Logging. Looks pretty cool. I see that the default logging level is now Verbose for non System/Microsoft loggers right? That might help a lot.

I also read in another issue that you guys are considering merging Debug and Verbose? Is that why you chose to use Verbose as the new default log level?

@Eilon
Copy link
Member

Eilon commented Nov 4, 2015

BTW the defaults themselves haven't changed per se, but the project templates will set some particular log levels.

@ashimoon
Copy link
Author

ashimoon commented Nov 4, 2015

Right, yes that is more accurate.

notice that the ILoggerFactory minimum level was removed from the template (web app) as well but it seems that the property still exists. Is that to discourage people to use that property?

@muratg
Copy link

muratg commented Dec 18, 2015

MinimumLevel was removed as part of this: #298

Closing this per the discussion above.

@muratg muratg closed this as completed Dec 18, 2015
@ashimoon
Copy link
Author

I noticed that, Latest version of logging looks really nice and intuitive. Good stuff guys, thanks for following up.

@MNF
Copy link

MNF commented May 7, 2017

@muratg, I do not understand why removing MinimumLevel property made AddDebug default less confusing.
I suggest

  1. to mark loggerFactory.AddDebug(); without parameters as obsolete, explaining that recommended use is
    loggerFactory.AddDebug(minLevel: LogLevel.Information);
  2. In AddDebug(this ILoggerFactory factory, LogLevel minLevel) Summary explain that LogLevel.Debug can be too verbose and LogLevel.Information is recommended (as Eilon commented on Oct 29 2015)

Can you re-open the issue or prefer to raise a new one?

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

No branches or pull requests

4 participants