Bug 1233986 - Move clientId loading, generation and serialization earlier during startup. r=gfritzsche, data-review=bsmedberg

MozReview-Commit-ID: LgBGLY8gbLB

toolkit/components/telemetry/Histograms.json

@@ -5137,6 +5137,14 @@
     "kind": "count",
     "description": "Number of archived Telemetry pings discarded because they exceeded the maximum size"
   },
+  "TELEMETRY_PING_SUBMISSION_WAITING_CLIENTID": {
+    "alert_emails": ["telemetry-client-dev@mozilla.com"],
+    "expires_in_version": "never",
+    "kind": "count",
+    "releaseChannelCollection": "opt-out",
+    "bug_numbers": [1233986],
+    "description": "The number of pings that were submitted and had to wait for a client id (i.e. before it was cached or loaded from disk)"
+  },
   "TELEMETRY_DISCARDED_PENDING_PINGS_SIZE_MB": {
     "alert_emails": ["telemetry-client-dev@mozilla.com"],
     "expires_in_version": "never",

toolkit/components/telemetry/TelemetryController.jsm

@@ -299,15 +299,6 @@ this.TelemetryController = Object.freeze({
     return Impl.savePing(aType, aPayload, aFilePath, options);
   },
 
-  /**
-   * The client id send with the telemetry ping.
-   *
-   * @return The client id as string, or null.
-   */
-  get clientID() {
-    return Impl.clientID;
-  },
-
   /**
    * The session recorder instance managed by Telemetry.
    * @return {Object} The active SessionRecorder instance or null if not available.
@@ -444,8 +435,10 @@ var Impl = {
   },
 
   /**
-   * Submit ping payloads to Telemetry. This will assemble a complete ping, adding
-   * environment data, client id and some general info.
+   * Internal function to assemble a complete ping, adding environment data, client id
+   * and some general info. This waits on the client id to be loaded/generated if it's
+   * not yet available. Note that this function is synchronous unless we need to load
+   * the client id.
    * Depending on configuration, the ping will be sent to the server (immediately or later)
    * and archived locally.
    *
@@ -459,6 +452,44 @@ var Impl = {
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
    * @returns {Promise} Test-only - a promise that is resolved with the ping id once the ping is stored or sent.
    */
+  _submitPingLogic: Task.async(function* (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) {
+      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 = yield ClientID.getClientID();
+    }
+
+    const pingData = this.assemblePing(aType, aPayload, aOptions);
+    this._log.trace("submitExternalPing - ping assembled, id: " + pingData.id);
+
+    // Always persist the pings if we are allowed to. We should not yield on any of the
+    // following operations to keep this function synchronous for the majority of the calls.
+    let archivePromise = TelemetryArchive.promiseArchivePing(pingData)
+      .catch(e => this._log.error("submitExternalPing - Failed to archive ping " + pingData.id, e));
+    let p = [ archivePromise ];
+
+    p.push(TelemetrySend.submitPing(pingData));
+
+    return Promise.all(p).then(() => pingData.id);
+  }),
+
+  /**
+   * Submit ping payloads to Telemetry.
+   *
+   * @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=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.
+   * @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));
 
@@ -471,19 +502,9 @@ var Impl = {
       return Promise.reject(new Error("Invalid type string submitted."));
     }
 
-    const pingData = this.assemblePing(aType, aPayload, aOptions);
-    this._log.trace("submitExternalPing - ping assembled, id: " + pingData.id);
-
-    // Always persist the pings if we are allowed to.
-    let archivePromise = TelemetryArchive.promiseArchivePing(pingData)
-      .catch(e => this._log.error("submitExternalPing - Failed to archive ping " + pingData.id, e));
-    let p = [ archivePromise ];
-
-    p.push(TelemetrySend.submitPing(pingData));
-
-    let promise = Promise.all(p);
+    let promise = this._submitPingLogic(aType, aPayload, aOptions);
     this._trackPendingPingTask(promise);
-    return promise.then(() => pingData.id);
+    return promise;
   },
 
   /**
@@ -814,10 +835,6 @@ var Impl = {
     return undefined;
   },
 
-  get clientID() {
-    return this._clientID;
-  },
-
   /**
    * Get an object describing the current state of this module for AsyncShutdown diagnostics.
    */