1287481 - [meta] Remove the "saved-session" pings

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P3)

Description

[Georg Fritzsche [:gfritzsche]]

The last dependencies for removing the "saved-session" pings are:
* the aggregator needs to use main pings (bug 1286868)
* the ongoing e10s analysis needs to switch to main pings

Once those are cleared, we should stop sending the "saved-session" ping.

Comment 1

[Georg Fritzsche [:gfritzsche]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #0)
> * the ongoing e10s analysis needs to switch to main pings

Chris, do you know if that analysis still ongoing? And does it still use saved-session pings?

Comment 2

[Georg Fritzsche [:gfritzsche]]

Mark, are you aware of any remaining pipeline dependencies?
Any jobs or monitoring that would expect saved-session pings?

Comment 3

[Chris H-C :chutten]

The beta e10s analysis is ongoing to test the effects of adding various cohorts of addons users to the e10s population. The relevant code is here: https://github.com/mozilla/telemetry-batch-view/blob/master/src/main/scala/com/mozilla/telemetry/views/E10sExperiment.scala#L61

Comment 4

[Georg Fritzsche [:gfritzsche]]

(In reply to Chris H-C :chutten from comment #3)
> The beta e10s analysis is ongoing to test the effects of adding various
> cohorts of addons users to the e10s population. The relevant code is here:
> https://github.com/mozilla/telemetry-batch-view/blob/master/src/main/scala/
> com/mozilla/telemetry/views/E10sExperiment.scala#L61

Is that expensive to migrate? Is it better to just wait until that staged rollout is complete?

Comment 5

[Chris H-C :chutten]

I dunno. If we can just s/saved_session/main/ and call it a day, I think we can probably do that whenever. Not sure what the effect would be on the analysis, though.

:dzeber and :ben_ are the ones running the analyses these days. Maybe put a bug in their court to try tweaking the scala and running the analysis on the resultant data to see what happens?

Comment 6

[Georg Fritzsche [:gfritzsche]]

Dave, what is your take on that?

Comment 7

[Dave Zeber [:dzeber]]

The analysis should remain valid provided the measures we look at don't depend on how far into the session they are collected (eg. they don't occur disproportionately near the start of the session), which I believe to be the case. In cases where we normalize by session time, we'd have to make sure to switch to subsessionLength (currently totalTime).

That said, I would rather wait until the end of the e10s staged rollout if possible. The analysis aggregates together full-session histograms (1 per client) and draws comparisons between e10s and non-e10s. If we switch to main pings, we will be selecting a random portion of the session, rather than aggregating full sessions per client. The comparisons will still be valid but will have added noise, and this may raise questions about whether something changed with e10s. The best approach from the point of view of analysis is to use a consistent dataset week over week.

If it is necessary to migrate sooner, another solution would be to modify the Scala job to output all main pings belonging to a randomly selected session, for each client.

Also, I wanted to call out that the arewee10syet data pull (https://github.com/dzeber/new-arewee10syet.com/blob/atmo-v2/shim-perf.ipynb) currently uses saved-session pings, but that could be easily switched over to main pings.

Comment 8

[Mark Reid [:mreid]]

Aside from the uses already outlined in this bug, I can't think of anything that relies upon saved-session pings.

Comment 9

[Mark Reid [:mreid]]

One other case has come up. Fennec still reports data using saved-session pings, which are (or were, until recently) included in the aggregates on t.m.o

Comment 10

[Georg Fritzsche [:gfritzsche]]

We are not planning to remove saved-session for Fennec right now.
There are decisions to be made at some point on how/if to move Fennec Telemetry forward.

Comment 11

[Chris H-C :chutten]

Work's done.