Remove code for expired telemetry histograms

RESOLVED FIXED in Firefox 40

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: ttaubert, Assigned: ttaubert)

Tracking

Trunk
Firefox 40
Points:
3
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify -

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment)

These telemetry histograms are going away:

FX_SESSION_RESTORE_COLLECT_SINGLE_WINDOW_DATA_MS
FX_SESSION_RESTORE_EXTRACTING_STATISTICS_DURATION_MS
FX_SESSION_RESTORE_INDIVIDUAL_CLOSED_TABS_IN_OPEN_WINDOWS_SIZE_BYTES
FX_SESSION_RESTORE_INDIVIDUAL_CLOSED_WINDOWS_SIZE_BYTES
FX_SESSION_RESTORE_INDIVIDUAL_COOKIES_SIZE_BYTES
FX_SESSION_RESTORE_INDIVIDUAL_FORMDATA_SIZE_BYTES
FX_SESSION_RESTORE_INDIVIDUAL_HISTORY_SIZE_BYTES
FX_SESSION_RESTORE_INDIVIDUAL_OPEN_WINDOWS_SIZE_BYTES
FX_SESSION_RESTORE_SEND_SERIALIZED_STATE_LONGEST_OP_MS
FX_SESSION_RESTORE_SESSION_LENGTH
FX_SESSION_RESTORE_TOTAL_CLOSED_TABS_IN_OPEN_WINDOWS_SIZE_BYTES
FX_SESSION_RESTORE_TOTAL_CLOSED_WINDOWS_SIZE_BYTES
FX_SESSION_RESTORE_TOTAL_COOKIES_SIZE_BYTES
FX_SESSION_RESTORE_TOTAL_DOM_STORAGE_SIZE_BYTES
FX_SESSION_RESTORE_TOTAL_FORMDATA_SIZE_BYTES
FX_SESSION_RESTORE_WRITE_FILE_LONGEST_OP_MS
FX_SESSION_RESTORE_WRITE_STATE_LONGEST_OP_MS

The code that submits values to those should be removed.
Depends on: 1156565
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Attachment #8600562 - Flags: review?(dteller)
Iteration: --- → 40.3 - 11 May
Points: --- → 3
Flags: qe-verify-
Flags: firefox-backlog+
Comment on attachment 8600562 [details] [diff] [review]
0001-Bug-1150529-Remove-code-for-expired-telemetry-histog.patch

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

::: browser/components/sessionstore/SessionStore.jsm
@@ -2928,5 @@
>      if (state.session && state.session.startTime) {
>        this._sessionStartTime = state.session.startTime;
> -
> -      // ms to days
> -      let sessionLength = (Date.now() - this._sessionStartTime) / MS_PER_DAY;

Is MS_PER_DAY still used?

@@ +2911,5 @@
>     */
>    _updateSessionStartTime: function ssi_updateSessionStartTime(state) {
>      // Attempt to load the session start time from the session state
>      if (state.session && state.session.startTime) {
>        this._sessionStartTime = state.session.startTime;

This histogram strikes me as useful (although I'm not 100% sure about the implementation). Do we have any other source of data that can replace it?
Attachment #8600562 - Flags: review?(dteller) → review+
(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo" - away until May 6th) from comment #2)
> > -      // ms to days
> > -      let sessionLength = (Date.now() - this._sessionStartTime) / MS_PER_DAY;
> 
> Is MS_PER_DAY still used?

No, good catch!

> @@ +2911,5 @@
> >     */
> >    _updateSessionStartTime: function ssi_updateSessionStartTime(state) {
> >      // Attempt to load the session start time from the session state
> >      if (state.session && state.session.startTime) {
> >        this._sessionStartTime = state.session.startTime;
> 
> This histogram strikes me as useful (although I'm not 100% sure about the
> implementation). Do we have any other source of data that can replace it?

I don't know of any other source that would give us that kind of data. The current FX_SESSION_RESTORE_SESSION_LENGTH data looks rather equally distributed and doesn't tell a lot.

I'm not entirely sure if that kind of data, at least this implementation, is useful. All it tells us is that there are people that always start a new session and people that always restore their old session and thus send ping N today and N+1 tomorrow. I would argue that there are no users that would intentionally start a new session by resetting the prefs or removing the sessionstore data files. We simply have no support for starting a new session.

If we find a good definition for what a session length might be and how to implement that I wouldn't object re-adding a measurement like this. That should then just go into a separate/new bug.

I'd go and remove it. What do you think?
Flags: needinfo?(dteller)
Sounds good to me.
Flags: needinfo?(dteller)
https://hg.mozilla.org/mozilla-central/rev/f392bbb83496
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
You need to log in before you can comment on or make changes to this bug.