Closed Bug 1333043 Opened 5 years ago Closed 5 years ago

Add windows_build_number to the ping also for Windows versions != 10

Categories

(Toolkit :: Telemetry, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: marco, Assigned: marco)

Details

(Whiteboard: [measurement:client])

Attachments

(1 file, 2 obsolete files)

It is available on Socorro, so it would be nice to have it in Telemetry too.
Attached patch windows_build_number (obsolete) — Splinter Review
This is the patch, in case it's OK to add it.

It wasn't added in bug 1255472 to save on bandwidth/storage.
Assignee: nobody → mcastelluccio
Status: NEW → ASSIGNED
If its ok to add it is probably best left to the required data collection review [0].
From the Telemetry client perspective this seems fine if it is actually needed to answer questions.

0: https://wiki.mozilla.org/Firefox/Data_Collection
Comment on attachment 8829403 [details] [diff] [review]
windows_build_number

Alessio, can you review this?
Attachment #8829403 - Flags: review?(alessio.placitelli)
Comment on attachment 8829403 [details] [diff] [review]
windows_build_number

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

Thanks Marco, this looks ok: the test doesn't need to change as it's already checking the field, if available.

However, the documentation needs to be updated: could you please update environment.rst?
Attachment #8829403 - Flags: review?(alessio.placitelli) → feedback+
Marco, would you kindly take care of asking to the data reviewers if it's ok to add this bit of info (and get a data review :-) ) to the other version of windows as well?
Flags: needinfo?(mcastelluccio)
Priority: -- → P3
Whiteboard: [measurement:client]
Comment on attachment 8829403 [details] [diff] [review]
windows_build_number

data-review? bsmedberg

This would be useful on Telemetry, as it is available on Socorro.
Flags: needinfo?(mcastelluccio)
Attachment #8829403 - Flags: review?(benjamin)
Attached patch Patch (obsolete) — Splinter Review
Attachment #8829403 - Attachment is obsolete: true
Attachment #8837253 - Flags: review?(benjamin)
Attachment #8837253 - Flags: review?(alessio.placitelli)
Comment on attachment 8837253 [details] [diff] [review]
Patch

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

Thanks Marco. This looks good to me with the docs fixed.

::: toolkit/components/telemetry/docs/data/environment.rst
@@ +116,5 @@
>              version: <string>, // e.g. "6.1", null on failure
>              kernelVersion: <string>, // android/b2g only or null on failure
>              servicePackMajor: <number>, // windows only or null on failure
>              servicePackMinor: <number>, // windows only or null on failure
> +            windowsBuildNumber: <number>, // windows only or null on failure

You should also update here: https://dxr.mozilla.org/mozilla-central/rev/0eef1d5a39366059677c6d7944cfe8a97265a011/toolkit/components/telemetry/docs/data/environment.rst#365
Attachment #8837253 - Flags: review?(alessio.placitelli) → review+
Attached patch PatchSplinter Review
(In reply to Alessio Placitelli [:Dexter] from comment #9)
> You should also update here:
> https://dxr.mozilla.org/mozilla-central/rev/
> 0eef1d5a39366059677c6d7944cfe8a97265a011/toolkit/components/telemetry/docs/
> data/environment.rst#365

Thanks, I hadn't noticed it was referenced twice!
Attachment #8837253 - Attachment is obsolete: true
Attachment #8837253 - Flags: review?(benjamin)
Attachment #8837258 - Flags: review?(benjamin)
Comment on attachment 8837258 [details] [diff] [review]
Patch

data-review+codereview=me
Attachment #8837258 - Flags: review?(benjamin) → review+
Pushed by mcastelluccio@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9228f08cacde
Add windows build number to the environment data for all Windows versions. r=Dexter,data-r=bsmedberg
https://hg.mozilla.org/mozilla-central/rev/9228f08cacde
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.