Bug 1450090 - Add study branch to preference study unenroll telemetry event r?Gijs draft
authorMike Cooper <mcooper@mozilla.com>
Wed, 16 May 2018 11:14:38 -0700
changeset 795846 f719268d909a8ab9fe8abf219b466d332474f2e6
parent 795657 3c9d69736f4a421218e5eb01b6571d535d38318a
push id110102
push userbmo:mcooper@mozilla.com
push dateWed, 16 May 2018 18:19:08 +0000
reviewersGijs
bugs1450090
milestone62.0a1
Bug 1450090 - Add study branch to preference study unenroll telemetry event r?Gijs MozReview-Commit-ID: FSLb8TSQhL2
toolkit/components/normandy/lib/PreferenceExperiments.jsm
toolkit/components/normandy/lib/TelemetryEvents.jsm
toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
--- a/toolkit/components/normandy/lib/PreferenceExperiments.jsm
+++ b/toolkit/components/normandy/lib/PreferenceExperiments.jsm
@@ -507,16 +507,17 @@ var PreferenceExperiments = {
     }
 
     experiment.expired = true;
     store.saveSoon();
 
     TelemetryEnvironment.setExperimentInactive(experimentName);
     TelemetryEvents.sendEvent("unenroll", "preference_study", experimentName, {
       didResetValue: resetValue ? "true" : "false",
+      branch: experiment.branch,
       reason,
     });
     await this.saveStartupPrefs();
   },
 
   /**
    * Get the experiment object for the named experiment.
    * @param {string} experimentName
--- a/toolkit/components/normandy/lib/TelemetryEvents.jsm
+++ b/toolkit/components/normandy/lib/TelemetryEvents.jsm
@@ -31,17 +31,17 @@ const TelemetryEvents = {
       objects: ["preference_rollout"],
       extra_keys: ["previousState"],
       record_on_release: true,
     },
 
     unenroll: {
       methods: ["unenroll"],
       objects: ["preference_study", "addon_study", "preference_rollback"],
-      extra_keys: ["reason", "didResetValue", "addonId", "addonVersion"],
+      extra_keys: ["reason", "didResetValue", "addonId", "addonVersion", "branch"],
       record_on_release: true,
     },
 
     unenroll_failed: {
       methods: ["unenrollFailed"],
       objects: ["preference_rollback"],
       extra_keys: ["reason"],
       record_on_release: true,
--- a/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
+++ b/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
@@ -415,16 +415,17 @@ decorate_task(
     // sure that tests clean up correctly.
     is(Preferences.get("fake.preference"), null, "preference should start unset");
 
     mockPreferences.set(`${startupPrefs}.fake.preference`, "experimentvalue", "user");
     mockPreferences.set("fake.preference", "experimentvalue", "default");
     experiments.test = experimentFactory({
       name: "test",
       expired: false,
+      branch: "fakebranch",
       preferenceName: "fake.preference",
       preferenceValue: "experimentvalue",
       preferenceType: "string",
       previousPreferenceValue: "oldvalue",
       preferenceBranchType: "default",
     });
     PreferenceExperiments.startObserver("test", "fake.preference", "string", "experimentvalue");
 
@@ -437,21 +438,22 @@ decorate_task(
       "stop reverted the preference to its previous value",
     );
     ok(
       !Services.prefs.prefHasUserValue(`${startupPrefs}.fake.preference`),
       "stop cleared the startup preference for fake.preference.",
     );
 
     Assert.deepEqual(
-      sendEventStub.getCall(0).args,
-      ["unenroll", "preference_study", experiments.test.name, {
+      sendEventStub.args,
+      [["unenroll", "preference_study", experiments.test.name, {
         didResetValue: "true",
         reason: "test-reason",
-      }],
+        branch: "fakebranch",
+      }]],
       "stop should send the correct telemetry event"
     );
 
     PreferenceExperiments.stopAllObservers();
   },
 );
 
 // stop should also support user pref experiments
@@ -521,35 +523,37 @@ decorate_task(
   withMockPreferences,
   withStub(PreferenceExperiments, "stopObserver"),
   withSendEventStub,
   async function testStopReset(experiments, mockPreferences, stopObserverStub, sendEventStub) {
     mockPreferences.set("fake.preference", "customvalue", "default");
     experiments.test = experimentFactory({
       name: "test",
       expired: false,
+      branch: "fakebranch",
       preferenceName: "fake.preference",
       preferenceValue: "experimentvalue",
       preferenceType: "string",
       previousPreferenceValue: "oldvalue",
       peferenceBranchType: "default",
     });
 
     await PreferenceExperiments.stop("test", {reason: "test-reason", resetValue: false});
     is(
       DefaultPreferences.get("fake.preference"),
       "customvalue",
       "stop did not modify the preference",
     );
     Assert.deepEqual(
-      sendEventStub.getCall(0).args,
-      ["unenroll", "preference_study", experiments.test.name, {
+      sendEventStub.args,
+      [["unenroll", "preference_study", experiments.test.name, {
         didResetValue: "false",
         reason: "test-reason",
-      }],
+        branch: "fakebranch"
+      }]],
       "stop should send the correct telemetry event"
     );
   }
 );
 
 // get should throw if no experiment exists with the given name
 decorate_task(
   withMockExperiments,
@@ -716,31 +720,29 @@ decorate_task(
       setActiveStub.getCall(0).args,
       ["test", "branch", {type: "normandy-exp"}],
       "Experiment is registered by start()",
     );
     await PreferenceExperiments.stop("test", {reason: "test-reason"});
     Assert.deepEqual(setInactiveStub.args, [["test"]], "Experiment is unregistered by stop()");
 
     Assert.deepEqual(
-      sendEventStub.getCall(0).args,
-      ["enroll", "preference_study", "test", {
-        experimentType: "exp",
-        branch: "branch",
-      }],
-      "PreferenceExperiments.start() should send the correct telemetry event"
-    );
-
-    Assert.deepEqual(
-      sendEventStub.getCall(1).args,
-      ["unenroll", "preference_study", "test", {
-        reason: "test-reason",
-        didResetValue: "true",
-      }],
-      "PreferenceExperiments.stop() should send the correct telemetry event"
+      sendEventStub.args,
+      [
+        ["enroll", "preference_study", "test", {
+          experimentType: "exp",
+          branch: "branch",
+        }],
+        ["unenroll", "preference_study", "test", {
+          reason: "test-reason",
+          didResetValue: "true",
+          branch: "branch",
+        }],
+      ],
+      "PreferenceExperiments.start() and stop() should send the correct telemetry event"
     );
   },
 );
 
 // starting experiments should use the provided experiment type
 decorate_task(
   withMockExperiments,
   withStub(TelemetryEnvironment, "setExperimentActive"),
@@ -1051,32 +1053,34 @@ decorate_task(
   withSendEventStub,
   withMockExperiments,
   async function testPrefChangeEventTelemetry(mockPreferences, sendEventStub, mockExperiments) {
     is(Preferences.get("fake.preference"), null, "preference should start unset");
     mockPreferences.set("fake.preference", "oldvalue", "default");
     mockExperiments.test = experimentFactory({
       name: "test",
       expired: false,
+      branch: "fakebranch",
       preferenceName: "fake.preference",
       preferenceValue: "experimentvalue",
       preferenceType: "string",
       previousPreferenceValue: "oldvalue",
       preferenceBranchType: "default",
     });
     PreferenceExperiments.startObserver("test", "fake.preference", "string", "experimentvalue");
 
     // setting the preference on the user branch should trigger the observer to stop the experiment
     mockPreferences.set("fake.preference", "uservalue", "user");
 
     // let the event loop tick to run the observer
     await Promise.resolve();
 
     Assert.deepEqual(
-      sendEventStub.getCall(0).args,
-      ["unenroll", "preference_study", "test", {
+      sendEventStub.args,
+      [["unenroll", "preference_study", "test", {
         didResetValue: "false",
         reason: "user-preference-changed",
-      }],
+        branch: "fakebranch",
+      }]],
       "stop should send a telemetry event indicating the user unenrolled manually",
     );
   },
 );