1458643 - os name for crash pings on fennec is "Linux", could it be Android?

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Metrics, enhancement)

Description

[William Lachance (:wlach)]

It appears that the OS for crash pings sent by fennec is specified as Linux. It would be nice if this were "Android", to match the core ping. This would save a bit of etl code inside our error aggregator (https://github.com/mozilla/telemetry-streaming/pull/129)

It appears that the telemetry environment things that the operating system is Linux, which might make this a bit annoying:

https://searchfox.org/mozilla-central/rev/795575287259a370d00518098472eaa5b362bfa7/mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetryCrashPingBuilder.java#102

:gsvelto, do you have any ideas here? Or should we just try to live with things as they are?

Comment 1

[Gabriele Svelto [:gsvelto]]

The core ping builder hardcodes the os name as "Android" [1]. My guess is that this was done because otherwise the platform is indistinguishable from Linux. I think we can do the some for the crash ping, being consistent is always less confusing. Patch coming.

[1] https://searchfox.org/mozilla-central/rev/795575287259a370d00518098472eaa5b362bfa7/mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetryCorePingBuilder.java#57

Comment 2

[Frank Bertsch [:frank]]

Note that this would make historical aggregates in both error_aggregator and crash_aggregates different, and would mean the aggregates would be different while the change rode the trains. We can solve that in the ETL layer, but we should check if there are any other users of the Fennec crash ping data.

Comment 3

[Gabriele Svelto [:gsvelto]]

Fennec crash ping data is quite new and I haven't seen it used in the dashboards yet so IMHO it's a good idea to do the change soon. I leave it up to you though, I'll post the patch and if there's consensus for it we can land it.

Comment 4

[William Lachance (:wlach)]

(In reply to Gabriele Svelto [:gsvelto] from comment #3)
> Fennec crash ping data is quite new and I haven't seen it used in the
> dashboards yet so IMHO it's a good idea to do the change soon. I leave it up
> to you though, I'll post the patch and if there's consensus for it we can
> land it.

I am not aware of any dashboards which make use of the fennec crash ping data. This would affect crash_aggregates but I haven't heard of anything which uses the fennec portion of that. 

Do you know of anything :chutten?

Comment 5

[Gabriele Svelto [:gsvelto]]

BTW what I'm proposing is to override the environment.system.os.name field to use "Android" in the crash ping builder. Or was this bug meant to change the way that field is populated within the telemetry core code itself?

Comment 6

[William Lachance (:wlach)]

(In reply to Gabriele Svelto [:gsvelto] from comment #5)
> BTW what I'm proposing is to override the environment.system.os.name field
> to use "Android" in the crash ping builder. Or was this bug meant to change
> the way that field is populated within the telemetry core code itself?

No, this bug is just about the functionality rather than code layout.

Comment 7

[Chris H-C :chutten]

(In reply to William Lachance (:wlach) (use needinfo!) from comment #4)
> (In reply to Gabriele Svelto [:gsvelto] from comment #3)
> > Fennec crash ping data is quite new and I haven't seen it used in the
> > dashboards yet so IMHO it's a good idea to do the change soon. I leave it up
> > to you though, I'll post the patch and if there's consensus for it we can
> > land it.
> 
> I am not aware of any dashboards which make use of the fennec crash ping
> data. This would affect crash_aggregates but I haven't heard of anything
> which uses the fennec portion of that. 
> 
> Do you know of anything :chutten?

I haven't written any, at the very least :)

And also I haven't heard of any being written. Might be worth adding in :frank to this discussion given his mobile dealings in the recent past.

Comment 8

[Gabriele Svelto [:gsvelto]]

Attachment: Bug 1458643 - Use 'Android' as the OS name for Fennec crash pings;

Review commit: https://reviewboard.mozilla.org/r/241596/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/241596/

Comment 9

[Gabriele Svelto [:gsvelto]]

The attached patch overrides the OS name as I described in comment 5. This is an example crash ping obtained with the change applied: https://jsoneditoronline.org/?id=be683f26b6ca49e0bcca0939cde8cd46

William, is this what you expected?

Comment 10

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8973065 [details]
Bug 1458643 - Use 'Android' as the OS name for Fennec crash pings;

https://reviewboard.mozilla.org/r/241596/#review247432

Comment 11

[William Lachance (:wlach)]

(In reply to Gabriele Svelto [:gsvelto] from comment #9)
> The attached patch overrides the OS name as I described in comment 5. This
> is an example crash ping obtained with the change applied:
> https://jsoneditoronline.org/?id=be683f26b6ca49e0bcca0939cde8cd46
> 
> William, is this what you expected?

Yup, exactly what I'd expect.

Comment 12

[Frank Bertsch [:frank]]

I looked through and didn't find anything other than the ones I mentioned.

Comment 13

[Gabriele Svelto [:gsvelto]]

Good, let's land this.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3c6468b1512e
Use 'Android' as the OS name for Fennec crash pings; r=jchen

Comment 15

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/3c6468b1512e