787280 - log4moz is not available on Fennec from its central location

Status: RESOLVED WONTFIX

(Remote Protocol :: Marionette, defect)

Description

[David Burns :automatedtester]

We are either going to have to remove log4moz or have a build step to copy it in to the components and then load it that way.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

that would require getting http://mxr.mozilla.org/mozilla-central/source/services/common/log4moz.js installed as part of services-common.  I am not sure if this is intended to be working in Fennec or not.  

I would assume asking the mobile or services teams would get the answer fastest.  If all else fails it wouldn't be too hard to write a mock version of this and put some exception handling around where we are importing it.

Comment 2

[David Burns :automatedtester]

@wesj in #mobile suggested that I just put log4moz in the directory and just use that. I am happy to do either really.

If we put log4moz in the directory I would prefer to have a build step that puts it there to prevent bit rot.

What is the norm for Mobile in situations like this?

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

I am concerned because this needs to be in the correct application space.  We might need additional manifest or similar files to make it accessible.

Comment 4

[David Burns :automatedtester]

Attachment: updating build.mk to let us use log4moz

Comment 5

[David Burns :automatedtester]

This is one approach that we can use or as :jmaher suggests we can build our own stub. The reason why it is not available is because it is part of the services code which now has a full java implementation and doesnt require this code so its not packaged.

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 657296 [details] [diff] [review]
updating build.mk to let us use log4moz

Review of attachment 657296 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/build.mk
@@ +22,5 @@
>  ifdef MOZ_EXTENSIONS
>  tier_app_dirs += extensions
>  endif
>  
> +ifdef ENABLE_MARIONETTE

this should still allow the original ifdef, I am sure there is some crazy form of makefile syntax to make that work.

Comment 7

[David Burns :automatedtester]

Attachment: updated since my repo was out of date and to handle comments

Comment 8

[David Burns :automatedtester]

Comment on attachment 657314 [details] [diff] [review]
updated since my repo was out of date and to handle comments

This allows use to use log4moz.js with Marionette. Since this is only with Marionette it shouldnt impact the end build

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 657314 [details] [diff] [review]
updated since my repo was out of date and to handle comments

Seems like a sane way to make this work

Comment 10

[Mike Hommey [:glandium]]

(In reply to David Burns :automatedtester from comment #8)
> Comment on attachment 657314 [details] [diff] [review]
> updated since my repo was out of date and to handle comments
> 
> This allows use to use log4moz.js with Marionette. Since this is only with
> Marionette it shouldnt impact the end build

... for now. How sure are you that marionette is not going to be enabled in end builds like it is on desktop nightlies and b2g?

Comment 11

[Mike Hommey [:glandium]]

That being said, is there any reason not to make log4moz.js part of the platform, under a resource://gre/ url?

Comment 12

[Mike Hommey [:glandium]]

Comment on attachment 657314 [details] [diff] [review]
updated since my repo was out of date and to handle comments

Resetting flag until this is figured out.

Comment 13

[Gregory Szorc [:gps]]

(In reply to Mike Hommey [:glandium] from comment #11)
> That being said, is there any reason not to make log4moz.js part of the
> platform, under a resource://gre/ url?

I think that discussion should be had in bug 451283.

But, I was told to weigh in here, so I will.

log4moz by itself is nice. But, it's far from perfect. If you are using log4moz, you are ultimately going to need to hook up plumbing. You'll want a integration with the Error Console, preferences to control logging levels, saving log files to disk, pruning old logs from disk, rotation of log files, forwarding logs to remote debuggers, etc. This is all missing from log4moz today. This means people like Sync and AITC come around and re-implement this functionality as one-offs.

As part of moving log4moz to the core platform, I would like to see a core logging service be introduced with it. We would have a unified logging bus under the hood that different features/subsystems could feed into. That bus/service would handle preferences to control settings, saving logs to disk, forwarding events, etc. As a consumer, you would just need to obtain a named logger and write data. If you are introducing a new subsystem, maybe you need to perform one-time registration (via prefs?) to let the bus know how it should handle events.

The central logging service also has a nice advantage: unified I/O handling. This is very difficult to get right. You can't have synchronous writes to disk for every log message because it's inefficient for the I/O subsystem and is blocking I/O. You don't want to have a synchronous API for dropping a log message from the application because that would result in horrendous JS with excessive closures and callbacks. A central logging service could allow messages to be passed to it with a synchronous API call which then forwarded messages to appropriate async APIs to do aggregation, I/O, etc. Complexity abstracted.

Anyway, log4moz currently exceeds at being an API for applications to send individual log messages somewhere. There are backend APIs in log4moz for doing something with these messages (appenders), but they are fragile and easy to use incorrectly. For that reason, I'm advocating for a central logging service as part of the platform. Subsystems inevitably reinvent this wheel, so we might as well provide it to everyone. And, who knows, maybe it leads to better introspection of Firefox since a unified messaging bus with interleaved events from all subsystems is pretty damn handy.

Comment 14

[Mike Hommey [:glandium]]

Note that this crosses the path of bug 602467. Although the intent of that bug was for C++ code, a single facility for all logging would surely be helpful.

Comment 15

[David Burns :automatedtester]

Could we spin off a separate bug for how this should be dealt with or fix this in bug 451283 or bug 602467. Once things have been moved there will probably be code updates anyway and I can update my code reliant on this there.

Comment 16

[David Burns :automatedtester]

Comment on attachment 657314 [details] [diff] [review]
updated since my repo was out of date and to handle comments

Since there hasnt been any visible discussion of this I am resetting the review flag. We can always spin off a separate bug or fix the log4moz to toolkit bug.

Comment 17

[Mike Hommey [:glandium]]

Comment on attachment 657314 [details] [diff] [review]
updated since my repo was out of date and to handle comments

Review of attachment 657314 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/build.mk
@@ +22,5 @@
>  ifdef MOZ_EXTENSIONS
>  tier_app_dirs += extensions
>  endif
>  
> +ifdef ENABLE_MARIONETTE

Please add a condition on the build being for fennec.

Comment 18

[David Burns :automatedtester]

closing this in favour of work being done in bug 451283