Bug 1231812 - Don't send empty keyed histograms in Telemetry pings. r=gfritzsche
authorClaas Augner <mozilla@caugner.de>
Thu, 14 Apr 2016 15:57:10 -0700
changeset 331297 a61539ca56bd03c517697c8bf873ed4f2efd1148
parent 331296 41fa5b9cf8dc4961c3103398805a44133ff0b9c5
child 331298 e687d8e0871dc79a26ed6e0b59f12e4e3cb64551
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfritzsche
bugs1231812
milestone48.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 1231812 - Don't send empty keyed histograms in Telemetry pings. r=gfritzsche MozReview-Commit-ID: 2Vl3IjjeaHj
toolkit/components/telemetry/TelemetrySession.jsm
toolkit/components/telemetry/docs/main-ping.rst
toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
--- a/toolkit/components/telemetry/TelemetrySession.jsm
+++ b/toolkit/components/telemetry/TelemetrySession.jsm
@@ -956,26 +956,33 @@ var Impl = {
     let ret = {};
 
     for (let id of registered) {
       // Omit telemetry test histograms outside of tests.
       if (id.startsWith('TELEMETRY_TEST_') && this._testing == false) {
         this._log.trace("getKeyedHistograms - Skipping test histogram: " + id);
         continue;
       }
-      ret[id] = {};
       let keyed = Telemetry.getKeyedHistogramById(id);
       let snapshot = null;
       if (subsession) {
         snapshot = clearSubsession ? keyed.snapshotSubsessionAndClear()
                                    : keyed.subsessionSnapshot();
       } else {
         snapshot = keyed.snapshot();
       }
-      for (let key of Object.keys(snapshot)) {
+
+      let keys = Object.keys(snapshot);
+      if (keys.length == 0) {
+        // Skip empty keyed histogram.
+        continue;
+      }
+
+      ret[id] = {};
+      for (let key of keys) {
         ret[id][key] = this.packHistogram(snapshot[key]);
       }
     }
 
     return ret;
   },
 
   getThreadHangStats: function getThreadHangStats(stats) {
--- a/toolkit/components/telemetry/docs/main-ping.rst
+++ b/toolkit/components/telemetry/docs/main-ping.rst
@@ -173,17 +173,19 @@ pingsOverdue
 Integer count of pending pings that are overdue.
 
 histograms
 ----------
 This section contains the histograms that are valid for the current platform. ``Flag`` and ``count`` histograms are always created and submitted, with their default value being respectively ``false`` and ``0``. Other histogram types (`see here <https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Adding_a_new_Telemetry_probe#Choosing_a_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. Unlike the ``histograms`` section, this section always reports all the keyed histograms, even though they contain no data.
+This section contains the keyed histograms available for the current platform.
+
+As of Firefox 48, this section does not contain empty keyed histograms anymore.
 
 threadHangStats
 ---------------
 Contains the statistics about the hangs in main and background threads. Note that hangs in this section capture the [C++ pseudostack](https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Profiling_with_the_Built-in_Profiler#Native_stack_vs._Pseudo_stack) and an incomplete JS stack, which is not 100% precise.
 
 To avoid submitting overly large payloads, some limits are applied:
 
 * Identical, adjacent "(chrome script)" or "(content script)" stack entries are collapsed together. If a stack is reduced, the "(reduced stack)" frame marker is added as the oldest frame.
--- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
@@ -360,21 +360,19 @@ function checkPayload(payload, reason, s
   Assert.ok(("IceCandidatesStats" in payload.webrtc) &&
                 ("webrtc" in payload.webrtc.IceCandidatesStats) &&
                 ("loop" in payload.webrtc.IceCandidatesStats));
 
   // Check keyed histogram payload.
 
   Assert.ok("keyedHistograms" in payload);
   let keyedHistograms = payload.keyedHistograms;
-  Assert.ok(TELEMETRY_TEST_KEYED_FLAG in keyedHistograms);
+  Assert.ok(!(TELEMETRY_TEST_KEYED_FLAG in keyedHistograms));
   Assert.ok(TELEMETRY_TEST_KEYED_COUNT in keyedHistograms);
 
-  Assert.deepEqual({}, keyedHistograms[TELEMETRY_TEST_KEYED_FLAG]);
-
   const expected_keyed_count = {
     "a": {
       range: [1, 2],
       bucket_count: 3,
       histogram_type: 4,
       values: {0:2, 1:0},
       sum: 2,
     },
@@ -638,20 +636,18 @@ add_task(function* test_checkSubsessionH
   keyed.clear();
   let classic = TelemetrySession.getPayload();
   let subsession = TelemetrySession.getPayload("environment-change");
 
   Assert.equal(classic.info.reason, "gather-payload");
   Assert.equal(subsession.info.reason, "environment-change");
   Assert.ok(!(COUNT_ID in classic.histograms));
   Assert.ok(!(COUNT_ID in subsession.histograms));
-  Assert.ok(KEYED_ID in classic.keyedHistograms);
-  Assert.ok(KEYED_ID in subsession.keyedHistograms);
-  Assert.deepEqual(classic.keyedHistograms[KEYED_ID], {});
-  Assert.deepEqual(subsession.keyedHistograms[KEYED_ID], {});
+  Assert.ok(!(KEYED_ID in classic.keyedHistograms));
+  Assert.ok(!(KEYED_ID in subsession.keyedHistograms));
 
   checkHistograms(classic.histograms, subsession.histograms);
   checkKeyedHistograms(classic.keyedHistograms, subsession.keyedHistograms);
 
   // Adding values should get picked up in both.
   count.add(1);
   keyed.add("a", 1);
   keyed.add("b", 1);
@@ -672,19 +668,18 @@ add_task(function* test_checkSubsessionH
   // Values should still reset properly.
   count.clear();
   keyed.clear();
   classic = TelemetrySession.getPayload();
   subsession = TelemetrySession.getPayload("environment-change");
 
   Assert.ok(!(COUNT_ID in classic.histograms));
   Assert.ok(!(COUNT_ID in subsession.histograms));
-  Assert.ok(KEYED_ID in classic.keyedHistograms);
-  Assert.ok(KEYED_ID in subsession.keyedHistograms);
-  Assert.deepEqual(classic.keyedHistograms[KEYED_ID], {});
+  Assert.ok(!(KEYED_ID in classic.keyedHistograms));
+  Assert.ok(!(KEYED_ID in subsession.keyedHistograms));
 
   checkHistograms(classic.histograms, subsession.histograms);
   checkKeyedHistograms(classic.keyedHistograms, subsession.keyedHistograms);
 
   // Adding values should get picked up in both.
   count.add(1);
   keyed.add("a", 1);
   keyed.add("b", 1);
@@ -720,20 +715,19 @@ add_task(function* test_checkSubsessionH
   subsession = TelemetrySession.getPayload("environment-change");
 
   Assert.ok(COUNT_ID in classic.histograms);
   Assert.ok(COUNT_ID in subsession.histograms);
   Assert.equal(classic.histograms[COUNT_ID].sum, 1);
   Assert.equal(subsession.histograms[COUNT_ID].sum, 0);
 
   Assert.ok(KEYED_ID in classic.keyedHistograms);
-  Assert.ok(KEYED_ID in subsession.keyedHistograms);
+  Assert.ok(!(KEYED_ID in subsession.keyedHistograms));
   Assert.equal(classic.keyedHistograms[KEYED_ID]["a"].sum, 1);
   Assert.equal(classic.keyedHistograms[KEYED_ID]["b"].sum, 1);
-  Assert.deepEqual(subsession.keyedHistograms[KEYED_ID], {});
 
   // Adding values should get picked up in both again.
   count.add(1);
   keyed.add("a", 1);
   keyed.add("b", 1);
   classic = TelemetrySession.getPayload();
   subsession = TelemetrySession.getPayload("environment-change");
 
@@ -867,17 +861,17 @@ add_task(function* test_dailyCollection(
   Assert.ok(!!ping);
 
   Assert.equal(ping.type, PING_TYPE_MAIN);
   Assert.equal(ping.payload.info.reason, REASON_DAILY);
   subsessionStartDate = new Date(ping.payload.info.subsessionStartDate);
   Assert.equal(subsessionStartDate.toISOString(), expectedDate.toISOString());
 
   Assert.equal(ping.payload.histograms[COUNT_ID].sum, 0);
-  Assert.deepEqual(ping.payload.keyedHistograms[KEYED_ID], {});
+  Assert.ok(!(KEYED_ID in ping.payload.keyedHistograms));
 
   // Trigger and collect another daily ping, with the histograms being set again.
   count.add(1);
   keyed.add("a", 1);
   keyed.add("b", 1);
 
   // The daily ping is rescheduled for "tomorrow".
   expectedDate = futureDate(expectedDate, MS_IN_ONE_DAY);
@@ -1074,17 +1068,17 @@ add_task(function* test_environmentChang
 
   Assert.equal(ping.type, PING_TYPE_MAIN);
   Assert.equal(ping.environment.settings.userPrefs[PREF_TEST], 1);
   Assert.equal(ping.payload.info.reason, REASON_ENVIRONMENT_CHANGE);
   subsessionStartDate = new Date(ping.payload.info.subsessionStartDate);
   Assert.equal(subsessionStartDate.toISOString(), startDay.toISOString());
 
   Assert.equal(ping.payload.histograms[COUNT_ID].sum, 0);
-  Assert.deepEqual(ping.payload.keyedHistograms[KEYED_ID], {});
+  Assert.ok(!(KEYED_ID in ping.payload.keyedHistograms));
 });
 
 add_task(function* test_savedPingsOnShutdown() {
   // 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 TelemetrySession shutdown to trigger a session save.