Closed
Bug 1309648
Opened 8 years ago
Closed 8 years ago
Telemetry should always report OS Version as a string
Categories
(Toolkit :: Telemetry, defect, P1)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: mreid, Assigned: gfritzsche)
Details
(Whiteboard: [measurement:client])
Attachments
(2 files, 1 obsolete file)
87.01 KB,
text/plain
|
Details | |
8.75 KB,
patch
|
gfritzsche
:
review+
|
Details | Diff | Splinter Review |
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•8 years ago
|
||
I'm attaching one such ping (in lsb_heka_cat output form) with client id redacted. Looks like it's possibly an Android issue.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → gfritzsche
Priority: -- → P2
Whiteboard: [measurement:client]
Assignee | ||
Updated•8 years ago
|
Priority: P2 → P1
Assignee | ||
Comment 2•8 years ago
|
||
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.
Attachment #8801856 -
Flags: review?(alessio.placitelli)
Comment 3•8 years ago
|
||
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?
Attachment #8801856 -
Flags: review?(alessio.placitelli) → review+
Assignee | ||
Comment 4•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d46122eeee806416ed87d5441ae7fd4fc6b6b18
Assignee | ||
Comment 5•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8801856 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8802102 -
Flags: review+
Assignee | ||
Comment 6•8 years ago
|
||
https://hg.mozilla.org/try/rev/1c39f058edac986b117a9b71901db2cc1cabeca3
Assignee | ||
Comment 7•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c39f058edac986b117a9b71901db2cc1cabeca3
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/353f4ca09a50 Constrain some environment/system/os fields to string types. r=dexter
Keywords: checkin-needed
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/353f4ca09a50
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•