Open Bug 1021545 Opened 10 years ago Updated 2 years ago

Create simple, preference-based logging configuration

Categories

(Toolkit :: General, defect)

defect

Tracking

()

People

(Reporter: markh, Unassigned)

References

Details

There is lots of cargo-culted code in modules that setup logging. In general, they all do the same basic thing: * Create a logger and set its default level, usually to ERROR * Read a pref to override the level. * Create one or both of a ConsoleAppender and DumpAppender. * Read a pref to set that appender's level. Note also that some modules grab and use a logger, but *do not* add appenders (and don't have 'parent' loggers that do either). IIUC, the output from these modules go to a black-hole at runtime (they may be configured by tests, but not in the running code.) Note that some modules are more sophisticated users of logging - eg, Sync and DataReporting/HealthReport - this bug is *not* intended to replace how they configure logging. It is also not intended to prevent future code from doing more sophisticated stuff, like file appenders etc - this bug is to handle the simple case in an easy, consistent way. For reference, modules with cargo-culted code: * http://mxr.mozilla.org/mozilla-central/source/browser/experiments/Experiments.jsm#130 * FxAccountsCommon.jsm * browserid_identity.js * HawkClient.jsm (probably not yet in the tree, but I just reluctantly r+'d a patch that did the same, which prompted me to open this bug) * http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/DeferredSave.jsm#26 And logs that seem to be created without appenders: * http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/Sqlite.jsm#330 (I think I found another but forgot to note it, but you get the idea) I want to propose something like: * A new method ".configureFromPrefs(defaults)" - either on the log itself, the repository (which would mean adding a logName param), or something else or with a different/better name - I don't care. * The basic outline of this code would be: ** Look for a pref named "logging.{log-name}.dumpLevel - if it exists, create a new DumpAppender with the specified level, and add it to the log ** Look for a pref named "logging.{log-name}.consoleLevel - if it exists, same as above but for a ConsoleAppender. ** Look for a pref named "logging.{log-name}.level - if it exists, use it. If not, use the minimal level specified for one of the above appenders (eg, if the the .dump-level is Error bug .console-level is Debug, the log gets Debug.) If all else fails, the log will be set to Error. ** Add observers for these prefs, so they can be tweaked at runtime and will update immediately without a restart. ** Allow the "defaults" param to be an object with, say, dumpLevel, consoleLevel and level keys, which would specify the default values if no prefs exist - eg, defaults={dumpLevel: Log.Level.Error} would create a dumpAppender even though the pref doesn't exist. We don't need to boil the ocean here - the goal is (a) for developers to be able to "casually" and simply use the logging module and (b) for people to have a fairly consistent way to configure the logging for a module without needing to read the same-but-different cargo-culted code modules are now adding. I'm happy to work on this (slowly), but thought I'd run it up the bugzilla flagpole and see who salutes! Thoughts? Bike-sheds?
I've been thinking along similar lines, although what I'd like is the ability to control this from the environment, so something like NSPR_LOG_MODULES. MOZ_LOG_DUMP=experiments:0,asyncshutdown:0,... or even "trace everything" MOZ_LOG_DUMP=*:0 MOZ_LOG_DUMP would go to dump() output or to a file via MOZ_LOG_DUMP_FILE we could also have a version MOZ_LOG_CONSOLE which makes logging go to the browser console by default.
There is an implementation in Thunderbird's fork of log4moz, see bug 956817
This bug is possibly a dupe of bug 956817. But comment 0 is so great that I like this bug better already. See also bug 451283 comment #19 for my take on this over a year ago. tl;dr is comment 0 is very similar to my initial plans for JS logging when I moved log4moz.js into Toolkit to become Log.jsm. This feature would be a very welcome addition to our logging story and I believe it would increase developer and QA productivity in the long run.
(In reply to Benjamin Smedberg [:bsmedberg] from comment #1) > I've been thinking along similar lines, although what I'd like is the > ability to control this from the environment, so something like > NSPR_LOG_MODULES. > > MOZ_LOG_DUMP=experiments:0,asyncshutdown:0,... > or even "trace everything" MOZ_LOG_DUMP=*:0 > > MOZ_LOG_DUMP would go to dump() output or to a file via MOZ_LOG_DUMP_FILE > > we could also have a version MOZ_LOG_CONSOLE which makes logging go to the > browser console by default. I agree that configuring from the environment would be awesome. Assuming we agree on something along the lines of comment 0, another approach to this might be something like: MOZ_LOG_DUMP=/path/to/logging_config.json The json file could have data in the same logical format as the prefs. So, for example, it could contain: { experiments: { consoleLevel: Trace, dumpLevel: Info }, ... } The underlying code could then be very similar. It would even allow us to have a number of different logging config files in the tree used for different purposes - eg, we could allow these files to appear in the various mochitest directories, so tests run from that directory have that logging config applied. Another advantage would be that if this scheme gets improved over time (eg, supporting different preferences and facilities), we'd basically get it for free here - so long as the JSON in this file matches the representation supported for prefs, it should "just work". Benjamin, would that keep you happy, or is that sounding too magic/verbose?
*sigh* - that's what happens when I try and quickly knock work off on a public holiday :) * MOZ_LOG_DUMP would be a bad name - something like MOZ_LOG_CONFIG would be better * My example JSON is invalid and missing some quotes - it should be 'consoleLevel: "Trace",' etc...
Connecting to Log.jsm since it sounds like this would be adding functionality to it.
Blocks: Log.jsm
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.