1036719 - [Search experiment] Search experiment should only target en-US

Status: VERIFIED FIXED

(Firefox Health Report Graveyard :: Client: Desktop, defect)

Description

[:Felipe Gomes (needinfo for replies!)]

We forgot to add this in bug 1029799.

Comment 1

[:Felipe Gomes (needinfo for replies!)]

To test:

- With a current beta build of Firefox, get the published experiment installed from the official server

- After the experiment has been received and properly installed, change the settings:
  - experiments.manifest.cert.checkAttributes => false
  - experiments.manifest.uri => https://people.mozilla.org/~fgomes/searchtest/locale-fixed/firefox-manifest.json

- Force a manifest update

- Make sure that using an en-US build will not have the experiment uninstalled, and any other locale will have the experiment immediatelly uninstalled and the search engine reset to Google.


Note: these two prefs are useful when testing:
  - experiments.logging.dump  => true
  - experiments.logging.level => 0

And to trigger a manifest update:
  Cu.import("resource:///modules/experiments/Experiments.jsm");
  Experiments.instance().updateManifest();

Comment 2

[:Felipe Gomes (needinfo for replies!)]

Attachment: Add locale restriction

Comment 3

[Marco Mucci [:MarcoM]]

Hi Felipe, can you provide a point value and if the bug should be marked as [qa+] or [qa-] for verification.

Comment 4

[:Felipe Gomes (needinfo for replies!)]

Kamil, could you QA this?

Comment 6

[Benjamin Smedberg]

Comment on attachment 8453468 [details] [diff] [review]
Add locale restriction

The only note I have here is that users who were already in the experiment will have it removed without any notice. I want to make sure Chad is ok with that.

Comment 7

[Kamil Jozwiak [:kjozwiak]]

(In reply to :Felipe Gomes from comment #4)
> Kamil, could you QA this?

Sure thing, assigned it to myself!

Comment 8

[Chad Weiner [:chadw]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #6)
> Comment on attachment 8453468 [details] [diff] [review]
> Add locale restriction
> 
> The only note I have here is that users who were already in the experiment
> will have it removed without any notice. I want to make sure Chad is ok with
> that.

As long as this is only true for non-EN-US users (meaning we're not ending the experiment for anyone currently enrolled who is using EN-US) I'm fine with no notification to end the experiment.

Comment 9

[Benjamin Smedberg]

Pushed and tagged. I'm going to reuse this same bug to actually push. cturra can you push telemetry-experiment-server today (should be revision 62b1cc91b070).

Comment 10

[Chris Turra [:cturra]]

:bsmedberg - please file a bug for this push under 'infrastructure & operations' > 'it managed services':

  https://bugzilla.mozilla.org/enter_bug.cgi?product=Infrastructure%20%26%20Operations

Comment 11

[Benjamin Smedberg]

Should be live.

Comment 12

[Kamil Jozwiak [:kjozwiak]]

Sorry for the delay, was on PTO today due to a personal appointment.

Went through the following test cases:

- ensured that the search experiment is being installed only on en-US locales
- ensured that users with the search experiment installed are being placed into the different branches
- ensured that the search experiment is not being installed on pl, zh-cn, nn-NO, hy-AM, ru, sw, mai
- ensured that the correct message is being displayed in the browser console when it fails due to an incorrect locale:
--> 1405046433078 Browser.Experiments.Experiments DEBUG	ExperimentEntry #0::isApplicable() - id=[e-mail] - test 'locale' failed
- ensured that sampling is still working and displaying the correct error message under browser console
--> 1405051453466 Browser.Experiments.Experiments DEBUG	ExperimentEntry #0::isApplicable() - id=[e-mail] - test 'sample' failed

I cloned the the code and updated to revision c129e4e9e51d from http://hg.mozilla.org/webtools/telemetry-experiment-server/ and went through the following test cases:

** Note: I went through the above test cases using several different locales **

- ensured that the experiment was disabled if it was already installed on a locale other than en-US
- ensured experiments.activeExperiment;false once the experiment has been removed
- ensured that the search experiment appears grayed in the "Experiment" container in about:addons
- ensured that the search experiment appears disabled under about:support
- ensured that Google Search is set as default under about:newtab, about:home, url bar and the search toolbar
- ensured extensions.bootstrappedAddons;{} (should be empty)
- ensured that the search experiment stay's disabled after fx restarts
- ensured that if a user changed their default search engine during the experiment, the experiment is removed but Google is not restored as default
- ensured that the branch information is removed from the telemetry payload
- ensured that the experiment is not removed when a user is on the en-US locale
- ensured that the search engine is not restored to Google if the user has selected another search engine while the experiment is still active on the en-US locale
- ensured that the user still gets a notification when there's three days remaining if they are under one of the notification branches

Comment 13

[Kamil Jozwiak [:kjozwiak]]

Possible issue:

I'm not sure if this is an issue on my side or legitimate problem. I've tried this on 3 different machines and I get the same behavior. The default link to the manifest in BETA is currently:
--> experiments.manifest.uri;https://telemetry-experiment.cdn.mozilla.net/manifest/v1/firefox/%VERSION%/%CHANNEL%

When I try installing the experiment using the above manifest, I get the following:

1405058008663 Browser.Experiments.Experiments TRACE Experiments #0::_updateExperiments() - experiments: {"experiments":[{"xpiURL":"https://telemetry-experiment.cdn.mozilla.net/fx-searchtest-en-beta31%40mozilla.org/experiment.xpi","xpiHash":"sha256:967b0870d0c08ad74d39d12cca34cf40cab7d344aa93180495366a2e4105b9fd","appName":["Firefox"],"maxActiveSeconds":2419200,"maxVersion":"32.*","sample":0.5,"minBuildID":"20140630185627","startTime":1404216000,"minVersion":"31.0a","endTime":1412035200,"id":"[e-mail]","channel":["beta"]}

Notice that the locale information is missing hence the experiment is being installed on all the locales. However when I use the following link, everything works fine:
--> https://telemetry-experiment.cdn.mozilla.net/firefox-manifest.json

Benjamin, everything else is working correctly other than the possible issue mentioned above.

Comment 14

[:Felipe Gomes (needinfo for replies!)]

Sounds like the caching is stronger in the rewrite URL than in the direct url to the manifest. The full URL is returning the correct manifest for me, so maybe it has flushed already or we are hitting different servers.

Kamil, next time you see this, can you access the url like https://telemetry-experiment.cdn.mozilla.net/manifest/v1/firefox/aaa/bbb (the values don't matter) using a different profile or browser, to see if this is local caching or from the CDN, and figure out if we should do something about it.

Comment 15

[Benjamin Smedberg]

Reopened bug 1037121 for CDN cache clearing.

Comment 16

[Kamil Jozwiak [:kjozwiak]]

Went through verification using the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/31.0b9/

bug # 1037121 has been fixed and I went through some of the test cases in comment # 12 without any issues. I've also verified bug # 1037121 to make sure that the manifest link is working correctly against other locales.