Fixed Enabled() for lazy evaluation and supporting >2 V-level

[hn8]

• Support logr.Enabled() usage for lazy argument preparation.
• Add SetMaxV function to change max V-level without knowledge of Zerolog's internal.
• Add support and warning for >2 V-level usage.

Close PR #22, #23, #24. Issue #20

[thockin]

So you don't want to document how to make higher V levels work at all? This puts zerologr into the "not interesting" category, IMO.

[hn8]

@thockin @pohly As I mentioned in the other thread, zapr is definitely much better choice than zerologr if higher than 2 verbosity (TraceLevel) is needed. Here are the reasons:

1. To use an undocumented zerolog levels, user need to figure out which zerolog levels to use by digging out zerolog source code. And zerolog's level value is in opposite direction of logr verbosity. It is easy to use zerolog.DebugLevel or zerolog.TraceLevel. It is confusing to set zerolog level to -5 to allow logr.V(6).
2. In zerolog, there are two level checks: one is global atomic variable, the other is stored in zerolog.Logger passing in zerologr.New(). By default, zerolog.Logger is set to TraceLevel, so most zerolog users only need to adjust global level, rarely need to adjust the level of local logger. To use undocumented zerolog levels, users will need to remember to set level in both zerolog.Logger and globally. And once zerologr.New is called, users have no easy way to adjust level of underlying zerolog.Logger locally. Example code by @tonglil

l := zerolog.Level(-2)
zerolog.SetGlobalLevel(l)
zl := zerolog.New(os.Stdout).Level(l)
log := zerologr.New(&zl)

3. Undocumented level can not support at least two zerolog features: Hooks and Sampling. And probably never since those two are implemented as struct.
4. By default, zerologr outputs both zerolog and logr label { "level": "trace", "v": 2 }, and users can disable either if needed. Otherwises, undocumented zerolog level would have output like { "level": "-2", "v": 3 } which is quite confusing.

logr is greate for Go library users since any logging libraries (often the same one used in application code) can swap in without hardcoded. If users choose to use zerolog for application code, it is likely they know they can only use highest verbosity up to TraceLevel without reading zerolog's source code and can use all features like Sampling. On the other hand, if users prefer to using logr directly in application code, they have more freedom to choose from logr implementation.

zerologr is the minimal glue between logr and zerolog and is limited by both. For example, logr does not support zerolog's WarnLevel/FatalLevel/PanicLevel. And zerolog does not support higher than logr.V(2) by default. Now, with this PR, it is possible for zerologr to support higher than logr.V(2) by setting EnforceLevelCheck=true but it breaks zerolog if zerolog's Hooks/Sampling is used.

IMHO, it is better to keep zerologr less interesting and also less confusing by default. If users want to hack zerolog's undocumented level, there is an option to do so, but it is not offically supported.

[hn8]

@thockin @pohly Also, maybe the root cause of this PR is where Enabled() should be called. I can move the dicussion to logr instead. About this line https://github.com/go-logr/logr/blob/master/logr.go#L274

Among all listed logr implementations, some (funcr, testr, glogr, klogr, ktesting, stdr, genericr, logfmtr) rely on logr.Enabled() check and the rest (zapr, zerologr, logrusr, gokitlogr, bufrlogr) check level again either in LogSink.Info() or underlying logging library calls. Since some established logging library already check level internally, it might be nice to move Enabled() call from Logr.Info() to LogSink.Info() to prevent the redundant Enabled() call in the logging hotpath, but that ship has sailed.

[pohly]

it might be nice to move Enabled() call from Logr.Info() to LogSink.Info() to prevent the redundant Enabled() call in the logging hotpath, but that ship has sailed.

The reason for it being in Logger.Info is that the check is visible to the compiler. The hope was (and still is) that in the common case of not emitting a debug message, the compiler will generate optimized code where the expensive work of preparing parameters for the Info call can be skipped - see golang/go#53465 (comment)

[hn8]

Understood the point of logr.Enabled() for lazy argument preparation. But it means that level checks will run two times for most logr implementation, three times for zerologr, may four times for zapr for single output. Don't have a better solution though.

I will update PR to default to use redundant level check but give a fast path option for users who don't need it.

[hn8]

@thockin @pohly Thanks for the feedback. I added SetMaxV function to reduce level-mapping confusion. Hopefully all related issues for non-conforming Enabled() and high verbosity are resolved.

[tonglil]

Thank you for taking additional verbosity into consideration, this implementation is much more graceful than mine. LGTM!

[thockin]

LGTM