bug 1397293 - Don't try to send Telemetry after net shutdown r=Dexter
authorChris H-C <chutten@mozilla.com>
Fri, 22 Sep 2017 12:28:06 -0400
changeset 383275 e663536181b03e7fa5635617004d73a1f1b16780
parent 383274 9e57229ccdb9c231c9506e115fd7f64fd84cb1b6
child 383276 49ebf04b82914b43466590fd93e6ad61e701a78c
push id95539
push userkwierso@gmail.com
push dateThu, 28 Sep 2017 00:01:12 +0000
treeherdermozilla-inbound@72de90e66155 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersDexter
bugs1397293
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1397293 - Don't try to send Telemetry after net shutdown r=Dexter Once networking starts tearing down, our attempts to use XHR to send pings will fail. This might contibute to high intermittent send failures reported in the wild. So let's not even try. But, let's be sure to mention when we decide not to try so we can ensure the numbers line up before and after. MozReview-Commit-ID: 71QGM7Xk5oc
toolkit/components/telemetry/Histograms.json
toolkit/components/telemetry/TelemetrySend.jsm
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -7284,17 +7284,18 @@
     "kind": "categorical",
     "labels": [
       "eOK",
       "eRequest",
       "eUnreachable",
       "eChannelOpen",
       "eRedirect",
       "abort",
-      "timeout"
+      "timeout",
+      "eTooLate"
     ],
     "description": "Counts of the different ways sending a Telemetry ping can fail."
   },
   "TELEMETRY_STRINGIFY" : {
     "record_in_processes": ["main", "content"],
     "alert_emails": ["telemetry-client-dev@mozilla.com"],
     "expires_in_version": "never",
     "kind": "exponential",
--- a/toolkit/components/telemetry/TelemetrySend.jsm
+++ b/toolkit/components/telemetry/TelemetrySend.jsm
@@ -48,16 +48,17 @@ const LOGGER_NAME = "Toolkit.Telemetry";
 const LOGGER_PREFIX = "TelemetrySend::";
 
 const TOPIC_IDLE_DAILY = "idle-daily";
 // The following topics are notified when Firefox is closing
 // because the OS is shutting down.
 const TOPIC_QUIT_APPLICATION_GRANTED = "quit-application-granted";
 const TOPIC_QUIT_APPLICATION_FORCED = "quit-application-forced";
 const PREF_CHANGED_TOPIC = "nsPref:changed";
+const TOPIC_PROFILE_CHANGE_NET_TEARDOWN = "profile-change-net-teardown";
 
 // Whether the FHR/Telemetry unification features are enabled.
 // Changing this pref requires a restart.
 const IS_UNIFIED_TELEMETRY = Services.prefs.getBoolPref(TelemetryUtils.Preferences.Unified, false);
 
 const PING_FORMAT_VERSION = 4;
 
 const MS_IN_A_MINUTE = 60 * 1000;
@@ -581,21 +582,24 @@ var TelemetrySendImpl = {
   // 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,
   ],
 
   OBSERVED_PREFERENCES: [
     TelemetryUtils.Preferences.TelemetryEnabled,
     TelemetryUtils.Preferences.FhrUploadEnabled,
   ],
 
   // Whether sending pings has been overridden.
@@ -640,16 +644,17 @@ var TelemetrySendImpl = {
 
   async setup(testing) {
     this._log.trace("setup");
 
     this._testMode = testing;
     this._sendingEnabled = true;
 
     Services.obs.addObserver(this, TOPIC_IDLE_DAILY);
+    Services.obs.addObserver(this, TOPIC_PROFILE_CHANGE_NET_TEARDOWN);
 
     this._server = Services.prefs.getStringPref(TelemetryUtils.Preferences.Server, undefined);
 
     // Annotate crash reports so that crash pings are sent correctly and listen
     // to pref changes to adjust the annotations accordingly.
     for (let pref of this.OBSERVED_PREFERENCES) {
       Services.prefs.addObserver(pref, this, true);
     }
@@ -762,16 +767,17 @@ var TelemetrySendImpl = {
   },
 
   reset() {
     this._log.trace("reset");
 
     this._shutdown = false;
     this._currentPings = new Map();
     this._overduePingCount = 0;
+    this._tooLateToSend = false;
     this._isOSShutdown = false;
 
     const histograms = [
       "TELEMETRY_SUCCESS",
       "TELEMETRY_SEND_SUCCESS",
       "TELEMETRY_SEND_FAILURE",
     ];
 
@@ -810,16 +816,19 @@ var TelemetrySendImpl = {
         setOSShutdown();
       }
       break;
     case PREF_CHANGED_TOPIC:
       if (this.OBSERVED_PREFERENCES.includes(data)) {
         this._annotateCrashReport();
       }
       break;
+    case TOPIC_PROFILE_CHANGE_NET_TEARDOWN:
+      this._tooLateToSend = true;
+      break;
     }
   },
 
   /**
    * Spawn the PingSender process that sends a ping. This function does
    * not return an error or throw, it only logs an error.
    *
    * Even if the function doesn't fail, it doesn't mean that the ping was
@@ -1071,16 +1080,22 @@ var TelemetrySendImpl = {
 
   _doPing(ping, id, isPersisted) {
     if (!this.sendingEnabled(ping)) {
       // We can't send the pings to the server, so don't try to.
       this._log.trace("_doPing - Can't send ping " + ping.id);
       return Promise.resolve();
     }
 
+    if (this._tooLateToSend) {
+      this._log.trace("_doPing - Too late to send ping " + ping.id);
+      Telemetry.getHistogramById("TELEMETRY_SEND_FAILURE_TYPE").add("eTooLate");
+      return Promise.resolve();
+    }
+
     this._log.trace("_doPing - server: " + this._server + ", persisted: " + isPersisted +
                     ", id: " + id);
 
     const url = this._buildSubmissionURL(ping);
 
     let request = new ServiceRequest();
     request.mozBackgroundRequest = true;
     request.timeout = Policy.pingSubmissionTimeout();