Bug 1376599 - Allow annotating experiments with a type. r=Dexter

--HG--
extra : rebase_source : b62b3a3c0a3b507190e8ccddb4721555254555ee

toolkit/components/telemetry/TelemetryEnvironment.jsm

@@ -43,6 +43,7 @@ const MAX_ATTRIBUTION_STRING_LENGTH = 100;
 // The maximum lengths for the experiment id and branch in the experiments section.
 const MAX_EXPERIMENT_ID_LENGTH = 100;
 const MAX_EXPERIMENT_BRANCH_LENGTH = 100;
+const MAX_EXPERIMENT_TYPE_LENGTH = 20;
 
 /**
  * This is a policy object used to override behavior for testing.
@@ -87,9 +88,11 @@ this.TelemetryEnvironment = {
    *
    * @param {String} id The id of the active experiment.
    * @param {String} branch The experiment branch.
+   * @param {Object} [options] Optional object with options.
+   * @param {String} [options.type=false] The specific experiment type.
    */
-  setExperimentActive(id, branch) {
+  setExperimentActive(id, branch, options={}) {
-    return getGlobal().setExperimentActive(id, branch);
+    return getGlobal().setExperimentActive(id, branch, options);
   },
 
   /**
@@ -892,7 +895,7 @@ EnvironmentCache.prototype = {
     this._changeListeners.delete(name);
   },
 
-  setExperimentActive(id, branch) {
+  setExperimentActive(id, branch, options) {
     this._log.trace("setExperimentActive");
     // Make sure both the id and the branch have sane lengths.
     const saneId = limitStringToLength(id, MAX_EXPERIMENT_ID_LENGTH);
@@ -907,10 +910,22 @@ EnvironmentCache.prototype = {
       this._log.warn("setExperimentActive - the experiment id or branch were truncated.");
     }
 
+    // Truncate the experiment type if present.
+    if (options.hasOwnProperty("type")) {
+      let type = limitStringToLength(options.type, MAX_EXPERIMENT_TYPE_LENGTH);
+      if (type.length != options.type.length) {
+        options.type = type;
+        this._log.warn("setExperimentActive - the experiment type was truncated.");
+      }
+    }
+
     let oldEnvironment = Cu.cloneInto(this._currentEnvironment, myScope);
     // Add the experiment annotation.
     let experiments = this._currentEnvironment.experiments || {};
     experiments[saneId] = { "branch": saneBranch };
+    if (options.hasOwnProperty("type")) {
+      experiments[saneId].type = options.type;
+    }
     this._currentEnvironment.experiments = experiments;
     // Notify of the change.
     this._onEnvironmentChange("experiment-annotation-changed", oldEnvironment);

toolkit/components/telemetry/docs/collection/experiments.rst

@@ -9,11 +9,12 @@ The JS API
 ==========
 Privileged JavaScript code can annotate experiments using the functions exposed by ``TelemetryEnvironment.jsm``.
 
-The following function adds an annotation to the environment for the provided ``id`` and ``branch``. Calling this function repeatedly with the same ``id`` will overwrite the state and trigger new subsessions (subject to throttling).
+The following function adds an annotation to the environment for the provided ``id``, ``branch`` and ``options``. Calling this function repeatedly with the same ``id`` will overwrite the state and trigger new subsessions (subject to throttling).
+``options`` is an object that currently may contain ``type``, to tag the experiment with a specific type.
 
 .. code-block:: js
 
-    TelemetryEnvironment.setExperimentActive(id, branch)
+    TelemetryEnvironment.setExperimentActive(id, branch, [options={}}])
 
 This removes the annotation for the experiment with the provided ``id``.
 
@@ -36,3 +37,4 @@ Limits and restrictions
 -----------------------
 To prevent abuses, the content of both the experiment ``id`` and ``branch`` is limited to
 100 characters in length.
+``type`` is limited to a lenght of 20 characters.

toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js

@@ -809,6 +809,9 @@ function checkExperimentsSection(data) {
     let experimentData = experiments[id];
     Assert.ok("branch" in experimentData, "The experiment must have branch data.")
     Assert.ok(checkString(experimentData.branch), "The experiment data must be valid.");
+    if ("type" in experimentData) {
+      Assert.ok(checkString(experimentData.type));
+    }
   }
 }
 
@@ -1666,7 +1669,7 @@ add_task(async function test_experimentsAPI() {
   const EXPERIMENT2 = "experiment-2";
   const EXPERIMENT2_BRANCH = "other-branch";
 
-  let checkExperiment = (id, branch, environmentData) => {
+  let checkExperiment = (environmentData, id, branch, type=null) => {
     Assert.ok("experiments" in environmentData,
               "The current environment must report the experiment annotations.");
     Assert.ok(id in environmentData.experiments,
@@ -1701,7 +1704,7 @@ add_task(async function test_experimentsAPI() {
   // Check that the current environment contains the right experiment.
   data = TelemetryEnvironment.currentEnvironment;
   checkEnvironmentData(data);
-  checkExperiment(EXPERIMENT1, EXPERIMENT1_BRANCH, data);
+  checkExperiment(data, EXPERIMENT1, EXPERIMENT1_BRANCH);
 
   TelemetryEnvironment.unregisterChangeListener("test_experimentsAPI");
 
@@ -1716,11 +1719,11 @@ add_task(async function test_experimentsAPI() {
   // Check that the current environment contains both the experiment.
   data = TelemetryEnvironment.currentEnvironment;
   checkEnvironmentData(data);
-  checkExperiment(EXPERIMENT1, EXPERIMENT1_BRANCH, data);
+  checkExperiment(data, EXPERIMENT1, EXPERIMENT1_BRANCH);
-  checkExperiment(EXPERIMENT2, EXPERIMENT2_BRANCH, data);
+  checkExperiment(data, EXPERIMENT2, EXPERIMENT2_BRANCH);
 
   // The previous environment should only contain the first experiment.
-  checkExperiment(EXPERIMENT1, EXPERIMENT1_BRANCH, eventEnvironmentData);
+  checkExperiment(eventEnvironmentData, EXPERIMENT1, EXPERIMENT1_BRANCH);
   Assert.ok(!(EXPERIMENT2 in eventEnvironmentData),
             "The old environment must not contain the new experiment annotation.");
 
@@ -1752,13 +1755,16 @@ add_task(async function test_experimentsAPI() {
   checkEnvironmentData(data);
   Assert.ok(!(EXPERIMENT1 in data),
             "The current environment must not contain the removed experiment annotation.");
-  checkExperiment(EXPERIMENT2, EXPERIMENT2_BRANCH, data);
+  checkExperiment(data, EXPERIMENT2, EXPERIMENT2_BRANCH);
 
   // The previous environment should contain both annotations.
-  checkExperiment(EXPERIMENT1, EXPERIMENT1_BRANCH, eventEnvironmentData);
+  checkExperiment(eventEnvironmentData, EXPERIMENT1, EXPERIMENT1_BRANCH);
-  checkExperiment(EXPERIMENT2, EXPERIMENT2_BRANCH, eventEnvironmentData);
+  checkExperiment(eventEnvironmentData, EXPERIMENT2, EXPERIMENT2_BRANCH);
 
-  TelemetryEnvironment.unregisterChangeListener("test_experimentsAPI5");
+  // Set an experiment with a type and check that it correctly shows up.
+  TelemetryEnvironment.setExperimentActive("typed-experiment", "random-branch", {type: "ab-test"});
+  data = TelemetryEnvironment.currentEnvironment;
+  checkExperiment(data, "typed-experiment", "random-branch", "ab-test");
 });
 
 add_task(async function test_experimentsAPI_limits() {
@@ -1800,6 +1806,12 @@ add_task(async function test_experimentsAPI_limits() {
             "The experiments must be reporting the truncated branch.");
 
   TelemetryEnvironment.unregisterChangeListener("test_experimentsAPI");
+
+  // Check that an overly long type is truncated.
+  const longType = "a0123456678901234567890123456789";
+  TelemetryEnvironment.setExperimentActive("exp", "some-branch", {type: longType});
+  data = TelemetryEnvironment.currentEnvironment;
+  Assert.equal(data.experiments["exp"].type, longType.substring(0, 20));
 });
 
 add_task(async function test_environmentShutdown() {