1923985 - Unify Console and MOZ_LOG levels

Status: NEW

(DevTools :: Console, enhancement)

Description

[Alexandre Poirot [:ochameau]]

Bug 1828100 is wiring the console API to MOZ_LOG.
But this is brittle around log level as MOZ_LOG level is used to control actual logs of console api calls to MOZ_LOG,
but console.shouldLog(...) ignores the MOZ_LOG level.
Instead it relies on ConsoleInstance maxLogLevel optional argument.
This could create some confusion and prevents using console.shouldLog to avoid costly logging operation when you are considering using only the MOZ_LOG output of console API calls.

This bug is a followup based on this review discussion:
https://phabricator.services.mozilla.com/D224104?id=927367#inline-1250562

Comment 1

[Mark Banner (:standard8)]

With the change in bug 1828100, we are better integrating console logging with MOZ_LOG. After the change we'll be in the state where:

1. console.log etc will log to the browser console regardless.
2. Logging via a console.createInstance will log according to the passed maxLogLevel or maxLogLevelPref.
3. Enabling MOZ_LOG=console:5 or one of the prefixes for console.createInstance will log to the defined MOZ_LOG location (i.e. stdout, profiler, or a file). We also have the option to use preferences for enabling these.

In the case of 1 and 2, we also have devtools.console.stdout.chrome and devtools.console.stdout.content that determine if console messages are logged to stdout or not. For developer builds devtools.console.stdout.chrome defaults to true.

Hence for JavaScript, we'll now have two ways for logging to stdout, with different level options. My thoughts were could we drop maxLogLevel etc and only use the MOZ_LOG env/preferences to control the logging levels for console.createInstance?

Looking at it preference wise, it could simplify the random logging preferences that we have e.g. browser.search.log which is a boolean, browser.download.loglevel which is a string and other inconsistencies. The MOZ_LOG preferences are all logging.<prefix> which is much simpler, although you do need to know the prefix (though maybe we can add those as default prefs?). The slightly confusing thing would be that the C++ prefixes wouldn't log to the browser console, whereas the JavaScript ones would.

The potential downside though is that for console.createInstance prefixes we would loose the output of errors/warnings to the browser console, unless we also logged those to stdout by default. This might affect end users slightly. Generally there shouldn't be lots of those occurring, though it could make broken situations worse wrt performance.

We would probably have to do something special for general console.* logging so that it would always log to the browser console, or we would have to output that to stdout all the time.

Overall, there does seem to be some overlap in the situation after bug 1828100 which is why I was thinking about potentially unifying these, but it does appear there are various issues that we'll need to work through to get there.