Bug 1017806 - Record and report on optional branches of experiments, so that each experiment doesn't have to re-invent data collection. r=gfritzsche

browser/experiments/Experiments.jsm

@@ -628,6 +628,54 @@ Experiments.Experiments.prototype = {
     return info;
   },
 
+  /**
+   * Experiment "branch" support. If an experiment has multiple branches, it
+   * can record the branch with the experiment system and it will
+   * automatically be included in data reporting (FHR/telemetry payloads).
+   */
+
+  /**
+   * Set the experiment branch for the specified experiment ID.
+   * @returns Promise<>
+   */
+  setExperimentBranch: Task.async(function*(id, branchstr) {
+    yield this._loadTask;
+    let e = this._experiments.get(id);
+    if (!e) {
+      throw new Error("Experiment not found");
+    }
+    e.branch = String(branchstr);
+    this._dirty = true;
+    Services.obs.notifyObservers(null, EXPERIMENTS_CHANGED_TOPIC, null);
+    yield this._run();
+  }),
+  /**
+   * Get the branch of the specified experiment. If the experiment is unknown,
+   * throws an error.
+   *
+   * @param id The ID of the experiment. Pass null for the currently running
+   *           experiment.
+   * @returns Promise<string|null>
+   * @throws Error if the specified experiment ID is unknown, or if there is no
+   *         current experiment.
+   */
+  getExperimentBranch: Task.async(function*(id=null) {
+    yield this._loadTask;
+    let e;
+    if (id) {
+      e = this._experiments.get(id);
+      if (!e) {
+        throw new Error("Experiment not found");
+      }
+    } else {
+      e = this._getActiveExperiment();
+      if (e === null) {
+	throw new Error("No active experiment");
+      }
+    }
+    return e.branch;
+  }),
+
   /**
    * Determine whether another date has the same UTC day as now().
    */
@@ -1013,6 +1061,17 @@ Experiments.Experiments.prototype = {
     return e.id;
   },
 
+  getActiveExperimentBranch: function() {
+    if (!this._experiments) {
+      return null;
+    }
+    let e = this._getActiveExperiment();
+    if (!e) {
+      return null;
+    }
+    return e.branch;
+  },
+
   _getActiveExperiment: function () {
     let enabled = [experiment for ([,experiment] of this._experiments) if (experiment._enabled)];
 
@@ -1258,6 +1317,8 @@ Experiments.ExperimentEntry = function (policy) {
   this._lastChangedDate = null;
   // Has this experiment failed to activate before?
   this._failedStart = false;
+  // The experiment branch
+  this._branch = null;
 
   // We grab these from the addon after download.
   this._name = null;
@@ -1306,6 +1367,7 @@ Experiments.ExperimentEntry.prototype = {
     "_addonId",
     "_startDate",
     "_endDate",
+    "_branch",
   ]),
 
   DATE_KEYS: new Set([
@@ -1313,6 +1375,10 @@ Experiments.ExperimentEntry.prototype = {
     "_endDate",
   ]),
 
+  UPGRADE_KEYS: new Map([
+    ["_branch", null],
+  ]),
+
   ADDON_CHANGE_NONE: 0,
   ADDON_CHANGE_INSTALL: 1,
   ADDON_CHANGE_UNINSTALL: 2,
@@ -1344,6 +1410,14 @@ Experiments.ExperimentEntry.prototype = {
     return this._manifestData.id;
   },
 
+  get branch() {
+    return this._branch;
+  },
+
+  set branch(v) {
+    this._branch = v;
+  },
+
   get startDate() {
     return this._startDate;
   },
@@ -1376,6 +1450,12 @@ Experiments.ExperimentEntry.prototype = {
    * @return boolean Whether initialization succeeded.
    */
   initFromCacheData: function (data) {
+    for (let [key, dval] of this.UPGRADE_KEYS) {
+      if (!(key in data)) {
+        data.set(key, dval);
+      }
+    }
+
     for (let key of this.SERIALIZE_KEYS) {
       if (!(key in data) && !this.DATE_KEYS.has(key)) {
         this._log.error("initFromCacheData() - missing required key " + key);
@@ -1970,6 +2050,9 @@ let stripDateToMidnight = function (d) {
 function ExperimentsLastActiveMeasurement1() {
   Metrics.Measurement.call(this);
 }
+function ExperimentsLastActiveMeasurement2() {
+  Metrics.Measurement.call(this);
+}
 
 const FIELD_DAILY_LAST_TEXT = {type: Metrics.Storage.FIELD_DAILY_LAST_TEXT};
 
@@ -1983,6 +2066,17 @@ ExperimentsLastActiveMeasurement1.prototype = Object.freeze({
     lastActive: FIELD_DAILY_LAST_TEXT,
   }
 });
+ExperimentsLastActiveMeasurement2.prototype = Object.freeze({
+  __proto__: Metrics.Measurement.prototype,
+
+  name: "info",
+  version: 2,
+
+  fields: {
+    lastActive: FIELD_DAILY_LAST_TEXT,
+    lastActiveBranch: FIELD_DAILY_LAST_TEXT,
+  }
+});
 
 this.ExperimentsProvider = function () {
   Metrics.Provider.call(this);
@@ -1997,6 +2091,7 @@ ExperimentsProvider.prototype = Object.freeze({
 
   measurementTypes: [
     ExperimentsLastActiveMeasurement1,
+    ExperimentsLastActiveMeasurement2,
   ],
 
   _OBSERVERS: [
@@ -2040,8 +2135,8 @@ ExperimentsProvider.prototype = Object.freeze({
       this._experiments = Experiments.instance();
     }
 
-    let m = this.getMeasurement(ExperimentsLastActiveMeasurement1.prototype.name,
-                                ExperimentsLastActiveMeasurement1.prototype.version);
+    let m = this.getMeasurement(ExperimentsLastActiveMeasurement2.prototype.name,
+                                ExperimentsLastActiveMeasurement2.prototype.version);
 
     return this.enqueueStorageOperation(() => {
       return Task.spawn(function* recordTask() {
@@ -2055,6 +2150,11 @@ ExperimentsProvider.prototype = Object.freeze({
         this._log.info("Recording last active experiment: " + todayActive.id);
         yield m.setDailyLastText("lastActive", todayActive.id,
                                  this._experiments._policy.now());
+        let branch = todayActive.branch;
+        if (branch) {
+          yield m.setDailyLastText("lastActiveBranch", branch,
+                                   this._experiments._policy.now());
+        }
       }.bind(this));
     });
   },

browser/experiments/test/xpcshell/head.js

@@ -76,6 +76,7 @@ const FAKE_EXPERIMENTS_1 = [
     description: "experiment 1",
     active: true,
     detailUrl: "https://dummy/experiment1",
+    branch: "foo",
   },
 ];
 
@@ -87,6 +88,7 @@ const FAKE_EXPERIMENTS_2 = [
     active: false,
     endDate: new Date(2014, 2, 11, 2, 4, 35, 42).getTime(),
     detailUrl: "https://dummy/experiment2",
+    branch: null,
   },
   {
     id: "id1",
@@ -95,6 +97,7 @@ const FAKE_EXPERIMENTS_2 = [
     active: false,
     endDate: new Date(2014, 2, 10, 0, 0, 0, 0).getTime(),
     detailURL: "https://dummy/experiment1",
+    branch: null,
   },
 ];
 

browser/experiments/test/xpcshell/test_api.js

@@ -171,6 +171,14 @@ add_task(function* test_getExperiments() {
   let addons = yield getExperimentAddons();
   Assert.equal(addons.length, 0, "Precondition: No experiment add-ons are installed.");
 
+  try {
+    let b = yield experiments.getExperimentBranch();
+    Assert.ok(false, "getExperimentBranch should fail with no experiment");
+  }
+  catch (e) {
+    Assert.ok(true, "getExperimentBranch correctly threw");
+  }
+
   // Trigger update, clock set for experiment 1 to start.
 
   now = futureDate(startDate1, 5 * MS_IN_ONE_DAY);
@@ -196,6 +204,19 @@ add_task(function* test_getExperiments() {
                  "Property " + k + " should match reference data.");
   }
 
+  let b = yield experiments.getExperimentBranch();
+  Assert.strictEqual(b, null, "getExperimentBranch should return null by default");
+
+  b = yield experiments.getExperimentBranch(EXPERIMENT1_ID);
+  Assert.strictEqual(b, null, "getExperimentsBranch should return null (with id)");
+
+  yield experiments.setExperimentBranch(EXPERIMENT1_ID, "foo");
+  b = yield experiments.getExperimentBranch();
+  Assert.strictEqual(b, "foo", "getExperimentsBranch should return the set value");
+
+  Assert.equal(observerFireCount, ++expectedObserverFireCount,
+               "Experiments observer should have been called.");
+
   Assert.equal(gTimerScheduleOffset, 10 * MS_IN_ONE_DAY,
                "Experiment re-evaluation should have been scheduled correctly.");
 

browser/experiments/test/xpcshell/test_cache.js

@@ -190,6 +190,13 @@ add_task(function* test_cache() {
   checkExperimentListsEqual(experimentListData.slice(1), list);
   checkExperimentSerializations(experiments._experiments.values());
 
+  let branch = yield experiments.getExperimentBranch(EXPERIMENT1_ID);
+  Assert.strictEqual(branch, null);
+
+  yield experiments.setExperimentBranch(EXPERIMENT1_ID, "testbranch");
+  branch = yield experiments.getExperimentBranch(EXPERIMENT1_ID);
+  Assert.strictEqual(branch, "testbranch");
+
   // Re-init, clock set for experiment 1 to stop.
 
   now = futureDate(now, 20 * MS_IN_ONE_DAY);
@@ -207,6 +214,9 @@ add_task(function* test_cache() {
   checkExperimentListsEqual(experimentListData.slice(1), list);
   checkExperimentSerializations(experiments._experiments.values());
 
+  branch = yield experiments.getExperimentBranch(EXPERIMENT1_ID);
+  Assert.strictEqual(branch, "testbranch");
+
   // Re-init, clock set for experiment 2 to start.
 
   now = futureDate(startDates[1], 20 * MS_IN_ONE_DAY);

browser/experiments/test/xpcshell/test_healthreport.js

@@ -9,6 +9,8 @@ Cu.import("resource:///modules/experiments/Experiments.jsm");
 Cu.import("resource://testing-common/services/healthreport/utils.jsm");
 Cu.import("resource://testing-common/services-common/logging.js");
 
+const kMeasurementVersion = 2;
+
 function getStorageAndProvider(name) {
   return Task.spawn(function* get() {
     let storage = yield Metrics.Storage(name);
@@ -53,7 +55,7 @@ add_task(function* test_collect() {
 
   // Initial state should not report anything.
   yield provider.collectDailyData();
-  let m = provider.getMeasurement("info", 1);
+  let m = provider.getMeasurement("info", kMeasurementVersion);
   let values = yield m.getValues();
   Assert.equal(values.days.size, 0, "Have no data if no experiments known.");
 
@@ -69,6 +71,8 @@ add_task(function* test_collect() {
   let day = values.days.getDay(now);
   Assert.ok(day.has("lastActive"), "Has lastActive field.");
   Assert.equal(day.get("lastActive"), "id2", "Last active ID is sane.");
+  Assert.strictEqual(day.get("lastActiveBranch"), undefined,
+		     "no branch should be set yet");
 
   // Making an experiment active replaces the lastActive value.
   replaceExperiments(provider._experiments, FAKE_EXPERIMENTS_1);
@@ -76,6 +80,8 @@ add_task(function* test_collect() {
   values = yield m.getValues();
   day = values.days.getDay(now);
   Assert.equal(day.get("lastActive"), "id1", "Last active ID is the active experiment.");
+  Assert.equal(day.get("lastActiveBranch"), "foo",
+	       "Experiment branch should be visible");
 
   // And make sure the observer works.
   replaceExperiments(provider._experiments, FAKE_EXPERIMENTS_2);

services/healthreport/docs/dataformat.rst

@@ -1639,13 +1639,22 @@ lastActive
     ID of the final Telemetry Experiment that is active on a given day, if any.
 
 
+Version 2
+^^^^^^^^^
+
+Adds an additional optional property:
+
+lastActiveBranch
+    If the experiment uses branches, the branch identifier string.
+
 Example
 ^^^^^^^
 
 ::
 
     "org.mozilla.experiments.info": {
-      "_v": 1,
-      "lastActive": "some.experiment.id"
+      "_v": 2,
+      "lastActive": "some.experiment.id",
+      "lastActiveBranch": "control"
     }
 

toolkit/components/telemetry/TelemetryPing.jsm

@@ -535,9 +535,11 @@ let Impl = {
     try {
       let scope = {};
       Cu.import("resource:///modules/experiments/Experiments.jsm", scope);
-      let activeExperiment = scope.Experiments.instance().getActiveExperimentID();
+      let experiments = scope.Experiments.instance()
+      let activeExperiment = experiments.getActiveExperimentID();
       if (activeExperiment) {
         ret.activeExperiment = activeExperiment;
+	ret.activeExperimentBranch = experiments.getActiveExperimentBranch();
       }
     } catch(e) {
       // If this is not Firefox, the import will fail.