1758795 - Histogram mirroring from child into parent was broken

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Bas Schouten (:bas.schouten)]

As far as I can tell, glean mirroring to telemetry from child processes to the parent was broken by bug 1751448. Since before that we called the GIFFT code even in the child (unconditionally from the C++ code), and after that we called it only from a Parent TimingDistributionMetric.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1751448

Comment 2

[Bas Schouten (:bas.schouten)]

Attachment: Bug 1758795: Also call GIFFT code when calling members on TimingDistributionMetric::Child. r=chutten!

Depends on D140681

Comment 3

[Chris H-C :chutten]

Attachment: Bug 1758795 - Test all IPC-available Glean metric types over GIFFT r?TravisLong!

Depends on D140691

Comment 4

[Chris H-C :chutten]

Oh yeah, should probably confirm here that yes: indeed this was broken in Fx99 by bug 1751448 so we should seek uplift once we get this slathered in tests and landed.

For the record, I had the misapprehension that the IPC replay would have all the samples come through (though all attributed to the parent process). And GIFFT over IPC was not covered by tests (because why would it be? It had no IPC-sensitive code in it (until bug 1751448)). So this escaped on me.

Comment 5

[Donal Meehan [:dmeehan]]

:bas.schouten is this something we need a beta uplift for 99, or let it ride the trains when it lands in central?
This is the final week for beta releases for 99

Comment 6

[Chris H-C :chutten]

Attachment: Bug 1758795 - Run cleanup registration on main thread r?TravisLong!

If we don't, we assert if the first GIFFT-enabled timing distribution's
instrumentation thread on a given process is not the main thread.

Depends on D140772

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ca181fe0c88d
Also call GIFFT code when calling members on TimingDistributionMetric::Child. r=chutten
https://hg.mozilla.org/integration/autoland/rev/1f93f26f3878
Test all IPC-available Glean metric types over GIFFT r=TravisLong
https://hg.mozilla.org/integration/autoland/rev/344d1bbe7ed5
Run cleanup registration on main thread r=TravisLong

Comment 8

[Bas Schouten (:bas.schouten)]

(In reply to Donal Meehan [:dmeehan] from comment #5)

:bas.schouten is this something we need a beta uplift for 99, or let it ride the trains when it lands in central?
This is the final week for beta releases for 99

Yes, I will request approval once it's seen a day of nightly testing.

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/ca181fe0c88d
https://hg.mozilla.org/mozilla-central/rev/1f93f26f3878
https://hg.mozilla.org/mozilla-central/rev/344d1bbe7ed5

Comment 10

[Bas Schouten (:bas.schouten)]

Comment on attachment 9268702 [details]
Bug 1758795 - Run cleanup registration on main thread r?TravisLong!

Beta/Release Uplift Approval Request

• User impact if declined: None. This hurts our telemetry collection.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This only impacts telemetry code.
• String changes made/needed: None

Comment 11

[Donal Meehan [:dmeehan]]

Comment on attachment 9267099 [details]
Bug 1758795: Also call GIFFT code when calling members on TimingDistributionMetric::Child. r=chutten!

Approved for 99.0b8. Thanks.

Comment 12

[Donal Meehan [:dmeehan]]

Comment on attachment 9267270 [details]
Bug 1758795 - Test all IPC-available Glean metric types over GIFFT r?TravisLong!

Approved for 99.0b8. Thanks.

Comment 13

[Donal Meehan [:dmeehan]]

Comment on attachment 9268702 [details]
Bug 1758795 - Run cleanup registration on main thread r?TravisLong!

Approved for 99.0b8. Thanks.

Comment 14

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e1260fc4ae9f
https://hg.mozilla.org/releases/mozilla-beta/rev/937d704d8a19
https://hg.mozilla.org/releases/mozilla-beta/rev/0cca633a7e88