Bug 1302663 - Part 3 - Add events to the main ping. r=dexter
☠☠ backed out by f20e3b45a156 ☠ ☠
authorGeorg Fritzsche <georg.fritzsche@googlemail.com>
Sun, 13 Nov 2016 01:52:29 +0700
changeset 349038 574bf5f49185f8fd4d89aa4daac3ea73677b78d2
parent 349037 ab64c55508aabf1af5063cda3322c526c67422ad
child 349039 a4471f64e67a13257db4ebaa68b8de6595acff01
push id10298
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:33:03 +0000
treeherdermozilla-aurora@7e29173b1641 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdexter
bugs1302663
milestone52.0a1
Bug 1302663 - Part 3 - Add events to the main ping. r=dexter
toolkit/components/telemetry/TelemetrySession.jsm
toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
--- a/toolkit/components/telemetry/TelemetrySession.jsm
+++ b/toolkit/components/telemetry/TelemetrySession.jsm
@@ -1003,16 +1003,34 @@ var Impl = {
       } else {
         ret[name] = scalarsSnapshot[name];
       }
     }
 
     return ret;
   },
 
+  getEvents: function(isSubsession, clearSubsession) {
+    if (!isSubsession) {
+      // We only support scalars for subsessions.
+      this._log.trace("getEvents - We only support events in subsessions.");
+      return [];
+    }
+
+    let events = Telemetry.snapshotBuiltinEvents(this.getDatasetType(),
+                                                 clearSubsession);
+
+    // Don't return the test events outside of test environments.
+    if (!this._testing) {
+      events = events.filter(e => e[1].startsWith("telemetry.test"));
+    }
+
+    return events;
+  },
+
   getThreadHangStats: function getThreadHangStats(stats) {
     this._log.trace("getThreadHangStats");
 
     stats.forEach((thread) => {
       thread.activity = this.packHistogram(thread.activity);
       thread.hangs.forEach((hang) => {
         hang.histogram = this.packHistogram(hang.histogram);
       });
@@ -1275,16 +1293,17 @@ var Impl = {
     let histograms = protect(() => this.getHistograms(isSubsession, clearSubsession), {});
     let keyedHistograms = protect(() => this.getKeyedHistograms(isSubsession, clearSubsession), {});
     payloadObj.histograms = histograms[HISTOGRAM_SUFFIXES.PARENT] || {};
     payloadObj.keyedHistograms = keyedHistograms[HISTOGRAM_SUFFIXES.PARENT] || {};
     payloadObj.processes = {
       parent: {
         scalars: protect(() => this.getScalars(isSubsession, clearSubsession)),
         keyedScalars: protect(() => this.getScalars(isSubsession, clearSubsession, true)),
+        events: protect(() => this.getEvents(isSubsession, clearSubsession)),
       },
       content: {
         histograms: histograms[HISTOGRAM_SUFFIXES.CONTENT],
         keyedHistograms: keyedHistograms[HISTOGRAM_SUFFIXES.CONTENT],
       },
     };
 
     // Only include the GPU process if we've accumulated data for it.
--- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
@@ -286,16 +286,63 @@ function checkScalars(processes) {
     for (let key in keyedScalars[name]) {
       Assert.equal(typeof key, "string", "Keyed scalar keys must be strings.");
       Assert.ok(key.length <= 70, "Keyed scalar keys can't have more than 70 characters.");
       checkScalar(scalar[name][key]);
     }
   }
 }
 
+function checkEvents(processes) {
+  // Check that the events section is available in the ping payload.
+  const parent = processes.parent;
+  Assert.ok("events" in parent, "The events section must be available in the parent process.");
+
+  // Check that the events section has the right format.
+  Assert.ok(Array.isArray(parent.events), "The events entry must be an array.");
+  for (let [ts, category, method, object, value, extra] of parent.events) {
+    Assert.equal(typeof(ts), "number", "Timestamp field should be a number.");
+    Assert.greaterOrEqual(ts, 0, "Timestamp should be >= 0.");
+
+    Assert.equal(typeof(category), "string", "Category should have the right type.");
+    Assert.lessOrEqual(category.length, 100, "Category should have the right string length.");
+
+    Assert.equal(typeof(method), "string", "Method should have the right type.");
+    Assert.lessOrEqual(method.length, 40, "Method should have the right string length.");
+
+    Assert.equal(typeof(object), "string", "Object should have the right type.");
+    Assert.lessOrEqual(object.length, 40, "Object should have the right string length.");
+
+    Assert.ok(value === null || typeof(value) === "string",
+              "Value should be null or a string.");
+    if (value) {
+      Assert.lessOrEqual(value.length, 100, "Value should have the right string length.");
+    }
+
+    Assert.ok(extra === null || typeof(extra) === "object",
+              "Extra should be null or an object.");
+    if (extra) {
+      let keys = Object.keys(extra);
+      let keyTypes = keys.map(k => typeof(k));
+      Assert.lessOrEqual(keys.length, 20, "Should not have too many extra keys.");
+      Assert.ok(keyTypes.every(t => t === "string"),
+                "All extra keys should be strings.");
+      Assert.ok(keys.every(k => k.length <= 20),
+                "All extra keys should have the right string length.");
+
+      let values = Object.values(extra);
+      let valueTypes = values.map(v => typeof(v));
+      Assert.ok(valueTypes.every(t => t === "string"),
+                "All extra values should be strings.");
+      Assert.ok(values.every(v => v.length <= 100),
+                "All extra values should have the right string length.");
+    }
+  }
+}
+
 function checkPayload(payload, reason, successfulPings, savedPings) {
   Assert.ok("info" in payload, "Payload must contain an info section.");
   checkPayloadInfo(payload.info);
 
   Assert.ok(payload.simpleMeasurements.totalTime >= 0);
   Assert.ok(payload.simpleMeasurements.uptime >= 0);
   Assert.equal(payload.simpleMeasurements.startupInterrupted, 1);
   Assert.equal(payload.simpleMeasurements.shutdownDuration, SHUTDOWN_TIME);
@@ -417,16 +464,17 @@ function checkPayload(payload, reason, s
       sum: 1,
     },
   };
   Assert.deepEqual(expected_keyed_count, keyedHistograms[TELEMETRY_TEST_KEYED_COUNT]);
 
   Assert.ok("processes" in payload, "The payload must have a processes section.");
   Assert.ok("parent" in payload.processes, "There must be at least a parent process.");
   checkScalars(payload.processes);
+  checkEvents(payload.processes);
 }
 
 function writeStringToFile(file, contents) {
   let ostream = Cc["@mozilla.org/network/safe-file-output-stream;1"]
                 .createInstance(Ci.nsIFileOutputStream);
   ostream.init(file, PR_WRONLY | PR_CREATE_FILE | PR_TRUNCATE,
 	       RW_OWNER, ostream.DEFER_OPEN);
   ostream.write(contents, contents.length);
@@ -653,16 +701,61 @@ add_task(function* test_checkSubsessionS
   Telemetry.scalarSet(STRING_SCALAR, expectedString);
   subsession = TelemetrySession.getPayload("environment-change");
   Assert.equal(subsession.processes.parent.scalars[UINT_SCALAR], expectedUint,
                UINT_SCALAR + " must contain the expected value.");
   Assert.equal(subsession.processes.parent.scalars[STRING_SCALAR], expectedString,
                STRING_SCALAR + " must contain the expected value.");
 });
 
+add_task(function* test_checkSubsessionEvents() {
+  if (gIsAndroid) {
+    // We don't support subsessions yet on Android.
+    return;
+  }
+
+  // Clear the events.
+  Telemetry.clearEvents();
+  yield TelemetryController.testReset();
+
+  // Record some events.
+  let expected = [
+    ["telemetry.test", "test1", "object1", "a", null],
+    ["telemetry.test", "test1", "object1", null, {key1: "value"}],
+  ];
+  for (let event of expected) {
+    Telemetry.recordEvent(...event);
+  }
+
+  // Check that events are not available in classic pings but are in subsession
+  // pings. Also clear the subsession.
+  let classic = TelemetrySession.getPayload();
+  let subsession = TelemetrySession.getPayload("environment-change", true);
+
+  Assert.ok("events" in classic.processes.parent, "Should have an events field in classic payload.");
+  Assert.ok("events" in subsession.processes.parent, "Should have an events field in subsession payload.");
+
+  // They should be empty in the classic payload.
+  Assert.deepEqual(classic.processes.parent.events, [], "Events in classic payload should be empty.");
+
+  // In the subsession payload, they should contain the recorded test events.
+  let events = subsession.processes.parent.events.filter(e => e[1] === "telemetry.test");
+  Assert.equal(events.length, expected.length, "Should have the right amount of events in the payload.");
+  for (let i = 0; i < expected.length; ++i) {
+    Assert.deepEqual(events[i].slice(1), expected[i],
+                     "Should have the right event data in the ping.");
+  }
+
+  // As we cleared the subsession above, the events entry should now be empty.
+  subsession = TelemetrySession.getPayload("environment-change", false);
+  Assert.ok("events" in subsession.processes.parent, "Should have an events field in subsession payload.");
+  events = subsession.processes.parent.events.filter(e => e[1] === "telemetry.test");
+  Assert.equal(events.length, 0, "Should have no test events in the subsession payload now.");
+});
+
 add_task(function* test_checkSubsessionHistograms() {
   if (gIsAndroid) {
     // We don't support subsessions yet on Android.
     return;
   }
 
   let now = new Date(2020, 1, 1, 12, 0, 0);
   let expectedDate = new Date(2020, 1, 1, 0, 0, 0);