Bug 1184705 - Search A/B testing cohort identifier should be recorded in FHR. r=rnewman, a=lmandel
authorFlorian Quèze <florian@queze.net>
Mon, 20 Jul 2015 18:58:21 +0200
changeset 275422 bd619f8b7f9dda0930b632a262b5a1aadea7c8b0
parent 275421 8832754476316fc3b8a707e5dfb3594055f31d86
child 275423 810104107530d22731ca28fd1a48dfc8d199a228
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman, lmandel
bugs1184705
milestone40.0
Bug 1184705 - Search A/B testing cohort identifier should be recorded in FHR. r=rnewman, a=lmandel
browser/base/content/test/general/browser_urlbar_search_healthreport.js
services/healthreport/docs/dataformat.rst
services/healthreport/providers.jsm
services/healthreport/tests/xpcshell/test_provider_searches.js
toolkit/components/telemetry/TelemetryEnvironment.jsm
toolkit/components/telemetry/docs/environment.rst
toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js
--- a/browser/base/content/test/general/browser_urlbar_search_healthreport.js
+++ b/browser/base/content/test/general/browser_urlbar_search_healthreport.js
@@ -64,17 +64,17 @@ add_task(function* test_healthreport_sea
   ok(day.has(field), "Have a search count for the urlbar.");
   let newCount = day.get(field);
   is(newCount, oldCount + 1, "We recorded one new search.");
 
   // We should record the default search engine if Telemetry is enabled.
   let oldTelemetry = Services.prefs.getBoolPref("toolkit.telemetry.enabled");
   Services.prefs.setBoolPref("toolkit.telemetry.enabled", true);
 
-  m = provider.getMeasurement("engines", 1);
+  m = provider.getMeasurement("engines", 2);
   yield provider.collectDailyData();
   data = yield m.getValues();
 
   ok(data.days.hasDay(now), "Have engines data when Telemetry is enabled.");
   day = data.days.getDay(now);
   ok(day.has("default"), "We have default engine data.");
   is(day.get("default"), defaultEngineID, "The default engine is reported properly.");
 
--- a/services/healthreport/docs/dataformat.rst
+++ b/services/healthreport/docs/dataformat.rst
@@ -1598,16 +1598,25 @@ default
    engine exists but its identifier could not be determined.
 
    This field's contents are
    ``Services.search.defaultEngine.identifier`` (if defined) or
    ``"other-"`` + ``Services.search.defaultEngine.name`` if not.
    In other words, search engines without an ``.identifier``
    are prefixed with ``other-``.
 
+Version 2
+^^^^^^^^^
+
+Starting with Firefox 40, there is an additional optional value:
+
+cohort
+  Daily cohort string identifier, recorded if the user is part of
+  search defaults A/B testing.
+
 org.mozilla.sync.sync
 ---------------------
 
 This daily measurement contains information about the Sync service.
 
 Values should be recorded for every day FHR measurements occurred.
 
 Version 1
--- a/services/healthreport/providers.jsm
+++ b/services/healthreport/providers.jsm
@@ -55,16 +55,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 const LAST_NUMERIC_FIELD = {type: Metrics.Storage.FIELD_LAST_NUMERIC};
 const LAST_TEXT_FIELD = {type: Metrics.Storage.FIELD_LAST_TEXT};
 const DAILY_DISCRETE_NUMERIC_FIELD = {type: Metrics.Storage.FIELD_DAILY_DISCRETE_NUMERIC};
 const DAILY_LAST_NUMERIC_FIELD = {type: Metrics.Storage.FIELD_DAILY_LAST_NUMERIC};
 const DAILY_LAST_TEXT_FIELD = {type: Metrics.Storage.FIELD_DAILY_LAST_TEXT};
 const DAILY_COUNTER_FIELD = {type: Metrics.Storage.FIELD_DAILY_COUNTER};
 
 const TELEMETRY_PREF = "toolkit.telemetry.enabled";
+const SEARCH_COHORT_PREF = "browser.search.cohort";
 
 function isTelemetryEnabled(prefs) {
   return prefs.get(TELEMETRY_PREF, false);
 }
 
 /**
  * Represents basic application state.
  *
@@ -1625,20 +1626,21 @@ SearchCountMeasurement3.prototype = Obje
 function SearchEnginesMeasurement1() {
   Metrics.Measurement.call(this);
 }
 
 SearchEnginesMeasurement1.prototype = Object.freeze({
   __proto__: Metrics.Measurement.prototype,
 
   name: "engines",
-  version: 1,
+  version: 2,
 
   fields: {
     default: DAILY_LAST_TEXT_FIELD,
+    cohort: DAILY_LAST_TEXT_FIELD,
   },
 });
 
 this.SearchesProvider = function () {
   Metrics.Provider.call(this);
 
   this._prefs = new Preferences({defaultBranch: null});
 };
@@ -1683,16 +1685,19 @@ this.SearchesProvider.prototype = Object
         name = engine.identifier;
       } else if (engine.name) {
         name = "other-" + engine.name;
       } else {
         name = "UNDEFINED";
       }
 
       yield m.setDailyLastText("default", name);
+
+      if (Services.prefs.prefHasUserValue(SEARCH_COHORT_PREF))
+        yield m.setDailyLastText("cohort", Services.prefs.getCharPref(SEARCH_COHORT_PREF));
     }.bind(this));
   },
 
   /**
    * Record that a search occurred.
    *
    * @param engine
    *        (nsISearchEngine) The search engine used.
--- a/services/healthreport/tests/xpcshell/test_provider_searches.js
+++ b/services/healthreport/tests/xpcshell/test_provider_searches.js
@@ -141,17 +141,17 @@ add_task(function* test_includes_other_f
   yield storage.close();
 });
 
 add_task(function* test_default_search_engine() {
   let storage = yield Metrics.Storage("default_search_engine");
   let provider = new SearchesProvider();
   yield provider.init(storage);
 
-  let m = provider.getMeasurement("engines", 1);
+  let m = provider.getMeasurement("engines", 2);
 
   let now = new Date();
   yield provider.collectDailyData();
   let data = yield m.getValues();
   Assert.ok(data.days.hasDay(now));
 
   let day = data.days.getDay(now);
   Assert.equal(day.size, 1);
@@ -169,10 +169,19 @@ add_task(function* test_default_search_e
   let engine1 = Services.search.getEngineByName("testdefault");
   Assert.ok(engine1);
   Services.search.defaultEngine = engine1;
 
   yield provider.collectDailyData();
   data = yield m.getValues();
   Assert.equal(data.days.getDay(now).get("default"), "other-testdefault");
 
+  // If no cohort identifier is set, we shouldn't report a cohort.
+  Assert.equal(data.days.getDay(now).get("cohort"), undefined);
+
+  // Set a cohort identifier and verify we record it.
+  Services.prefs.setCharPref("browser.search.cohort", "testcohort");
+  yield provider.collectDailyData();
+  data = yield m.getValues();
+  Assert.equal(data.days.getDay(now).get("cohort"), "testcohort");
+
   yield storage.close();
 });
--- a/toolkit/components/telemetry/TelemetryEnvironment.jsm
+++ b/toolkit/components/telemetry/TelemetryEnvironment.jsm
@@ -148,16 +148,17 @@ const PREF_DISTRIBUTOR = "app.distributo
 const PREF_DISTRIBUTOR_CHANNEL = "app.distributor.channel";
 const PREF_E10S_ENABLED = "browser.tabs.remote.autostart";
 const PREF_HOTFIX_LASTVERSION = "extensions.hotfix.lastVersion";
 const PREF_APP_PARTNER_BRANCH = "app.partner.";
 const PREF_PARTNER_ID = "mozilla.partner.id";
 const PREF_TELEMETRY_ENABLED = "toolkit.telemetry.enabled";
 const PREF_UPDATE_ENABLED = "app.update.enabled";
 const PREF_UPDATE_AUTODOWNLOAD = "app.update.auto";
+const PREF_SEARCH_COHORT = "browser.search.cohort";
 
 const EXPERIMENTS_CHANGED_TOPIC = "experiments-changed";
 const SEARCH_ENGINE_MODIFIED_TOPIC = "browser-search-engine-modified";
 const SEARCH_SERVICE_TOPIC = "browser-search-service";
 
 /**
  * Get the current browser.
  * @return a string with the locale or null on failure.
@@ -873,16 +874,20 @@ EnvironmentCache.prototype = {
     }
 
     // Make sure we have a settings section.
     this._currentEnvironment.settings = this._currentEnvironment.settings || {};
     // Update the search engine entry in the current environment.
     this._currentEnvironment.settings.defaultSearchEngine = this._getDefaultSearchEngine();
     this._currentEnvironment.settings.defaultSearchEngineData =
       Services.search.getDefaultEngineInfo();
+
+    // Record the cohort identifier used for search defaults A/B testing.
+    if (Services.prefs.prefHasUserValue(PREF_SEARCH_COHORT))
+      this._currentEnvironment.settings.searchCohort = Services.prefs.getCharPref(PREF_SEARCH_COHORT);
   },
 
   /**
    * Update the default search engine value and trigger the environment change.
    */
   _onSearchEngineChange: function () {
     this._log.trace("_onSearchEngineChange");
 
--- a/toolkit/components/telemetry/docs/environment.rst
+++ b/toolkit/components/telemetry/docs/environment.rst
@@ -226,8 +226,13 @@ The object contains:
  [profile]/searchplugins/engine.xml
  [distribution]/searchplugins/common/engine.xml
  [other]/engine.xml
 
 - a ``submissionURL`` property with the HTTP url we would use to search.
   For privacy, we don't record this for user-installed engines.
 
 ``loadPath`` and ``submissionURL`` are not present if ``name`` is ``NONE``.
+
+searchCohort
+~~~~~~~~~~~~
+
+If the user has been enrolled into a search default change experiment, this contains the string identifying the experiment the user is taking part in. Most user profiles will never be part of any search default change experiment, and will not send this value.
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js
@@ -1033,13 +1033,22 @@ add_task(function* test_defaultSearchEng
   Preferences.set(PREF_TEST, 1);
   yield deferred.promise;
   TelemetryEnvironment.unregisterChangeListener("testSearchEngine_pref");
 
   // Check that the search engine information is correctly retained when prefs change.
   data = TelemetryEnvironment.currentEnvironment;
   checkEnvironmentData(data);
   Assert.equal(data.settings.defaultSearchEngine, EXPECTED_SEARCH_ENGINE);
+
+  // Check that by default we are not sending a cohort identifier...
+  Assert.equal(data.settings.searchCohort, undefined);
+
+  // ... but that if a cohort identifier is set, we send it.
+  Services.prefs.setCharPref("browser.search.cohort", "testcohort");
+  Services.obs.notifyObservers(null, "browser-search-service", "init-complete");
+  data = TelemetryEnvironment.currentEnvironment;
+  Assert.equal(data.settings.searchCohort, "testcohort");
 });
 
 add_task(function*() {
   do_test_finished();
 });