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

Rename LogLevel.Verbose to .Trace and modify its severity to be below .Debug #299

Closed
Eilon opened this issue Nov 20, 2015 · 18 comments
Closed
Assignees
Milestone

Comments

@Eilon
Copy link
Member

Eilon commented Nov 20, 2015

It has two things going against it:

  1. Its relative order within the LogLevel enum is inconsistent with other logging systems. This can easily be confusing to users.
  2. It doesn't exist at all in some logging systems. This means that it would have to flow through as a different level when using one of those sinks.

This is teh codez: https://github.com/aspnet/Logging/blob/dev/src/Microsoft.Extensions.Logging.Abstractions/LogLevel.cs#L11-L15

@lodejard @muratg @DamianEdwards

@lodejard
Copy link
Contributor

Good to make it consistent. How about changing the order to Verbose,Debug,Information,etc. but keeping the Verbose/Debug distinction for the benefit of the loggers which have them?

@muratg
Copy link

muratg commented Nov 20, 2015

👍

@Mertsch
Copy link
Contributor

Mertsch commented Nov 24, 2015

I agree with @lodejard
From my programming history I was absolutely sure that Debug is above Verbose

But you could also argue that the Debug level should be considered a special dev only level and therefore is even more detailed then a verbose setting.
I would be willing to see it as this, but then the documentation needs to be very explicit about this fact, to not confuse devs too much

@Eilon Eilon changed the title Remove LogLevel.Debug Change order of LogLevel.Debug and .Verbose Nov 24, 2015
@Eilon
Copy link
Member Author

Eilon commented Nov 24, 2015

@JunTaoLuo make sure to update the wiki in this repo as well.

@Eilon
Copy link
Member Author

Eilon commented Nov 24, 2015

And post an announcement.

@JunTaoLuo
Copy link
Contributor

@muratg I took a look at a few logging frameworks. Seems like not many loggers have both debug and verbose but SmartInspect does have the same order we do. Loggers that have both verbose and trace seem to put trace as the least severe level though.

Language .NET .NET .NET .NET .NET Java Java Java Java Java Java Javascript
Framework Gibraltar Loupe TheObjectGuy dotnetlog Nlog SmartInspect log4net log4j java.util.logging Apache Commons slf4j tinylog Logback log4javascript
Levels Critical Fatal Fatal Fatal Fatal Fatal Severe Fatal Fatal Error Error Fatal
Error Critical Error Error Error Error Warning Error Error Warning Warn Error
Warning Error Warn Warning Warn Warn Info Warn Warn Info Info Warn
Information Warning Info Message Info Info Config Info Info Debug Debug Info
Verbose Status Debug Verbose Debug Debug Fine Debug Debug Trace Trace Debug
Info Trace Debug All Trace Finer Trace Trace All Trace
Debug All Finest All

@lodejard
Copy link
Contributor

Nice research! There's Serilog also, which is Fatal/Error/Warning/Information/Debug/Verbose that corresponds to the Nlog Fatal/Error/Warn/Info/Debug/Trace. I believe what you would say is that Verbose == Trace when you map the levels to that system, which seems fine.

In that case only SmartInspect puts Debug below Verbose-or-Trace, all others put Verbose-or-Trace as the lowest value. We should do the same and move Verbose as the last entry in the enum below Debug.

@JunTaoLuo
Copy link
Contributor

Agreed. Just curious if we should rename it to Trace since that is uniformly less severe than Debug across the board.

@muratg
Copy link

muratg commented Nov 25, 2015

Thanks @JunTaoLuo! I think renaming it to Trace as well as reordering make sense looking at all the other ones.

@TJSoftware
Copy link

@JunTaoLuo Nice job! NLog, log4net, log4j, and probably the rest also have an OFF state. It may be useful to have an OFF state in the LogLevel enum.

image

@JunTaoLuo
Copy link
Contributor

@TJSoftware Maybe you are looking for None?

@TJSoftware
Copy link

@JunTaoLuo None would certainly work to turn show that I do not want to use logging in a certain scenario. Just some way to turn off (disable) the logging component using the logger factory ...

public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory)
{
  loggerFactory.MinimumLevel = LogLevel.None;
  //or
  loggerFactory.MinimumLevel = LogLevel.Off;

@JunTaoLuo
Copy link
Contributor

@TJSoftware We are removing loggerFactory.MinimumLevel altogether in #298.

@TJSoftware
Copy link

@JunTaoLuo Ahh, I missed that, I primarly was looking for the LogLevel enum. I see now that Louis DeJardin added None on Oct 29th. It must be for RC2, as I do not see it in RC1 yet. Thank you for following up.

Commit

9f38745

LogLevel.cs

https://github.com/aspnet/Logging/blob/dev/src/Microsoft.Extensions.Logging.Abstractions/LogLevel.cs#L11-L15

@JunTaoLuo JunTaoLuo changed the title Change order of LogLevel.Debug and .Verbose Rename LogLevel.Verbose to .Trace and modify its severity to be below .Debug Dec 2, 2015
@PureKrome
Copy link
Contributor

RE: the announcement and fix...

@nblumhardt
Copy link
Contributor

👍 :-)

@MaximRouiller
Copy link

Man.... that handshake to hug transformation. Epic. ;-)

@304NotModified
Copy link
Contributor

Great news! 👍 !!

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

No branches or pull requests

10 participants