855495 - Turn widget/winrt logging off by default in nightly release builds

Status: RESOLVED INVALID

(Core Graveyard :: Widget: WinRT, defect)

Description

[Jim Mathies [:jimm]]

Anyone have any issues with this? Seems pointless to have this on in nightly builds. We can turn it on manually when debugging local builds. I noticed this while getting console output hooked up for test runs in release builds.

Comment 1

[Brian R. Bondy [:bbondy]]

sounds good

Comment 2

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

We enabled release logging for widget/winrt in bug 837220.  It's not terribly uncommon for logging to be enabled for release builds [1], especially in widget code.

I'm not sure what the right decision is here, but I'd like to at least weigh the pros and cons of having logging available in release builds before we disable it.

Following are the ones I can think of off the top of my head

Pros to having release debugging:
  Easier to track down issues without reproducing in a local build - Leaving things the way they are, users (or developers) can enable logging by setting the NSPR_LOG_MODULES to "MetroWidget:5" or "all:5" and pointing NSPR_LOG_FILE to a file they want to log to.  If we disable logging in release builds, then logging will only be possible in local builds, after making a code change and then setting the environment variables listed above.

Cons to having release debugging:
  Possible perf gain - Maybe this is something we should try to measure?
  Cluttered logs - We could mitigate this by taking advantage of the fact that PR_LOG can log at different granularities, so that "MetroWidget:1" logs a different amount than "MetroWidget:5".

[1] https://mxr.mozilla.org/mozilla-central/search?string=FORCE_PR_LOG

Comment 3

[Jim Mathies [:jimm]]

I think the perf concern is somewhat valid, although measuring any small changes will probably be hard. My main concern is that with the currently setup / logging level all of our widget output will end up in tbpl logs, and may break test result parsing. We'll find out soon enough.

Comment 4

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Ah, I wasn't aware of the tbpl issue.  If enabling metro/widget logging in release builds hinders tbpl at all, then I'm in favor of disabling it.

Comment 5

[BMO Automation]

This component is no longer valid. Closing bug as INVALID.