Bug 1594035 - Don't send `null` enrollmentIds for Normandy telemetry r=rhelmer a=pascalc
authorMichael Cooper <mcooper@mozilla.com>
Tue, 26 Nov 2019 23:43:42 +0000
changeset 563494 9aaa2cc3ad5d439793ad13b24ae06786cbf82bf8
parent 563493 20b673a3bcd7678de723fcbc2fd729b183e00593
child 563495 b4717df2d39ff77a9d991d08657049d30494f0e4
push id2201
push usershindli@mozilla.com
push dateThu, 28 Nov 2019 10:02:20 +0000
treeherdermozilla-release@9aaa2cc3ad5d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrhelmer, pascalc
bugs1594035
milestone71.0
Bug 1594035 - Don't send `null` enrollmentIds for Normandy telemetry r=rhelmer a=pascalc Differential Revision: https://phabricator.services.mozilla.com/D54834
toolkit/components/normandy/actions/AddonRollbackAction.jsm
toolkit/components/normandy/actions/AddonRolloutAction.jsm
toolkit/components/normandy/actions/BranchedAddonStudyAction.jsm
toolkit/components/normandy/actions/PreferenceRollbackAction.jsm
toolkit/components/normandy/actions/PreferenceRolloutAction.jsm
toolkit/components/normandy/lib/AddonStudies.jsm
toolkit/components/normandy/lib/PreferenceExperiments.jsm
toolkit/components/normandy/lib/PreferenceRollouts.jsm
toolkit/components/normandy/lib/TelemetryEvents.jsm
--- a/toolkit/components/normandy/actions/AddonRollbackAction.jsm
+++ b/toolkit/components/normandy/actions/AddonRollbackAction.jsm
@@ -46,31 +46,37 @@ class AddonRollbackAction extends BaseAc
         if (addon) {
           try {
             await addon.uninstall();
           } catch (err) {
             TelemetryEvents.sendEvent(
               "unenrollFailed",
               "addon_rollback",
               rolloutSlug,
-              { reason: "uninstall-failed", enrollmentId: rollout.enrollmentId }
+              {
+                reason: "uninstall-failed",
+                enrollmentId:
+                  rollout.enrollmentId ||
+                  TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
+              }
             );
             throw err;
           }
         } else {
           this.log.warn(
             `Could not uninstall addon ${
               rollout.addonId
             } for rollback ${rolloutSlug}: it is not installed.`
           );
         }
 
         TelemetryEvents.sendEvent("unenroll", "addon_rollback", rolloutSlug, {
           reason: "rollback",
-          enrollmentId: rollout.enrollmentId,
+          enrollmentId:
+            rollout.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
         });
         TelemetryEnvironment.setExperimentInactive(rolloutSlug);
         break;
       }
 
       case AddonRollouts.STATE_ROLLED_BACK: {
         return; // Do nothing
       }
--- a/toolkit/components/normandy/actions/AddonRolloutAction.jsm
+++ b/toolkit/components/normandy/actions/AddonRolloutAction.jsm
@@ -113,38 +113,46 @@ class AddonRolloutAction extends BaseAct
       rollout =>
         rollout.slug !== slug &&
         rollout.addonId === extensionDetails.extension_id
     );
     if (conflictingRollout) {
       const conflictError = createError("conflict", {
         addonId: conflictingRollout.addonId,
         conflictingSlug: conflictingRollout.slug,
-        enrollmentId: conflictingRollout.enrollmentId,
+        enrollmentId:
+          conflictingRollout.enrollmentId ||
+          TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
       });
       this.reportError(conflictError, "enrollFailed");
       throw conflictError;
     }
 
     const onInstallStarted = (install, installDeferred) => {
       const existingAddon = install.existingAddon;
 
       if (existingRollout && existingRollout.addonId !== install.addon.id) {
         installDeferred.reject(
-          createError("addon-id-changed", { enrollmentId })
+          createError("addon-id-changed", {
+            enrollmentId:
+              enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
+          })
         );
         return false; // cancel the upgrade, the add-on ID has changed
       }
 
       if (
         existingAddon &&
         Services.vc.compare(existingAddon.version, install.addon.version) > 0
       ) {
         installDeferred.reject(
-          createError("upgrade-required", { enrollmentId })
+          createError("upgrade-required", {
+            enrollmentId:
+              enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
+          })
         );
         return false; // cancel the installation, must be an upgrade
       }
 
       return true;
     };
 
     const applyNormandyChanges = async install => {
@@ -163,17 +171,17 @@ class AddonRolloutAction extends BaseAct
           ...details,
         });
       } else {
         enrollmentId = NormandyUtils.generateUuid();
         await AddonRollouts.add({
           recipeId: recipe.id,
           state: AddonRollouts.STATE_ACTIVE,
           slug,
-          enrollmentId,
+          enrollmentId: enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
           ...details,
         });
       }
     };
 
     const undoNormandyChanges = async () => {
       if (existingRollout) {
         await AddonRollouts.update(existingRollout);
@@ -206,17 +214,17 @@ class AddonRolloutAction extends BaseAct
         }
       );
     }
 
     // All done, report success to Telemetry
     TelemetryEvents.sendEvent(eventName, "addon_rollout", slug, {
       addonId: installedId,
       addonVersion: installedVersion,
-      enrollmentId,
+      enrollmentId: enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
     });
   }
 
   reportError(error, eventName) {
     if (error instanceof AddonRolloutError) {
       // One of our known errors. Report it nicely to telemetry
       TelemetryEvents.sendEvent(
         eventName,
--- a/toolkit/components/normandy/actions/BranchedAddonStudyAction.jsm
+++ b/toolkit/components/normandy/actions/BranchedAddonStudyAction.jsm
@@ -348,17 +348,17 @@ class BranchedAddonStudyAction extends B
         throw err;
       }
 
       // All done, report success to Telemetry
       TelemetryEvents.sendEvent("enroll", "addon_study", slug, {
         addonId: AddonStudies.NO_ADDON_MARKER,
         addonVersion: AddonStudies.NO_ADDON_MARKER,
         branch: branch.slug,
-        enrollmentId,
+        enrollmentId: enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
       });
     } else {
       const extensionDetails = await NormandyApi.fetchExtensionDetails(
         branch.extensionApiId
       );
 
       const onInstallStarted = installDeferred => cbInstall => {
         const versionMatches =
@@ -430,23 +430,23 @@ class BranchedAddonStudyAction extends B
         reportError: this.reportEnrollError,
       });
 
       // All done, report success to Telemetry
       TelemetryEvents.sendEvent("enroll", "addon_study", slug, {
         addonId: installedId,
         addonVersion: installedVersion,
         branch: branch.slug,
-        enrollmentId,
+        enrollmentId: enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
       });
     }
 
     TelemetryEnvironment.setExperimentActive(slug, branch.slug, {
       type: "normandy-addonstudy",
-      enrollmentId,
+      enrollmentId: enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
     });
   }
 
   /**
    * Update the study represented by the given recipe.
    * @param recipe Object describing the study to be updated.
    * @param extensionDetails Object describing the addon to be installed.
    */
@@ -469,29 +469,31 @@ class BranchedAddonStudyAction extends B
     );
 
     let error;
 
     if (study.addonId && study.addonId !== extensionDetails.extension_id) {
       error = new AddonStudyUpdateError(slug, {
         branch: branch.slug,
         reason: "addon-id-mismatch",
-        enrollmentId: study.enrollmentId,
+        enrollmentId:
+          study.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
       });
     }
 
     const versionCompare = Services.vc.compare(
       study.addonVersion,
       extensionDetails.version
     );
     if (versionCompare > 0) {
       error = new AddonStudyUpdateError(slug, {
         branch: branch.slug,
         reason: "no-downgrade",
-        enrollmentId: study.enrollmentId,
+        enrollmentId:
+          study.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
       });
     } else if (versionCompare === 0) {
       return; // Unchanged, do nothing
     }
 
     if (error) {
       this.reportUpdateError(error);
       throw error;
@@ -502,26 +504,28 @@ class BranchedAddonStudyAction extends B
         cbInstall.addon.version === extensionDetails.version;
       const idMatches = cbInstall.addon.id === extensionDetails.extension_id;
 
       if (!cbInstall.existingAddon) {
         installDeferred.reject(
           new AddonStudyUpdateError(slug, {
             branch: branch.slug,
             reason: "addon-does-not-exist",
-            enrollmentId: study.enrollmentId,
+            enrollmentId:
+              study.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
           })
         );
         return false; // cancel the installation, must upgrade an existing add-on
       } else if (!versionMatches || !idMatches) {
         installDeferred.reject(
           new AddonStudyUpdateError(slug, {
             branch: branch.slug,
             reason: "metadata-mismatch",
-            enrollmentId: study.enrollmentId,
+            enrollmentId:
+              study.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
           })
         );
         return false; // cancel the installation, server metadata do not match downloaded add-on
       }
 
       return true;
     };
 
@@ -551,25 +555,29 @@ class BranchedAddonStudyAction extends B
       recipe,
       extensionDetails,
       branchSlug: branch.slug,
       onInstallStarted,
       onComplete,
       onFailedInstall,
       errorClass: AddonStudyUpdateError,
       reportError: this.reportUpdateError,
-      errorExtra: { enrollmentId: study.enrollmentId },
+      errorExtra: {
+        enrollmentId:
+          study.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
+      },
     });
 
     // All done, report success to Telemetry
     TelemetryEvents.sendEvent("update", "addon_study", slug, {
       addonId: installedId,
       addonVersion: installedVersion,
       branch: branch.slug,
-      enrollmentId: study.enrollmentId,
+      enrollmentId:
+        study.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
     });
   }
 
   reportEnrollError(error) {
     if (error instanceof AddonStudyEnrollError) {
       // One of our known errors. Report it nicely to telemetry
       TelemetryEvents.sendEvent(
         "enrollFailed",
--- a/toolkit/components/normandy/actions/PreferenceRollbackAction.jsm
+++ b/toolkit/components/normandy/actions/PreferenceRollbackAction.jsm
@@ -56,32 +56,40 @@ class PreferenceRollbackAction extends B
         for (const { preferenceName, previousValue } of rollout.preferences) {
           PrefUtils.setPref("default", preferenceName, previousValue);
         }
         await PreferenceRollouts.update(rollout);
         TelemetryEvents.sendEvent(
           "unenroll",
           "preference_rollback",
           rolloutSlug,
-          { reason: "rollback", enrollmentId: rollout.enrollmentId }
+          {
+            reason: "rollback",
+            enrollmentId:
+              rollout.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
+          }
         );
         TelemetryEnvironment.setExperimentInactive(rolloutSlug);
         break;
       }
       case PreferenceRollouts.STATE_ROLLED_BACK: {
         // The rollout has already been rolled back, so nothing to do here.
         break;
       }
       case PreferenceRollouts.STATE_GRADUATED: {
         // graduated rollouts can't be rolled back
         TelemetryEvents.sendEvent(
           "unenrollFailed",
           "preference_rollback",
           rolloutSlug,
-          { reason: "graduated", enrollmentId: rollout.enrollmentId }
+          {
+            reason: "graduated",
+            enrollmentId:
+              rollout.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
+          }
         );
         throw new Error(
           `Cannot rollback already graduated rollout ${rolloutSlug}`
         );
       }
       default: {
         throw new Error(
           `Unexpected state when rolling back ${rolloutSlug}: ${rollout.state}`
--- a/toolkit/components/normandy/actions/PreferenceRolloutAction.jsm
+++ b/toolkit/components/normandy/actions/PreferenceRolloutAction.jsm
@@ -76,17 +76,19 @@ class PreferenceRolloutAction extends Ba
         newRollout
       );
 
       // If anything was different about the new rollout, write it to the db and send an event about it
       if (anyChanged) {
         await PreferenceRollouts.update(newRollout);
         TelemetryEvents.sendEvent("update", "preference_rollout", args.slug, {
           previousState: existingRollout.state,
-          enrollmentId: existingRollout.enrollmentId,
+          enrollmentId:
+            existingRollout.enrollmentId ||
+            TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
         });
 
         switch (existingRollout.state) {
           case PreferenceRollouts.STATE_ACTIVE: {
             this.log.debug(`Updated preference rollout ${args.slug}`);
             break;
           }
           case PreferenceRollouts.STATE_GRADUATED: {
@@ -138,20 +140,20 @@ class PreferenceRolloutAction extends Ba
 
       for (const { preferenceName, value } of args.preferences) {
         PrefUtils.setPref("default", preferenceName, value);
       }
 
       this.log.debug(`Enrolled in preference rollout ${args.slug}`);
       TelemetryEnvironment.setExperimentActive(args.slug, newRollout.state, {
         type: "normandy-prefrollout",
-        enrollmentId,
+        enrollmentId: enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
       });
       TelemetryEvents.sendEvent("enroll", "preference_rollout", args.slug, {
-        enrollmentId,
+        enrollmentId: enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
       });
     }
   }
 
   /**
    * Check that all the preferences in a rollout are ok to set. This means 1) no
    * other rollout is managing them, and 2) they match the types of the builtin
    * values.
--- a/toolkit/components/normandy/lib/AddonStudies.jsm
+++ b/toolkit/components/normandy/lib/AddonStudies.jsm
@@ -347,17 +347,18 @@ var AddonStudies = {
     await getStore(db, "readwrite").put(study);
 
     Services.obs.notifyObservers(study, STUDY_ENDED_TOPIC, `${study.recipeId}`);
     TelemetryEvents.sendEvent("unenroll", "addon_study", study.slug, {
       addonId: study.addonId || AddonStudies.NO_ADDON_MARKER,
       addonVersion: study.addonVersion || AddonStudies.NO_ADDON_MARKER,
       reason,
       branch: study.branch,
-      enrollmentId: study.enrollmentId,
+      enrollmentId:
+        study.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
     });
     TelemetryEnvironment.setExperimentInactive(study.slug);
 
     await this.callUnenrollListeners(study.addonId, reason);
   },
 
   // Maps extension id -> Set(callbacks)
   _unenrollListeners: new Map(),
--- a/toolkit/components/normandy/lib/PreferenceExperiments.jsm
+++ b/toolkit/components/normandy/lib/PreferenceExperiments.jsm
@@ -270,17 +270,18 @@ var PreferenceExperiments = {
       }
 
       // Notify Telemetry of experiments we're running, since they don't persist between restarts
       TelemetryEnvironment.setExperimentActive(
         experiment.slug,
         experiment.branch,
         {
           type: EXPERIMENT_TYPE_PREFIX + experiment.experimentType,
-          enrollmentId: experiment.enrollmentId,
+          enrollmentId:
+            experiment.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
         }
       );
 
       // Watch for changes to the experiment's preference
       this.startObserver(experiment.slug, experiment.preferences);
     }
   },
 
@@ -546,22 +547,22 @@ var PreferenceExperiments = {
       enrollmentId,
     };
 
     store.data.experiments[slug] = experiment;
     store.saveSoon();
 
     TelemetryEnvironment.setExperimentActive(slug, branch, {
       type: EXPERIMENT_TYPE_PREFIX + experimentType,
-      enrollmentId,
+      enrollmentId: enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
     });
     TelemetryEvents.sendEvent("enroll", "preference_study", slug, {
       experimentType,
       branch,
-      enrollmentId,
+      enrollmentId: enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
     });
     await this.saveStartupPrefs();
 
     return experiment;
   },
 
   /**
    * Register a preference observer that stops an experiment when the user
@@ -707,17 +708,21 @@ var PreferenceExperiments = {
     }
 
     const experiment = store.data.experiments[experimentSlug];
     if (experiment.expired) {
       TelemetryEvents.sendEvent(
         "unenrollFailed",
         "preference_study",
         experimentSlug,
-        { reason: "already-unenrolled", enrollmentId: experiment.enrollmentId }
+        {
+          reason: "already-unenrolled",
+          enrollmentId:
+            experiment.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
+        }
       );
       throw new Error(
         `Cannot stop preference experiment "${experimentSlug}" because it is already expired`
       );
     }
 
     if (PreferenceExperiments.hasObserver(experimentSlug)) {
       PreferenceExperiments.stopObserver(experimentSlug);
@@ -759,17 +764,18 @@ var PreferenceExperiments = {
     experiment.expired = true;
     store.saveSoon();
 
     TelemetryEnvironment.setExperimentInactive(experimentSlug);
     TelemetryEvents.sendEvent("unenroll", "preference_study", experimentSlug, {
       didResetValue: resetValue ? "true" : "false",
       branch: experiment.branch,
       reason,
-      enrollmentId: experiment.enrollmentId,
+      enrollmentId:
+        experiment.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
     });
     await this.saveStartupPrefs();
   },
 
   /**
    * Clone an experiment using knowledge of its structure to avoid
    * having to serialize/deserialize it.
    *
--- a/toolkit/components/normandy/lib/PreferenceRollouts.jsm
+++ b/toolkit/components/normandy/lib/PreferenceRollouts.jsm
@@ -151,33 +151,35 @@ var PreferenceRollouts = {
         rollout.state = this.STATE_GRADUATED;
         changed = true;
         log.debug(`Graduating rollout: ${rollout.slug}`);
         TelemetryEvents.sendEvent(
           "graduate",
           "preference_rollout",
           rollout.slug,
           {
-            enrollmentId: rollout.enrollmentId,
+            enrollmentId:
+              rollout.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
           }
         );
       }
 
       if (changed) {
         const db = await getDatabase();
         await getStore(db, "readwrite").put(rollout);
       }
     }
   },
 
   async init() {
     for (const rollout of await this.getAllActive()) {
       TelemetryEnvironment.setExperimentActive(rollout.slug, rollout.state, {
         type: "normandy-prefrollout",
-        enrollmentId: rollout.enrollmentId,
+        enrollmentId:
+          rollout.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
       });
     }
   },
 
   async uninit() {
     await this.saveStartupPrefs();
   },
 
--- a/toolkit/components/normandy/lib/TelemetryEvents.jsm
+++ b/toolkit/components/normandy/lib/TelemetryEvents.jsm
@@ -5,21 +5,30 @@
 
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 var EXPORTED_SYMBOLS = ["TelemetryEvents"];
 
 const TELEMETRY_CATEGORY = "normandy";
 
 const TelemetryEvents = {
+  NO_ENROLLMENT_ID_MARKER: "__NO_ENROLLMENT_ID__",
+
   init() {
     Services.telemetry.setEventRecordingEnabled(TELEMETRY_CATEGORY, true);
   },
 
   sendEvent(method, object, value, extra) {
+    for (const val of Object.values(extra)) {
+      if (val == null) {
+        throw new Error(
+          "Extra parameters in telemetry events must not be null"
+        );
+      }
+    }
     Services.telemetry.recordEvent(
       TELEMETRY_CATEGORY,
       method,
       object,
       value,
       extra
     );
   },