Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Don't use the root logger

VERIFIED FIXED in mozilla7

Status

Cloud Services
Firefox Sync: Backend
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: philikon, Assigned: philikon)

Tracking

unspecified
mozilla7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [verified in services])

Attachments

(2 attachments)

We'd like to uplift log4moz to toolkit at some point (bug 451283). One prerequisite is to get Sync to stop taking control of the root logger.
Created attachment 538295 [details] [diff] [review]
v1
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #538295 - Flags: review?(rnewman)
Comment on attachment 538295 [details] [diff] [review]
v1

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

What's this, 23 files? :D

Pleasantly formulaic!
Attachment #538295 - Flags: review?(rnewman) → review+
Actually, the more I think about it, the more I wonder whether we should perhaps just get rid of the default LoggerRepository (Log4Moz.repository) and make every component that wants to use log4moz create its own logger hierarchy in its own repository. Then we don't need the redundant "Sync." prefix everywhere.
(In reply to comment #3)
> Actually, the more I think about it, the more I wonder whether we should
> perhaps just get rid of the default LoggerRepository (Log4Moz.repository)
> and make every component that wants to use log4moz create its own logger
> hierarchy in its own repository. Then we don't need the redundant "Sync."
> prefix everywhere.

Would this make it impossible to simply turn on all logging? If so I would be against that.
(In reply to comment #4)
> Would this make it impossible to simply turn on all logging? If so I would
> be against that.

What does "all logging" mean? What does "turn on" mean?

Having each component use its own repository would mean there's no single root logger which means you can't just attach a, say, DumpAppender to the one and only root logger to get all logging output from all components. Is that what you want? It doesn't sound very useful to me...
Being able to turn on logging for everything is often pretty useful if you have no idea what is causing some disk thrashing at a particular time etc. As more things start to use log4moz I'd expect to see something like the error console start to just log things and having custom repositories for everything seems more complex than just having everything use a sensible namespace
Created attachment 538532 [details] [diff] [review]
alternate patch: use a distinct repository
(In reply to comment #6)
> Being able to turn on logging for everything is often pretty useful if you
> have no idea what is causing some disk thrashing at a particular time etc.
> As more things start to use log4moz I'd expect to see something like the
> error console start to just log things and having custom repositories for
> everything seems more complex than just having everything use a sensible
> namespace

Fair enough. I wonder if we should disallow separate repositories then.
(In reply to comment #8)
> (In reply to comment #6)
> > Being able to turn on logging for everything is often pretty useful if you
> > have no idea what is causing some disk thrashing at a particular time etc.
> > As more things start to use log4moz I'd expect to see something like the
> > error console start to just log things and having custom repositories for
> > everything seems more complex than just having everything use a sensible
> > namespace
> 
> Fair enough. I wonder if we should disallow separate repositories then.

The repository never seemed like a worthwhile abstraction to me, I commented as such in bug 451283 comment 16
(In reply to comment #9)
> The repository never seemed like a worthwhile abstraction to me, I commented
> as such in bug 451283 comment 16

Yup. I was just curious since you were critiquing dissimilarities with Log4J similarities (Formatter v. Layout), and Log4J *does* have repositories.
(In reply to comment #10)
> (In reply to comment #9)
> > The repository never seemed like a worthwhile abstraction to me, I commented
> > as such in bug 451283 comment 16
> 
> Yup. I was just curious since you were critiquing dissimilarities with Log4J
> similarities (Formatter v. Layout), and Log4J *does* have repositories.

I think that must have been a newer addition since I last used log4J and seems to be an optional feature (in that you can use log4J quite happily by ignoring it).
http://hg.mozilla.org/services/services-central/rev/0122402eba05
Whiteboard: [fixed in services]
STRs for QA:

The Sync log (or logs, since bug 610832) should all look like this:

1307991868256	Sync.Service	INFO	Loading Weave 1.10.0
1307991868309	Sync.Engine.Bookmarks	DEBUG	Engine initialized
1307991868313	Sync.Engine.Forms	DEBUG	Engine initialized
1307991868316	Sync.Engine.History	DEBUG	Engine initialized
1307991868320	Sync.Engine.Passwords	DEBUG	Engine initialized
1307991868323	Sync.Engine.Prefs	DEBUG	Engine initialized
1307991868326	Sync.Engine.Tabs	DEBUG	Engine initialized
...

Notice the "Sync." prefix before each component.
verified with latest s-c builds
Whiteboard: [fixed in services] → [fixed in services][verified in services]
http://hg.mozilla.org/mozilla-central/rev/0122402eba05
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in services][verified in services] → [verified in services]
Target Milestone: --- → mozilla7

Updated

6 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.