Bug 1469233 - remove pingsOverdue from simpleMeasurements; r?Dexter draft
authorArjun Krishna Babu <arjunkrishnababu96@gmail.com>
Wed, 20 Jun 2018 18:51:09 +0530
changeset 810747 b100a3a5b44e29c0be06e613e6fe38d8f219f8e3
parent 808264 1e2c9151a09e43613a79daa8d4a94dc3e314020c
push id114084
push userbmo:arjunkrishnababu96@gmail.com
push dateTue, 26 Jun 2018 11:34:53 +0000
reviewersDexter
bugs1469233
milestone62.0a1
Bug 1469233 - remove pingsOverdue from simpleMeasurements; r?Dexter pingsOverdue is a telemetry-specific data field that is not used anymore. Therefore it is being removed from both the docs and TelemetrySession.jsm The logic that exports and computes the overdue pings count, and all related code, is also removed. Associated test failure (due to performing the above) is fixed by removing the offending test code. MozReview-Commit-ID: DZUapvZbC9U
toolkit/components/telemetry/TelemetrySend.jsm
toolkit/components/telemetry/TelemetrySession.jsm
toolkit/components/telemetry/docs/data/main-ping.rst
toolkit/components/telemetry/tests/unit/test_TelemetrySendOldPings.js
--- a/toolkit/components/telemetry/TelemetrySend.jsm
+++ b/toolkit/components/telemetry/TelemetrySend.jsm
@@ -77,19 +77,16 @@ const MAX_PING_SENDS_PER_MINUTE = 10;
 // send for after SEND_TICK_DELAY.
 const SEND_TICK_DELAY = 1 * MS_IN_A_MINUTE;
 // If we had any ping send failures since the last ping, we use a backoff timeout
 // for the next ping sends. We increase the delay exponentially up to a limit of
 // SEND_MAXIMUM_BACKOFF_DELAY_MS.
 // This exponential backoff will be reset by external ping submissions & idle-daily.
 const SEND_MAXIMUM_BACKOFF_DELAY_MS = 120 * MS_IN_A_MINUTE;
 
-// The age of a pending ping to be considered overdue (in milliseconds).
-const OVERDUE_PING_FILE_AGE = 7 * 24 * 60 * MS_IN_A_MINUTE; // 1 week
-
 // Strings to map from XHR.errorCode to TELEMETRY_SEND_FAILURE_TYPE.
 // Echoes XMLHttpRequestMainThread's ErrorType enum.
 // Make sure that any additions done to XHR_ERROR_TYPE enum are also mirrored in
 // TELEMETRY_SEND_FAILURE_TYPE's labels.
 const XHR_ERROR_TYPE = [
   "eOK",
   "eRequest",
   "eUnreachable",
@@ -171,23 +168,16 @@ function gzipCompressString(string) {
   converter.onStartRequest(null, null);
   converter.onDataAvailable(null, null, stringStream, 0, string.length);
   converter.onStopRequest(null, null, null);
   return observer.buffer;
 }
 
 var TelemetrySend = {
 
-  /**
-   * Age in ms of a pending ping to be considered overdue.
-   */
-  get OVERDUE_PING_FILE_AGE() {
-    return OVERDUE_PING_FILE_AGE;
-  },
-
   get pendingPingCount() {
     return TelemetrySendImpl.pendingPingCount;
   },
 
   /**
    * Partial setup that runs immediately at startup. This currently triggers
    * the crash report annotations.
    */
@@ -227,23 +217,16 @@ var TelemetrySend = {
    * @return {Promise} Test-only - a promise that is resolved when the ping is sent or saved.
    */
   submitPing(ping, options = {}) {
     options.usePingSender = options.usePingSender || false;
     return TelemetrySendImpl.submitPing(ping, options);
   },
 
   /**
-   * Count of pending pings that were found to be overdue at startup.
-   */
-  get overduePingsCount() {
-    return TelemetrySendImpl.overduePingsCount;
-  },
-
-  /**
    * Notify that we can start submitting data to the servers.
    */
   notifyCanUpload() {
     return TelemetrySendImpl.notifyCanUpload();
   },
 
   /**
    * Only used in tests. Used to reset the module data to emulate a restart.
@@ -592,18 +575,16 @@ var TelemetrySendImpl = {
   // This tracks all the pending async ping activity.
   _pendingPingActivity: new Set(),
   // This is true when running in the test infrastructure.
   _testMode: false,
   // This holds pings that we currently try and haven't persisted yet.
   _currentPings: new Map(),
   // Used to skip spawning the pingsender if OS is shutting down.
   _isOSShutdown: false,
-  // Count of pending pings that were overdue.
-  _overduePingCount: 0,
   // Has the network shut down, making it too late to send pings?
   _tooLateToSend: false,
 
   OBSERVER_TOPICS: [
     TOPIC_IDLE_DAILY,
     TOPIC_QUIT_APPLICATION_GRANTED,
     TOPIC_QUIT_APPLICATION_FORCED,
     TOPIC_PROFILE_CHANGE_NET_TEARDOWN,
@@ -622,20 +603,16 @@ var TelemetrySendImpl = {
   get _log() {
     if (!this._logger) {
       this._logger = Log.repository.getLoggerWithMessagePrefix(LOGGER_NAME, LOGGER_PREFIX);
     }
 
     return this._logger;
   },
 
-  get overduePingsCount() {
-    return this._overduePingCount;
-  },
-
   get pendingPingRequests() {
     return this._pendingPingRequests;
   },
 
   get pendingPingCount() {
     return TelemetryStorage.getPendingPingList().length + this._currentPings.size;
   },
 
@@ -726,21 +703,16 @@ var TelemetrySendImpl = {
     this._log.info("_checkPendingPings - pending ping count: " + infos.length);
     if (infos.length == 0) {
       this._log.trace("_checkPendingPings - no pending pings");
       return;
     }
 
     const now = Policy.now();
 
-    // Check for overdue pings.
-    const overduePings = infos.filter((info) =>
-      (now.getTime() - info.lastModificationDate) > OVERDUE_PING_FILE_AGE);
-    this._overduePingCount = overduePings.length;
-
     // Submit the age of the pending pings.
     for (let pingInfo of infos) {
       const ageInDays =
         Utils.millisecondsToDays(Math.abs(now.getTime() - pingInfo.lastModificationDate));
       Telemetry.getHistogramById("TELEMETRY_PENDING_PINGS_AGE").add(ageInDays);
     }
    },
 
@@ -778,17 +750,16 @@ var TelemetrySendImpl = {
     await this._persistCurrentPings();
   },
 
   reset() {
     this._log.trace("reset");
 
     this._shutdown = false;
     this._currentPings = new Map();
-    this._overduePingCount = 0;
     this._tooLateToSend = false;
     this._isOSShutdown = false;
     this._sendingEnabled = true;
 
     const histograms = [
       "TELEMETRY_SUCCESS",
       "TELEMETRY_SEND_SUCCESS",
       "TELEMETRY_SEND_FAILURE",
--- a/toolkit/components/telemetry/TelemetrySession.jsm
+++ b/toolkit/components/telemetry/TelemetrySession.jsm
@@ -9,17 +9,16 @@ ChromeUtils.import("resource://gre/modul
 ChromeUtils.import("resource://gre/modules/Services.jsm", this);
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm", this);
 ChromeUtils.import("resource://gre/modules/DeferredTask.jsm", this);
 ChromeUtils.import("resource://gre/modules/Timer.jsm");
 ChromeUtils.import("resource://gre/modules/TelemetryUtils.jsm", this);
 ChromeUtils.import("resource://gre/modules/AppConstants.jsm");
 
 XPCOMUtils.defineLazyModuleGetters(this, {
-  TelemetrySend: "resource://gre/modules/TelemetrySend.jsm",
   AddonManagerPrivate: "resource://gre/modules/AddonManager.jsm",
   TelemetryController: "resource://gre/modules/TelemetryController.jsm",
   TelemetryStorage: "resource://gre/modules/TelemetryStorage.jsm",
   UITelemetry: "resource://gre/modules/UITelemetry.jsm",
   GCTelemetry: "resource://gre/modules/GCTelemetry.jsm",
   TelemetryEnvironment: "resource://gre/modules/TelemetryEnvironment.jsm",
   TelemetryReportingPolicy: "resource://gre/modules/TelemetryReportingPolicy.jsm",
 });
@@ -801,18 +800,16 @@ var Impl = {
     }
 
     if (clearSubsession) {
       this._subsessionStartActiveTicks = activeTicks;
     }
 
     ret.activeTicks = activeTicks;
 
-    ret.pingsOverdue = TelemetrySend.overduePingsCount;
-
     return ret;
   },
 
   /**
    * Get the type of the dataset that needs to be collected, based on the preferences.
    * @return {Integer} A value from nsITelemetry.DATASET_*.
    */
   getDatasetType() {
--- a/toolkit/components/telemetry/docs/data/main-ping.rst
+++ b/toolkit/components/telemetry/docs/data/main-ping.rst
@@ -224,20 +224,16 @@ savedPings
 Integer count of the number of pings that need to be sent.
 
 activeTicks
 ~~~~~~~~~~~
 Integer count of the number of five-second intervals ('ticks') the user was considered 'active' (sending UI events to the window). An extra event is fired immediately when the user becomes active after being inactive. This is for some mouse and gamepad events, and all touch, keyboard, wheel, and pointer events (see `EventStateManager.cpp <https://dxr.mozilla.org/mozilla-central/rev/e6463ae7eda2775bc84593bb4a0742940bb87379/dom/events/EventStateManager.cpp#549>`_).
 This measure might be useful to give a trend of how much a user actually interacts with the browser when compared to overall session duration. It does not take into account whether or not the window has focus or is in the foreground. Just if it is receiving these interaction events.
 Note that in ``main`` pings, this measure is reset on subsession splits, while in ``saved-session`` pings it covers the whole browser session.
 
-pingsOverdue
-~~~~~~~~~~~~
-Integer count of pending pings that are overdue.
-
 histograms
 ----------
 This section contains the histograms that are valid for the current platform. ``Flag`` histograms are always created and submitted with a default value of ``false`` if a value of ``true`` is not recorded during the time period. Other histogram types (see :ref:`choosing-histogram-type`) are not created nor submitted if no data was added to them. The type and format of the reported histograms is described by the ``Histograms.json`` file. Its most recent version is available `here <https://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/Histograms.json>`_. The ``info.revision`` field indicates the revision of the file that describes the reported histograms.
 
 keyedHistograms
 ---------------
 This section contains the keyed histograms available for the current platform.
 
--- a/toolkit/components/telemetry/tests/unit/test_TelemetrySendOldPings.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySendOldPings.js
@@ -11,30 +11,21 @@
 
 ChromeUtils.import("resource://gre/modules/Services.jsm", this);
 ChromeUtils.import("resource://gre/modules/TelemetryStorage.jsm", this);
 ChromeUtils.import("resource://gre/modules/TelemetryController.jsm", this);
 ChromeUtils.import("resource://gre/modules/TelemetrySend.jsm", this);
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 const {OS: {File, Path, Constants}} = ChromeUtils.import("resource://gre/modules/osfile.jsm", {});
 
-// We increment TelemetryStorage's MAX_PING_FILE_AGE and
-// OVERDUE_PING_FILE_AGE by 1 minute so that our test pings exceed
-// those points in time, even taking into account file system imprecision.
-const ONE_MINUTE_MS = 60 * 1000;
-const OVERDUE_PING_FILE_AGE = TelemetrySend.OVERDUE_PING_FILE_AGE + ONE_MINUTE_MS;
-
 const PING_SAVE_FOLDER = "saved-telemetry-pings";
 const PING_TIMEOUT_LENGTH = 5000;
-const OVERDUE_PINGS = 6;
 const OLD_FORMAT_PINGS = 4;
 const RECENT_PINGS = 4;
 
-const TOTAL_EXPECTED_PINGS = OVERDUE_PINGS + RECENT_PINGS + OLD_FORMAT_PINGS;
-
 var gCreatedPings = 0;
 var gSeenPings = 0;
 
 /**
  * Creates some Telemetry pings for the and saves them to disk. Each ping gets a
  * unique ID based on an incrementor.
  *
  * @param {Array} aPingInfos An array of ping type objects. Each entry must be an
@@ -180,17 +171,17 @@ add_task(async function test_recent_ping
   assertReceivedPings(RECENT_PINGS);
 
   await TelemetryStorage.testClearPendingPings();
 });
 
 /**
  * Create an overdue ping in the old format and try to send it.
  */
-add_task(async function test_overdue_old_format() {
+add_task(async function test_old_formats() {
   // A test ping in the old, standard format.
   const PING_OLD_FORMAT = {
     slug: "1234567abcd",
     reason: "test-ping",
     payload: {
       info: {
         reason: "test-ping",
         OS: "XPCShell",
@@ -225,26 +216,22 @@ add_task(async function test_overdue_old
 
   const PING_FILES_PATHS = [
     getSavePathForPingId(PING_OLD_FORMAT.slug),
     getSavePathForPingId(PING_NO_INFO.slug),
     getSavePathForPingId(PING_NO_PAYLOAD.slug),
     getSavePathForPingId("no-slug-file"),
   ];
 
-  // Write the ping to file and make it overdue.
+  // Write the ping to file
   await TelemetryStorage.savePing(PING_OLD_FORMAT, true);
   await TelemetryStorage.savePing(PING_NO_INFO, true);
   await TelemetryStorage.savePing(PING_NO_PAYLOAD, true);
   await TelemetryStorage.savePingToFile(PING_NO_SLUG, PING_FILES_PATHS[3], true);
 
-  for (let f in PING_FILES_PATHS) {
-    await File.setDates(PING_FILES_PATHS[f], null, Date.now() - OVERDUE_PING_FILE_AGE);
-  }
-
   gSeenPings = 0;
   await TelemetryController.testReset();
   await TelemetrySend.testWaitOnOutgoingPings();
   assertReceivedPings(OLD_FORMAT_PINGS);
 
   // |TelemetryStorage.cleanup| doesn't know how to remove a ping with no slug or id,
   // so remove it manually so that the next test doesn't fail.
   await OS.File.remove(PING_FILES_PATHS[3]);
@@ -301,41 +288,16 @@ add_task(async function test_corrupted_p
 
   let exists = await OS.File.exists(getSavePathForPingId(pendingPingId));
   Assert.ok(!exists, "The unparseable ping should have been removed");
 
   await TelemetryStorage.testClearPendingPings();
 });
 
 /**
- * Create some recent and overdue pings and verify that they get sent.
- */
-add_task(async function test_overdue_pings_trigger_send() {
-  let pingTypes = [
-    { num: RECENT_PINGS },
-    { num: OVERDUE_PINGS, age: OVERDUE_PING_FILE_AGE },
-  ];
-  let pings = await createSavedPings(pingTypes);
-  let recentPings = pings.slice(0, RECENT_PINGS);
-  let overduePings = pings.slice(-OVERDUE_PINGS);
-
-  await TelemetryController.testReset();
-  await TelemetrySend.testWaitOnOutgoingPings();
-  assertReceivedPings(TOTAL_EXPECTED_PINGS);
-
-  await assertNotSaved(recentPings);
-  await assertNotSaved(overduePings);
-
-  Assert.equal(TelemetrySend.overduePingsCount, overduePings.length,
-               "Should have tracked the correct amount of overdue pings");
-
-  await TelemetryStorage.testClearPendingPings();
-});
-
-/**
  * Create a ping in the old format, send it, and make sure the request URL contains
  * the correct version query parameter.
  */
 add_task(async function test_overdue_old_format() {
   // A test ping in the old, standard format.
   const PING_OLD_FORMAT = {
     slug: "1234567abcd",
     reason: "test-ping",
@@ -348,22 +310,18 @@ add_task(async function test_overdue_old
         appName: "XPCShell",
         appBuildID: "123456789",
         appUpdateChannel: "Test",
         platformBuildID: "987654321",
       },
     },
   };
 
-  const filePath =
-    Path.join(Constants.Path.profileDir, PING_SAVE_FOLDER, PING_OLD_FORMAT.slug);
-
-  // Write the ping to file and make it overdue.
+  // Write the ping to file
   await TelemetryStorage.savePing(PING_OLD_FORMAT, true);
-  await File.setDates(filePath, null, Date.now() - OVERDUE_PING_FILE_AGE);
 
   let receivedPings = 0;
   // Register a new prefix handler to validate the URL.
   PingServer.registerPingHandler(request => {
     // Check that we have a version query parameter in the URL.
     Assert.notEqual(request.queryString, "");
 
     // Make sure the version in the query string matches the old ping format version.