diff --git a/toolkit/components/telemetry/TelemetryEnvironment.jsm b/toolkit/components/telemetry/TelemetryEnvironment.jsm index ed81c897c538..d805c4c295a5 100644 --- a/toolkit/components/telemetry/TelemetryEnvironment.jsm +++ b/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) { - return getGlobal().setExperimentActive(id, branch); + setExperimentActive(id, branch, options={}) { + 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); diff --git a/toolkit/components/telemetry/docs/collection/experiments.rst b/toolkit/components/telemetry/docs/collection/experiments.rst index 14ece210d24c..1e1b58b642aa 100644 --- a/toolkit/components/telemetry/docs/collection/experiments.rst +++ b/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. diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js b/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js index 6f7b2b116e07..a4d8f9df26c4 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js +++ b/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(EXPERIMENT2, EXPERIMENT2_BRANCH, data); + checkExperiment(data, EXPERIMENT1, EXPERIMENT1_BRANCH); + 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(EXPERIMENT2, EXPERIMENT2_BRANCH, eventEnvironmentData); + checkExperiment(eventEnvironmentData, EXPERIMENT1, EXPERIMENT1_BRANCH); + 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() {