Closed
Bug 1137139
Opened 10 years ago
Closed 10 years ago
Save main ping with "shutdown" reason when Telemetry shuts down
Categories
(Toolkit :: Telemetry, defect)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla39
Tracking | Status | |
---|---|---|
firefox39 | --- | fixed |
People
(Reporter: gfritzsche, Assigned: Dexter)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Dexter
:
review+
|
Details | Diff | Splinter Review |
We currently only trigger "saved-session", we also need to store a (subsession) main ping with reason "shutdown".
Note: We only want do this on desktop for now.
Assignee | ||
Comment 1•10 years ago
|
||
This patch saves a session ping with the reason "shutdown" when Firefox shuts down, only on Desktop. Adjusts the test_TelemetrySession.js accordingly.
Attachment #8569815 -
Flags: review?(gfritzsche)
Assignee | ||
Updated•10 years ago
|
Attachment #8569815 -
Flags: review?(gfritzsche)
Assignee | ||
Comment 2•10 years ago
|
||
Improved test coverage.
Attachment #8569815 -
Attachment is obsolete: true
Attachment #8569838 -
Flags: review?(gfritzsche)
Reporter | ||
Updated•10 years ago
|
Attachment #8569838 -
Flags: review?(gfritzsche) → review?(vdjeric)
Comment 3•10 years ago
|
||
Comment on attachment 8569838 [details] [diff] [review]
bug1137139.patch - v2
Review of attachment 8569838 [details] [diff] [review]:
-----------------------------------------------------------------
::: toolkit/components/telemetry/TelemetrySession.jsm
@@ +1248,3 @@
> };
> +
> + let savePending = () => TelemetryPing.savePendingPings(getPingType(savedSessionPayload),
make savePending a method in the Impl object and call it saveClassicPing or something similar
@@ +1249,5 @@
> +
> + let savePending = () => TelemetryPing.savePendingPings(getPingType(savedSessionPayload),
> + savedSessionPayload, options);
> +#ifndef MOZ_WIDGET_ANDROID
> + let shutdownPayload = this.getSessionPayload(REASON_SHUTDOWN, false);
is there a reason for clearSubsession = false here?
Attachment #8569838 -
Flags: review?(vdjeric) → review+
Assignee | ||
Comment 4•10 years ago
|
||
I assumed we didn't need a new subsession when shutting down, as we are creating a new one when starting Firefox again (and the "saved-session" ping isn't clearing the subsession either). Should I create a new subsession for saving the "shutdown" ping?
Flags: needinfo?(vdjeric)
Assignee | ||
Comment 5•10 years ago
|
||
This patch adds a |savePendingPingsClassic| function.
As discussed over IRC, subsessions should not be created when saving shutdown pings.
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=58341471c0d9
Attachment #8569838 -
Attachment is obsolete: true
Flags: needinfo?(vdjeric)
Attachment #8572745 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 6•10 years ago
|
||
Comment 7•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla39
You need to log in
before you can comment on or make changes to this bug.
Description
•