bug 1450690 - Ensure extra_keys applies to more than just the first dynamic Telemetry Event r?froydnj,Dexter
(This is a partial revert of
bug 1443587 part 2 where the `Move`s were added.)
When registering multiple methods and objects within the same `eventData` item,
we must copy the list of acceptable event keys into each new DynamicEventInfo.
Also includes a test.
MozReview-Commit-ID: 5PSH15nSWEB
--- a/toolkit/components/telemetry/TelemetryEvent.cpp
+++ b/toolkit/components/telemetry/TelemetryEvent.cpp
@@ -121,22 +121,22 @@ typedef nsClassHashtable<nsCStringHashKe
struct EventKey {
uint32_t id;
bool dynamic;
};
struct DynamicEventInfo {
DynamicEventInfo(const nsACString& category, const nsACString& method,
- const nsACString& object, nsTArray<nsCString>&& extra_keys,
+ const nsACString& object, nsTArray<nsCString>& extra_keys,
bool recordOnRelease)
: category(category)
, method(method)
, object(object)
- , extra_keys(Move(extra_keys))
+ , extra_keys(extra_keys)
, recordOnRelease(recordOnRelease)
{}
DynamicEventInfo(const DynamicEventInfo&) = default;
DynamicEventInfo& operator=(const DynamicEventInfo&) = delete;
const nsCString category;
const nsCString method;
@@ -1061,17 +1061,17 @@ TelemetryEvent::RegisterEvents(const nsA
}
// Append event infos to be registered.
for (auto& method : methods) {
for (auto& object : objects) {
// We defer the actual registration here in case any other event description is invalid.
// In that case we don't need to roll back any partial registration.
DynamicEventInfo info{aCategory, method, object,
- Move(extra_keys), recordOnRelease};
+ extra_keys, recordOnRelease};
newEventInfos.AppendElement(info);
newEventExpired.AppendElement(expired);
}
}
}
{
StaticMutexAutoLock locker(gTelemetryEventsMutex);
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryEvents.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryEvents.js
@@ -292,17 +292,17 @@ add_task(async function test_unicodeValu
let snapshot = Telemetry.snapshotEvents(OPTIN, true);
Assert.ok(("parent" in snapshot), "Should have entry for main process.");
let events = snapshot.parent;
Assert.equal(events.length, 2, "Should have recorded 2 events.");
Assert.equal(events[0][4], value, "Should have recorded the right value.");
Assert.equal(events[1][5].key1, value, "Should have recorded the right extra value.");
});
-add_task(function* test_dynamicEvents() {
+add_task(async function test_dynamicEvents() {
Telemetry.clearEvents();
Telemetry.canRecordExtended = true;
// Register some test events.
Telemetry.registerEvents("telemetry.test.dynamic", {
// Event with only required fields.
"test1": {
methods: ["test1"],
@@ -327,31 +327,34 @@ add_task(function* test_dynamicEvents()
record_on_release: true,
},
});
// Record some valid events.
Telemetry.recordEvent("telemetry.test.dynamic", "test1", "object1");
Telemetry.recordEvent("telemetry.test.dynamic", "test2", "object1", null,
{"key1": "foo", "key2": "bar"});
+ Telemetry.recordEvent("telemetry.test.dynamic", "test2b", "object1", null,
+ {"key1": "foo", "key2": "bar"});
Telemetry.recordEvent("telemetry.test.dynamic", "test3", "object1", "some value");
Telemetry.recordEvent("telemetry.test.dynamic", "test4", "object1", null);
// Test recording an unknown event.
Assert.throws(() => Telemetry.recordEvent("telemetry.test.dynamic", "unknown", "unknown"),
/Error: Unknown event: \["telemetry\.test\.dynamic", "unknown", "unknown"\]/,
"Should throw when recording an unknown dynamic event.");
// Now check that the snapshot contains the expected data.
let snapshot = Telemetry.snapshotEvents(OPTIN, false);
Assert.ok(("dynamic" in snapshot), "Should have dynamic events in the snapshot.");
let expected = [
["telemetry.test.dynamic", "test1", "object1"],
["telemetry.test.dynamic", "test2", "object1", null, {key1: "foo", key2: "bar"}],
+ ["telemetry.test.dynamic", "test2b", "object1", null, {key1: "foo", key2: "bar"}],
// "test3" is epxired, so it should not be recorded.
["telemetry.test.dynamic", "test4", "object1"],
];
let events = snapshot.dynamic;
Assert.equal(events.length, expected.length, "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.");