Bug 1127914 - Part 3 - Submit duplicate histogram data for 'non-classic' telemetry sessions. r=vladan

toolkit/components/telemetry/TelemetrySession.jsm

@@ -170,10 +170,11 @@ this.TelemetrySession = Object.freeze({
   },
   /**
    * Returns the current telemetry payload.
+   * @param reason Optional, the reason to trigger the payload.
    * @returns Object
    */
-  getPayload: function() {
-    return Impl.getPayload();
+  getPayload: function(reason) {
+    return Impl.getPayload(reason);
   },
   /**
    * Save histograms to a file.
@@ -458,11 +459,12 @@ let Impl = {
     return retgram;
   },
 
-  getHistograms: function getHistograms(hls) {
+  getHistograms: function getHistograms(subsession) {
     this._log.trace("getHistograms");
 
     let registered =
       Telemetry.registeredHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, []);
+    let hls = subsession ? Telemetry.subsessionHistogramSnapshots : Telemetry.histogramSnapshots;
     let ret = {};
 
     for (let name of registered) {
@@ -495,7 +497,7 @@ let Impl = {
     return ret;
   },
 
-  getKeyedHistograms: function() {
+  getKeyedHistograms: function(subsession) {
     this._log.trace("getKeyedHistograms");
 
     let registered =
@@ -505,7 +507,7 @@ let Impl = {
     for (let id of registered) {
       ret[id] = {};
       let keyed = Telemetry.getKeyedHistogramById(id);
-      let snapshot = keyed.snapshot();
+      let snapshot = subsession ? keyed.subsessionSnapshot() : keyed.snapshot();
       for (let key of Object.keys(snapshot)) {
         ret[id][key] = this.packHistogram(snapshot[key]);
       }
@@ -792,15 +794,23 @@ let Impl = {
    * to |this.getSimpleMeasurements| and |this.getMetadata|,
    * respectively.
    */
-  assemblePayloadWithMeasurements: function assemblePayloadWithMeasurements(simpleMeasurements, info) {
-    this._log.trace("assemblePayloadWithMeasurements");
+  assemblePayloadWithMeasurements: function(simpleMeasurements, info, reason) {
+    const classicReasons = [
+      "saved-session",
+      "idle-daily",
+      "gather-payload",
+      "test-ping",
+    ];
+    const isSubsession = classicReasons.indexOf(reason) == -1;
+    this._log.trace("assemblePayloadWithMeasurements - reason: " + reason +
+                    ", submitting subsession data: " + isSubsession);
 
     // Payload common to chrome and content processes.
     let payloadObj = {
       ver: PAYLOAD_VERSION,
       simpleMeasurements: simpleMeasurements,
-      histograms: this.getHistograms(Telemetry.histogramSnapshots),
-      keyedHistograms: this.getKeyedHistograms(),
+      histograms: this.getHistograms(isSubsession),
+      keyedHistograms: this.getKeyedHistograms(isSubsession),
       chromeHangs: Telemetry.chromeHangs,
       threadHangStats: this.getThreadHangStats(Telemetry.threadHangStats),
       log: TelemetryLog.entries(),
@@ -840,7 +850,7 @@ let Impl = {
     this._log.trace("getSessionPayload - Reason " + reason);
     let measurements = this.getSimpleMeasurements(reason == "saved-session");
     let info = !IS_CONTENT_PROCESS ? this.getMetadata(reason) : null;
-    return this.assemblePayloadWithMeasurements(measurements, info);
+    return this.assemblePayloadWithMeasurements(measurements, info, reason);
   },
 
   assemblePing: function assemblePing(payloadObj, reason) {
@@ -1093,8 +1103,9 @@ let Impl = {
 #endif
   },
 
-  getPayload: function getPayload() {
+  getPayload: function getPayload(reason) {
     this._log.trace("getPayload");
+    reason = reason || "gather-payload";
     // This function returns the current Telemetry payload to the caller.
     // We only gather startup info once.
     if (Object.keys(this._slowSQLStartup).length == 0) {
@@ -1102,7 +1113,7 @@ let Impl = {
       this._slowSQLStartup = Telemetry.slowSQL;
     }
     this.gatherMemory();
-    return this.getSessionPayload("gather-payload");
+    return this.getSessionPayload(reason);
   },
 
   gatherStartup: function gatherStartup() {

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

@@ -606,6 +606,173 @@ add_task(function* test_saveLoadPing() {
   }
 });
 
+add_task(function* test_checkSubsession() {
+  const COUNT_ID = "TELEMETRY_TEST_COUNT";
+  const KEYED_ID = "TELEMETRY_TEST_KEYED_COUNT";
+  const count = Telemetry.getHistogramById(COUNT_ID);
+  const keyed = Telemetry.getKeyedHistogramById(KEYED_ID);
+  const registeredIds =
+    new Set(Telemetry.registeredHistograms(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, []));
+
+  const stableHistograms = new Set([
+    "TELEMETRY_TEST_FLAG",
+    "TELEMETRY_TEST_COUNT",
+    "TELEMETRY_TEST_RELEASE_OPTOUT",
+    "TELEMETRY_TEST_RELEASE_OPTIN",
+    "STARTUP_CRASH_DETECTED",
+  ]);
+
+  const stableKeyedHistograms = new Set([
+    "TELEMETRY_TEST_KEYED_FLAG",
+    "TELEMETRY_TEST_KEYED_COUNT",
+    "TELEMETRY_TEST_KEYED_RELEASE_OPTIN",
+    "TELEMETRY_TEST_KEYED_RELEASE_OPTOUT",
+  ]);
+
+  // Compare the two sets of histograms.
+  // The "subsession" histograms should match the registered
+  // "classic" histograms. However, histograms can change
+  // between us collecting the different payloads, so we only
+  // check for deep equality on known stable histograms.
+  checkHistograms = (classic, subsession) => {
+    for (let id of Object.keys(classic)) {
+      if (!registeredIds.has(id)) {
+        continue;
+      }
+
+      Assert.ok(id in subsession);
+      if (stableHistograms.has(id)) {
+        Assert.deepEqual(classic[id],
+                         subsession[id]);
+      } else {
+        Assert.equal(classic[id].histogram_type,
+                     subsession[id].histogram_type);
+      }
+    }
+  };
+
+  // Same as above, except for keyed histograms.
+  checkKeyedHistograms = (classic, subsession) => {
+    for (let id of Object.keys(classic)) {
+      if (!registeredIds.has(id)) {
+        continue;
+      }
+
+      Assert.ok(id in subsession);
+      if (stableKeyedHistograms.has(id)) {
+        Assert.deepEqual(classic[id],
+                         subsession[id]);
+      }
+    }
+  };
+
+  // Both classic and subsession payload histograms should start the same.
+  // The payloads should be identical for now except for the reason.
+  count.clear();
+  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], {});
+
+  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);
+  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.equal(classic.histograms[COUNT_ID].sum, 1);
+  Assert.equal(classic.keyedHistograms[KEYED_ID]["a"].sum, 1);
+  Assert.equal(classic.keyedHistograms[KEYED_ID]["b"].sum, 1);
+
+  checkHistograms(classic.histograms, subsession.histograms);
+  checkKeyedHistograms(classic.keyedHistograms, subsession.keyedHistograms);
+
+  // 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], {});
+
+  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);
+  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.equal(classic.histograms[COUNT_ID].sum, 1);
+  Assert.equal(classic.keyedHistograms[KEYED_ID]["a"].sum, 1);
+  Assert.equal(classic.keyedHistograms[KEYED_ID]["b"].sum, 1);
+
+  checkHistograms(classic.histograms, subsession.histograms);
+  checkKeyedHistograms(classic.keyedHistograms, subsession.keyedHistograms);
+
+  // We should be able to reset only the subsession histograms.
+  count.clear(true);
+  keyed.clear(true);
+  classic = TelemetrySession.getPayload();
+  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.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");
+
+  Assert.ok(COUNT_ID in classic.histograms);
+  Assert.ok(COUNT_ID in subsession.histograms);
+  Assert.equal(classic.histograms[COUNT_ID].sum, 2);
+  Assert.equal(subsession.histograms[COUNT_ID].sum, 1);
+
+  Assert.ok(KEYED_ID in classic.keyedHistograms);
+  Assert.ok(KEYED_ID in subsession.keyedHistograms);
+  Assert.equal(classic.keyedHistograms[KEYED_ID]["a"].sum, 2);
+  Assert.equal(classic.keyedHistograms[KEYED_ID]["b"].sum, 2);
+  Assert.equal(subsession.keyedHistograms[KEYED_ID]["a"].sum, 1);
+  Assert.equal(subsession.keyedHistograms[KEYED_ID]["b"].sum, 1);
+});
+
 // Checks that an expired histogram file is deleted when loaded.
 add_task(function* test_runOldPingFile() {
   let histogramsFile = getSavedHistogramsFile("old-histograms.dat");