1430115 - all_children change wasn't backwards-compatible, causes upstream headaches

Status: RESOLVED DUPLICATE of bug 1363725

(Toolkit :: Telemetry, enhancement, P1)

Description

[Chris H-C :chutten]

bug 1363725 introduced what seemed like a harmless patch that improved the grammar of working with Telemetry.

It wasn't so harmless, unfortunately, as now the modern parsing scripts will fail to parse historical .json/.yaml files, and historical parsing scripts will fail to parse modern .json/.yaml files.

We need to make this change backwards-compatible _and_ update upstream consumers (probe-scraper, moztelemetry) with the changed script.

Comment 1

[Chris H-C :chutten]

Attachment: bug 1430115 - Add back historical record_in_processes value 'all_childs'

Once again I forgot that shared_telemetry_utils is used to help parse
historical Histograms.json/Scalars.yaml/Events.yaml files in addition to modern
ones. We need to maintain backwards-compatibility.

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

Comment 2

[Alessio Placitelli [:Dexter]]

Comment on attachment 8942176 [details]
bug 1430115 - Add back historical record_in_processes value 'all_childs'

https://reviewboard.mozilla.org/r/212456/#review218176

Comment 3

[Alessio Placitelli [:Dexter]]

Comment on attachment 8942176 [details]
bug 1430115 - Add back historical record_in_processes value 'all_childs'

https://reviewboard.mozilla.org/r/212456/#review218180

::: toolkit/components/telemetry/shared_telemetry_utils.py:24
(Diff revision 1)
>      'all': 'All',
>      'all_children': 'AllChildren',
>      'main': 'Main',
>      'content': 'Content',
>      'gpu': 'Gpu',
> +    'all_childs': 'AllChildren',

Let's mark this as a "legacy" value or something, so that people reading code know

Comment 4

[Chris H-C :chutten]

Comment on attachment 8942176 [details]
bug 1430115 - Add back historical record_in_processes value 'all_childs'

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

Comment 5

[Chris H-C :chutten]

Comment on attachment 8942176 [details]
bug 1430115 - Add back historical record_in_processes value 'all_childs'

With bug 1363725's revert, this patch will no longer apply.

We need a considered plan for how to update the upstream consumers of the json+yaml files so that when we change their format, we update those consumers with backwards-compatible tools.

Comment 6

[Chris H-C :chutten]

Might be a case to update moztelemetry to the probe info service.

Comment 7

[Chris H-C :chutten]

All the work's being done in bug 1363725