1923035 - Figure out a glean migration strategy for the media.video_hardware_decoding_support and media.video_hd_hardware_decoding_support boolean scalars

Status: RESOLVED FIXED

(Toolkit :: Telemetry, task, P1)

Description

[Florian Quèze [:florian]]

These 2 scalars are the only boolean scalars that seem intentionally recorded outside the parent process. This is annoying to migrate them to Glean as the Boolean metric type in Glean is only supported on the parent process.

Possible strategies:

• change Glean to support this (unfortunate as it looks like these 2 probes would be the only consumer)
• check if the probe is really useful, and if not remove it.
• change the code to IPC the value to the parent, and always record on the parent.

These probes landed in bug 1733722 and bug 1741286.

Alastor, Chris, opinions on the best path forward?

Comment 1

[Chris H-C :chutten]

Some background for those coming at this fresh: FOG specifically didn't implement non-commutative metric operations over IPC because it couldn't guarantee order. If, for example, two child processes A and B both set a value for a boolean metric but disagree on the value: how can FOG ensure that it gets the "correct" one?

This approach is in line with Glean's philosophy of being opinionated about design in ways that result in predictable and consistent data quality. But it misses the "unsafety valve" that characterizes how we express those opinions in code: e.g. yes, we're going to ask you to let us do timing for you because we can guarantee things about the timing sources we have access to within the SDK, but we'll also give you a raw API if you feel you can provide the same guarantees or educate your consumers about the differences.

All this is to say that it's not merely a choice of "do we augment FOG to support IPC for exactly two boolean metrics", this is a "do we commit to supplying an unsafety valve for all non-commutative metric operations". We have space in our philosophy for it now, so maybe it's time. But if we don't need it because the probe isn't used or useful, then maybe this decision can wait for another day.

Comment 2

[Alastor Wu [:alwu]]

(In reply to Florian Quèze [:florian] from comment #0)

• change the code to IPC the value to the parent, and always record on the parent.

We should do this way, as those probes are still helpful for us to monitor the hardware decoding status, eg. [1]

[1]
https://sql.telemetry.mozilla.org/queries/82362/source#204048
https://sql.telemetry.mozilla.org/queries/82391/source#204112

Comment 3

[Chris H-C :chutten]

Sounds like we need to permit non-commutative operations for a minimum of boolean (and, thus, labeled_boolean) metrics. I'll write a quick design proposal.

Comment 4

[Chris H-C :chutten]

Attachment: Proposal: [bug 1923035] Permit non-commutative metric operations on non-parent processes in Firefox Desktop (FOG)

ni?janerik as Glean End-to-end lead for sign-off.

I don't expect this to be controversial. I just know that it's easier to change words than an implemented system currently in use. (So get your feedback in now when it's cheap to change)

Comment 5

[Jan-Erik Rediger [:janerik]]

commented.

Comment 6

[Chris H-C :chutten]

Attachment: Bug 1923035 - Support non-commutative operations in child processes for boolean and labeled_boolean metrics r?janerik

Comment 7

[Chris H-C :chutten]

Attachment: Bug 1923035 - Remove extraneous printfs r?TravisLong

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/32783242e758
Support non-commutative operations in child processes for boolean and labeled_boolean metrics r=janerik
https://hg.mozilla.org/integration/autoland/rev/47d997a65266
Remove extraneous printfs r=TravisLong

Comment 9

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/32783242e758
https://hg.mozilla.org/mozilla-central/rev/47d997a65266