bug 1501303 - Adjust Telemetry Send meta-metrics r=Dexter
authorChris H-C <chutten@mozilla.com>
Mon, 05 Nov 2018 14:53:21 +0000
changeset 444427 4fe52b757719f8085a62a14fd87f6bc8a8c846e2
parent 444426 68e3e10e800346ffe93b453a3f8d150b1dc35b73
child 444428 5da3c0dad67a20c04730cd0480844132b38f66c4
push id72328
push userchutten@mozilla.com
push dateMon, 05 Nov 2018 14:54:38 +0000
treeherderautoland@4fe52b757719 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersDexter
bugs1501303
milestone65.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 1501303 - Adjust Telemetry Send meta-metrics r=Dexter TELEMETRY_FAILED_SEND_PINGS_SIZE_KB and TELEMETRY_SUCCESSFUL_SEND_PINGS_SIZE_KB are unmonitored and don't show evidence consistent with the original hypothesis that size correlates with send failures. So let's remove them. TELEMETRY_SEND_FAILURE_TYPE is a useful measure to track numbers and types of send failures, in case they change. So let's make that permanent. MozReview-Commit-ID: GWaKhrzIvph Differential Revision: https://phabricator.services.mozilla.com/D10284
toolkit/components/telemetry/Histograms.json
toolkit/components/telemetry/app/TelemetrySend.jsm
toolkit/components/telemetry/tests/unit/test_TelemetrySend.js
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -7093,38 +7093,16 @@
     "record_in_processes": ["main"],
     "alert_emails": ["telemetry-client-dev@mozilla.com"],
     "expires_in_version": "never",
     "kind": "exponential",
     "high": 300000,
     "n_buckets": 20,
     "description": "Time (ms) it takes for checking if the pending pings are over-quota"
   },
-  "TELEMETRY_SUCCESSFUL_SEND_PINGS_SIZE_KB": {
-    "record_in_processes": ["main"],
-    "alert_emails": ["telemetry-client-dev@mozilla.com", "chutten@mozilla.com"],
-    "expires_in_version": "66",
-    "kind": "exponential",
-    "low": 10,
-    "high": 1024,
-    "n_buckets": 20,
-    "bug_numbers": [1367094],
-    "description": "The size (KB) of the Telemetry successfully sent pings"
-  },
-  "TELEMETRY_FAILED_SEND_PINGS_SIZE_KB": {
-    "record_in_processes": ["main"],
-    "alert_emails": ["telemetry-client-dev@mozilla.com", "chutten@mozilla.com"],
-    "expires_in_version": "66",
-    "kind": "exponential",
-    "low": 10,
-    "high": 1024,
-    "n_buckets": 20,
-    "bug_numbers": [1367094],
-    "description": "The size (KB) of the Telemetry unsuccessfully sent pings"
-  },
   "TELEMETRY_PING_SIZE_EXCEEDED_SEND": {
     "record_in_processes": ["main"],
     "alert_emails": ["telemetry-client-dev@mozilla.com"],
     "expires_in_version": "never",
     "kind": "count",
     "description": "Number of Telemetry pings discarded before sending because they exceeded the maximum size"
   },
   "TELEMETRY_PING_SIZE_EXCEEDED_PENDING": {
@@ -7204,18 +7182,18 @@
     "kind": "exponential",
     "high": 120000,
     "n_buckets": 20,
     "description": "Time needed (in ms) for a failed send of a Telemetry ping to the servers and getting a reply back."
   },
   "TELEMETRY_SEND_FAILURE_TYPE" : {
     "record_in_processes": ["main"],
     "alert_emails": ["telemetry-client-dev@mozilla.com", "chutten@mozilla.com"],
-    "bug_numbers": [1367110],
-    "expires_in_version": "66",
+    "bug_numbers": [1367110, 1501303],
+    "expires_in_version": "never",
     "kind": "categorical",
     "labels": [
       "eOK",
       "eRequest",
       "eUnreachable",
       "eChannelOpen",
       "eRedirect",
       "abort",
--- a/toolkit/components/telemetry/app/TelemetrySend.jsm
+++ b/toolkit/components/telemetry/app/TelemetrySend.jsm
@@ -1112,22 +1112,18 @@ var TelemetrySendImpl = {
     request.channel.loadFlags &= ~Ci.nsIChannel.LOAD_CLASSIFY_URI;
 
     const monotonicStartTime = Utils.monotonicNow();
     let deferred = PromiseUtils.defer();
 
     let onRequestFinished = (success, event) => {
       let onCompletion = () => {
         if (success) {
-          let histogram = Telemetry.getHistogramById("TELEMETRY_SUCCESSFUL_SEND_PINGS_SIZE_KB");
-          histogram.add(compressedPingSizeKB);
           deferred.resolve();
         } else {
-          let histogram = Telemetry.getHistogramById("TELEMETRY_FAILED_SEND_PINGS_SIZE_KB");
-          histogram.add(compressedPingSizeKB);
           deferred.reject(event);
         }
       };
 
       this._pendingPingRequests.delete(id);
       this._onPingRequestFinished(success, monotonicStartTime, id, isPersisted)
         .then(() => onCompletion(),
               (error) => {
@@ -1224,17 +1220,16 @@ var TelemetrySendImpl = {
                .add(Math.floor(compressedPingSizeBytes / 1024 / 1024));
       // We don't need to call |request.abort()| as it was not sent yet.
       this._pendingPingRequests.delete(id);
 
       TelemetryHealthPing.recordDiscardedPing(ping.type);
       return TelemetryStorage.removePendingPing(id);
     }
 
-    const compressedPingSizeKB = Math.floor(payloadStream.data.length / 1024);
     Telemetry.getHistogramById("TELEMETRY_COMPRESS").add(Utils.monotonicNow() - startTime);
     request.sendInputStream(payloadStream);
 
     this.payloadStream = payloadStream;
 
     return deferred.promise;
   },
 
--- a/toolkit/components/telemetry/tests/unit/test_TelemetrySend.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySend.js
@@ -548,64 +548,16 @@ add_task(async function testCookies() {
     var request = await PingServer.promiseNextRequest();
     var ping = decodeRequestPayload(request);
     foundit = id === ping.id;
   }
   Assert.equal(id, ping.id, "We're testing the right ping's request, right?");
   Assert.equal(false, request.hasHeader("Cookie"), "Request should not have Cookie header");
 });
 
-add_task(async function test_measurePingsSize() {
-  const TEST_TYPE = "test-measure-ping-size";
-
-  let histSuccessPingSize = Telemetry.getHistogramById("TELEMETRY_SUCCESSFUL_SEND_PINGS_SIZE_KB");
-  let histFailedPingSize = Telemetry.getHistogramById("TELEMETRY_FAILED_SEND_PINGS_SIZE_KB");
-
-  for (let h of [histSuccessPingSize, histFailedPingSize]) {
-    h.clear();
-  }
-
-  await TelemetryController.submitExternalPing(TEST_TYPE, {});
-  await TelemetrySend.testWaitOnOutgoingPings();
-
-  // Check that we recorded the ping sizes correctly into histograms.
-  Assert.equal(histogramValueCount(histSuccessPingSize.snapshot()), 1,
-    "Should have recorded 1 successful ping into histogram.");
-  Assert.equal(histogramValueCount(histFailedPingSize.snapshot()), 0,
-    "Should have recorded 0 failed ping into histogram.");
-
-  // Submit the same ping a second time.
-  await TelemetryController.submitExternalPing(TEST_TYPE, {});
-  await TelemetrySend.testWaitOnOutgoingPings();
-
-  // Check that we recorded the ping sizes correctly into histograms.
-  Assert.equal(histogramValueCount(histSuccessPingSize.snapshot()), 2,
-    "Should have recorded 2 successful ping into histogram.");
-  Assert.equal(histogramValueCount(histFailedPingSize.snapshot()), 0,
-    "Should have recorded 0 failed ping into histogram.");
-
-  // Register a custom ping handler which will return 601.
-  PingServer.registerPingHandler((req, res) => {
-    res.setStatusLine(null, 601, "Not Implemented");
-    res.processAsync();
-    res.finish();
-  });
-
-  await TelemetryController.submitExternalPing(TEST_TYPE, {});
-  await ContentTaskUtils.waitForCondition(() => {
-    return histogramValueCount(histFailedPingSize.snapshot()) > 0;
-  });
-
-  // Check that we recorded the ping sizes correctly into histograms.
-  Assert.equal(histogramValueCount(histSuccessPingSize.snapshot()), 2,
-    "Should have recorded 2 successful ping into histogram.");
-  Assert.equal(histogramValueCount(histFailedPingSize.snapshot()), 1,
-    "Should have recorded 1 failed ping into histogram.");
-});
-
 add_task(async function test_pref_observer() {
   // This test requires the presence of the crash reporter component.
   let registrar = Components.manager.QueryInterface(Ci.nsIComponentRegistrar);
   if (!registrar.isContractIDRegistered("@mozilla.org/toolkit/crash-reporter;1")) {
     return;
   }
 
   await TelemetrySend.setup(true);