Closed
Bug 1455096
Opened 7 years ago
Closed 7 years ago
extra_keys for non-first dynamically-registered Events are never valid
Categories
(Toolkit :: Telemetry, enhancement, P1)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
People
(Reporter: chutten, Assigned: chutten)
References
Details
While investigating bug 1450690 we discovered that one of the optimizations in bug 1443587 was a little over-eager. This means that accumulations with extra_keys to non-first-registered dynamic events are never correct.
Put in example form:
If your registration is, say:
Services.telemetry.registerEvents(TELEMETRY_CATEGORY, {
enroll: {
methods: ["enroll"],
objects: ["preference_study", "addon_study"],
extra_keys: ["experimentType", "branch", "addonId", "addonVersion"],
record_on_release: true,
},
enroll_failure: {
methods: ["enrollFailed"],
objects: ["addon_study"],
extra_keys: ["reason"],
record_on_release: true,
},
unenroll: {
methods: ["unenroll"],
objects: ["preference_study", "addon_study"],
extra_keys: ["reason", "didResetValue", "addonId", "addonVersion"],
record_on_release: true,
},
});
Then the following, though valid, will not work:
Services.telemetry.recordEvent("normandy", "enroll", "addon_study", {"addonId": "some-addon-id"});
(because "addon_study" isn't the first object registered under the event name `enroll`)
Updated•7 years ago
|
Assignee | ||
Comment 1•7 years ago
|
||
Oh right! As part of bug 1450690 we successfully landed the patch I opened this bug to land.
I was getting confused that applying the patch resulted in an empty commit. But it makes sense, as the code on central right now is the code I wanted.
Not sure if I should close this bug or not...
Comment 2•7 years ago
|
||
This is why I was asking for clarification in the other bug about where things stand. My understanding from the QA folks is that there were still issues on trunk even after that patch landed. We could *really* use some sort of high-level summary for the current status of 61 at the moment...
Assignee | ||
Comment 3•7 years ago
|
||
Well, -I'm- happy that events can be registered :D
I guess the shortest path to everyone being happy is if I figure out this whole "VPN" thing and make a nuisance of myself by poking around in bug 1450690 again...
/me goes to mana to figure out mozVPN
Assignee | ||
Comment 4•7 years ago
|
||
Since the patch for this landed on bug 1450690, I'll count this as fixed.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•