latestLogs implementation in Experiments async shutdown instrumentation may be broken

RESOLVED FIXED in Firefox 34

Status

Firefox Health Report
Client: Desktop
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: gfritzsche, Assigned: Benjamin Smedberg)

Tracking

unspecified
Firefox 34
Points:
2
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
Per the data collected in bug 1012924 we have no logs being submitted.
This might point to the latestLogs implementation being broken.
Flags: firefox-backlog+
(Assignee)

Comment 1

3 years ago
It is broken, and I know why... not sure yet what to do about it.
(Assignee)

Comment 2

3 years ago
The code is broken because we're not subclassing a Logger object: we're using getLoggerWithMessagePrefix which uses a proxy object: http://hg.mozilla.org/mozilla-central/annotate/5299864050ee/toolkit/modules/Log.jsm#l508

I think I can fix this in Log.jsm by making the proxy only override the log() method, not all of the others.
(Assignee)

Comment 3

3 years ago
Created attachment 8473231 [details] [diff] [review]
fix-prefixLogger-subclassing
Assignee: nobody → benjamin
Status: NEW → ASSIGNED
Attachment #8473231 - Flags: review?(gps)
(Assignee)

Updated

3 years ago
Points: 3 → 2
Comment on attachment 8473231 [details] [diff] [review]
fix-prefixLogger-subclassing

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

I'm kinda surprised this survived 2 code reviews.
Attachment #8473231 - Flags: review?(gps) → review+

Updated

3 years ago
Iteration: --- → 34.2
QA Whiteboard: [qa?]
(Assignee)

Comment 5

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e75d77ba981
Target Milestone: --- → Firefox 34
Hi Benjamin, can you mark this bug as [qa+] or [qa-] for verificaiton.
Flags: needinfo?(benjamin)
(Assignee)

Comment 7

3 years ago
I don't think external verification is required for this: I checked that it fixed the error locally and that should be sufficient.
QA Whiteboard: [qa?] → [qa-]
Flags: needinfo?(benjamin)
https://hg.mozilla.org/mozilla-central/rev/0e75d77ba981
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.