Improve logging for TelemetryStorage.loadAbortedSessionPing

RESOLVED FIXED in Firefox 44

Status

()

P1
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: gfritzsche, Assigned: Dexter)

Tracking

Trunk
mozilla44
Points:
1
Dependency tree / graph

Firefox Tracking Flags

(firefox43 affected, firefox44 fixed)

Details

(Whiteboard: [unifiedTelemetry][measurement:client])

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
loadAbortedSessionPing() logs about failing to remove a ping when it actually failed to read or parse a ping:
https://dxr.mozilla.org/mozilla-central/rev/9ed17db42e3e46f1c712e4dffd62d54e915e0fac/toolkit/components/telemetry/TelemetryStorage.jsm#1540

Additionally Vladan requested we make this log the file path too.
(Assignee)

Comment 1

3 years ago
Doesn't the aborted-session ping always have the same path (relative to the profile)?
(Reporter)

Comment 2

3 years ago
It does ($profile/datareporting/aborted-session-ping). Vladan, do you really want that part?
Flags: needinfo?(vladan.bugzilla)

Updated

3 years ago
Points: --- → 1
Priority: -- → P1
Whiteboard: [unifiedTelemetry] → [unifiedTelemetry][measurement:client]
I'd just like to know which ping Telemetry had a problem with. If it's the aborted session ping, then we can just say it's the aborted ping. 
For other pings, the path relative to the profile directory would be enough.
Flags: needinfo?(vladan.bugzilla)
(Assignee)

Comment 4

3 years ago
The path for the other pings is already logged (trace level), see [0]/|TelemetryStorage.loadPingFile). I'll change [1] to a more clear error message ("error loading ping" instead of "error removing ping").

[0] - https://dxr.mozilla.org/mozilla-central/rev/9ed17db42e3e46f1c712e4dffd62d54e915e0fac/toolkit/components/telemetry/TelemetryStorage.jsm?offset=200#1448

[1] - https://dxr.mozilla.org/mozilla-central/rev/9ed17db42e3e46f1c712e4dffd62d54e915e0fac/toolkit/components/telemetry/TelemetryStorage.jsm?offset=200#1540
(Assignee)

Comment 5

3 years ago
Created attachment 8664200 [details] [diff] [review]
bug1205976.patch

This patch changes the log line to a more appropriate one.
Assignee: nobody → alessio.placitelli
Status: NEW → ASSIGNED
Attachment #8664200 - Flags: review?(gfritzsche)
(Reporter)

Updated

3 years ago
Attachment #8664200 - Flags: review?(gfritzsche) → review+
(Assignee)

Comment 7

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/0f1f7daec31f73372eb11b47157d557a553bc8cf
Bug 1205976 - Improve logging for TelemetryStorage.loadAbortedSessionPing. r=gfritzsche
https://hg.mozilla.org/mozilla-central/rev/0f1f7daec31f
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.