711815 - Provide an alternative to console.log which checks a debug flag before logging

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P2)

Description

[Kris Maglione [:kmag]]

One of the most common reasons I'm withholding full review from Jetpack add-ons lately is that they tend to produce large amounts of debugging Error Console output. I think that a prominent API with builtin consideration for debugging flags would go a long way towards making authors conscious of the need to stem their debugging output in release versions.

The flag should probably be stored as a preference which defaults to false. As to how authors should enable it in their own builds and setups, I have no idea.

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

There actually are already a bunch of methods on the console object: https://addons.mozilla.org/en-US/developers/docs/sdk/1.3/dev-guide/addon-development/console.html including a console.debug(). I think that might only do stuff in actual debug builds of Firefox, though.

Comment 2

[Kris Maglione [:kmag]]

If the debug method suits the purposes of this bug then a documentation update might be sufficient. I think, given the complexity of the console API, that a parallel 'debug' object with the same message might be a better route. I.e., debug.log(), debug.trace()

Comment 3

[Myk Melez [:myk] [@mykmelez]]

The conventional solution to this problem is to specify a default log level for production environments that is stricter than that for testing environments.  For example, testing environments might log all messages that are at the "info" level of severity or higher by default, while production environments log all messages that are at the "warn" or "error" level of severity.

The Jetpack console object supports the following four levels of severity and corresponding methods for logging at those levels:

* debug: debug()
* info: info(), log()
* warning: warn()
* error: error()

We should set the default log level to "info" for testing environments, i.e. when running automated tests for an addon via `cfx test` and testing the addon manually via `cfx run`; and "error" for production environments, i.e. when using an addon by installing it from a package created by `cfx xpi`.

We should also allow users to specify a log level, so developers can choose to see "debug" messages while debugging their addons and users can choose to see less severe messages while gathering information to report a bug.  But that can be done in a separate bug.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Attachment: pull request 689 - set a default log level and add prefs for changing it

This adds a default log level to the plain-text-console module along with a mechanism to change the log level for a specific addon or all SDK-based addons.

The log levels, and the console methods they enable, are:

    all: all methods
    debug: debug(), log(), info(), warn(), error()
    info: log(), info(), warn(), error()
    warn: warn(), error()
    error: error()
    off: no methods

The names of these levels are taken from log4javascript, which is based on log4j. The default log level for cfx run is info; the default level for an installed addon is error.

The only (cosmetic) change to existing functionality is that warn() messages now get prefixed with warn instead of warning for consistency with the name of the method and the name of the log level.

I intentionally placed the addon-specific preference inside the extensions.[id] namespace, so addons can create a simple pref for it. I also intentionally placed it inside an sdk subnamespace, i.e. extensions.[id].sdk, to isolate it from an addon's own preferences. This establishes a precedent for other such prefs, should we choose to add some in the future.

Comment 5

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 692503 [details]
pull request 689 - set a default log level and add prefs for changing it

That's really cool Myk, thanks!! Could you please hold on landing it though, tree is orange now, I'll land this as soon as we'll make it green again.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Sure thing, I'll let you land it at your convenience!

Comment 7

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/fb6b4a92f7969090acd766fe24b8237aa7805ec5
fix Bug 711815 - set a default log level and add prefs for changing it r=@gozala

Comment 8

[[github robot]]

Commit pushed to integration at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/fb6b4a92f7969090acd766fe24b8237aa7805ec5
fix Bug 711815 - set a default log level and add prefs for changing it r=@gozala