Bug 1686685 - Implement the deletion request ping (#42)

Author: Beatriz Rizental

src/constants.ts

@@ -35,3 +35,6 @@ export const KNOWN_CLIENT_ID = "c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0";
 
 // The default server pings are sent to.
 export const DEFAULT_TELEMETRY_ENDPOINT = "https://incoming.telemetry.mozilla.org";
+
+// The name of the deletion-request ping.
+export const DELETION_REQUEST_PING_NAME = "deletion-request";

src/glean.ts

@@ -14,6 +14,7 @@ import EventsDatabase from "metrics/events_database";
 import UUIDMetricType from "metrics/types/uuid";
 import DatetimeMetricType, { DatetimeMetric } from "metrics/types/datetime";
 import Dispatcher from "dispatcher";
+import CorePings from "internal_pings";
 
 class Glean {
   // The Glean singleton.
@@ -29,6 +30,8 @@ class Glean {
   private _initialized: boolean;
   // Instances of Glean's core metrics.
   private _coreMetrics: CoreMetrics;
+  // Instances of Glean's core pings.
+  private _corePings: CorePings;
   // The ping uploader.
   private _pingUploader: PingUploader
   // A task dispatcher to help execute in order asynchronous external API calls.
@@ -53,6 +56,7 @@ class Glean {
     this._dispatcher = new Dispatcher();
     this._pingUploader = new PingUploader();
     this._coreMetrics = new CoreMetrics();
+    this._corePings = new CorePings();
     this._initialized = false;
     this._db = {
       metrics: new MetricsDatabase(),
@@ -77,6 +81,10 @@ class Glean {
     return Glean.instance._coreMetrics;
   }
 
+  private static get corePings(): CorePings {
+    return Glean.instance._corePings;
+  }
+
   private static get uploadEnabled(): boolean {
     return Glean.instance._uploadEnabled || false;
   }
@@ -109,6 +117,9 @@ class Glean {
   private static async onUploadDisabled(): Promise<void> {
     Glean.uploadEnabled = false;
     await Glean.clearMetrics();
+    // Note that `submit` is a dispatched function.
+    // The actual submission will only happen after we leave `onUploadDisabled`.
+    Glean.corePings.deletionRequest.submit();
   }
 
   /**
@@ -126,7 +137,7 @@ class Glean {
     //
     // Note: This will throw in case the stored metric is incorrect or inexistent.
     // The most likely is that it throws if the metrics hasn't been set,
-    // e.g. we start Glean for the first with tupload disabled.
+    // e.g. we start Glean for the first with upload disabled.
     let firstRunDate: Date;
     try {
       firstRunDate = new DatetimeMetric(
@@ -220,6 +231,13 @@ class Glean {
       // IMPORTANT!
       // Any pings we want to send upon initialization should happen before this.
       await Glean.metricsDatabase.clear(Lifetime.Application);
+
+      // We need to mark Glean as initialized before dealing with the upload status,
+      // otherwise we will not be able to submit deletion-request pings if necessary.
+      //
+      // This is fine, we are inside a dispatched task that is guaranteed to run before any
+      // other task. No external API call will be executed before we leave this task.
+      Glean.instance._initialized = true;
 
       // The upload enabled flag may have changed since the last run, for
       // example by the changing of a config file.
@@ -242,96 +260,52 @@ class Glean {
 
         if (clientId) {
           if (clientId !== KNOWN_CLIENT_ID) {
-            // Temporarily enable uploading so we can submit a
-            // deletion request ping.
-            Glean.uploadEnabled = true;
             await Glean.onUploadDisabled();
           }
         } else {
+          // Call `clearMetrics` directly here instead of `onUploadDisabled` to avoid sending
+          // a deletion-request ping for a user that has already done that.
           await Glean.clearMetrics();
         }
       }
 
-      Glean.instance._initialized = true;
-
       await Glean.pingUploader.scanPendingPings();
 
       // Even though this returns a promise, there is no need to block on it returning.
       //
-      // On the contrary we _want_ the dispatcher to execute tasks async.
+      // On the contrary we _want_ the uploading tasks to be executed async.
       void Glean.pingUploader.triggerUpload();
     });
   }
 
-  /**
-   * Gets this Glean's instance metrics database.
-   *
-   * @returns This Glean's instance metrics database.
-   */
   static get metricsDatabase(): MetricsDatabase {
     return Glean.instance._db.metrics;
   }
 
-  /**
-   * Gets this Glean's instance events database.
-   *
-   * @returns This Glean's instance events database.
-   */
   static get eventsDatabase(): EventsDatabase {
     return Glean.instance._db.events;
   }
 
-
-  /**
-   * Gets this Glean's instance pings database.
-   *
-   * @returns This Glean's instance pings database.
-   */
   static get pingsDatabase(): PingsDatabase {
     return Glean.instance._db.pings;
   }
 
-  /**
-   * Gets this Glean's instance initialization status.
-   *
-   * @returns Whether or not the Glean singleton has been initialized.
-   */
   static get initialized(): boolean {
     return Glean.instance._initialized;
   }
 
-  /**
-   * Gets this Glean's instance application id.
-   *
-   * @returns The application id or `undefined` in case Glean has not been initialized yet.
-   */
   static get applicationId(): string | undefined {
     return Glean.instance._applicationId;
   }
 
-  /**
-   * Gets this Glean's instance server endpoint.
-   *
-   * @returns The server endpoint or `undefined` in case Glean has not been initialized yet.
-   */
   static get serverEndpoint(): string | undefined {
     return Glean.instance._config?.serverEndpoint;
   }
 
-  /**
-   * Whether or not to log pings upon collection.
-   *
-   * @returns Whether or not to log pings upon collection.
-   */
   static get logPings(): boolean {
     return Glean.instance._config?.debug?.logPings || false;
   }
 
-  /**
-   * Gets this Gleans's instance dispatcher.
-   *
-   * @returns The dispatcher instance.
-   */
   static get dispatcher(): Dispatcher {
     return Glean.instance._dispatcher;
   }

src/internal_pings.ts

@@ -0,0 +1,31 @@
+/* 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/. */
+
+import { DELETION_REQUEST_PING_NAME } from "./constants";
+import PingType from "pings";
+
+/**
+ * Glean-provided pings, all enabled by default.
+ *
+ * Pings initialized here should be defined in `pings.yaml`
+ * and for now manually translated into JS code.
+ *
+ * TODO: This file should be auto-generated once Bug 1691365 is resolved.
+ */
+class CorePings {
+  // This ping is intended to communicate to the Data Pipeline
+  // that the user wishes to have their reported Telemetry data deleted.
+  // As such it attempts to send itself at the moment the user opts out of data collection.
+  readonly deletionRequest: PingType;
+
+  constructor() {
+    this.deletionRequest = new PingType(
+      DELETION_REQUEST_PING_NAME,
+      /* include client id */ true,
+      /* send if empty */ true
+    );
+  }
+}
+
+export default CorePings;

src/pings/index.ts

@@ -2,9 +2,9 @@
  * 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/. */
 
-
-import collectAndStorePing from "pings/maker";
+import { DELETION_REQUEST_PING_NAME } from "../constants";
 import { generateUUIDv4 } from "utils";
+import collectAndStorePing from "pings/maker";
 import Glean from "glean";
 
 /**
@@ -21,15 +21,19 @@ class PingType {
    * @param name  The name of the ping.
    * @param includeClientId Whether to include the client ID in the assembled ping when submitting.
    * @param sendIfEmtpy Whether the ping should be sent empty or not.
-   * @param reasonCodes The valid reason codes for this ping.
+   * @param reasonCodes Optional. The valid reason codes for this ping.
    */
   constructor (
     readonly name: string,
     readonly includeClientId: boolean,
     readonly sendIfEmtpy: boolean,
-    readonly reasonCodes: string[]
+    readonly reasonCodes: string[] = []
   ) {}
 
+  private isDeletionRequest(): boolean {
+    return this.name === DELETION_REQUEST_PING_NAME;
+  }
+
   /**
    * Collects and submits a ping for eventual uploading.
    *
@@ -48,9 +52,9 @@ class PingType {
         console.info("Glean must be initialized before submitting pings.");
         return;
       }
-  
-      if (!Glean.isUploadEnabled()) {
-        console.info("Glean disabled: not submitting any pings.");
+
+      if (!Glean.isUploadEnabled() && !this.isDeletionRequest()) {
+        console.info("Glean disabled: not submitting pings. Glean may still submit the deletion-request ping.");
         return;
       }
 

src/upload/index.ts

@@ -148,7 +148,7 @@ class PingUploader implements PingsDatabaseObserver {
    * @returns The status number of the response or `undefined` if unable to attempt upload.
    */
   private async attemptPingUpload(ping: QueuedPing): Promise<UploadResult> {
-    if (!Glean.initialize) {
+    if (!Glean.initialized) {
       console.warn("Attempted to upload a ping, but Glean is not initialized yet. Ignoring.");
       return { result: UploadResultStatus.RecoverableFailure };
     }

tests/glean.spec.ts

@@ -5,7 +5,7 @@
 import assert from "assert";
 import sinon from "sinon";
 
-import { CLIENT_INFO_STORAGE, KNOWN_CLIENT_ID } from "../src/constants";
+import { CLIENT_INFO_STORAGE, DELETION_REQUEST_PING_NAME, KNOWN_CLIENT_ID } from "../src/constants";
 import Glean from "glean";
 import { Lifetime } from "metrics";
 import StringMetricType from "metrics/types/string";
@@ -210,10 +210,75 @@ describe("Glean", function() {
     ping.submit();
 
     await Glean.dispatcher.testBlockOnQueue();
+    // TODO: Make this nicer once we resolve Bug 1691033 is resolved.
+    await Glean["pingUploader"]["currentJob"];
 
+    // Check that one ping was sent,
+    // but that ping is not our custom ping, but the deletion-request.
+    assert.ok(postSpy.getCall(0).args[0].indexOf(DELETION_REQUEST_PING_NAME) !== -1);
+    assert.strictEqual(postSpy.callCount, 1);
+  });
+
+  it("deletion request is sent when toggling upload from on to off", async function() {
+    const postSpy = sandbox.spy(Uploader, "post");
+
+    Glean.setUploadEnabled(false);
+    await Glean.dispatcher.testBlockOnQueue();
+
+    assert.strictEqual(postSpy.callCount, 1);
+    assert.ok(postSpy.getCall(0).args[0].indexOf(DELETION_REQUEST_PING_NAME) !== -1);
+
+    postSpy.resetHistory();
+    Glean.setUploadEnabled(true);
+    await Glean.dispatcher.testBlockOnQueue();
     assert.strictEqual(postSpy.callCount, 0);
+  });
+
+  it("deletion request ping is sent when toggling upload status between runs", async function() {
+    const postSpy = sandbox.spy(Uploader, "post");
 
-    // TODO: Check that a deletion request ping was sent after Bug 1686685 is resolved.
+    Glean.setUploadEnabled(true);
+    await Glean.dispatcher.testBlockOnQueue();
+
+    // Can't use testResetGlean here because it clears all stores
+    // and when there is no client_id at all stored, a deletion ping is also not set.
+    await Glean.testUninitialize();
+    Glean.initialize(GLOBAL_APPLICATION_ID, false);
+    await Glean.dispatcher.testBlockOnQueue();
+    // TODO: Make this nicer once we resolve Bug 1691033 is resolved.
+    await Glean["pingUploader"]["currentJob"];
+
+    // A deletion request is sent
+    assert.strictEqual(postSpy.callCount, 1);
+    assert.ok(postSpy.getCall(0).args[0].indexOf(DELETION_REQUEST_PING_NAME) !== -1);
+  });
+
+  it("deletion request ping is not sent if upload status does not change between runs", async function () {
+    const postSpy = sandbox.spy(Uploader, "post");
+
+    Glean.setUploadEnabled(false);
+    await Glean.dispatcher.testBlockOnQueue();
+
+    // A deletion request is sent
+    assert.strictEqual(postSpy.callCount, 1);
+    assert.ok(postSpy.getCall(0).args[0].indexOf(DELETION_REQUEST_PING_NAME) !== -1);
+
+    // Can't use testResetGlean here because it clears all stores
+    // and when there is no client_id at all stored, a deletion ping is also not set.
+    await Glean.testUninitialize();
+    Glean.initialize(GLOBAL_APPLICATION_ID, false);
+    await Glean.dispatcher.testBlockOnQueue();
+    // TODO: Make this nicer once we resolve Bug 1691033 is resolved.
+    await Glean["pingUploader"]["currentJob"];
+
+    postSpy.resetHistory();
+    assert.strictEqual(postSpy.callCount, 0);
+  });
+
+  it("deletion request ping is not sent when user starts Glean for the first time with upload disabled", async function () {
+    const postSpy = sandbox.spy(Uploader, "post");
+    await Glean.testResetGlean(GLOBAL_APPLICATION_ID, false);
+    assert.strictEqual(postSpy.callCount, 0);
   });
 
   it("setting log pings works before and after glean and on initialize", async function () {