1241336 - Run an e10s A/B experiment on Beta 45 (first experiment)

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Vladan Djeric (:vladan)]

We want to run an A/B experiment on Beta 45 on 50% of all profiles (with and without extensions).

Comment 1

[Vladan Djeric (:vladan)]

Attachment: Experiment code, mostly unchanged.patch

Start time: beginning of Monday, Jan 25th GMT
End time: beginning of Thursday, Feb 11th GMT
Maximum duration: 15 days
Sampling: 50% of Beta population (25% + 25%)
Filtered only on profiles that have extended Telemetry enabled (default on Beta)

Comment 2

[Vladan Djeric (:vladan)]

Is there some way to determine that e10s is blocked because of a11y now? Maybe experiment state "force-disabled" combined with the the values in Telemetry's A11Y_ histograms?

Comment 3

[Vladan Djeric (:vladan)]

Note that this experiment doesn't have the extension blacklist yet. I will add it tomorrow.

Comment 4

[:Felipe Gomes (needinfo for replies!)]

(In reply to Vladan Djeric (:vladan) -- please needinfo from comment #2)
> Is there some way to determine that e10s is blocked because of a11y now?
> Maybe experiment state "force-disabled" combined with the the values in
> Telemetry's A11Y_ histograms?

The E10S_STATUS histogram will have a value of 4. Possible values are here: http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp?rev=1626afe51b11#4646
(soon to be added 6 = disabled for certain locales, 7 = disabled for addons)

Comment 6

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8710228 [details] [diff] [review]
Experiment code, mostly unchanged.patch

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

I'll give r+ after the addon whitelist is added, but the code here looks good.

::: experiments/e10s-enabled-beta45.1/code/bootstrap.js
@@ +32,5 @@
> +      case null:
> +        // Profile is ineligible if either of the two e10s prefs is non-default
> +        if (Preferences.isSet(OPTIN_PREF) || Preferences.isSet(PREF)) {
> +          TelemetryLog.log(SELF_ID, ["Setting branch to 'user-disabled' because e10s prefs have non-default values."]);
> +          yield Experiments.instance().setExperimentBranch(SELF_ID, "user-disabled");

if optin_pref = true, e10s will be enabled, so user-disabled is a bit of a misnomer here.. i.e., they will be running e10s, but just not due to the experiment.  Not sure how to better tag that, that's up to how the data analysis will be done afterwards.

Although that is a small concern since the optin_pref was never exposed to beta, so only a few users messing with about:config or reusing profiles will fall into this.

::: experiments/e10s-enabled-beta45.1/code/install.rdf
@@ +1,4 @@
> +<?xml version="1.0" encoding="utf-8"?>
> +<RDF xmlns="http://www.w3.org/1999/02/22-rdf-syntax-ns#" xmlns:em="http://www.mozilla.org/2004/em-rdf#">
> +  <Description about="urn:mozilla:install-manifest">
> +    <em:id>[e-mail]</em:id>

very nitpicky, but can you change the name to "e10s-beta45-withaddons", just so that this is more descriptive and easy to differentiate with a quick glance?  The ID in the manifest and bootstrap.js also needs to be changed.

::: experiments/e10s-enabled-beta45.1/manifest.json
@@ +1,3 @@
> +{
> +  "publish"     : true,
> +  "priority"    : 2,

I talked with bsmedberg about this. Since there's another experiment running at the same time (the unified urlbar), and that experiment has a smaller sample size than this one, this one needs to have a lower priority. That one is set to 2, so please set the priority here to 1.

@@ +11,5 @@
> +    "maxActiveSeconds" : 1296000,
> +    "appName"          : ["Firefox"],
> +    "channel"          : ["beta"],
> +    "minVersion"       : "44.0",
> +    "maxVersion"       : "44.0",

need to update min/max Version to 45.

Comment 7

[:Felipe Gomes (needinfo for replies!)]

Attachment: in progress experiment.xpi (no addons blacklist)

Hello Jason, could you sign this xpi for us?

FWIW, there will be another signing needed soon, but we want to start testing this asap so this version is good enough for this. Hope that is ok!

Comment 8

[Jason Thomas [:jason]]

Attachment: signed experiment.xpi

signed experiment.xpi

Comment 9

[:Felipe Gomes (needinfo for replies!)]

So I made the changes I mentioned here and published an experiment server at https://people.mozilla.org/~fgomes/e10s-beta45-withaddons/ .

With the standard experiment instructions, one can set experiments.manifest.uri to:
  
  https://people.mozilla.org/~fgomes/e10s-beta45-withaddons/firefox-manifest.json

To start testing it. The difference from what needs to land is that I set the sample size to 100% and I removed the other running experiment, in order to make it easier to test this one.

Comment 10

[:Felipe Gomes (needinfo for replies!)]

It's still an open question the about whether we'll include an add-on blocklist in this experiment or not, so I also landed what we already have: (which is everything except the blocklist)

https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/b5ddc65fd536

Comment 11

[:Felipe Gomes (needinfo for replies!)]

Attachment: Filter old versions of lastpass

Filters out users of old versions of LastPass from being part of the experiment

Comment 12

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8711893 [details] [diff] [review]
Filter old versions of lastpass

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

::: experiments/e10s-beta45-withaddons/filter.js
@@ +1,5 @@
>  function filter(c) {
> +  let activeAddons = {};
> +  try {
> +    activeAddons = c.telemetryEnvironment.addons.activeAddons;
> +  } catch (e) {}

How about we return false if we get an exception here?

Comment 13

[:Felipe Gomes (needinfo for replies!)]

Pushed the filtering of LastPass 3.* to the manifest

https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/73b05e4d57c6

Once QA signs off I'll push the release_tag on this experiment and close this bug

Comment 14

[:Felipe Gomes (needinfo for replies!)]

Attachment: Block users of certain locales

The in-tree code that does the locale blocking blocks based on the OS's locale, rather than the actual Firefox locale. Not a huge deal (would miss users of these locales if they're running the OS in en-US for example), but also not ideal .. So let's block here in the manifest and let the code be fixed for beta2.

Comment 15

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8713316 [details] [diff] [review]
Block users of certain locales

Actually, forget that. The bidi issue is minor and affects very few people. It will be better to have the in-tree code running to test it instead of filtering out these users

Comment 16

[:Felipe Gomes (needinfo for replies!)]

QA has signed off on the experiment, so I updated release_tag to point to it:

https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/31f336659583

Final deployment to production will occur in bug 1243867