Backed out 2 changesets (bug 1585410) for causing xpcshell failures in test_TelemetryController.js CLOSED TREE
authorMihai Alexandru Michis <malexandru@mozilla.com>
Thu, 14 Nov 2019 18:40:04 +0200
changeset 501975 8c1ac1675109471d8e302229f34998b6d4e73928
parent 501974 f09b9242197689e2949df164e6d628a01593b526
child 501976 fe3a07722fee5e181b33b93240634bdb7639dde2
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1585410
milestone72.0a1
backs outbbe85df3365a23b6400c9f0b453f316d950b64c4
474430e7ea37c836c40d3fb9f93a3566f7e36af1
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
Backed out 2 changesets (bug 1585410) for causing xpcshell failures in test_TelemetryController.js CLOSED TREE Backed out changeset bbe85df3365a (bug 1585410) Backed out changeset 474430e7ea37 (bug 1585410)
toolkit/components/telemetry/app/ClientID.jsm
toolkit/components/telemetry/app/TelemetryController.jsm
toolkit/components/telemetry/app/TelemetrySend.jsm
toolkit/components/telemetry/docs/concepts/pings.rst
toolkit/components/telemetry/docs/data/deletion-request-ping.rst
toolkit/components/telemetry/docs/data/index.rst
toolkit/components/telemetry/docs/data/optout-ping.rst
toolkit/components/telemetry/docs/obsolete/index.rst
toolkit/components/telemetry/docs/obsolete/optout-ping.rst
toolkit/components/telemetry/tests/marionette/harness/telemetry_harness/ping_filters.py
toolkit/components/telemetry/tests/marionette/tests/client/manifest.ini
toolkit/components/telemetry/tests/marionette/tests/client/test_deletion_request_ping.py
toolkit/components/telemetry/tests/marionette/tests/client/test_optout_ping.py
toolkit/components/telemetry/tests/unit/head.js
toolkit/components/telemetry/tests/unit/test_TelemetryClientID_reset.js
toolkit/components/telemetry/tests/unit/test_TelemetryController.js
--- a/toolkit/components/telemetry/app/ClientID.jsm
+++ b/toolkit/components/telemetry/app/ClientID.jsm
@@ -154,55 +154,51 @@ var ClientIDImpl = {
     return this._loadClientIdTask;
   },
 
   /**
    * Load the Client ID from the DataReporting Service state file.
    * If no Client ID is found, we generate a new one.
    */
   async _doLoadClientID() {
-    this._log.trace(`_doLoadClientID`);
     // If there's a removal in progress, let's wait for it
     await this._removeClientIdTask;
 
     // Try to load the client id from the DRS state file.
     try {
       let state = await CommonUtils.readJSON(gStateFilePath);
       if (AppConstants.platform == "android" && state && "wasCanary" in state) {
         this._wasCanary = state.wasCanary;
       }
       if (state && this.updateClientID(state.clientID)) {
-        this._log.trace(`_doLoadClientID: Client id loaded from state.`);
         return this._clientID;
       }
     } catch (e) {
       // fall through to next option
     }
 
     // We dont have an id from the DRS state file yet, generate a new ID.
     this.updateClientID(CommonUtils.generateUUID());
     this._saveClientIdTask = this._saveClientID();
 
     // Wait on persisting the id. Otherwise failure to save the ID would result in
     // the client creating and subsequently sending multiple IDs to the server.
     // This would appear as multiple clients submitting similar data, which would
     // result in orphaning.
     await this._saveClientIdTask;
 
-    this._log.trace("_doLoadClientID: New client id loaded and persisted.");
     return this._clientID;
   },
 
   /**
    * Save the client ID to the client ID file.
    *
    * @return {Promise} A promise resolved when the client ID is saved to disk.
    */
   async _saveClientID() {
-    this._log.trace(`_saveClientID`);
     let obj = { clientID: this._clientID };
     // We detected a canary client ID when resetting, storing this as a flag
     if (AppConstants.platform == "android" && this._wasCanary) {
       obj.wasCanary = true;
     }
     await OS.File.makeDir(gDatareportingPath);
     await CommonUtils.writeJSON(obj, gStateFilePath);
     this._saveClientIdTask = null;
@@ -290,45 +286,38 @@ var ClientIDImpl = {
   async _reset() {
     await this._loadClientIdTask;
     await this._saveClientIdTask;
     this._clientID = null;
     this._clientIDHash = null;
   },
 
   async setClientID(id) {
-    this._log.trace("setClientID");
     if (!this.updateClientID(id)) {
       throw new Error("Invalid client ID: " + id);
     }
 
     this._saveClientIdTask = this._saveClientID();
     await this._saveClientIdTask;
     return this._clientID;
   },
 
   async _doRemoveClientID() {
-    this._log.trace("_doRemoveClientID");
-
     // Reset stored id.
     this._clientID = null;
     this._clientIDHash = null;
 
     // Clear the client id from the preference cache.
     Services.prefs.clearUserPref(PREF_CACHED_CLIENTID);
 
-    // If there is a save in progress, wait for it to complete.
-    await this._saveClientIdTask;
-
     // Remove the client id from disk
     await OS.File.remove(gStateFilePath, { ignoreAbsent: true });
   },
 
   async resetClientID() {
-    this._log.trace("resetClientID");
     let oldClientId = this._clientID;
 
     // Wait for the removal.
     // Asynchronous calls to getClientID will also be blocked on this.
     this._removeClientIdTask = this._doRemoveClientID();
     let clear = () => (this._removeClientIdTask = null);
     this._removeClientIdTask.then(clear, clear);
 
--- a/toolkit/components/telemetry/app/TelemetryController.jsm
+++ b/toolkit/components/telemetry/app/TelemetryController.jsm
@@ -40,17 +40,17 @@ const TELEMETRY_DELAY =
 const TELEMETRY_TEST_DELAY = 1;
 
 // How long to wait (ms) before sending the new profile ping on the first
 // run of a new profile.
 const NEWPROFILE_PING_DEFAULT_DELAY = 30 * 60 * 1000;
 
 // Ping types.
 const PING_TYPE_MAIN = "main";
-const PING_TYPE_DELETION_REQUEST = "deletion-request";
+const PING_TYPE_OPTOUT = "optout";
 
 // Session ping reasons.
 const REASON_GATHER_PAYLOAD = "gather-payload";
 const REASON_GATHER_SUBSESSION_PAYLOAD = "gather-subsession-payload";
 
 XPCOMUtils.defineLazyServiceGetter(
   this,
   "Telemetry",
@@ -195,18 +195,16 @@ var TelemetryController = Object.freeze(
    * @param {Object} aPayload The actual data payload for the ping.
    * @param {Object} [aOptions] Options object.
    * @param {Boolean} [aOptions.addClientId=false] true if the ping should contain the client
    *                  id, false otherwise.
    * @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
    *                  environment data.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
    * @param {Boolean} [aOptions.usePingSender=false] if true, send the ping using the PingSender.
-   * @param {String} [aOptions.overrideClientId=undefined] if set, override the
-   *                 client id to the provided value. Implies aOptions.addClientId=true.
    * @returns {Promise} Test-only - a promise that resolves with the ping id once the ping is stored or sent.
    */
   submitExternalPing(aType, aPayload, aOptions = {}) {
     aOptions.addClientId = aOptions.addClientId || false;
     aOptions.addEnvironment = aOptions.addEnvironment || false;
     aOptions.usePingSender = aOptions.usePingSender || false;
 
     return Impl.submitExternalPing(aType, aPayload, aOptions);
@@ -230,18 +228,16 @@ var TelemetryController = Object.freeze(
    * @param {Object} [aOptions] Options object.
    * @param {Boolean} [aOptions.addClientId=false] true if the ping should contain the client
    *                  id, false otherwise.
    * @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
    *                  environment data.
    * @param {Boolean} [aOptions.overwrite=false] true overwrites a ping with the same name,
    *                  if found.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
-   * @param {String} [aOptions.overrideClientId=undefined] if set, override the
-   *                 client id to the provided value. Implies aOptions.addClientId=true.
    *
    * @returns {Promise} A promise that resolves with the ping id when the ping is saved to
    *                    disk.
    */
   addPendingPing(aType, aPayload, aOptions = {}) {
     let options = aOptions;
     options.addClientId = aOptions.addClientId || false;
     options.addEnvironment = aOptions.addEnvironment || false;
@@ -378,18 +374,16 @@ var Impl = {
    * @param {String} aType The type of the ping.
    * @param {Object} aPayload The actual data payload for the ping.
    * @param {Object} aOptions Options object.
    * @param {Boolean} aOptions.addClientId true if the ping should contain the client
    *                  id, false otherwise.
    * @param {Boolean} aOptions.addEnvironment true if the ping should contain the
    *                  environment data.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
-   * @param {String} [aOptions.overrideClientId=undefined] if set, override the
-   *                 client id to the provided value. Implies aOptions.addClientId=true.
    *
    * @returns {Object} An object that contains the assembled ping data.
    */
   assemblePing: function assemblePing(aType, aPayload, aOptions = {}) {
     this._log.trace(
       "assemblePing - Type " + aType + ", aOptions " + JSON.stringify(aOptions)
     );
 
@@ -403,18 +397,18 @@ var Impl = {
       type: aType,
       id: Policy.generatePingId(),
       creationDate: Policy.now().toISOString(),
       version: PING_FORMAT_VERSION,
       application: this._getApplicationSection(),
       payload,
     };
 
-    if (aOptions.addClientId || aOptions.overrideClientId) {
-      pingData.clientId = aOptions.overrideClientId || this._clientID;
+    if (aOptions.addClientId) {
+      pingData.clientId = this._clientID;
     }
 
     if (aOptions.addEnvironment) {
       pingData.environment =
         aOptions.overrideEnvironment || TelemetryEnvironment.currentEnvironment;
 
       // On Android store a flag if the client ID was reset from a canary ID.
       if (AppConstants.platform == "android" && ClientID.wasCanaryClientID()) {
@@ -448,26 +442,23 @@ var Impl = {
    * @param {Object} aPayload The actual data payload for the ping.
    * @param {Object} [aOptions] Options object.
    * @param {Boolean} [aOptions.addClientId=false] true if the ping should contain the client
    *                  id, false otherwise.
    * @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
    *                  environment data.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
    * @param {Boolean} [aOptions.usePingSender=false] if true, send the ping using the PingSender.
-   * @param {String} [aOptions.overrideClientId=undefined] if set, override the
-   *                 client id to the provided value. Implies aOptions.addClientId=true.
    * @returns {Promise} Test-only - a promise that is resolved with the ping id once the ping is stored or sent.
    */
   async _submitPingLogic(aType, aPayload, aOptions) {
     // Make sure to have a clientId if we need one. This cover the case of submitting
     // a ping early during startup, before Telemetry is initialized, if no client id was
     // cached.
-    if (!this._clientID && aOptions.addClientId && !aOptions.overrideClientId) {
-      this._log.trace("_submitPingLogic - Waiting on client id");
+    if (!this._clientID && aOptions.addClientId) {
       Telemetry.getHistogramById(
         "TELEMETRY_PING_SUBMISSION_WAITING_CLIENTID"
       ).add();
       // We can safely call |getClientID| here and during initialization: we would still
       // spawn and return one single loading task.
       this._clientID = await ClientID.getClientID();
     }
 
@@ -501,18 +492,16 @@ var Impl = {
    * @param {Object} aPayload The actual data payload for the ping.
    * @param {Object} [aOptions] Options object.
    * @param {Boolean} [aOptions.addClientId=false] true if the ping should contain the client
    *                  id, false otherwise.
    * @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
    *                  environment data.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
    * @param {Boolean} [aOptions.usePingSender=false] if true, send the ping using the PingSender.
-   * @param {String} [aOptions.overrideClientId=undefined] if set, override the
-   *                 client id to the provided value. Implies aOptions.addClientId=true.
    * @returns {Promise} Test-only - a promise that is resolved with the ping id once the ping is stored or sent.
    */
   submitExternalPing: function send(aType, aPayload, aOptions) {
     this._log.trace(
       "submitExternalPing - type: " +
         aType +
         ", aOptions: " +
         JSON.stringify(aOptions)
@@ -565,18 +554,16 @@ var Impl = {
    * @param {Object} aPayload The actual data payload for the ping.
    * @param {Object} aOptions Options object.
    * @param {Boolean} aOptions.addClientId true if the ping should contain the client id,
    *                  false otherwise.
    * @param {Boolean} aOptions.addEnvironment true if the ping should contain the
    *                  environment data.
    * @param {Boolean} aOptions.overwrite true overwrites a ping with the same name, if found.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
-   * @param {String} [aOptions.overrideClientId=undefined] if set, override the
-   *                 client id to the provided value. Implies aOptions.addClientId=true.
    *
    * @returns {Promise} A promise that resolves with the ping id when the ping is saved to
    *                    disk.
    */
   addPendingPing: function addPendingPing(aType, aPayload, aOptions) {
     this._log.trace(
       "addPendingPing - Type " +
         aType +
@@ -1010,17 +997,17 @@ var Impl = {
       connectionsBarrier: this._connectionsBarrier.state,
       sendModule: TelemetrySend.getShutdownState(),
       haveDelayedNewProfileTask: !!this._delayedNewPingTask,
     };
   },
 
   /**
    * Called whenever the FHR Upload preference changes (e.g. when user disables FHR from
-   * the preferences panel), this triggers sending the "deletion-request" ping.
+   * the preferences panel), this triggers sending the optout ping.
    */
   _onUploadPrefChange() {
     const uploadEnabled = Services.prefs.getBoolPref(
       TelemetryUtils.Preferences.FhrUploadEnabled,
       false
     );
     if (uploadEnabled) {
       this._log.trace(
@@ -1058,42 +1045,37 @@ var Impl = {
           "_onUploadPrefChange - error clearing pending pings",
           e
         );
       } finally {
         // 4. Reset session and subsession counter
         TelemetrySession.resetSubsessionCounter();
 
         // 5. Set ClientID to a known value
-        let oldClientId = this._clientID;
         this._clientID = await ClientID.setClientID(
           TelemetryUtils.knownClientID
         );
 
-        // 6. Send the deletion-request ping.
-        this._log.trace("_onUploadPrefChange - Sending deletion-request ping.");
-        this.submitExternalPing(
-          PING_TYPE_DELETION_REQUEST,
-          {},
-          { overrideClientId: oldClientId }
-        );
+        // 6. Send the optout ping.
+        this._log.trace("_onUploadPrefChange - Sending optout ping.");
+        this.submitExternalPing(PING_TYPE_OPTOUT, {}, { addClientId: false });
       }
     })();
 
     this._shutdownBarrier.client.addBlocker(
       "TelemetryController: removing pending pings after data upload was disabled",
       p
     );
   },
 
   QueryInterface: ChromeUtils.generateQI([Ci.nsISupportsWeakReference]),
 
   _attachObservers() {
     if (IS_UNIFIED_TELEMETRY) {
-      // Watch the FHR upload setting to trigger "deletion-request" pings.
+      // Watch the FHR upload setting to trigger optout pings.
       Services.prefs.addObserver(
         TelemetryUtils.Preferences.FhrUploadEnabled,
         this,
         true
       );
     }
   },
 
--- a/toolkit/components/telemetry/app/TelemetrySend.jsm
+++ b/toolkit/components/telemetry/app/TelemetrySend.jsm
@@ -65,17 +65,17 @@ const TOPIC_PROFILE_CHANGE_NET_TEARDOWN 
 // Changing this pref requires a restart.
 const IS_UNIFIED_TELEMETRY = Services.prefs.getBoolPref(
   TelemetryUtils.Preferences.Unified,
   false
 );
 
 const MS_IN_A_MINUTE = 60 * 1000;
 
-const PING_TYPE_DELETION_REQUEST = "deletion-request";
+const PING_TYPE_OPTOUT = "optout";
 
 // We try to spread "midnight" pings out over this interval.
 const MIDNIGHT_FUZZING_INTERVAL_MS = 60 * MS_IN_A_MINUTE;
 // We delay sending "midnight" pings on this client by this interval.
 const MIDNIGHT_FUZZING_DELAY_MS = Math.random() * MIDNIGHT_FUZZING_INTERVAL_MS;
 
 // Timeout after which we consider a ping submission failed.
 const PING_SUBMIT_TIMEOUT_MS = 1.5 * MS_IN_A_MINUTE;
@@ -128,22 +128,22 @@ function isV4PingFormat(aPing) {
     "id" in aPing &&
     "application" in aPing &&
     "version" in aPing &&
     aPing.version >= 2
   );
 }
 
 /**
- * Check if the provided ping is a deletion-request ping.
+ * Check if the provided ping is an optout ping.
  * @param {Object} aPing The ping to check.
- * @return {Boolean} True if the ping is a deletion-request ping, false otherwise.
+ * @return {Boolean} True if the ping is an optout ping, false otherwise.
  */
-function isDeletionRequestPing(aPing) {
-  return isV4PingFormat(aPing) && aPing.type == PING_TYPE_DELETION_REQUEST;
+function isOptoutPing(aPing) {
+  return isV4PingFormat(aPing) && aPing.type == PING_TYPE_OPTOUT;
 }
 
 /**
  * Save the provided ping as a pending ping.
  * @param {Object} aPing The ping to save.
  * @return {Promise} A promise resolved when the ping is saved.
  */
 function savePing(aPing) {
@@ -234,17 +234,17 @@ var TelemetrySend = {
   submitPing(ping, options = {}) {
     options.usePingSender = options.usePingSender || false;
     return TelemetrySendImpl.submitPing(ping, options);
   },
 
   /**
    * Check if sending is disabled. If Telemetry is not allowed to upload,
    * pings are not sent to the server.
-   * If trying to send a deletion-request ping, don't block it.
+   * If trying to send an optout ping, don't block it.
    *
    * @param {Object} [ping=null] A ping to be checked.
    * @return {Boolean} True if pings can be send to the servers, false otherwise.
    */
   sendingEnabled(ping = null) {
     return TelemetrySendImpl.sendingEnabled(ping);
   },
 
@@ -481,32 +481,32 @@ var SendScheduler = {
 
       if (this._shutdown) {
         this._log.trace("_doSendTask - shutting down, bailing out");
         this._sendTaskState = "bail out - shutdown check";
         return;
       }
 
       // Get a list of pending pings, sorted by last modified, descending.
-      // Filter out all the pings we can't send now. This addresses scenarios like "deletion-request" pings
+      // Filter out all the pings we can't send now. This addresses scenarios like "optout" pings
       // which can be sent even when upload is disabled.
       let pending = TelemetryStorage.getPendingPingList();
       let current = TelemetrySendImpl.getUnpersistedPings();
       this._log.trace(
         "_doSendTask - pending: " +
           pending.length +
           ", current: " +
           current.length
       );
       // Note that the two lists contain different kind of data. |pending| only holds ping
       // info, while |current| holds actual ping data.
       if (!TelemetrySendImpl.sendingEnabled()) {
-        // If sending is disabled, only handle deletion-request pings
+        // If sending is disabled, only handle an unpersisted optout ping
         pending = [];
-        current = current.filter(p => isDeletionRequestPing(p));
+        current = current.filter(p => isOptoutPing(p));
       }
       this._log.trace(
         "_doSendTask - can send - pending: " +
           pending.length +
           ", current: " +
           current.length
       );
 
@@ -1078,21 +1078,29 @@ var TelemetrySendImpl = {
     ];
 
     for (let current of currentPings) {
       let ping = current;
       let p = (async () => {
         try {
           await this._doPing(ping, ping.id, false);
         } catch (ex) {
-          this._log.info(
-            "sendPings - ping " + ping.id + " not sent, saving to disk",
-            ex
-          );
-          await savePing(ping);
+          if (isOptoutPing(ping)) {
+            // Optout pings should only be tried once and then discarded.
+            this._log.info(
+              "sendPings - optout ping " + ping.id + " not sent, discarding",
+              ex
+            );
+          } else {
+            this._log.info(
+              "sendPings - ping " + ping.id + " not sent, saving to disk",
+              ex
+            );
+            await savePing(ping);
+          }
         } finally {
           this._currentPings.delete(ping.id);
         }
       })();
 
       this._trackPendingPingTask(p);
       pingSends.push(p);
     }
@@ -1445,17 +1453,17 @@ var TelemetrySendImpl = {
     }
 
     return this._sendingEnabled;
   },
 
   /**
    * Check if sending is disabled. If Telemetry is not allowed to upload,
    * pings are not sent to the server.
-   * If trying to send a "deletion-request" ping, don't block it.
+   * If trying to send an optout ping, don't block it.
    * If unified telemetry is off, don't send pings if Telemetry is disabled.
    *
    * @param {Object} [ping=null] A ping to be checked.
    * @return {Boolean} True if pings can be send to the servers, false otherwise.
    */
   sendingEnabled(ping = null) {
     // We only send pings from official builds, but allow overriding this for tests.
     if (
@@ -1464,18 +1472,18 @@ var TelemetrySendImpl = {
       !this._overrideOfficialCheck
     ) {
       return false;
     }
 
     // With unified Telemetry, the FHR upload setting controls whether we can send pings.
     // The Telemetry pref enables sending extended data sets instead.
     if (IS_UNIFIED_TELEMETRY) {
-      // "deletion-request" pings are sent once even if the upload is disabled.
-      if (ping && isDeletionRequestPing(ping)) {
+      // Optout pings are sent once even if the upload is disabled.
+      if (ping && isOptoutPing(ping)) {
         return true;
       }
       return Services.prefs.getBoolPref(
         TelemetryUtils.Preferences.FhrUploadEnabled,
         false
       );
     }
 
@@ -1510,18 +1518,21 @@ var TelemetrySendImpl = {
     );
     p.push(SendScheduler.waitOnSendTask());
     return Promise.all(p);
   },
 
   async _persistCurrentPings() {
     for (let [id, ping] of this._currentPings) {
       try {
-        await savePing(ping);
-        this._log.trace("_persistCurrentPings - saved ping " + id);
+        // Never save an optout ping to disk
+        if (!isOptoutPing(ping)) {
+          await savePing(ping);
+          this._log.trace("_persistCurrentPings - saved ping " + id);
+        }
       } catch (ex) {
         this._log.error("_persistCurrentPings - failed to save ping " + id, ex);
       } finally {
         this._currentPings.delete(id);
       }
     }
   },
 
--- a/toolkit/components/telemetry/docs/concepts/pings.rst
+++ b/toolkit/components/telemetry/docs/concepts/pings.rst
@@ -21,9 +21,9 @@ Pings sent from code that ships with Fir
 
 Important examples are:
 
 * :doc:`main <../data/main-ping>` - contains the information collected by Telemetry (Histograms, Scalars, ...)
 * :doc:`saved-session <../data/main-ping>` - has the same format as a main ping, but it contains the *"classic"* Telemetry payload with measurements covering the whole browser session. This is only a separate type to make storage of saved-session easier server-side. As of Firefox 61 this is sent on Android only.
 * :doc:`crash <../data/crash-ping>` - a ping that is captured and sent after a Firefox process crashes.
 * :doc:`new-profile <../data/new-profile-ping>` - sent on the first run of a new profile.
 * :doc:`update <../data/update-ping>` - sent right after an update is downloaded.
-* :doc:`deletion-request <../data/deletion-request-ping>` - sent when FHR upload is disabled
+* :doc:`optout <../data/optout-ping>` - sent when FHR upload is disabled
deleted file mode 100644
--- a/toolkit/components/telemetry/docs/data/deletion-request-ping.rst
+++ /dev/null
@@ -1,35 +0,0 @@
-"deletion-request" ping
-=======================
-
-This ping is submitted when a user opts out of sending technical and interaction data to Mozilla.
-(In other words, when the
-``datareporting.healthreport.uploadEnabled``
-:doc:`preference <../internals/preferences>` is set to ``false``.)
-
-This ping contains the client id.
-This ping does not contain any environment data.
-
-Structure:
-
-.. code-block:: js
-
-    {
-      version: 4,
-      type: "deletion-request",
-      ... common ping data (including clientId)
-      payload: { }
-    }
-
-Expected behaviours
--------------------
-The following is a list of expected behaviours for the ``deletion-request`` ping:
-
-- Telemetry will try to send the ping even if upload is disabled.
-- Telemetry may persist this ping if it can't be immediately sent, and may try to resend it later.
-
-Version History
----------------
-
-- Firefox 70:
-
-  - "deletion-request" ping replaces the "optout" ping (`bug 1585410 <https://bugzilla.mozilla.org/show_bug.cgi?id=1585410>`_).
--- a/toolkit/components/telemetry/docs/data/index.rst
+++ b/toolkit/components/telemetry/docs/data/index.rst
@@ -5,16 +5,18 @@ Data documentation
 .. toctree::
    :maxdepth: 2
    :titlesonly:
    :glob:
 
    common-ping
    environment
    main-ping
+   optout-ping
+   deletion-ping
    crash-ping
    backgroundhangmonitor-ping
    anonymous-ping
    first-shutdown-ping
    *-ping
    ecosystem-telemetry
 
 The `mozilla-pipeline-schemas repository <https://github.com/mozilla-services/mozilla-pipeline-schemas/>`_ contains schemas for some of the pings.
rename from toolkit/components/telemetry/docs/obsolete/optout-ping.rst
rename to toolkit/components/telemetry/docs/data/optout-ping.rst
--- a/toolkit/components/telemetry/docs/obsolete/optout-ping.rst
+++ b/toolkit/components/telemetry/docs/data/optout-ping.rst
@@ -1,11 +1,11 @@
 
-"optout" ping (obsolete)
-========================
+"optout" ping
+=============
 
 This ping is generated when a user turns off FHR upload from the Preferences panel, changing the related ``datareporting.healthreport.uploadEnabled`` :doc:`preference <../internals/preferences>`.
 
 This ping contains no client id and no environment data.
 
 Structure:
 
 .. code-block:: js
--- a/toolkit/components/telemetry/docs/obsolete/index.rst
+++ b/toolkit/components/telemetry/docs/obsolete/index.rst
@@ -6,9 +6,8 @@ Obsolete Documentation
 
 .. toctree::
    :maxdepth: 5
    :titlesonly:
 
    uitelemetry/index
    fhr/index
    hybrid-content
-   optout-ping
--- a/toolkit/components/telemetry/tests/marionette/harness/telemetry_harness/ping_filters.py
+++ b/toolkit/components/telemetry/tests/marionette/harness/telemetry_harness/ping_filters.py
@@ -5,26 +5,16 @@
 
 class PingFilter(object):
     """Ping filter that accepts any pings."""
 
     def __call__(self, ping):
         return True
 
 
-class DeletionRequestPingFilter(PingFilter):
-    """Ping filter that accepts deletion-request pings."""
-
-    def __call__(self, ping):
-        if not super(DeletionRequestPingFilter, self).__call__(ping):
-            return False
-
-        return ping["type"] == "deletion-request"
-
-
 class EventPingFilter(PingFilter):
     """Ping filter that accepts event pings."""
 
     def __call__(self, ping):
         if not super(EventPingFilter, self).__call__(ping):
             return False
 
         return ping["type"] == "event"
@@ -61,15 +51,25 @@ class MainPingReasonFilter(MainPingFilte
 
     def __call__(self, ping):
         if not super(MainPingReasonFilter, self).__call__(ping):
             return False
 
         return ping["payload"]["info"]["reason"] == self.reason
 
 
+class OptoutPingFilter(PingFilter):
+    """Ping filter that accepts optout pings."""
+
+    def __call__(self, ping):
+        if not super(OptoutPingFilter, self).__call__(ping):
+            return False
+
+        return ping["type"] == "optout"
+
+
 ANY_PING = PingFilter()
-DELETION_REQUEST_PING = DeletionRequestPingFilter()
 EVENT_PING = EventPingFilter()
 FIRST_SHUTDOWN_PING = FirstShutdownPingFilter()
 MAIN_PING = MainPingFilter()
 MAIN_SHUTDOWN_PING = MainPingReasonFilter("shutdown")
 MAIN_ENVIRONMENT_CHANGE_PING = MainPingReasonFilter("environment-change")
+OPTOUT_PING = OptoutPingFilter()
--- a/toolkit/components/telemetry/tests/marionette/tests/client/manifest.ini
+++ b/toolkit/components/telemetry/tests/marionette/tests/client/manifest.ini
@@ -1,8 +1,8 @@
 [DEFAULT]
 tags = client
 
-[test_deletion_request_ping.py]
 [test_event_ping.py]
 [test_main_tab_scalars.py]
+[test_optout_ping.py]
 [test_search_counts_across_sessions.py]
 [test_subsession_management.py]
rename from toolkit/components/telemetry/tests/marionette/tests/client/test_deletion_request_ping.py
rename to toolkit/components/telemetry/tests/marionette/tests/client/test_optout_ping.py
--- a/toolkit/components/telemetry/tests/marionette/tests/client/test_deletion_request_ping.py
+++ b/toolkit/components/telemetry/tests/marionette/tests/client/test_optout_ping.py
@@ -1,56 +1,56 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 from telemetry_harness.testcase import TelemetryTestCase
-from telemetry_harness.ping_filters import ANY_PING, DELETION_REQUEST_PING, MAIN_SHUTDOWN_PING
+from telemetry_harness.ping_filters import ANY_PING, OPTOUT_PING, MAIN_SHUTDOWN_PING
 
 
-class TestDeletionRequestPing(TelemetryTestCase):
-    """Tests for "deletion-request" ping."""
+class TestOptoutPing(TelemetryTestCase):
+    """Tests for "optout" ping."""
 
     def disable_telemetry(self):
         self.marionette.instance.profile.set_persistent_preferences(
             {"datareporting.healthreport.uploadEnabled": False})
         self.marionette.set_pref("datareporting.healthreport.uploadEnabled", False)
 
     def enable_telemetry(self):
         self.marionette.instance.profile.set_persistent_preferences(
             {"datareporting.healthreport.uploadEnabled": True})
         self.marionette.set_pref("datareporting.healthreport.uploadEnabled", True)
 
     def test_optout_ping_across_sessions(self):
-        """Test the "deletion-request" ping behaviour across sessions."""
+        """Test the "optout" ping behaviour across sessions."""
 
         # Get the client_id.
         client_id = self.wait_for_ping(self.install_addon, ANY_PING)["clientId"]
         self.assertIsValidUUID(client_id)
 
-        # Trigger an "deletion-request" ping.
-        ping = self.wait_for_ping(self.disable_telemetry, DELETION_REQUEST_PING)
+        # Trigger an "optout" ping.
+        optout_ping = self.wait_for_ping(self.disable_telemetry, OPTOUT_PING)
 
-        self.assertIn("clientId", ping)
-        self.assertIn("payload", ping)
-        self.assertNotIn("environment", ping["payload"])
+        self.assertNotIn("clientId", optout_ping)
+        self.assertIn("payload", optout_ping)
+        self.assertNotIn("environment", optout_ping["payload"])
 
         # Close Firefox cleanly.
         self.marionette.quit(in_app=True)
 
         # TODO: Check pending pings aren't persisted
 
         # Start Firefox.
         self.marionette.start_session()
 
         # Trigger an environment change, which isn't allowed to send a ping.
         self.install_addon()
 
         # Ensure we've sent no pings since "optout".
-        self.assertEqual(self.ping_server.pings[-1], ping)
+        self.assertEqual(self.ping_server.pings[-1], optout_ping)
 
         # Turn Telemetry back on.
         self.enable_telemetry()
 
         # Close Firefox cleanly, collecting its "main"/"shutdown" ping.
         main_ping = self.wait_for_ping(self.restart_browser, MAIN_SHUTDOWN_PING)
 
         # Ensure the "main" ping has changed its client id.
--- a/toolkit/components/telemetry/tests/unit/head.js
+++ b/toolkit/components/telemetry/tests/unit/head.js
@@ -204,17 +204,17 @@ function decodeRequestPayload(request) {
     let bytes = NetUtil.readInputStream(s, s.available());
     payload = JSON.parse(new TextDecoder().decode(bytes));
   }
 
   // Check for canary value
   Assert.notEqual(
     TelemetryUtils.knownClientID,
     payload.clientId,
-    `Known clientId shouldn't appear in a "${payload.type}" ping on the server.`
+    "Known clientId should never appear in a ping on the server"
   );
 
   return payload;
 }
 
 function checkPingFormat(aPing, aType, aHasClientId, aHasEnvironment) {
   const APP_VERSION = "1";
   const APP_NAME = "XPCShell";
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryClientID_reset.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryClientID_reset.js
@@ -8,17 +8,17 @@ ChromeUtils.import("resource://gre/modul
 ChromeUtils.import("resource://gre/modules/TelemetryStorage.jsm", this);
 ChromeUtils.import("resource://gre/modules/TelemetrySend.jsm", this);
 ChromeUtils.import("resource://gre/modules/TelemetryUtils.jsm", this);
 const { Preferences } = ChromeUtils.import(
   "resource://gre/modules/Preferences.jsm"
 );
 
 const PING_FORMAT_VERSION = 4;
-const DELETION_REQUEST_PING_TYPE = "deletion-request";
+const OPTOUT_PING_TYPE = "optout";
 const TEST_PING_TYPE = "test-ping-type";
 
 function sendPing(addEnvironment = false) {
   let options = {
     addClientId: true,
     addEnvironment,
   };
   return TelemetryController.submitExternalPing(TEST_PING_TYPE, {}, options);
@@ -50,41 +50,37 @@ add_task(async function test_setup() {
  * 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 deletion-request ping reset included.
+ * with and without the optout ping reset included.
  */
 add_task(async function test_clientid_reset_after_reenabling() {
   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 deletion-request ping.
+  // Disable FHR upload: this should trigger a optout ping.
   Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, false);
 
   ping = await PingServer.promiseNextPing();
-  Assert.equal(
-    ping.type,
-    DELETION_REQUEST_PING_TYPE,
-    "The ping must be a deletion-request ping"
-  );
-  Assert.equal(ping.clientId, firstClientId);
+  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
@@ -114,41 +110,37 @@ add_task(async function test_clientid_re
  * 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 deletion-request ping reset included.
+ * with and without the optout ping reset included.
  */
 add_task(async function test_clientid_canary_after_disabling() {
   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 deletion-request ping.
+  // Disable FHR upload: this should trigger a optout ping.
   Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, false);
 
   ping = await PingServer.promiseNextPing();
-  Assert.equal(
-    ping.type,
-    DELETION_REQUEST_PING_TYPE,
-    "The ping must be a deletion-request ping"
-  );
-  Assert.equal(ping.clientId, firstClientId);
+  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(
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryController.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryController.js
@@ -20,17 +20,17 @@ ChromeUtils.import("resource://gre/modul
 ChromeUtils.import("resource://gre/modules/TelemetryArchive.jsm", this);
 ChromeUtils.import("resource://gre/modules/TelemetryUtils.jsm", this);
 const { Preferences } = ChromeUtils.import(
   "resource://gre/modules/Preferences.jsm"
 );
 ChromeUtils.import("resource://testing-common/ContentTaskUtils.jsm", this);
 
 const PING_FORMAT_VERSION = 4;
-const DELETION_REQUEST_PING_TYPE = "deletion-request";
+const OPTOUT_PING_TYPE = "optout";
 const TEST_PING_TYPE = "test-ping-type";
 
 const PLATFORM_VERSION = "1.9.2";
 const APP_VERSION = "1";
 const APP_NAME = "XPCShell";
 
 var gClientID = null;
 
@@ -154,17 +154,18 @@ add_task(async function test_simplePing(
   let ping = decodeRequestPayload(request);
   checkPingFormat(ping, TEST_PING_TYPE, false, false);
 });
 
 add_task(async function test_disableDataUpload() {
   const OPTIN_PROBE = "telemetry.data_upload_optin";
   const isUnified = Preferences.get(TelemetryUtils.Preferences.Unified, false);
   if (!isUnified) {
-    // Skipping the test if unified telemetry is off, as no deletion-request ping will be generated.
+    // Skipping the test if unified telemetry is off, as no optout ping will
+    // be generated.
     return;
   }
 
   // Check that the optin probe is not set.
   // (If there are no recorded scalars, "parent" will be undefined).
   let snapshot = Telemetry.getSnapshotForScalars("main", false).parent || {};
   Assert.ok(
     !(OPTIN_PROBE in snapshot),
@@ -178,28 +179,28 @@ add_task(async function test_disableData
   let firstClientId = ping.clientId;
   Assert.ok(firstClientId, "Test ping needs a client ID");
   Assert.notEqual(
     TelemetryUtils.knownClientID,
     firstClientId,
     "Client ID should be valid and random"
   );
 
-  // Disable FHR upload: this should trigger a deletion-request ping.
+  // Disable FHR upload: this should trigger a optout ping.
   Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, false);
 
   ping = await PingServer.promiseNextPing();
-  checkPingFormat(ping, DELETION_REQUEST_PING_TYPE, true, false);
+  checkPingFormat(ping, OPTOUT_PING_TYPE, false, false);
   // Wait on ping activity to settle.
   await TelemetrySend.testWaitOnOutgoingPings();
 
   snapshot = Telemetry.getSnapshotForScalars("main", false).parent || {};
   Assert.ok(
     !(OPTIN_PROBE in snapshot),
-    "Data optin scalar should not be set after opt out"
+    "Data optin scalar should not be set after optout"
   );
 
   // Restore FHR Upload.
   Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, true);
 
   // We need to wait until the scalar is set
   await ContentTaskUtils.waitForCondition(() => {
     const scalarSnapshot = Telemetry.getSnapshotForScalars("main", false);
@@ -210,49 +211,40 @@ add_task(async function test_disableData
   });
 
   snapshot = Telemetry.getSnapshotForScalars("main", false).parent || {};
   Assert.ok(
     snapshot[OPTIN_PROBE],
     "Enabling data upload should set optin probe"
   );
 
-  // The clientId should've been reset when we restored FHR Upload.
-  let secondClientId = TelemetryController.getCurrentPingData().clientId;
-  Assert.notEqual(
-    firstClientId,
-    secondClientId,
-    "The client id must have changed"
-  );
-
-  // Simulate a failure in sending the deletion-request ping by disabling the HTTP server.
+  // Simulate a failure in sending the optout ping by disabling the HTTP server.
   await PingServer.stop();
 
   // Try to send a ping. It will be saved as pending  and get deleted when disabling upload.
   TelemetryController.submitExternalPing(TEST_PING_TYPE, {});
 
-  // Disable FHR upload to send a deletion-request ping again.
+  // Disable FHR upload to send a optout ping again.
   Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, false);
 
   // Wait on sending activity to settle, as |TelemetryController.testReset()| doesn't do that.
   await TelemetrySend.testWaitOnOutgoingPings();
   // Wait for the pending pings to be deleted. Resetting TelemetryController doesn't
   // trigger the shutdown, so we need to call it ourselves.
   await TelemetryStorage.shutdown();
   // Simulate a restart, and spin the send task.
   await TelemetryController.testReset();
 
   // Disabling Telemetry upload must clear out all the pending pings.
   let pendingPings = await TelemetryStorage.loadPendingPingList();
   Assert.equal(
     pendingPings.length,
-    1,
-    "All the pending pings should have been deleted, except the deletion-request ping"
+    0,
+    "All the pending pings should have been deleted, including the optout ping"
   );
-  Assert.ok(true, JSON.stringify(PingServer._defers) + "\n\n\n");
 
   // Enable the ping server again.
   PingServer.start();
   // We set the new server using the pref, otherwise it would get reset with
   // |TelemetryController.testReset|.
   Preferences.set(
     TelemetryUtils.Preferences.Server,
     "http://localhost:" + PingServer.port
@@ -264,41 +256,32 @@ add_task(async function test_disableData
   await TelemetryController.testReset();
 
   // Re-enable Telemetry
   Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, true);
 
   // Send a test ping
   await sendPing(true, false);
 
-  // We should have received the test ping first.
+  // We should have only received the test ping
   ping = await PingServer.promiseNextPing();
   checkPingFormat(ping, TEST_PING_TYPE, true, false);
 
   // The data in the test ping should be different than before
   Assert.notEqual(
     TelemetryUtils.knownClientID,
     ping.clientId,
     "Client ID should be reset to a random value"
   );
   Assert.notEqual(
     firstClientId,
     ping.clientId,
     "Client ID should be different from the previous value"
   );
 
-  // The "deletion-request" ping should come next, as it was pending.
-  ping = await PingServer.promiseNextPing();
-  checkPingFormat(ping, DELETION_REQUEST_PING_TYPE, true, false);
-  Assert.equal(
-    secondClientId,
-    ping.clientId,
-    "Deletion must be requested for correct client id"
-  );
-
   // Wait on ping activity to settle before moving on to the next test. If we were
   // to shut down telemetry, even though the PingServer caught the expected pings,
   // TelemetrySend could still be processing them (clearing pings would happen in
   // a couple of ticks). Shutting down would cancel the request and save them as
   // pending pings.
   await TelemetrySend.testWaitOnOutgoingPings();
 });
 
@@ -416,20 +399,21 @@ add_task(async function test_archivePing
   // With unified telemetry the FHR upload pref controls this,
   // with non-unified telemetry the Telemetry enabled pref.
   const isUnified = Preferences.get(TelemetryUtils.Preferences.Unified, false);
   const uploadPref = isUnified
     ? TelemetryUtils.Preferences.FhrUploadEnabled
     : TelemetryUtils.Preferences.TelemetryEnabled;
   Preferences.set(uploadPref, false);
 
-  // If we're using unified telemetry, disabling ping upload will generate a "deletion-request" ping. Catch it.
+  // If we're using unified telemetry, disabling ping upload will generate a "optout"
+  // ping. Catch it.
   if (isUnified) {
     let ping = await PingServer.promiseNextPing();
-    checkPingFormat(ping, DELETION_REQUEST_PING_TYPE, true, false);
+    checkPingFormat(ping, OPTOUT_PING_TYPE, false, false);
   }
 
   // Register a new Ping Handler that asserts if a ping is received, then send a ping.
   PingServer.registerPingHandler(() =>
     Assert.ok(false, "Telemetry must not send pings if not allowed to.")
   );
   let pingId = await sendPing(true, true);