If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Include OS data in sync ping

RESOLVED FIXED in Firefox 56

Status

()

Firefox
Sync
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: markh, Unassigned)

Tracking

Trunk
Firefox 56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

3 months ago
Now that Android and iOS are submitting sync pings, we are having trouble determining where some pings originate from - eg, Grisha is reporting that some desktop pings seem to be submitting with "xpcomAbi=arm-eabi-gcc3", which we can't quite explain, but the content of those pings certainly imply they are coming from desktop.

It seems that we should include the OS data in the sync pings. Adding |{addEnvironment: true}| to our call to |TelemetryController.submitExternalPing()| works, but includes way more information than we need. However, including |TelemetryEnvironment.currentEnvironment["system"]["os"]| in the body of our ping might be acceptable.

Georg, does that sound reasonable to you?
Flags: needinfo?(gfritzsche)
This sounds reasonable in general.
I think we want to solve this kind of problem more generically, but this will not help you in the short-term.
Flags: needinfo?(gfritzsche)

Comment 2

3 months ago
Some more information about what other clients are currently submitting:

Android:
- xpcomAbi=arm-eabi-gcc3
- architecture=armeabi-v7a

iOS:
- architecture=arm

Furthermore, for application.name, Android is always submitting Fennec, while iOS is submitting Fennec, Firefox Beta, and Firefox depending on the channel.

environment.os definitely seems like a very nice addition to pings from all devices.

Updated

3 months ago
See Also: → bug 1374758

Updated

3 months ago
See Also: → bug 1374760
Comment hidden (mozreview-request)
(Reporter)

Updated

3 months ago
Blocks: 1375353

Comment 4

3 months ago
mozreview-review
Comment on attachment 8880233 [details]
Bug 1373093 - Add operating system information to the desktop Sync ping.

https://reviewboard.mozilla.org/r/151586/#review156822

LGTM modulo nits.

::: commit-message-416c3:1
(Diff revision 1)
> +Bug 1373093 - Add operating system information to the destkop Sync ping. r?tcsc

nit: typo in commit message

::: services/sync/tests/unit/test_telemetry.js:99
(Diff revision 1)
> +
> +  // Check the "os" block - we can't really check specific values, but can
> +  // check it smells sane.
> +  ok(ping.os, "there is an OS block");
> +  ok(ping.os.name, "there is an OS name");
> +  ok(ping.os.name, "there is an OS version");

This probably should be `ok(ping.os.version`, not name (which is checked on the previous line).

Or maybe even `"version" in ping.os`, which would avoid some awkward situation where an OS releases version 0 or something (although it's possible this isn't necessary, up to you)

::: toolkit/components/telemetry/docs/data/sync-ping.rst:21
(Diff revision 1)
>        version: 4,
>        type: "sync",
>        ... common ping data
>        payload: {
>          version: 1,
> +        os : { ... }, // os data from the current telemetry environment,

Nit: Probably worth describing it's format here? e.g. at least that it contains `{name, version}` and what their types are.
Attachment #8880233 - Flags: review?(tchiovoloni) → review+

Comment 5

3 months ago
mozreview-review
Comment on attachment 8880233 [details]
Bug 1373093 - Add operating system information to the desktop Sync ping.

https://reviewboard.mozilla.org/r/151586/#review156884

::: services/sync/tests/unit/test_telemetry.js:99
(Diff revision 1)
> +
> +  // Check the "os" block - we can't really check specific values, but can
> +  // check it smells sane.
> +  ok(ping.os, "there is an OS block");
> +  ok(ping.os.name, "there is an OS name");
> +  ok(ping.os.name, "there is an OS version");

locale?
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 8

3 months ago
Pushed by mhammond@skippinet.com.au:
https://hg.mozilla.org/integration/autoland/rev/2efe95aabff8
Add operating system information to the desktop Sync ping. r=tcsc

Comment 9

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2efe95aabff8
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
You need to log in before you can comment on or make changes to this bug.