1405614 - (telemetry-update_parquet-beta_version) Firefox Beta doesn't report the beta suffix in the environment's build info

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[Rémy Hubscher (:natim)]

If you look at the list of versions here: https://sql.telemetry.mozilla.org/queries/47012/source

You will see that the nightly channel provides the full version number (57.0a1 and 58.0a1) while for beta the version number doesn't contain it.

We would need to have it for beta in order to differentiate records between beta versions without having to fetch first the list of build IDs for that version.

Comment 1

[Alessio Placitelli [:Dexter]]

This is not a bug in the dataset, but rather a different behaviour in the two builds:

- Nightly is reporting the "a1" suffix for *all* the Nightly builds, and it's adding that to both the "version" [1] and "displayVersion" [2].
- Beta is only adding the beta suffix (e.g. "b9") to the "displayVersion", while "version" still shows up without the suffix.

The fix here is either changing the behaviour in the client or adding the "displayVersion" field to the dataset.

[1] - https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/data/environment.html
[2] - https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/data/common-ping.html

Comment 2

[Rémy Hubscher (:natim)]

Thank you for your answer.

If possible I would be in favour of adding app_display_version to the dataset for the three places:

- target_display_version
- previous_display_version
- environment.build.display_version

If not I would need to first do a request on main_summary to findout the build IDs for the given display_version and then use that list of build IDs to query the update_parquet dataset.

Comment 3

[Alessio Placitelli [:Dexter]]

(In reply to Rémy Hubscher (:natim) from comment #2)
> Thank you for your answer.
> 
> If possible I would be in favour of adding app_display_version to the
> dataset for the three places:
> 
> - target_display_version
> - previous_display_version
> - environment.build.display_version

We can only add the "displayVersion" field from the common ping format to the dataset without any additional client change. I'm not sure we have that piece of information in the update blob (that powers the previous/target sections).

@mhowell, do you know if the "update packet" for Beta contains the information about the beta suffix? e.g. b1 for 57.0b1?

> If not I would need to first do a request on main_summary to findout the build IDs for the given display_version and then use that list of build IDs to query the update_parquet dataset.

That would not be ideal. I think there should be a dataset that contains build id/version information.

Comment 4

[Molly Howell (she/her) (mostly inactive)]

The update ping does have application.displayVersion, and it does include the correct beta number. We don't get the beta number of the previous version, so if that's needed it would have to be looked up by build ID.

Comment 5

[Alessio Placitelli [:Dexter]]

(In reply to Matt Howell [:mhowell] from comment #4)
> The update ping does have application.displayVersion, and it does include
> the correct beta number. We don't get the beta number of the previous
> version, so if that's needed it would have to be looked up by build ID.

After having a chat with Matt, turns out the update MAR (what I called "update packet") does not contain the "b2" suffix. Instead, it reports the displayVersion in a different form: "57.0 Beta 2".

Comment 6

[Chris H-C :chutten]

Mapping build ids to version strings is one of the stated goals of the Buildhub project: https://github.com/mozilla-services/buildhub/

Now, it's a service not a dataset, so it's not something you can inner join in re:dash. Maybe a feature request for a parquet product from buildhub would not be out of line?

Or maybe we should derive our own utility dataset?

Comment 7

[Rémy Hubscher (:natim)]

Yes using buildhub is actually my plan B :)

Comment 8

[Alessio Placitelli [:Dexter]]

(In reply to Rémy Hubscher (:natim) from comment #7)
> Yes using buildhub is actually my plan B :)

If you need other analyses that require the specific beta information for the target update or the previous update, this is your best option: any change to the client code will take a bit to propagate, so you won't see 57 data with the different build suffixes.

(In reply to Chris H-C :chutten from comment #6)
> Mapping build ids to version strings is one of the stated goals of the
> Buildhub project: https://github.com/mozilla-services/buildhub/
> 
> Now, it's a service not a dataset, so it's not something you can inner join
> in re:dash. Maybe a feature request for a parquet product from buildhub
> would not be out of line?

I think that would ideal, since it might be useful for other cases as well.

Comment 9

[Alessio Placitelli [:Dexter]]

Filed https://github.com/mozilla-services/buildhub/issues/260

Comment 10

[Alessio Placitelli [:Dexter]]

:natim, adding  "target_display_version" and "previous_display_version" would ride the trains, it will definitely not make it to 57. Also, looks like the issue from comment 9 is actionable and can be used to fix the problem in a much cleaner way.

Can we close this bug?

Comment 11

[Rémy Hubscher (:natim)]

Unfortunately the solution of comment 9 is not sufficient for querying the ready state for a give version.
It is fine for me if it can ride the trains.

Comment 12

[Alessio Placitelli [:Dexter]]

Attachment: Bug 1405614 - Add 'displayVersion' to the update ping with reason 'ready'.

This is needed in addition to "version" as, for some channel, it will report
the release suffix.

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

Comment 13

[Alessio Placitelli [:Dexter]]

data-review?

Rebecca, this bug adds a new field to the "update" ping with reason "ready", the "targetDisplayVersion". This is needed because, for certain channels, the suffix information (e.g. the "Beta 1" part in "58.0 Beta 1") is not available as part of the "targetVersion" field. You can find the documentation update in the attached patch (I wasn't able to add you as a reviewer there).

Comment 14

[Chris H-C :chutten]

Comment on attachment 8917263 [details]
Bug 1405614 - Add 'displayVersion' to the update ping with reason 'ready'.

https://reviewboard.mozilla.org/r/188276/#review193576

The update manager knows the target display version? Nice.

Comment 15

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 8917263 [details]
Bug 1405614 - Add 'displayVersion' to the update ping with reason 'ready'.

https://reviewboard.mozilla.org/r/188276/#review193636

::: toolkit/components/telemetry/docs/data/update-ping.rst:21
(Diff revision 1)
>        payload: {
>          reason: <string>, // "ready", "success"
>          targetChannel: <string>, // "nightly" (only present for reason = "ready")
>          targetVersion: <string>, // "56.01a" (only present for reason = "ready")
>          targetBuildId: <string>, // "20080811053724" (only present for reason = "ready")
> +        targetDisplayVersion: <string>, // "56.01a" (only present for reason = "ready")

This looks like a typo for "56.0a1", but since this string was already here for other fields I'm not sure...

Comment 16

[Alessio Placitelli [:Dexter]]

Comment on attachment 8917263 [details]
Bug 1405614 - Add 'displayVersion' to the update ping with reason 'ready'.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/188276/diff/1-2/

Comment 17

[Alessio Placitelli [:Dexter]]

Comment on attachment 8917263 [details]
Bug 1405614 - Add 'displayVersion' to the update ping with reason 'ready'.

https://reviewboard.mozilla.org/r/188276/#review193636

> This looks like a typo for "56.0a1", but since this string was already here for other fields I'm not sure...

Ha! Good catch! Just fixed it )

Comment 18

[Alessio Placitelli [:Dexter]]

data-r? :francois, in case you can get there before Rebecca does ;) Details are in comment 13!

Comment 19

[François Marier [:francois]]

(In reply to Alessio Placitelli [:Dexter] from comment #18)
> data-r? :francois, in case you can get there before Rebecca does ;) Details
> are in comment 13!

This is Category 1 data.

datareview+

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dab164aaabfd
Add 'displayVersion' to the update ping with reason 'ready'. r=chutten,mhowell

Comment 21

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/dab164aaabfd

Comment 22

[Alessio Placitelli [:Dexter]]

The schema for this ping addition is being updated at https://github.com/mozilla-services/mozilla-pipeline-schemas/pull/93