Bug 1443603 - Stop sending `saved-session` pings from Firefox Desktop r=chutten

--HG--
extra : rebase_source : 1dc21f77aae95580761ae8a4b5fd907f8553493a

toolkit/components/telemetry/TelemetrySession.jsm

@@ -1678,16 +1678,16 @@ var Impl = {
   },
 
    /**
-    * Save both the "saved-session" and the "shutdown" pings to disk.
+    * On Desktop: Save the "shutdown" ping to disk.
+    * On Android: Save the "saved-session" ping to disk.
     * This needs to be called after TelemetrySend shuts down otherwise pings
     * would be sent instead of getting persisted to disk.
     */
   saveShutdownPings() {
     this._log.trace("saveShutdownPings");
 
-    // We don't wait for "shutdown" pings to be written to disk before gathering the
-    // "saved-session" payload. Instead we append the promises to this list and wait
-    // on both to be saved after kicking off their collection.
+    // We append the promises to this list and wait
+    // on all pings to be saved after kicking off their collection.
     let p = [];
 
     if (IS_UNIFIED_TELEMETRY) {
@@ -1727,9 +1727,7 @@ var Impl = {
       }
     }
 
-    // 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) {
+    if (AppConstants.platform == "android" && Telemetry.canRecordExtended) {
       let payload = this.getSessionPayload(REASON_SAVED_SESSION, false);
 
       let options = {
@@ -1830,7 +1828,6 @@ var Impl = {
       // content-child-shutdown is only registered for content processes.
       this.uninstall();
       Telemetry.flushBatchedChildTelemetry();
-      this.sendContentProcessPing(REASON_SAVED_SESSION);
       break;
     case TOPIC_CYCLE_COLLECTOR_BEGIN:
       let now = new Date();

toolkit/components/telemetry/docs/concepts/pings.rst

@@ -22,7 +22,7 @@ Pings sent from code that ships with Firefox are listed in the :doc:`data docume
 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. This is temporary and will be removed soon.
+* :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.

toolkit/components/telemetry/docs/data/main-ping.rst

@@ -14,7 +14,7 @@ This ping may be triggered for one of many reasons documented by the ``reason``
 * ``environment-change`` - the :doc:`environment` changed, so the session measurements got reset and a new subsession starts
 * ``shutdown`` - triggered when the browser session ends. For the first browsing session, this ping is saved to disk and sent on the next browser restart. From the second browsing session on, this ping is sent immediately on shutdown using the :doc:`../internals/pingsender`, unless the OS is shutting down
 * ``daily`` - a session split triggered in 24h hour intervals at local midnight. If an ``environment-change`` ping is generated by the time it should be sent, the daily ping is rescheduled for the next midnight
-* ``saved-session`` - the *"classic"* Telemetry payload with measurements covering the whole browser session (only submitted for a transition period)
+* ``saved-session`` - the *"classic"* Telemetry payload with measurements covering the whole browser session (only submitted on Android)
 
 Most reasons lead to a session split, initiating a new *subsession*. We reset important measurements for those subsessions.
 
@@ -22,7 +22,7 @@ After a new subsession split, the ``internal-telemetry-after-subsession-split``
 
 .. note::
 
-  ``saved-session`` is sent with a different ping type (``saved-session``, not ``main``), but otherwise has the same format as discussed here.
+  ``saved-session`` is sent with a different ping type (``saved-session``, not ``main``), but otherwise has the same format as discussed here. As of Firefox 61 this is sent on Android only.
 
 Structure:
 
@@ -691,3 +691,11 @@ Structure:
       }
       ...
     ],
+
+Version History
+===============
+
+- Firefox 61:
+
+  - Stopped reporting ``childPayloads`` (`bug 1443599 <https://bugzilla.mozilla.org/show_bug.cgi?id=1443599>`_).
+  - Stopped reporting ``saved-session`` pings on Firefox Desktop (`bug 1443603 <https://bugzilla.mozilla.org/show_bug.cgi?id=1443603>`_).

toolkit/components/telemetry/tests/unit/test_TelemetrySession.js

@@ -1349,9 +1349,6 @@ add_task(async function test_experimentAnnotations_subsession() {
 add_task(async function test_savedPingsOnShutdown() {
   await TelemetryController.testReset();
 
-  // On desktop, we expect both "saved-session" and "shutdown" pings. We only expect
-  // the former on Android.
-  const expectedPingCount = (gIsAndroid) ? 1 : 2;
   // Assure that we store the ping properly when saving sessions on shutdown.
   // We make the TelemetryController shutdown to trigger a session save.
   const dir = TelemetryStorage.pingDirectoryPath;
@@ -1362,18 +1359,14 @@ add_task(async function test_savedPingsOnShutdown() {
   PingServer.clearRequests();
   await TelemetryController.testReset();
 
-  const pings = await PingServer.promiseNextPings(expectedPingCount);
+  const ping = await PingServer.promiseNextPing();
 
-  for (let ping of pings) {
-    Assert.ok("type" in ping);
+  let expectedType   = gIsAndroid ? PING_TYPE_SAVED_SESSION : PING_TYPE_MAIN;
+  let expectedReason = gIsAndroid ? REASON_SAVED_SESSION : REASON_SHUTDOWN;
 
-    let expectedReason =
-      (ping.type == PING_TYPE_SAVED_SESSION) ? REASON_SAVED_SESSION : REASON_SHUTDOWN;
-
-    checkPingFormat(ping, ping.type, true, true);
-    Assert.equal(ping.payload.info.reason, expectedReason);
-    Assert.equal(ping.clientId, gClientID);
-  }
+  checkPingFormat(ping, expectedType, true, true);
+  Assert.equal(ping.payload.info.reason, expectedReason);
+  Assert.equal(ping.clientId, gClientID);
 });
 
 add_task(async function test_sendShutdownPing() {
@@ -1388,18 +1381,11 @@ add_task(async function test_sendShutdownPing() {
 
   let checkPendingShutdownPing = async function() {
     let pendingPings = await TelemetryStorage.loadPendingPingList();
-    Assert.equal(pendingPings.length, 2,
-                 "We expect 2 pending pings: shutdown and saved-session.");
+    Assert.equal(pendingPings.length, 1,
+                 "We expect 1 pending ping: shutdown.");
     // Load the pings off the disk.
-    const pings = [
-      await TelemetryStorage.loadPendingPing(pendingPings[0].id),
-      await TelemetryStorage.loadPendingPing(pendingPings[1].id)
-    ];
-    // Find the shutdown main ping and check that it contains the right data.
-    const shutdownPing = pings.find(p => p.type == "main");
+    const shutdownPing = await TelemetryStorage.loadPendingPing(pendingPings[0].id);
     Assert.ok(shutdownPing, "The 'shutdown' ping must be saved to disk.");
-    Assert.ok(pings.find(p => p.type == "saved-session"),
-              "The 'saved-session' ping must be saved to disk.");
     Assert.equal("shutdown", shutdownPing.payload.info.reason,
                  "The 'shutdown' ping must be saved to disk.");
   };