1679837 - Provide OS version detection for Glean RLB

Status: RESOLVED FIXED

(Data Platform and Tools :: Glean: SDK, enhancement, P1)

Description

[Alessio Placitelli [:Dexter]]

The RLB currently report unknown as the OS version. We can probably do better than that!

This bug is for providing a better implementation for the version detection.

Comment 1

[Chin-Ying Li]

I would like to try tackle this one if the bug is still relevant!

Comment 2

[Jan-Erik Rediger [:janerik]]

The bug is still valid but ... it's complicated.
Right now we report the os version for Desktop in a separate metric.
For macOS that's the Darwin version (e.g. I think 20.4.0 for my Big Sur right now).
For Windows it's the NT Kernel version (e.g. 10.0 for Windows 10, 6.1 for Windows 7).
For Linux it's the Kernel version (e.g. 5.10.12-1-vanilla, but easily something different for self-built Kernels).

So the problem is: what is the right data we actually need?

Crate wise there's sysinfo, which probably has most of the information.
But it's a heavy dependency and we should check that it's worthwhile adding that or if we only need a subset of it.

That is all to say: I don't think you should tackle this right now because of the unresolved questions.

Comment 3

[Jan-Erik Rediger [:janerik]]

I wonder if we can make this more actionable.

What is it that we want to report here?
Our docs say client.os_version is The user-visible version of the operating system (e.g. "1.2.3").

That's easy (easier?) to define for macOS and Windows.
But for Linux is that the Kernel version? Or is it the Distribution version? But then just the version doesn't help, we need the distribution name (and os is already taken to be Linux). What if it's an unknown distribution and we don't know the version?

ni?:chutten: How did you end up using fog_validation_os_version? Did it provide anything useful for Linux to you?

(We can of course go back to allowing this version to be passed in and let Glean take the stance of not caring about it much)

Comment 4

[Chin-Ying Li]

(In reply to Jan-Erik Rediger [:janerik] from comment #2)
I see, thank you for the detailed explanation! Do you have any suggestions for bugs that would be more "approachable" for me?

Comment 5

[Chris H-C :chutten]

(In reply to Jan-Erik Rediger [:janerik] from comment #3)

ni?:chutten: How did you end up using fog_validation_os_version? Did it provide anything useful for Linux to you?

(We can of course go back to allowing this version to be passed in and let Glean take the stance of not caring about it much)

For Linux I took the leftmost substring adjacent to a '.', tried to cast it as a number, and accepted that NULL was possible.

Some notes from how Linux versions are being handled:

• We now have a UDF for turning X.Y.Z into just X or X.Y explicitly for helping with OS version strings.
• TMO Measurement Dashboards takes the leftmost three characters of the kernel version for its Linux-specific OS version dimension.
• GLAM does away with versions altogether, having found that folks most often just need the OS family name.

I think we have a lot of latitude on what to provide for Linux here. Anyone who needs more detailed information (crashes, gfx maybe?) can supply it as a metric they control and understand, whereas we can focus on only the broadest possible cases.

Comment 6

[Jan-Erik Rediger [:janerik]]

I write up a small proposal next.

Comment 7

[Jan-Erik Rediger [:janerik]]

(Can't attach this as a file right now, so doing it manually)

Proposal: OS version detection

One-pager! Proposal as discussed in yesterday's meeting.

Who else should I get to take a look at? Who from data science?

Comment 8

[Jan-Erik Rediger [:janerik]]

Attachment: Proposal: OS Version detection

Comment 9

[Alessio Placitelli [:Dexter]]

I left a few questions in the doc.

Comment 10

[Chris H-C :chutten]

Proposal looks fine to me, with Alessio's points about bringing in DS addressed.

See also Crash Stats folks' experiences with Linux (and Mac) OS versions, summarized here: https://bugzilla.mozilla.org/show_bug.cgi?id=1693579#c8

Comment 11

[Jan-Erik Rediger [:janerik]]

Hey Leif, you might be the right person to give this a look, with your background of knowing a lot about desktop telemetry.

Comment 12

[Leif Oines [:loines]]

Yes, I think this is the right approach. No need to deviate too much from what we already do, its been serving its purpose fine. And the change to how linux is handled seems reasonable. We should still be able to normalize the os values in the pipeline using code in [1] if needed.

[1] https://github.com/mozilla/bigquery-etl/blob/a962b729b2745e338ba8ef939bb6b63a9bb0670c/sql/mozfun/norm/os/udf.sql

Comment 13

[Chin-Ying Li]

Here's the WIP Rust snippet for detecting OS version; a bit of refactoring can be done.
Details of how the OS version string are represented can still be changed, and I would like to know if there's any suggestion on my approach.
Using std::process::Command to query the machine itself is lightest dependency I can think of.

Output on macOS Big Sur 11.0.1: 20.1.0
Output on my Ubuntu 20.04.2 LTS: 5.8
Output on Windows 10: 10.0

Comment 14

[Jan-Erik Rediger [:janerik]]

(In reply to Chin-Ying Li from comment #13)

Here's the WIP Rust snippet for detecting OS version; a bit of refactoring can be done.

Oh, thanks for some early work! We'll definitely wait for the proposal to be fully accepted before we land any of this.
Quick note on your code: We should avoid running an external process. We can't guarantee that this succeeds.

The sysinfo crate would have the information we need. We should check if just depending on it brings too much additional stuff with it. If not, then it's best to just use it. If it is, we could see about extracting the relevant parts into a smaller crate to use.

Comment 15

[Jan-Erik Rediger [:janerik]]

Hey Romain, we're trying to figure out what it is that Glean should report for the OS version.
Will brought up the point about reporting the actual Linux distribution and version if possible. He recommended also asking you.
Would you give the linked proposal a look?

I'm leaning toward not breaking what we're used to from legacy Firefox Telemetry and keep os=Linux and os.version=Kernel version, but eventually extend it with reporting the distribution if detected.

Comment 16

[Romain Testard [:RT]]

(In reply to Jan-Erik Rediger [:janerik] from comment #15)

Hey Romain, we're trying to figure out what it is that Glean should report for the OS version.
Will brought up the point about reporting the actual Linux distribution and version if possible. He recommended also asking you.
Would you give the linked proposal a look?

I'm leaning toward not breaking what we're used to from legacy Firefox Telemetry and keep os=Linux and os.version=Kernel version, but eventually extend it with reporting the distribution if detected.

Thanks for the ping, personally I find I need both distribution_id and version given that most linux users have distros (https://sql.telemetry.mozilla.org/queries/78320/source#194750) but we have to start somewhere so OK with your approach.
I ping mkaply who may have further ideas since he's closer than I am to distro work.

Comment 17

[Jan-Erik Rediger [:janerik]]

Thanks!
Some extended context after further discussions last week:
We're considering supporting the os and os version as proposed, with the distribution id being the shared responsibility of someone on the Firefox side (of course we'd be around for helping with that, the details are not yet clear or who needs to own that).

I'm still interested in hearing how either the os version or distribution name and version are or would be used.

Comment 18

[Mike Kaply [:mkaply]]

distribution name is a proxy for getting the Linux distro since most Linux distros use a distribution.ini which tells on the Linux distro.

If we could get the Linux distro name some other way, that would be find. Distro version isn't really used.

Comment 19

[Jan-Erik Rediger [:janerik]]

Thanks!

With that the proposal is accepted.
We're going to auto-fillin os and os.version as specified.
I captured the work on distribution or similar in a "future possibilities" section.

Keeping this issue open to track the implementation.

Chin-Ying: are you still up for doing the implementation, as specified above by first looking at the existing crate and if we can just use that?

Comment 20

[Chin-Ying Li]

Sure, I'll look into possible crate(s) and share my findings here.

Comment 21

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/glean/pull/1538

Comment 22

[Jan-Erik Rediger [:janerik]]

badboy merged PR #1538: "Bug 1679837: Set internal_metrics::os_version for specific os" in 9a36da3.