For pre-release channels like beta, we have a "bN" or "aN" suffix (e.g. "45.0b6" vs. "45.0"). This is called the "display version" and we don't have this data point anywhere in Telemetry. Bug 1240522 and other use-cases looking at specific betas would need this (to avoid doing buildid-based lookups). This data seems to come from browser/config/version_display.txt & browser/config/version.txt: * https://dxr.mozilla.org/mozilla-central/rev/5b2baa5e9356644a7ed0b73e422eaff62e159ffb/configure.in#1764 * http://hg.mozilla.org/releases/mozilla-beta/file/tip/browser/config/version.txt * http://hg.mozilla.org/releases/mozilla-beta/file/tip/browser/config/version_display.txt

We need this data point in more than just the "main" ping (e.g. "activation" and "upgrade"). Consequently, i think we should put this into the ping.application section [0]. I would say this could be called "buildNo", but Sylvestre mentioned that part in the display version is not only used for build numbers. 0: https://dxr.mozilla.org/mozilla-central/rev/5b2baa5e9356644a7ed0b73e422eaff62e159ffb/toolkit/components/telemetry/TelemetryController.jsm#362

(In reply to Georg Fritzsche [:gfritzsche] from comment #1) > We need this data point in more than just the "main" ping (e.g. "activation" > and "upgrade"). > Consequently, i think we should put this into the ping.application section > [0]. > > I would say this could be called "buildNo", but Sylvestre mentioned that > part in the display version is not only used for build numbers. Benjamin, are you ok with putting this info into the application section?

Sylvestre wanted to look into this.

I'm fine with it. My understanding is that we do this mapping server-side for crash-stats, so that might be a reasonable stopgap as well if there's something that this is urgently blocking.

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

Comment on attachment 8723998 [details] MozReview Request: Bug 1250897 - Add build number data in Telemetry pings Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36807/diff/1-2/

https://reviewboard.mozilla.org/r/36807/#review33361 First version, this is not fully working as the test is failing AppConstants.MOZ_APP_VERSION_DISPLAY is provided by the local application. Here, it is a1. Should I move AppConstants.MOZ_APP_VERSION_DISPLAY into Services.appinfo?

Comment on attachment 8723998 [details] MozReview Request: Bug 1250897 - Add build number data in Telemetry pings Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36807/diff/2-3/

Comment on attachment 8723998 [details] MozReview Request: Bug 1250897 - Add build number data in Telemetry pings https://reviewboard.mozilla.org/r/36807/#review33389 Lets also add documentation for the new field in `common-ping.rst`. ::: toolkit/components/telemetry/TelemetryController.jsm:392 (Diff revision 3) > + detailedVersion: detailed, Per IRC discussion, lets just submit the display version directly here (e.g. "45.0b6") as `displayVersion`. The solution here will behave a bit inconsistent and doesn't seem well-behaved for analysis. ::: toolkit/components/telemetry/tests/unit/test_TelemetryController.js:27 (Diff revision 3) > -const APP_VERSION = "1"; > +const APP_VERSION = "1b2"; This version here is different from the "display version", we should not change it. ::: toolkit/components/telemetry/tests/unit/test_TelemetryController.js:62 (Diff revision 3) > + detailedVersion: APP_DETAILED_VERSION, With the changes above we can just check against AppConstants here. ::: toolkit/components/telemetry/tests/unit/test_TelemetryController.js:99 (Diff revision 3) > - loadAddonManager("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2"); > + loadAddonManager("xpcshell@tests.mozilla.org", "XPCShell", "1b2", "1.9.2"); This version here is different from the "display version", we should not change it.

Comment on attachment 8723998 [details] MozReview Request: Bug 1250897 - Add build number data in Telemetry pings Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36807/diff/3-4/

Comment on attachment 8723998 [details] MozReview Request: Bug 1250897 - Add build number data in Telemetry pings https://reviewboard.mozilla.org/r/36807/#review33401 Thanks, this looks good except for the test update. ::: toolkit/components/telemetry/tests/unit/test_TelemetryController.js (Diff revisions 3 - 4) > - detailedVersion: APP_DETAILED_VERSION, We can compare displayVersion against AppConstants.MOZ_APP_VERSION_DISPLAY here instead of the changes below. While that is not the most sophisticated testing, it gives us some sanity-checking.

Comment on attachment 8723998 [details] MozReview Request: Bug 1250897 - Add build number data in Telemetry pings Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36807/diff/4-5/

Comment on attachment 8723998 [details] MozReview Request: Bug 1250897 - Add build number data in Telemetry pings https://reviewboard.mozilla.org/r/36807/#review33405 ::: toolkit/components/telemetry/tests/unit/test_TelemetryController.js:74 (Diff revisions 4 - 5) > + Assert.equal(aPing.application.displayVersion, AppConstants.MOZ_APP_VERSION_DISPLAY, Let's just put this into the `APPLICATION_TEST_DATA` above, no need for a separate assertion here.

Comment on attachment 8723998 [details] MozReview Request: Bug 1250897 - Add build number data in Telemetry pings Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36807/diff/5-6/

Comment on attachment 8723998 [details] MozReview Request: Bug 1250897 - Add build number data in Telemetry pings https://reviewboard.mozilla.org/r/36807/#review33415 Thanks, this looks good. We need Benjamin to have a quick look (per [0]), but per the previous comment we should be fine here. 0: https://wiki.mozilla.org/Firefox/Data_Collection

Comment on attachment 8723998 [details] MozReview Request: Bug 1250897 - Add build number data in Telemetry pings https://reviewboard.mozilla.org/r/36807/#review33417

NI Benjamin for comment #15.

This is fine. Please make sure that this gets added to the schemas.

> Please make sure that this gets added to the schemas. Georg, as discussed on irc

(In reply to Sylvestre Ledru [:sylvestre] from comment #19) > > Please make sure that this gets added to the schemas. > > Georg, as discussed on irc PR is up: https://github.com/mozilla-services/mozilla-pipeline-schemas/pull/1

Comment on attachment 8723998 [details] MozReview Request: Bug 1250897 - Add build number data in Telemetry pings Approval Request Comment [Feature/regressing bug #]: Not a new issue, we want data sooner [User impact if declined]: We will have to wait longer to have better partial for beta builds. See bug 1229682 [Describe test coverage new/current, TreeHerder]: Nightly has the new field [Risks and why]: New field in an existing structure + has a test covering the change [String/UUID change made/needed]: None

Comment on attachment 8723998 [details] MozReview Request: Bug 1250897 - Add build number data in Telemetry pings Better data, so useful, adds in build/version. Please uplift to aurora