Bug 1166705 - Don't send a saved-session ping when extended Telemetry is off. r=vladan a=sledru
authorGeorg Fritzsche <georg.fritzsche@googlemail.com>
Fri, 22 May 2015 22:42:29 +0700
changeset 275128 1424c5c5e1d46175b36cefcdd60368ce7ca054df
parent 275127 a619e70d24ae00e30754fa1fb8bf08c48b52b811
child 275129 4f9750a7c70f454128d88979f7433569e9a4bbc7
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvladan, sledru
bugs1166705
milestone40.0a2
Bug 1166705 - Don't send a saved-session ping when extended Telemetry is off. r=vladan a=sledru This also cleans up the pending pings persistance, putting the control of saving them out of TelemetrySession.
toolkit/components/telemetry/TelemetryController.jsm
toolkit/components/telemetry/TelemetrySession.jsm
toolkit/components/telemetry/TelemetryStorage.jsm
--- a/toolkit/components/telemetry/TelemetryController.jsm
+++ b/toolkit/components/telemetry/TelemetryController.jsm
@@ -230,37 +230,16 @@ this.TelemetryController = Object.freeze
    * @param {bool} aSubsession Whether to get subsession data. Optional, defaults to false.
    * @return {object} The current ping data in object form.
    */
   getCurrentPingData: function(aSubsession = false) {
     return Impl.getCurrentPingData(aSubsession);
   },
 
   /**
-   * Add the ping to the pending ping list and save all pending pings.
-   *
-   * @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} A promise that resolves when the pings are saved.
-   */
-  savePendingPings: function(aType, aPayload, aOptions = {}) {
-    let options = aOptions;
-    options.addClientId = aOptions.addClientId || false;
-    options.addEnvironment = aOptions.addEnvironment || false;
-
-    return Impl.savePendingPings(aType, aPayload, options);
-  },
-
-  /**
    * Save a ping to disk.
    *
    * @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
@@ -645,38 +624,16 @@ let Impl = {
     })];
 
     let promise = Promise.all(p);
     this._trackPendingPingTask(promise);
     return promise;
   },
 
   /**
-   * Saves all the pending pings, plus the passed one, to disk.
-   *
-   * @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.
-   *
-   * @returns {Promise} A promise that resolves when all the pings are saved to disk.
-   */
-  savePendingPings: function savePendingPings(aType, aPayload, aOptions) {
-    this._log.trace("savePendingPings - Type " + aType + ", Server " + this._server +
-                    ", aOptions " + JSON.stringify(aOptions));
-
-    let pingData = this.assemblePing(aType, aPayload, aOptions);
-    return TelemetryStorage.savePendingPings(pingData);
-  },
-
-  /**
    * Save a ping to disk.
    *
    * @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
--- a/toolkit/components/telemetry/TelemetrySession.jsm
+++ b/toolkit/components/telemetry/TelemetrySession.jsm
@@ -1613,52 +1613,54 @@ let Impl = {
   sendContentProcessPing: function sendContentProcessPing(reason) {
     this._log.trace("sendContentProcessPing - Reason " + reason);
     const isSubsession = !this._isClassicReason(reason);
     let payload = this.getSessionPayload(reason, isSubsession);
     payload.childUUID = this._processUUID;
     cpmm.sendAsyncMessage(MESSAGE_TELEMETRY_PAYLOAD, payload);
   },
 
-  /**
-   * Save both the "saved-session" and the "shutdown" pings to disk.
-   */
-  savePendingPings: function savePendingPings() {
-    this._log.trace("savePendingPings");
+   /**
+    * Save both the "saved-session" and the "shutdown" pings to disk.
+    */
+  saveShutdownPings: Task.async(function*() {
+    this._log.trace("saveShutdownPings");
 
-    if (!IS_UNIFIED_TELEMETRY) {
-      return this.savePendingPingsClassic();
-    }
-
-    let options = {
-      addClientId: true,
-      addEnvironment: true,
-      overwrite: true,
-    };
+    if (IS_UNIFIED_TELEMETRY) {
+      try {
+        let shutdownPayload = this.getSessionPayload(REASON_SHUTDOWN, false);
 
-    let shutdownPayload = this.getSessionPayload(REASON_SHUTDOWN, false);
-    // Make sure we try to save the pending pings, even though we failed saving the shutdown
-    // ping.
-    return TelemetryController.addPendingPing(getPingType(shutdownPayload), shutdownPayload, options)
-                        .then(() => this.savePendingPingsClassic(),
-                              () => this.savePendingPingsClassic());
-  },
+        let options = {
+          addClientId: true,
+          addEnvironment: true,
+          overwrite: true,
+        };
+        yield TelemetryController.addPendingPing(getPingType(shutdownPayload), shutdownPayload, options);
+      } catch (ex) {
+        this._log.error("saveShutdownPings - failed to submit shutdown ping", ex);
+      }
+     }
 
-  /**
-   * Save the "saved-session" ping and make TelemetryController save all the pending pings to disk.
-   */
-  savePendingPingsClassic: function savePendingPingsClassic() {
-    this._log.trace("savePendingPingsClassic");
-    let payload = this.getSessionPayload(REASON_SAVED_SESSION, false);
-    let options = {
-      addClientId: true,
-      addEnvironment: true,
-    };
-    return TelemetryController.savePendingPings(getPingType(payload), payload, options);
-  },
+    // As a temporary measure, we want to submit saved-session too if extended Telemetry is enabled
+    // to keep existing performance analysis working.
+    if (Telemetry.canRecordExtended) {
+      try {
+        let payload = this.getSessionPayload(REASON_SAVED_SESSION, false);
+
+        let options = {
+          addClientId: true,
+          addEnvironment: true,
+        };
+        yield TelemetryController.addPendingPing(getPingType(payload), payload, options);
+      } catch (ex) {
+        this._log.error("saveShutdownPings - failed to submit saved-session ping", ex);
+      }
+    }
+  }),
+
 
   testSaveHistograms: function testSaveHistograms(file) {
     this._log.trace("testSaveHistograms - Path: " + file.path);
     let payload = this.getSessionPayload(REASON_SAVED_SESSION, false);
     let options = {
       addClientId: true,
       addEnvironment: true,
       overwrite: true,
@@ -1837,17 +1839,17 @@ let Impl = {
 
       let reset = () => {
         this._initStarted = false;
         this._initialized = false;
       };
 
       if (Telemetry.isOfficialTelemetry || testing) {
         return Task.spawn(function*() {
-          yield this.savePendingPings();
+          yield this.saveShutdownPings();
           yield this._stateSaveSerializer.flushTasks();
 
           if (IS_UNIFIED_TELEMETRY) {
             yield TelemetryController.removeAbortedSessionPing();
           }
 
           reset();
         }.bind(this));
--- a/toolkit/components/telemetry/TelemetryStorage.jsm
+++ b/toolkit/components/telemetry/TelemetryStorage.jsm
@@ -213,26 +213,16 @@ this.TelemetryStorage = {
    * if it exists.
    * @returns {promise}
    */
   savePing: function(ping, overwrite) {
     return TelemetryStorageImpl.savePing(ping, overwrite);
   },
 
   /**
-   * Save all pending pings.
-   *
-   * @param {object} sessionPing The additional session ping.
-   * @returns {promise}
-   */
-  savePendingPings: function(sessionPing) {
-    return TelemetryStorageImpl.savePendingPings(sessionPing);
-  },
-
-  /**
    * Add a ping to the saved pings directory so that it gets saved
    * and sent along with other pings.
    *
    * @param {Object} pingData The ping object.
    * @return {Promise} A promise resolved when the ping is saved to the pings directory.
    */
   addPendingPing: function(pingData) {
     return TelemetryStorageImpl.addPendingPing(pingData);
@@ -455,16 +445,17 @@ let TelemetryStorageImpl = {
   /**
    * Shutdown & block on any outstanding async activity in this module.
    *
    * @return {Promise} Promise that is resolved when shutdown is complete.
    */
   shutdown: Task.async(function*() {
     this._shutdown = true;
     yield this._abortedSessionSerializer.flushTasks();
+    yield this.savePendingPings();
     // If the archive cleaning task is running, block on it. It should bail out as soon
     // as possible.
     yield this._archiveCleanTask;
   }),
 
   /**
    * Save an archived ping to disk.
    *
@@ -753,27 +744,22 @@ let TelemetryStorageImpl = {
       let file = pingFilePath(ping);
       yield this.savePingToFile(ping, file, overwrite);
     }.bind(this));
   },
 
   /**
    * Save all pending pings.
    *
-   * @param {object} sessionPing The additional session ping.
    * @returns {promise}
    */
-  savePendingPings: function(sessionPing) {
-    let p = pendingPings.reduce((p, ping) => {
-      // Restore the files with the previous pings if for some reason they have
-      // been deleted, don't overwrite them otherwise.
-      p.push(this.savePing(ping, false));
-      return p;}, [this.savePing(sessionPing, true)]);
-
-    pendingPings = [];
+  savePendingPings: function() {
+    let p = [for (ping of pendingPings) this.savePing(ping, false).catch(ex => {
+      this._log.error("savePendingPings - failed to save pending pings.");
+    })];
     return Promise.all(p);
   },
 
   /**
    * Add a ping from an existing file to the saved pings directory so that it gets saved
    * and sent along with other pings.
    * Note: that the original ping file will not be modified.
    *