1455096 - extra_keys for non-first dynamically-registered Events are never valid

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P1)

Description

[Chris H-C :chutten]

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`)

Comment 1

[Chris H-C :chutten]

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

[Ryan VanderMeulen [:RyanVM]]

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...

Comment 3

[Chris H-C :chutten]

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

Comment 4

[Chris H-C :chutten]

Since the patch for this landed on bug 1450690, I'll count this as fixed.