fixes #807, use correct dynamic label for demographics survey

rhelmer commented 2 years ago

Checklist for reviewer:

[ ] The description should reference a bug or github issue, if relevant.
[ ] There must be a CHANGELOG.md entry for any non-test change.
[ ] Any change to the NPM commands must be carefully reviewed to make sure it won't break the Add-ons pipeline.
[ ] Any version increase must follow the release process.

rhelmer commented 2 years ago

@aaga this is a bit tricky to test... see https://github.com/mozilla-rally/rally-core-addon/issues/807 for the explanation, there's a bug filed on glean.js to possibly handle this differently (currently glean will silently substitute unknown dynamic labels with __other__, when maybe it should generate an error or at least log a warning).

I think it's pretty hard to have an automated test in the absence of clarity around the expected behavior. Maybe we could add a test that inspects the output of glean and ensures that __other__ is not present?

In the meantime, this can be manually tested by enabling glean and setting debug: { logPings: true } in Glean.initialize, which will cause Glean to log to the background script's console (in about:debugging in Firefox).

rhelmer commented 2 years ago

Was able to test and confirm this fixed the ping for me.

@rhelmer Worth creating a similar GH issue for the RWP? I noticed it is currently using the same incorrect label.

Thanks, filed https://github.com/mozilla-rally/rally-web-platform/issues/289

mozilla-rally / rally-core-addon

fixes #807, use correct dynamic label for demographics survey #808