Remove activeAddons/activeTheme/activePlugins from the legacy telemetry environment
Categories
(Toolkit :: Add-ons Manager, task, P2)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox156 | --- | fixed |
People
(Reporter: rpl, Assigned: rpl)
References
Details
(Whiteboard: [addons-jira])
Attachments
(2 files)
|
Bug 2055613 - Prepare telemetry/addons tests for legacy environment addons removal r=chutten!,robwu!
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review |
As part of Bug 1981496 we have moved the internals that are collecting telemetry data related to the active extensions, active theme and active GMPlugins from the telemetry environment internals to the AddonManager internals and we have started to collect them in Glean (and Bug 2000866 introduced an addons ping that include this data in addition to the metrics one), but we have kept this data also in the telemetry environment.
This bug is tracking following up on removing these telemetry data from the legacy telemetry environment.
In addition to remove the logic that is adding this data into the telemetry environment we should also:
- remove mention to the removed parts of the telemetry environment from the related in-tree docs: https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/data/environment.html
- update the schema in https://github.com/mozilla-services/mozilla-pipeline-schemas/blob/main/templates/include/telemetry/environment.1.schema.json
- cleanup the test coverage from asserting the data being removed
See the list of steps as documented in https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/start/remove-telemetry.html#environment-fields
|
||
Updated•1 month ago
|
|
||
Comment 1•1 month ago
|
||
And when you do so, remember to update the magic comments that power arewegleanyet (should just have to decrease both the Legacy and Glean numbers by 3 (activeAddons, activeGMPlugins, theme)) and the docs.
(( The accounting for the Environment is weird, thus the comments. The "Glean Count" is the number of Glean metrics mirroring the Environment pieces. The "Legacy Count" is the number of Environment pieces. You'd think you'd just reduce the "Legacy Count", but in removing the Environment pieces, the Glean metrics aren't mirrors any more, so you have to reduce that count too. ))
|
Assignee | |
Updated•1 month ago
|
|
|
Assignee | |
Comment 2•17 days ago
|
||
Removes activeAddons/theme/GMPlugins test coverage from TelemetryEnvironment and
AddonManager tests, ahead of removing the environment addons properties as part
of the child revision.
|
|
Assignee | |
Comment 3•17 days ago
|
||
Removes environment.addons along with cleaning up about:telemetry and EnvironmentAddonBuilder from
logic that was mainly needed due to the legacy telemetry environment.
Also updates the docs and and decrements the arewegleanyet Legacy/Glean counters.
|
||
Updated•16 days ago
|
|
|
||
Updated•16 days ago
|
|
||
Comment 6•8 days ago
•
|
||
Backed out for causing xpcshell failures on test_exposure.
These ones too:
- https://treeherder.mozilla.org/logviewer?job_id=586534869&repo=autoland&task=ACJJpjzpQ-O0JfidlY3dVg.0
- https://treeherder.mozilla.org/logviewer?job_id=586542976&repo=autoland&task=OW4u3d_HTimvCtfHGX5XqQ.1
And likely also this: https://treeherder.mozilla.org/logviewer?job_id=586537525&repo=autoland&task=UKUXi_lHSa6B1sCY4r9XpQ.0
|
|
Assignee | |
Comment 7•7 days ago
|
||
(In reply to Cosmin Sabou [:CosminS] from comment #6)
Backed out for causing xpcshell failures on test_exposure.
These ones too:
These xpcshell failures were definitely related to the change in the last of the two patches, the underlying issue was kind of pre-existing, due to the TelemetryEnvironment initialization waiting on browser-delayed-startup-finished which doesn't happen in xpcshell test unless explicitly notified, which was hidden by a rejection that was having the unintented side-effect of hiding that issue.
I was able to reproduce each of these test failures consistently and I've updated the patch to handle them.
And likely also this: https://treeherder.mozilla.org/logviewer?job_id=586537525&repo=autoland&task=UKUXi_lHSa6B1sCY4r9XpQ.0
I suspect this may be unrelated, the _telemetry in the test name was making suspicious but I wasn't unable to hit that locally and based on a quick look to the test I couldn't see how the change in the second of the two patches in this stack may be triggering that kind of assertion failure.
|
||
Comment 10•6 days ago
|
||
Backed out for causing xpc failures @ test_provider_markSafe.js
|
|
Assignee | |
Comment 11•3 days ago
|
||
(In reply to pstanciu from comment #10)
Backed out for causing xpc failures @ test_provider_markSafe.js
I've looked into it, the test AOM provider registered internally by test_provider_markSafe.js was missing a getAddonsByTypes method that the AddonManager expects to be available on every registered AOM provider, it wasn't noticed before the changes applied to the second patch because there wasn't anything calling that method and so the incomplete test AOM provider wasn't hitting the failure because the EnvironmentAddonBuilder instance was only created by the telemetry environment initialization (while it is now instantiated by AMTelemetry on the AddonManager startup also when the test doesn't trigger the telemetry environment initialization).
I've update the patch to add the getAddonsByTypes method to the test AOM mock providers created by test_provider_markSafe.js (and on a couple more xpcshell test that I've verified locally were also missing the getAddonsByTypes method and would be hitting a similar test failure).
|
||
Comment 12•3 days ago
|
||
|
||
Comment 13•2 days ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/8f10a945fa66
https://hg.mozilla.org/mozilla-central/rev/589a05df4bac
Description
•