Bug 1594035 - Don't send `null` enrollmentIds for Normandy telemetry r=rhelmer

Differential Revision: https://phabricator.services.mozilla.com/D54834 --HG-- extra : moz-landing-system : lando
2019-11-26 23:43:42 +00:00 · 2019-11-26 23:43:42 +00:00 · 5096499a64
--- a/toolkit/components/normandy/actions/AddonRollbackAction.jsm
+++ b/toolkit/components/normandy/actions/AddonRollbackAction.jsm
@ -51,7 +51,12 @@ class AddonRollbackAction extends BaseAction {
              "unenrollFailed",
              "addon_rollback",
              rolloutSlug,
-              { reason: "uninstall-failed", enrollmentId: rollout.enrollmentId }
+              {
+                reason: "uninstall-failed",
+                enrollmentId:
+                  rollout.enrollmentId ||
+                  TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
+              }
            );
            throw err;
          }
@ -65,7 +70,8 @@ class AddonRollbackAction extends BaseAction {

        TelemetryEvents.sendEvent("unenroll", "addon_rollback", rolloutSlug, {
          reason: "rollback",
-          enrollmentId: rollout.enrollmentId,
+          enrollmentId:
+            rollout.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
        });
        TelemetryEnvironment.setExperimentInactive(rolloutSlug);
        break;
--- a/toolkit/components/normandy/actions/AddonRolloutAction.jsm
+++ b/toolkit/components/normandy/actions/AddonRolloutAction.jsm
@ -118,7 +118,9 @@ class AddonRolloutAction extends BaseAction {
      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;
@ -129,7 +131,10 @@ class AddonRolloutAction extends BaseAction {

      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
      }
@ -139,7 +144,10 @@ class AddonRolloutAction extends BaseAction {
        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
      }
@ -168,7 +176,7 @@ class AddonRolloutAction extends BaseAction {
          recipeId: recipe.id,
          state: AddonRollouts.STATE_ACTIVE,
          slug,
-          enrollmentId,
+          enrollmentId: enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
          ...details,
        });
      }
@ -211,7 +219,7 @@ class AddonRolloutAction extends BaseAction {
    TelemetryEvents.sendEvent(eventName, "addon_rollout", slug, {
      addonId: installedId,
      addonVersion: installedVersion,
-      enrollmentId,
+      enrollmentId: enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
    });
  }

--- a/toolkit/components/normandy/actions/BranchedAddonStudyAction.jsm
+++ b/toolkit/components/normandy/actions/BranchedAddonStudyAction.jsm
@ -353,7 +353,7 @@ class BranchedAddonStudyAction extends BaseStudyAction {
        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(
@ -435,13 +435,13 @@ class BranchedAddonStudyAction extends BaseStudyAction {
        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,
    });
  }

@ -474,7 +474,8 @@ class BranchedAddonStudyAction extends BaseStudyAction {
      error = new AddonStudyUpdateError(slug, {
        branch: branch.slug,
        reason: "addon-id-mismatch",
-        enrollmentId: study.enrollmentId,
+        enrollmentId:
+          study.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
      });
    }

@ -486,7 +487,8 @@ class BranchedAddonStudyAction extends BaseStudyAction {
      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
@ -507,7 +509,8 @@ class BranchedAddonStudyAction extends BaseStudyAction {
          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
@ -516,7 +519,8 @@ class BranchedAddonStudyAction extends BaseStudyAction {
          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
@ -556,7 +560,10 @@ class BranchedAddonStudyAction extends BaseStudyAction {
      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
@ -564,7 +571,8 @@ class BranchedAddonStudyAction extends BaseStudyAction {
      addonId: installedId,
      addonVersion: installedVersion,
      branch: branch.slug,
-      enrollmentId: study.enrollmentId,
+      enrollmentId:
+        study.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
    });
  }

--- a/toolkit/components/normandy/actions/PreferenceRollbackAction.jsm
+++ b/toolkit/components/normandy/actions/PreferenceRollbackAction.jsm
@ -61,7 +61,11 @@ class PreferenceRollbackAction extends BaseAction {
          "unenroll",
          "preference_rollback",
          rolloutSlug,
-          { reason: "rollback", enrollmentId: rollout.enrollmentId }
+          {
+            reason: "rollback",
+            enrollmentId:
+              rollout.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
+          }
        );
        TelemetryEnvironment.setExperimentInactive(rolloutSlug);
        break;
@ -76,7 +80,11 @@ class PreferenceRollbackAction extends BaseAction {
          "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}`
--- a/toolkit/components/normandy/actions/PreferenceRolloutAction.jsm
+++ b/toolkit/components/normandy/actions/PreferenceRolloutAction.jsm
@ -81,7 +81,9 @@ class PreferenceRolloutAction extends BaseAction {
        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) {
@ -143,10 +145,10 @@ class PreferenceRolloutAction extends BaseAction {
      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,
      });
    }
  }
--- a/toolkit/components/normandy/lib/AddonStudies.jsm
+++ b/toolkit/components/normandy/lib/AddonStudies.jsm
@ -352,7 +352,8 @@ var AddonStudies = {
      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);

--- a/toolkit/components/normandy/lib/PreferenceExperiments.jsm
+++ b/toolkit/components/normandy/lib/PreferenceExperiments.jsm
@ -275,7 +275,8 @@ var PreferenceExperiments = {
        experiment.branch,
        {
          type: EXPERIMENT_TYPE_PREFIX + experiment.experimentType,
-          enrollmentId: experiment.enrollmentId,
+          enrollmentId:
+            experiment.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
        }
      );

@ -551,12 +552,12 @@ var PreferenceExperiments = {

    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();

@ -712,7 +713,11 @@ var PreferenceExperiments = {
        "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`
@ -764,7 +769,8 @@ var PreferenceExperiments = {
      didResetValue: resetValue ? "true" : "false",
      branch: experiment.branch,
      reason,
-      enrollmentId: experiment.enrollmentId,
+      enrollmentId:
+        experiment.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
    });
    await this.saveStartupPrefs();
  },
--- a/toolkit/components/normandy/lib/PreferenceRollouts.jsm
+++ b/toolkit/components/normandy/lib/PreferenceRollouts.jsm
@ -156,7 +156,8 @@ var PreferenceRollouts = {
          "preference_rollout",
          rollout.slug,
          {
-            enrollmentId: rollout.enrollmentId,
+            enrollmentId:
+              rollout.enrollmentId || TelemetryEvents.NO_ENROLLMENT_ID_MARKER,
          }
        );
      }
@ -172,7 +173,8 @@ var PreferenceRollouts = {
    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,
      });
    }
  },
--- a/toolkit/components/normandy/lib/TelemetryEvents.jsm
+++ b/toolkit/components/normandy/lib/TelemetryEvents.jsm
@ -10,11 +10,20 @@ 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,