Bug 1563239 - Do not summarize builtin events to the "dynamic" process r=janerik
authorsinguliere <singuliere@autistici.org>
Thu, 15 Aug 2019 16:27:54 +0000
changeset 488264 e7ff4966b99e57346c8bff748b0dc06c074cfb00
parent 488263 0f32ec38b8ff3c28ab0312d5503f3668105dfbeb
child 488265 41f6c078e950a7d4231745274047133b7372ebf4
push id36440
push userncsoregi@mozilla.com
push dateFri, 16 Aug 2019 03:57:48 +0000
treeherdermozilla-central@a58b7dc85887 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjanerik
bugs1563239
milestone70.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 1563239 - Do not summarize builtin events to the "dynamic" process r=janerik Check if the dynamic event is built-in or not before summarizing. Differential Revision: https://phabricator.services.mozilla.com/D41924
toolkit/components/telemetry/core/TelemetryEvent.cpp
toolkit/components/telemetry/tests/unit/test_TelemetryEvents_buildFaster.js
--- a/toolkit/components/telemetry/core/TelemetryEvent.cpp
+++ b/toolkit/components/telemetry/core/TelemetryEvent.cpp
@@ -438,17 +438,19 @@ RecordEventResult RecordEvent(const Stat
   if (IsExpired(*eventKey)) {
     mozilla::Telemetry::AccumulateCategorical(
         LABELS_TELEMETRY_EVENT_RECORDING_ERROR::Expired);
     return RecordEventResult::ExpiredEvent;
   }
 
   // Fixup the process id only for non-builtin (e.g. supporting build faster)
   // dynamic events.
-  if (eventKey->dynamic && !(*gDynamicEventInfo)[eventKey->id].builtin) {
+  auto dynamicNonBuiltin =
+      eventKey->dynamic && !(*gDynamicEventInfo)[eventKey->id].builtin;
+  if (dynamicNonBuiltin) {
     processType = ProcessID::Dynamic;
   }
 
   // Check whether the extra keys passed are valid.
   if (!CheckExtraKeysValid(*eventKey, extra)) {
     mozilla::Telemetry::AccumulateCategorical(
         LABELS_TELEMETRY_EVENT_RECORDING_ERROR::ExtraKey);
     return RecordEventResult::InvalidExtraKey;
@@ -457,17 +459,17 @@ RecordEventResult RecordEvent(const Stat
   // Check whether we can record this event.
   if (!CanRecordEvent(lock, *eventKey, processType)) {
     return RecordEventResult::Ok;
   }
 
   // Count the number of times this event has been recorded, even if its
   // category does not have recording enabled.
   TelemetryScalar::SummarizeEvent(UniqueEventName(category, method, object),
-                                  processType, eventKey->dynamic);
+                                  processType, dynamicNonBuiltin);
 
   // Check whether this event's category has recording enabled
   if (!gEnabledCategories.GetEntry(GetCategory(lock, *eventKey))) {
     return RecordEventResult::Ok;
   }
 
   static bool sEventPingEnabled = mozilla::Preferences::GetBool(
       "toolkit.telemetry.eventping.enabled", true);
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryEvents_buildFaster.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryEvents_buildFaster.js
@@ -159,16 +159,17 @@ add_task(
     // Clean up.
     await TelemetryController.testShutdown();
     await OS.File.remove(FILE_PATH);
   }
 );
 
 add_task(async function test_dynamicBuiltinEvents() {
   Telemetry.clearEvents();
+  Telemetry.clearScalars();
   Telemetry.canRecordExtended = true;
 
   const TEST_EVENT_NAME = "telemetry.test.dynamicbuiltin";
 
   // Register some dynamic builtin test events.
   Telemetry.registerBuiltinEvents(TEST_EVENT_NAME, {
     // Event with only required fields.
     test1: {
@@ -195,16 +196,23 @@ add_task(async function test_dynamicBuil
   });
   // Now check that the snapshot contains the expected data.
   let snapshot = Telemetry.snapshotEvents(
     Ci.nsITelemetry.DATASET_PRERELEASE_CHANNELS,
     false
   );
   Assert.ok("parent" in snapshot, "Should have parent events in the snapshot.");
 
+  // For checking event summaries
+  const scalars = Telemetry.getSnapshotForKeyedScalars("main", true);
+  Assert.ok(
+    "parent" in scalars,
+    "Should have parent scalars in the main snapshot."
+  );
+
   let expected = [
     [TEST_EVENT_NAME, "test1", "object1"],
     [TEST_EVENT_NAME, "test2", "object1", null, { key1: "foo", key2: "bar" }],
     [TEST_EVENT_NAME, "test2b", "object2", null, { key2: "bar" }],
   ];
   let events = snapshot.parent;
   Assert.equal(
     events.length,
@@ -212,16 +220,23 @@ add_task(async function test_dynamicBuil
     "Should have recorded the right amount of events."
   );
   for (let i = 0; i < expected.length; ++i) {
     Assert.deepEqual(
       events[i].slice(1),
       expected[i],
       "Should have recorded the expected event data."
     );
+
+    const uniqueEventName = `${expected[i][0]}#${expected[i][1]}#${
+      expected[i][2]
+    }`;
+    const summaryCount =
+      scalars.parent["telemetry.event_counts"][uniqueEventName];
+    Assert.equal(1, summaryCount, `${uniqueEventName} had wrong summary count`);
   }
 });
 
 add_task(async function test_dynamicBuiltinEventsDisabledByDefault() {
   Telemetry.clearEvents();
   Telemetry.canRecordExtended = true;
 
   const TEST_EVENT_NAME = "telemetry.test.offbydefault";