Glean events are not cleared when testResetFOG is called on Android
Categories
(Data Platform and Tools :: Glean: SDK, defect, P2)
Tracking
(Not tracked)
People
(Reporter: ltenenbaum, Unassigned)
Details
Attachments
(1 file)
When testResetFOG is called, metrics with type: event are not reset. This causes events from previous tests to carry over into new tests, which makes it harder to write tests for event metrics (see this try job for example, where adding a new test navigate.https.window.html caused the old test telemetry.sub.https.window.html to break).
| Reporter | ||
Comment 1•12 days ago
|
||
Nevermind I was misunderstanding what was happening
| Reporter | ||
Comment 2•11 days ago
•
|
||
Hmm no, there is something here, although it only seems to affect Android. For context, we have an event which is recorded in the parent process (here) and a mozilla WPT test which calls testResetFOG followed by testGetValue on the event. But some events which happened before the call to testResetFOG (in previous tests) are returned.
| Reporter | ||
Updated•11 days ago
|
Comment 3•4 days ago
|
||
Travis to take an initial look this week or next week!
| Reporter | ||
Comment 4•4 days ago
|
||
BTW we ended up marking the test as intermittent on Android: https://searchfox.org/firefox-main/rev/61c7b6d2fe5fc598e573df6528447ab48bc027b3/testing/web-platform/mozilla/meta/notifications/telemetry.sub.https.window.js.ini
So to reproduce the issue, you can remove that file then run ./mach wpt testing/web-platform/mozilla/tests/notifications/
Comment 5•4 days ago
|
||
glean_test_destroy_glean has two branches, and only the initialized one deals with pending work. If Glean was never initialized, the reset deletes the data directory but leaves every task sitting in the dispatcher's pre-init queue. test_reset_glean then initializes Glean, which flushes that queue and writes everything recorded since process start back into the store that was just cleared.
Only the first reset in a process is affected. Later ones take the initialized branch, which handles the queue correctly.
It reproduces on Android because nothing in Gecko initializes Glean there. On desktop initializeFOG() runs at startup, so the branch is never reached. The bug is not specific to events; every metric type is affected.
I'm working on a fix for this in glean_test_destroy_glean so it will now clear the pre-init queue when Glean isn't initialized so a reset no longer replays earlier recordings. Once I finish adding some test coverage for this I'll have a patch up.
This will require a Glean release and vendoring, of course, so not a quick fix at the moment. Hopefully this is compatible with your time frame.
| Reporter | ||
Comment 6•4 days ago
|
||
That's fine, it's not urgent at all for us to have this fixed :)
Comment 7•4 days ago
|
||
Comment 8•2 days ago
|
||
Welp, my first stab at fixing this in glean-core isn't going to work because we have a use-case that is supported by allowing the pre-init queuing prior to Glean being initialized. I might spend a little more time to see if I can find an alternative fix in glean-core, but it might be that the "workaround" to call testResetFOG in the test setup before any metrics are recorded is the easiest way to address this. If that's the case, I'll try and find somewhere to document the behaviour so it isn't surprising to anyone.
Description
•