We should be able to pref off the unified Telemetry behavior with one switch. Lets verify that.

Same for being able to disable FHR.

What do you mean by "the unified telemetry behavior"? I think for release management of 39, the thing we need to be able to turn off is disable this entire collection system for release users who haven't opted into telemetry.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #2) > What do you mean by "the unified telemetry behavior"? I think for release > management of 39, the thing we need to be able to turn off is disable this > entire collection system for release users who haven't opted into telemetry. For 39 this currently works fine. Per Brendan we don't need really need the 39 beta data though at this point, so shouldn't we make sure that we don't take additional submission & storage hits like bug 1140037?

For 40 we need to introduce a switch to disable always initializing the Telemetry feature (result from bug 1137252). I plan to support a hidden pref that can be set to disable this specific behavior (say "toolkit.telemetry.disableUnified"), then we should be good.

(In reply to Georg Fritzsche [:gfritzsche] from comment #3) > (In reply to Benjamin Smedberg [:bsmedberg] from comment #2) > > What do you mean by "the unified telemetry behavior"? I think for release > > management of 39, the thing we need to be able to turn off is disable this > > entire collection system for release users who haven't opted into telemetry. > > For 39 this currently works fine. > Per Brendan we don't need really need the 39 beta data though at this point, > so shouldn't we make sure that we don't take additional submission & storage > hits like bug 1140037?

I don't understand the proposal yet. For 39 beta, what things are you planning on changing? * disable the feature when FHR is disabled (a 39 shipping requirement) * disable ping retention? * disabled sending of pings other than the saved-session ping? * other? Presumably we have to keep sending the saved-session ping, because that's the basis for all the telemetry dashboards. But if we can keep sending the normal main pings, I think that would be helpful to have a larger basis for comparing things like search counts and other metrics.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #6) > I don't understand the proposal yet. For 39 beta, what things are you > planning on changing? > > * disable the feature when FHR is disabled (a 39 shipping requirement) > * disable ping retention? > * disabled sending of pings other than the saved-session ping? > * other? > > Presumably we have to keep sending the saved-session ping, because that's > the basis for all the telemetry dashboards. But if we can keep sending the > normal main pings, I think that would be helpful to have a larger basis for > comparing things like search counts and other metrics. Yes, we need to keep sending the saved-session ping. I plan to: * disabled sending of pings other than the saved-session ping Ping retention/archiving is not on 39 at this point. Telemetry recording is not happening on 39 at this point if the Telemetry pref is off. If we want to collect the main pings on 39 beta we will need further work on 39 (at least bug 1140037) and need to clear this up with release management. Either way, i want to be able to turn the unified features off in beta (either immediately or in reaction to issues).

We can cover bug 1153252 here too.

Yes, please fix bug 1140037 before beta. The spikes may cause trouble on the server with Beta request volume.

The above patches introduce a toolkit.telemetry.unified pref on Aurora and use it for: * only initializing the Telemetry modules if toolkit.telemetry.enabled==true or .unified==true * only sending/triggering the unified "main" pings if .unified==true * only enabling the aborted-session feature if .unified==true For the patch stack context see the try push (based on Aurora): https://treeherder.mozilla.org/#/jobs?repo=try&revision=74074cdae872 Upcoming patches will address 40.

Comment on attachment 8597259 [details] [diff] [review] Part 2 (Fx 39): Honor toolkit.telemetry.unified pref in Telemetry modules Review of attachment 8597259 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/telemetry/TelemetryPing.jsm @@ +693,5 @@ > + const enabled = Preferences.get(PREF_ENABLED, false); > + > + // Enable base Telemetry recording. > + // We always record base data if unified behavior is on. > + Telemetry.canRecordBase = enabled || IS_UNIFIED_TELEMETRY; Does all.js govern the behaviour of IS_UNIFIED_TELEMETRY also in tests (Mochitests, reftests, etc.)? If that's the case, and tests are crashing, you could need to stub out the telemetry server in runreftest.py and profile.py as done by bug 1137252.

New push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=10935c0d959b

Cleared up an oversight from friday, now try looks sane: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5bb29b99b645

Comment on attachment 8598035 [details] [diff] [review] Part 2 (Fx 39): Honor toolkit.telemetry.unified pref in Telemetry modules Review of attachment 8598035 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/telemetry/TelemetrySession.jsm @@ +1590,5 @@ > + yield this._saveAbortedSessionPing(); > + } > + > + // Start the scheduler. > + TelemetryScheduler.init(); In your previous patch, you had a comment describing why the scheduler init was being skipped. Do you think it could be worth keeping it to explain that this turns off the new unified pings but keeps sending the one in the old format?

Comment on attachment 8598067 [details] [diff] [review] Part 1 (Fx 40): Introduce a toolkit.telemetry.unified pref Review of attachment 8598067 [details] [diff] [review]: ----------------------------------------------------------------- Nevermind, this part doesn't really differ from the 39 version.

Addressed review comment and fixed not disabling 'shutdown' ping.

Updated to disable "shutdown" ping too. (Sorry about the above patch noise, bzexport doesn't seem to work that great for obsoleting)

Comment on attachment 8598515 [details] [diff] [review] Part 3 (Fx 40): Honor toolkit.telemetry.unified pref in Telemetry modules Review of attachment 8598515 [details] [diff] [review]: ----------------------------------------------------------------- It looks good, I just have this doubt: ::: toolkit/components/telemetry/TelemetryController.jsm @@ -882,5 @@ > > - // Initialize some probes that are kept in their own modules > - this._thirdPartyCookies = new ThirdPartyCookieProbe(); > - this._thirdPartyCookies.init(); > - Why are you removing this?

Comment on attachment 8598072 [details] [diff] [review] Part 4 (Fx 40): Disable Telemetry recording in reftests Review of attachment 8598072 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/tools/reftest/runreftest.py @@ +205,5 @@ > > # Ensure that telemetry is disabled, so we don't connect to the telemetry > # server in the middle of the tests. > prefs['toolkit.telemetry.enabled'] = False > + prefs['toolkit.telemetry.unified'] = False Do we need to do the same in profile.py [1]? [1] - https://hg.mozilla.org/mozilla-central/annotate/caf25344f73e/testing/mozbase/mozprofile/mozprofile/profile.py#l372

(In reply to Alessio Placitelli [:Dexter] from comment #28) > ::: toolkit/components/telemetry/TelemetryController.jsm > @@ -882,5 @@ > > > > - // Initialize some probes that are kept in their own modules > > - this._thirdPartyCookies = new ThirdPartyCookieProbe(); > > - this._thirdPartyCookies.init(); > > - > > Why are you removing this? We initialize that in TelemetrySession.jsm already. That module only pushes histograms, so it should be enough to initialize it there.

(In reply to Alessio Placitelli [:Dexter] from comment #29) > ::: layout/tools/reftest/runreftest.py > @@ +205,5 @@ > > > > # Ensure that telemetry is disabled, so we don't connect to the telemetry > > # server in the middle of the tests. > > prefs['toolkit.telemetry.enabled'] = False > > + prefs['toolkit.telemetry.unified'] = False > > Do we need to do the same in profile.py [1]? Looks like we should. FWIW, i cleared up what profile.py affects and added it here: https://developer.mozilla.org/en-US/docs/Mozilla/QA/Automated_testing#Need_to_set_preferences_for_test-suites.3F

Try push for the Fx 40 versions: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f00400820af

Repushed to try (with some other bugs) with minor fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9ebe645e2d2f

Minor Android xpcshell issue around the archiving API. Repushed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=58c713a66be2

https://hg.mozilla.org/integration/fx-team/rev/833051c87c7b https://hg.mozilla.org/integration/fx-team/rev/0df0d478d396 https://hg.mozilla.org/integration/fx-team/rev/b2c68526576d https://hg.mozilla.org/integration/fx-team/rev/01ff4e0d37ae

https://hg.mozilla.org/mozilla-central/rev/833051c87c7b https://hg.mozilla.org/mozilla-central/rev/0df0d478d396 https://hg.mozilla.org/mozilla-central/rev/b2c68526576d https://hg.mozilla.org/mozilla-central/rev/01ff4e0d37ae

Georg, does this need uplift to aurora?

We are currently discussing that on bug 1158317.