1309648 - Telemetry should always report OS Version as a string

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[Mark Reid [:mreid]]

According to [1], it is sometimes the case that the environment/system/os/version field, which is specified in the docs[2] as a string value, is reported as an integer.

We should be sure to cast it to a string when adding it to the payload.

[1] https://github.com/mozilla-services/mozilla-pipeline-schemas/pull/13#discussion_r83058306
[2] https://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/telemetry/data/environment.html

Comment 1

[Wesley Dawson [:whd]]

Attachment: ping.txt

I'm attaching one such ping (in lsb_heka_cat output form) with client id redacted. Looks like it's possibly an Android issue.

Comment 2

[Georg Fritzsche [:gfritzsche]]

Attachment: Constrain some environment/system/os fields to string types

I will need to file a follow-up bug on properly merging TelemetryEnvironment.testReset() and .testCleanRestart(). This currently runs makes at least two tests fail in mysterious ways though (e.g. test_SubsessionChaining.js) which are not easily diagnosed. This also includes a minor test cleanup change.

Comment 3

[Alessio Placitelli [:Dexter]]

Comment on attachment 8801856 [details] [diff] [review]
Constrain some environment/system/os fields to string types

Review of attachment 8801856 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good with the comment addressed, thanks! Would you mind testing for the null condition as well in the test?

::: toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js
@@ +1542,5 @@
>  
> +add_task(function* test_osstrings() {
> +  SysInfo.overrides = {
> +    version: 1,
> +    name: 2,

maybe you could make the name or version null to check that behaviour too?

Comment 4

[Georg Fritzsche [:gfritzsche]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d46122eeee806416ed87d5441ae7fd4fc6b6b18

Comment 5

[Georg Fritzsche [:gfritzsche]]

Attachment: Constrain some environment/system/os fields to string types

Comment 6

[Georg Fritzsche [:gfritzsche]]

https://hg.mozilla.org/try/rev/1c39f058edac986b117a9b71901db2cc1cabeca3

Comment 7

[Georg Fritzsche [:gfritzsche]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c39f058edac986b117a9b71901db2cc1cabeca3

Comment 8

[Georg Fritzsche [:gfritzsche]]

All green.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/353f4ca09a50
Constrain some environment/system/os fields to string types. r=dexter

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/353f4ca09a50