Bug 1489520 - Fix canary client ID on startup when Telemetry upload is enabled r=chutten
authorJan-Erik Rediger <jrediger@mozilla.com>
Mon, 17 Sep 2018 13:37:16 +0000
changeset 495155 961cfd2ee957fdba7a86f1e7881f8fcde0158da8
parent 495154 5c8ae3b994b5e903d9617939795fe2de0cdd58ae
child 495156 88b67d6b6a46325c652c3cde93dc6f21107638da
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten
bugs1489520
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1489520 - Fix canary client ID on startup when Telemetry upload is enabled r=chutten Differential Revision: https://phabricator.services.mozilla.com/D5771
toolkit/components/telemetry/app/TelemetryController.jsm
toolkit/components/telemetry/tests/unit/test_TelemetryClientID_reset.js
toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
toolkit/components/telemetry/tests/unit/xpcshell.ini
--- a/toolkit/components/telemetry/app/TelemetryController.jsm
+++ b/toolkit/components/telemetry/app/TelemetryController.jsm
@@ -653,16 +653,26 @@ var Impl = {
       try {
         // TODO: This should probably happen after all the delayed init here.
         this._initialized = true;
         TelemetryEnvironment.delayedInit();
 
         // Load the ClientID.
         this._clientID = await ClientID.getClientID();
 
+        // Fix-up a canary client ID if detected.
+        const uploadEnabled = Services.prefs.getBoolPref(TelemetryUtils.Preferences.FhrUploadEnabled, false);
+        if (uploadEnabled && this._clientID == Utils.knownClientID) {
+          this._log.trace("Upload enabled, but got canary client ID. Resetting.");
+          this._clientID = await ClientID.resetClientID();
+        } else if (!uploadEnabled && this._clientID != Utils.knownClientID) {
+          this._log.trace("Upload disabled, but got a valid client ID. Setting canary client ID.");
+          this._clientID = await ClientID.setClientID(TelemetryUtils.knownClientID);
+        }
+
         await TelemetrySend.setup(this._testMode);
 
         // Perform TelemetrySession delayed init.
         await TelemetrySession.delayedInit();
 
         if (Services.prefs.getBoolPref(TelemetryUtils.Preferences.NewProfilePingEnabled, false) &&
             !TelemetrySession.newProfilePingSent) {
           // Kick off the scheduling of the new-profile ping.
new file mode 100644
--- /dev/null
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryClientID_reset.js
@@ -0,0 +1,157 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/
+*/
+
+ChromeUtils.import("resource://services-common/utils.js");
+ChromeUtils.import("resource://gre/modules/ClientID.jsm");
+ChromeUtils.import("resource://gre/modules/Services.jsm");
+ChromeUtils.import("resource://gre/modules/TelemetryController.jsm", this);
+ChromeUtils.import("resource://gre/modules/TelemetryStorage.jsm", this);
+ChromeUtils.import("resource://gre/modules/TelemetrySend.jsm", this);
+ChromeUtils.import("resource://gre/modules/TelemetryUtils.jsm", this);
+ChromeUtils.import("resource://gre/modules/Preferences.jsm");
+
+const PING_FORMAT_VERSION = 4;
+const OPTOUT_PING_TYPE = "optout";
+const TEST_PING_TYPE = "test-ping-type";
+
+function sendPing() {
+  let options = {
+    addClientId: true,
+    addEnvironment: false,
+  };
+  return TelemetryController.submitExternalPing(TEST_PING_TYPE, {}, options);
+}
+
+add_task(async function test_setup() {
+  // Addon manager needs a profile directory
+  do_get_profile();
+  // Make sure we don't generate unexpected pings due to pref changes.
+  await setEmptyPrefWatchlist();
+
+  Services.prefs.setBoolPref(TelemetryUtils.Preferences.FhrUploadEnabled, true);
+
+  await new Promise(resolve =>
+    Telemetry.asyncFetchTelemetryData(wrapWithExceptionHandler(resolve)));
+
+  PingServer.start();
+  Preferences.set(TelemetryUtils.Preferences.Server, "http://localhost:" + PingServer.port);
+  await TelemetryController.testSetup();
+});
+
+/**
+ * Testing the following scenario:
+ *
+ * 1. Telemetry upload gets disabled
+ * 2. Canary client ID is set
+ * 3. Instance is shut down
+ * 4. Telemetry upload flag is toggled
+ * 5. Instance is started again
+ * 6. Detect that upload is enabled and reset client ID
+ *
+ * This scenario e.g. happens when switching between channels
+ * with and without the optout ping reset included.
+ */
+add_task(async function test_clientid_reset_after_reenabling() {
+  const isUnified = Preferences.get(TelemetryUtils.Preferences.Unified, false);
+  if (!isUnified) {
+    // Skipping the test if unified telemetry is off, as no optout ping will
+    // be generated.
+    return;
+  }
+
+  await sendPing();
+  let ping = await PingServer.promiseNextPing();
+  Assert.equal(ping.type, TEST_PING_TYPE, "The ping must be a test ping");
+  Assert.ok("clientId" in ping);
+
+  let firstClientId = ping.clientId;
+  Assert.notEqual(TelemetryUtils.knownClientID, firstClientId, "Client ID should be valid and random");
+
+  // Disable FHR upload: this should trigger a optout ping.
+  Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, false);
+
+  ping = await PingServer.promiseNextPing();
+  Assert.equal(ping.type, OPTOUT_PING_TYPE, "The ping must be an optout ping");
+  Assert.ok(!("clientId" in ping));
+  let clientId = await ClientID.getClientID();
+  Assert.equal(TelemetryUtils.knownClientID, clientId);
+
+  // Now shutdown the instance
+  await TelemetryController.testShutdown();
+  await TelemetryStorage.testClearPendingPings();
+
+  // Flip the pref again
+  Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, true);
+
+  // Start the instance
+  await TelemetryController.testReset();
+
+  let newClientId = await ClientID.getClientID();
+  Assert.notEqual(TelemetryUtils.knownClientID, newClientId, "Client ID should be valid and random");
+  Assert.notEqual(firstClientId, newClientId, "Client ID should be newly generated");
+});
+
+/**
+ * Testing the following scenario:
+ * (Reverse of the first test)
+ *
+ * 1. Telemetry upload gets disabled, canary client ID is set
+ * 2. Telemetry upload is enabled
+ * 3. New client ID is generated.
+ * 3. Instance is shut down
+ * 4. Telemetry upload flag is toggled
+ * 5. Instance is started again
+ * 6. Detect that upload is disabled and sets canary client ID
+ *
+ * This scenario e.g. happens when switching between channels
+ * with and without the optout ping reset included.
+ */
+add_task(async function test_clientid_canary_after_disabling() {
+  const isUnified = Preferences.get(TelemetryUtils.Preferences.Unified, false);
+  if (!isUnified) {
+    // Skipping the test if unified telemetry is off, as no optout ping will
+    // be generated.
+    return;
+  }
+
+  await sendPing();
+  let ping = await PingServer.promiseNextPing();
+  Assert.equal(ping.type, TEST_PING_TYPE, "The ping must be a test ping");
+  Assert.ok("clientId" in ping);
+
+  let firstClientId = ping.clientId;
+  Assert.notEqual(TelemetryUtils.knownClientID, firstClientId, "Client ID should be valid and random");
+
+  // Disable FHR upload: this should trigger a optout ping.
+  Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, false);
+
+  ping = await PingServer.promiseNextPing();
+  Assert.equal(ping.type, OPTOUT_PING_TYPE, "The ping must be an optout ping");
+  Assert.ok(!("clientId" in ping));
+  let clientId = await ClientID.getClientID();
+  Assert.equal(TelemetryUtils.knownClientID, clientId);
+
+  Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, true);
+  await sendPing();
+  ping = await PingServer.promiseNextPing();
+  Assert.equal(ping.type, TEST_PING_TYPE, "The ping must be a test ping");
+  Assert.notEqual(firstClientId, ping.clientId, "Client ID should be newly generated");
+
+  // Now shutdown the instance
+  await TelemetryController.testShutdown();
+  await TelemetryStorage.testClearPendingPings();
+
+  // Flip the pref again
+  Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, false);
+
+  // Start the instance
+  await TelemetryController.testReset();
+
+  let newClientId = await ClientID.getClientID();
+  Assert.equal(TelemetryUtils.knownClientID, newClientId, "Client ID should be a canary when upload disabled");
+});
+
+add_task(async function stopServer() {
+  await PingServer.stop();
+});
--- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
@@ -1074,16 +1074,18 @@ add_task(async function test_sendShutdow
   await TelemetryStorage.testClearPendingPings();
 
   // Enable ping upload and signal an OS shutdown. The pingsender
   // will not be spawned and no ping will be sent.
   Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, true);
   await TelemetryController.testReset();
   Services.obs.notifyObservers(null, "quit-application-forced");
   await TelemetryController.testShutdown();
+  // After re-enabling FHR, wait for the new client ID
+  gClientID = await ClientID.getClientID();
 
   // Check that the "shutdown" ping was correctly saved to disk.
   await checkPendingShutdownPing();
 
   // Make sure we have no pending pings between the runs.
   await TelemetryStorage.testClearPendingPings();
   Telemetry.clearScalars();
 
--- a/toolkit/components/telemetry/tests/unit/xpcshell.ini
+++ b/toolkit/components/telemetry/tests/unit/xpcshell.ini
@@ -40,16 +40,17 @@ tags = addons
 skip-if = os == "android"
 tags = addons
 [test_PingAPI.js]
 skip-if = os == "android"
 [test_TelemetryFlagClear.js]
 [test_TelemetryLateWrites.js]
 [test_TelemetryLockCount.js]
 [test_TelemetryController.js]
+[test_TelemetryClientID_reset.js]
 [test_HealthPing.js]
 skip-if = (verify && (os == 'win'))
 tags = addons
 [test_TelemetryController_idle.js]
 [test_TelemetryControllerShutdown.js]
 tags = addons
 [test_TelemetryStopwatch.js]
 [test_TelemetryControllerBuildID.js]