burner/logger

Should setting globalLogLevel also change stdlog.logLevel?

Closed this issue · 4 comments

This is not an issue for std.logger's backend classes, however for Log4D it raises an issue: code that sets globalLogLevel can bypass the rootLogger's directives in the Log4D config file. This isn't a problem when you set globalLogLevel once and walk away, but it is a problem if you set it multiple times. For Log4D I would rather let client code manipulate globalLogLevel independently than implicitly set it to match the rootLogger's level on every Log4D.init() call.

I think that other logging backends besides Log4D will want to replace stdlog as I do. Yet client code will still want globalLogLevel to be a hard floor separate from whatever stdlog is pointing to.

My proposal is simply removing the stdlog is null check and set block in the globalLogLevel setter. If clients want to explicitly set stdlog's logLevel to match globalLogLevel they can. Otherwise globalLogLevel behaves as a completely independent threshold.

The problem I see with that idea is:

stdlog.logLevel = LogLevel.info; // done somewhere you don't know about
globalLogLevel = LogLevel.warning; // user knows about this

log("Print me"); // user on irc: WTF is this not printed is set the globalLogLevel

The current impl doesn't have this problem.

In the end this is going to be a judgement call.

p.s. I found a bug looking at the code again stdlog never can be null. I will fix this before this gets merged (sometime soon hopefully).

anyway, thanks for caring

Hmm. So if we "fix" it for Logger objects, we make it more complicated for log() and logf(), am I reading you right?

It feels to me like globalLogLevel is mis-named. It is really being two thing right now: 1) the "defaultLogLevel" still referred to in the docs (but not in the code anywhere), and 2) the "globalLogLevel" that can be used to silence everything. What do you think about: 1) an alias for defaultLogLevel to stdlog.logLevel_, and 2) rename globalLogLevel to something else, maybe globalLogLevelMin?

I'm glad we're finally getting close to something in Phobos, I'm hoping Log4D can provide some further momentum for you.

With the thread changes from PR burner/phobos#2, globalLogLevel no longer sets stdlog's logLevel. This eliminates the ambiguity. Closing.

thanks