1248796 - Store on Telemetry whether the e10s blocking code successfully ran

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[:Felipe Gomes (needinfo for replies!)]

Attachment: patch

This pref is not defined by default (in prefs.js or firefox.js), and should be set (whether to true or false), to any user who got an add-on installed/uninstalled/updated [1], and any user during a Firefox update [2].

In order to make sure the code that blocks e10s for add-on users is running properly (being investigated in bug 1248130), we should check if it's actually set.

With [1], we can look into all users on beta who received the e10s telemetry experiment.
With [2], we can check for all users on release (with telemetry enabled) once they update to 45.


Georg, is it fine to uplift changes to the Telemetry environment? This is only useful if I can uplift it to beta.


[Tracking Requested - why for this release]:
Check in advance whether users' profiles (with add-ons) are ready to receive e10s.

Comment 1

[:Felipe Gomes (needinfo for replies!)]

Attachment: patch

Comment 2

[Georg Fritzsche [:gfritzsche]]

(In reply to :Felipe Gomes (needinfo me!) from comment #0)
> In order to make sure the code that blocks e10s for add-on users is running
> properly (being investigated in bug 1248130), we should check if it's
> actually set.

Is there any specific reason to put this into the environment instead of recording it in a histogram (limited to recording in the next few releases)?

Comment 3

[:Felipe Gomes (needinfo for replies!)]

Hm no strong reason, but the main data analysis for crashes is being done with info from the environment (e.g. https://github.com/vitillo/e10s_analyses/blob/f9ad11ff7fafaf55d8ca740bef660773b7057927/beta45-withaddons/e10s_crash_rate.ipynb), so I thought it would be easier here (also I don't need to find an appropriate location to add to the histogram)

Comment 4

[:Felipe Gomes (needinfo for replies!)]

Attachment: As a histogram

Per IRC, moved to a histogram and set expiry date to 48

Comment 5

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8720327 [details] [diff] [review]
As a histogram

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

Thanks, this looks good with the below addressed.
Over to Benjamin for data review.

::: toolkit/xre/nsAppRunner.cpp
@@ +4765,5 @@
>      mozilla::Telemetry::Accumulate(mozilla::Telemetry::E10S_BLOCKED_FROM_RUNNING,
>                                      !gBrowserTabsRemoteAutostart);
>    }
> +  mozilla::Telemetry::Accumulate(mozilla::Telemetry::E10S_ADDONS_BLOCKER_RAN,
> +                                 Preferences::HasUserValue("extensions.e10sBlockedByAddons"));

From what i see this is only recorded once. A flag histogram seems much better here then.
(boolean histograms allow recording multiple boolean values, flag only records one)

Comment 6

[:Felipe Gomes (needinfo for replies!)]

Attachment: As a flag histogram, r=gfritzche

Changed to a flag histogram, carrying forward r+.

Tagging Benjamin for data review

Comment 7

[Benjamin Smedberg]

Comment on attachment 8720356 [details] [diff] [review]
As a flag histogram, r=gfritzche

Clearly valuable immediately for experiments. This same code will be used in 46 release? Clear user value in making sure it works correctly: Felipe are you responsible for monitoring it?

Comment 8

[:Felipe Gomes (needinfo for replies!)]

Yeah, I'll be responsible for monitoring it.  For release, it will be directly useful in 45. Since the add-ons blocking code is already in 45 and thus will run one release in advance, this probe will give us confidence that the existing profiles out there are correctly prepared to enable e10s when it rolls out.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9f14c676b59a

Comment 10

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8720356 [details] [diff] [review]
As a flag histogram, r=gfritzche

Approval Request Comment
[Feature/regressing bug #]: adds a telemetry probe that will help verify the e10s activation code
[User impact if declined]: fewer data to drive decision
[Describe test coverage new/current, TreeHerder]: landed on inbound
[Risks and why]: slim to none, just adds a new telemetry probe
[String/UUID change made/needed]: none

It would be really nice if this made it to the beta build that is gonna be built this Thursday. Is there any chance for that to happen?

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f2fa3f9f3544

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9f14c676b59a
https://hg.mozilla.org/mozilla-central/rev/f2fa3f9f3544

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8720356 [details] [diff] [review]
As a flag histogram, r=gfritzche

Help e10s, taking it.
Should be in 45 beta 7

Comment 14

[:Felipe Gomes (needinfo for replies!)]

Thanks!
https://hg.mozilla.org/releases/mozilla-beta/rev/effb4c258fcc

Comment 15

[:Felipe Gomes (needinfo for replies!)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ce42b6e3986a