Bug 1585410 - Implement and document 'deletion-request' ping r=janerik,Dexter
☠☠ backed out by 8c1ac1675109 ☠ ☠
authorChris H-C <chutten@mozilla.com>
Tue, 12 Nov 2019 22:46:23 +0000
changeset 501936 bbe85df3365a23b6400c9f0b453f316d950b64c4
parent 501935 474430e7ea37c836c40d3fb9f93a3566f7e36af1
child 501937 a78842ddeb296e3093b51b4075a27cc617d42e4a
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)
reviewersjanerik, Dexter
bugs1585410
milestone72.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 1585410 - Implement and document 'deletion-request' ping r=janerik,Dexter The 'deletion-request' ping, which supercedes the 'optout' ping, notifies the pipeline when a profile opts out of FHR upload. (IOW, when a user on a specific profile unchecks the box in about:preferences#privacy about sharing technical and interaction data with Mozilla). This ping tries its best to reach the pipeline to let them know that we need to delete data associated with the provided clientId. This means it will remain pending on the client even after opt out and it will try to resend if upload is ever re-enabled. Differential Revision: https://phabricator.services.mozilla.com/D51710
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/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_OPTOUT = "optout";
+const PING_TYPE_DELETION_REQUEST = "deletion-request";
 
 // Session ping reasons.
 const REASON_GATHER_PAYLOAD = "gather-payload";
 const REASON_GATHER_SUBSESSION_PAYLOAD = "gather-subsession-payload";
 
 XPCOMUtils.defineLazyServiceGetter(
   this,
   "Telemetry",
@@ -195,16 +195,18 @@ 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);
@@ -228,16 +230,18 @@ 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;
@@ -374,16 +378,18 @@ 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)
     );
 
@@ -397,18 +403,18 @@ var Impl = {
       type: aType,
       id: Policy.generatePingId(),
       creationDate: Policy.now().toISOString(),
       version: PING_FORMAT_VERSION,
       application: this._getApplicationSection(),
       payload,
     };
 
-    if (aOptions.addClientId) {
-      pingData.clientId = this._clientID;
+    if (aOptions.addClientId || aOptions.overrideClientId) {
+      pingData.clientId = aOptions.overrideClientId || 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()) {
@@ -442,23 +448,26 @@ 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) {
+    if (!this._clientID && aOptions.addClientId && !aOptions.overrideClientId) {
+      this._log.trace("_submitPingLogic - Waiting on client id");
       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();
     }
 
@@ -492,16 +501,18 @@ 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)
@@ -554,16 +565,18 @@ 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 +
@@ -997,17 +1010,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 optout ping.
+   * the preferences panel), this triggers sending the "deletion-request" ping.
    */
   _onUploadPrefChange() {
     const uploadEnabled = Services.prefs.getBoolPref(
       TelemetryUtils.Preferences.FhrUploadEnabled,
       false
     );
     if (uploadEnabled) {
       this._log.trace(
@@ -1045,37 +1058,42 @@ 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 optout ping.
-        this._log.trace("_onUploadPrefChange - Sending optout ping.");
-        this.submitExternalPing(PING_TYPE_OPTOUT, {}, { addClientId: false });
+        // 6. Send the deletion-request ping.
+        this._log.trace("_onUploadPrefChange - Sending deletion-request ping.");
+        this.submitExternalPing(
+          PING_TYPE_DELETION_REQUEST,
+          {},
+          { overrideClientId: oldClientId }
+        );
       }
     })();
 
     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 optout pings.
+      // Watch the FHR upload setting to trigger "deletion-request" 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_OPTOUT = "optout";
+const PING_TYPE_DELETION_REQUEST = "deletion-request";
 
 // 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 an optout ping.
+ * Check if the provided ping is a deletion-request ping.
  * @param {Object} aPing The ping to check.
- * @return {Boolean} True if the ping is an optout ping, false otherwise.
+ * @return {Boolean} True if the ping is a deletion-request ping, false otherwise.
  */
-function isOptoutPing(aPing) {
-  return isV4PingFormat(aPing) && aPing.type == PING_TYPE_OPTOUT;
+function isDeletionRequestPing(aPing) {
+  return isV4PingFormat(aPing) && aPing.type == PING_TYPE_DELETION_REQUEST;
 }
 
 /**
  * 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 an optout ping, don't block it.
+   * If trying to send a deletion-request 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 "optout" pings
+      // Filter out all the pings we can't send now. This addresses scenarios like "deletion-request" 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 an unpersisted optout ping
+        // If sending is disabled, only handle deletion-request pings
         pending = [];
-        current = current.filter(p => isOptoutPing(p));
+        current = current.filter(p => isDeletionRequestPing(p));
       }
       this._log.trace(
         "_doSendTask - can send - pending: " +
           pending.length +
           ", current: " +
           current.length
       );
 
@@ -1078,29 +1078,21 @@ var TelemetrySendImpl = {
     ];
 
     for (let current of currentPings) {
       let ping = current;
       let p = (async () => {
         try {
           await this._doPing(ping, ping.id, false);
         } catch (ex) {
-          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);
-          }
+          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);
     }
@@ -1453,17 +1445,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 an optout ping, don't block it.
+   * If trying to send a "deletion-request" 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 (
@@ -1472,18 +1464,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) {
-      // Optout pings are sent once even if the upload is disabled.
-      if (ping && isOptoutPing(ping)) {
+      // "deletion-request" pings are sent once even if the upload is disabled.
+      if (ping && isDeletionRequestPing(ping)) {
         return true;
       }
       return Services.prefs.getBoolPref(
         TelemetryUtils.Preferences.FhrUploadEnabled,
         false
       );
     }
 
@@ -1518,21 +1510,18 @@ var TelemetrySendImpl = {
     );
     p.push(SendScheduler.waitOnSendTask());
     return Promise.all(p);
   },
 
   async _persistCurrentPings() {
     for (let [id, ping] of this._currentPings) {
       try {
-        // Never save an optout ping to disk
-        if (!isOptoutPing(ping)) {
-          await savePing(ping);
-          this._log.trace("_persistCurrentPings - saved ping " + id);
-        }
+        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:`optout <../data/optout-ping>` - sent when FHR upload is disabled
+* :doc:`deletion-request <../data/deletion-request-ping>` - sent when FHR upload is disabled
new file mode 100644
--- /dev/null
+++ b/toolkit/components/telemetry/docs/data/deletion-request-ping.rst
@@ -0,0 +1,35 @@
+"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,18 +5,16 @@ 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.
--- a/toolkit/components/telemetry/docs/obsolete/index.rst
+++ b/toolkit/components/telemetry/docs/obsolete/index.rst
@@ -6,8 +6,9 @@ Obsolete Documentation
 
 .. toctree::
    :maxdepth: 5
    :titlesonly:
 
    uitelemetry/index
    fhr/index
    hybrid-content
+   optout-ping
rename from toolkit/components/telemetry/docs/data/optout-ping.rst
rename to toolkit/components/telemetry/docs/obsolete/optout-ping.rst
--- a/toolkit/components/telemetry/docs/data/optout-ping.rst
+++ b/toolkit/components/telemetry/docs/obsolete/optout-ping.rst
@@ -1,11 +1,11 @@
 
-"optout" ping
-=============
+"optout" ping (obsolete)
+========================
 
 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/tests/marionette/harness/telemetry_harness/ping_filters.py
+++ b/toolkit/components/telemetry/tests/marionette/harness/telemetry_harness/ping_filters.py
@@ -5,16 +5,26 @@
 
 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"
@@ -51,25 +61,15 @@ 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_optout_ping.py
rename to toolkit/components/telemetry/tests/marionette/tests/client/test_deletion_request_ping.py
--- a/toolkit/components/telemetry/tests/marionette/tests/client/test_optout_ping.py
+++ b/toolkit/components/telemetry/tests/marionette/tests/client/test_deletion_request_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, OPTOUT_PING, MAIN_SHUTDOWN_PING
+from telemetry_harness.ping_filters import ANY_PING, DELETION_REQUEST_PING, MAIN_SHUTDOWN_PING
 
 
-class TestOptoutPing(TelemetryTestCase):
-    """Tests for "optout" ping."""
+class TestDeletionRequestPing(TelemetryTestCase):
+    """Tests for "deletion-request" 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 "optout" ping behaviour across sessions."""
+        """Test the "deletion-request" 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 "optout" ping.
-        optout_ping = self.wait_for_ping(self.disable_telemetry, OPTOUT_PING)
+        # Trigger an "deletion-request" ping.
+        ping = self.wait_for_ping(self.disable_telemetry, DELETION_REQUEST_PING)
 
-        self.assertNotIn("clientId", optout_ping)
-        self.assertIn("payload", optout_ping)
-        self.assertNotIn("environment", optout_ping["payload"])
+        self.assertIn("clientId", ping)
+        self.assertIn("payload", ping)
+        self.assertNotIn("environment", 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], optout_ping)
+        self.assertEqual(self.ping_server.pings[-1], 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 should never appear in a ping on the server"
+    `Known clientId shouldn't appear in a "${payload.type}" 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 OPTOUT_PING_TYPE = "optout";
+const DELETION_REQUEST_PING_TYPE = "deletion-request";
 const TEST_PING_TYPE = "test-ping-type";
 
 function sendPing(addEnvironment = false) {
   let options = {
     addClientId: true,
     addEnvironment,
   };
   return TelemetryController.submitExternalPing(TEST_PING_TYPE, {}, options);
@@ -50,37 +50,41 @@ 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 optout ping reset included.
+ * with and without the deletion-request 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 optout ping.
+  // Disable FHR upload: this should trigger a deletion-request 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));
+  Assert.equal(
+    ping.type,
+    DELETION_REQUEST_PING_TYPE,
+    "The ping must be a deletion-request ping"
+  );
+  Assert.equal(ping.clientId, firstClientId);
   let clientId = await ClientID.getClientID();
   Assert.equal(TelemetryUtils.knownClientID, clientId);
 
   // Now shutdown the instance
   await TelemetryController.testShutdown();
   await TelemetryStorage.testClearPendingPings();
 
   // Flip the pref again
@@ -110,37 +114,41 @@ 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 optout ping reset included.
+ * with and without the deletion-request 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 optout ping.
+  // Disable FHR upload: this should trigger a deletion-request 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));
+  Assert.equal(
+    ping.type,
+    DELETION_REQUEST_PING_TYPE,
+    "The ping must be a deletion-request ping"
+  );
+  Assert.equal(ping.clientId, firstClientId);
   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 OPTOUT_PING_TYPE = "optout";
+const DELETION_REQUEST_PING_TYPE = "deletion-request";
 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,18 +154,17 @@ 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 optout ping will
-    // be generated.
+    // Skipping the test if unified telemetry is off, as no deletion-request 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),
@@ -179,28 +178,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 optout ping.
+  // Disable FHR upload: this should trigger a deletion-request ping.
   Preferences.set(TelemetryUtils.Preferences.FhrUploadEnabled, false);
 
   ping = await PingServer.promiseNextPing();
-  checkPingFormat(ping, OPTOUT_PING_TYPE, false, false);
+  checkPingFormat(ping, DELETION_REQUEST_PING_TYPE, true, 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 optout"
+    "Data optin scalar should not be set after opt out"
   );
 
   // 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);
@@ -211,40 +210,49 @@ add_task(async function test_disableData
   });
 
   snapshot = Telemetry.getSnapshotForScalars("main", false).parent || {};
   Assert.ok(
     snapshot[OPTIN_PROBE],
     "Enabling data upload should set optin probe"
   );
 
-  // Simulate a failure in sending the optout ping by disabling the HTTP server.
+  // 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.
   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 optout ping again.
+  // Disable FHR upload to send a deletion-request 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,
-    0,
-    "All the pending pings should have been deleted, including the optout ping"
+    1,
+    "All the pending pings should have been deleted, except the deletion-request 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
@@ -256,32 +264,41 @@ 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 only received the test ping
+  // We should have received the test ping first.
   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();
 });
 
@@ -399,21 +416,20 @@ 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 "optout"
-  // ping. Catch it.
+  // If we're using unified telemetry, disabling ping upload will generate a "deletion-request" ping. Catch it.
   if (isUnified) {
     let ping = await PingServer.promiseNextPing();
-    checkPingFormat(ping, OPTOUT_PING_TYPE, false, false);
+    checkPingFormat(ping, DELETION_REQUEST_PING_TYPE, true, 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);