1746941 - Add tags for all glean metrics in Firefox

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement)

Description

[William Lachance (:wlach)]

This bug tracks adding tags to all Glean metrics in Firefox for Desktop, paving the way for bugzilla tag information to appear in the Glean Dictionary (and elsewhere).

Initially I thought we would need to implement bug 1745283 first, but I've come to think maybe a naive implementation might be a good thing to get started with.

Comment 1

[William Lachance (:wlach)]

Attachment: Bug 1746941 - Add tags to all Firefox-on-Glean metrics r?janerik!

Based on our experience with Firefox for Android, annotating Glean metrics
with issue tracker component information can provide valuable context to
anyone searching for metrics.

This adds a new set of tags corresponding to the components in the
tree, annotates the existing Glean metrics. Finally, it also adds a new
mach command called update-glean-tags to update the tags files based
on build metadata.

Comment 2

[William Lachance (:wlach)]

This seems to be working well on try: https://treeherder.mozilla.org/jobs?repo=try&revision=0047a0708ff6ded982c07281386bc7f08763c2d8

Will wait until the new year to ask for review.

Comment 3

[William Lachance (:wlach)]

Comment on attachment 9256246 [details]
Bug 1746941 - Add tags to all Firefox-on-Glean metrics r?janerik!

I'm not sure when I'm going to get re-setup to modify this, but figured I may as well tee up a review.

Comment 4

[Chris H-C :chutten]

Comment on attachment 9256246 [details]
Bug 1746941 - Add tags to all Firefox-on-Glean metrics r?janerik!

Huh, weird, the review flag stuck around here on BMO.

Comment 5

[Nick Alexander :nalexander [he/him]]

How does this accommodate BZ components that go away, or are renamed? Is the new tag strictly informational?

Comment 6

[Chris H-C :chutten]

(In reply to Nick Alexander :nalexander [he/him] from comment #5)

How does this accommodate BZ components that go away, or are renamed? Is the new tag strictly informational?

It is strictly informational/organizational. You'll see it on things like Glean Dictionary's search results to help you find out that this count of bookmarks is the count sync'd over sync, and that count of bookmarks is the number stored in places, and that count of bookmarks is the number displayed in the UI.

(...maybe. Though I really do hope they'll be named sufficiently descriptively that the product+component is just a hint for scanning like how German capitalizes nouns)

(( And if the component goes away, presumably so too does the code and its instrumentation? ))

Comment 7

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/probe-scraper/pull/393

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c0de94959204
Add tags to all Firefox-on-Glean metrics r=janerik

Comment 9

[Sandor Molnar[:smolnar]]

Backed out for causing python unit test failure.

Backout link:https://hg.mozilla.org/integration/autoland/rev/efe2ad36a5176703efb3a68d84be7b1fb7d017e3

Push with failures

Failure log

 toolkit/components/glean/tests/pytest/test_no_expired_metrics.py::test_no_metrics_expired TEST-UNEXPECTED-FAIL
[task 2022-01-14T16:50:24.713Z]  0:10.16 
[task 2022-01-14T16:50:24.713Z]  0:10.16 =================================== FAILURES ===================================
[task 2022-01-14T16:50:24.713Z]  0:10.16 ___________________________ test_no_metrics_expired ____________________________
[task 2022-01-14T16:50:24.713Z]  0:10.16 
[task 2022-01-14T16:50:24.713Z]  0:10.16     def test_no_metrics_expired():
[task 2022-01-14T16:50:24.713Z]  0:10.16         """
[task 2022-01-14T16:50:24.713Z]  0:10.16         Of all the metrics included in this build, are any expired?
[task 2022-01-14T16:50:24.713Z]  0:10.16         If so, they must be removed or renewed.
[task 2022-01-14T16:50:24.713Z]  0:10.16 
[task 2022-01-14T16:50:24.713Z]  0:10.16         (This also checks other lints, as a treat.)
[task 2022-01-14T16:50:24.713Z]  0:10.16         """
[task 2022-01-14T16:50:24.714Z]  0:10.16         with open("browser/config/version.txt", "r") as version_file:
[task 2022-01-14T16:50:24.714Z]  0:10.16             app_version = version_file.read().strip()
[task 2022-01-14T16:50:24.714Z]  0:10.16 
[task 2022-01-14T16:50:24.714Z]  0:10.16         options = run_glean_parser.get_parser_options(app_version)
[task 2022-01-14T16:50:24.714Z]  0:10.16         metrics_paths = [Path(x) for x in metrics_yamls]
[task 2022-01-14T16:50:24.714Z]  0:10.16         all_objs = parser.parse_objects(metrics_paths, options)
[task 2022-01-14T16:50:24.714Z]  0:10.16         assert not util.report_validation_errors(all_objs)
[task 2022-01-14T16:50:24.714Z]  0:10.16 >       assert not lint.lint_metrics(all_objs.value, options)
[task 2022-01-14T16:50:24.714Z]  0:10.16 E       AssertionError: assert not [<glean_parser.lint.GlinterNit object at 0x7f91aefae710>, <glean_parser.lint.GlinterNit object at 0x7f91aefae6a0>, <gl... <glean_parser.lint.GlinterNit object at 0x7f91aefaef28>, <glean_parser.lint.GlinterNit object at 0x7f91aefae9e8>, ...]
[task 2022-01-14T16:50:24.714Z]  0:10.16 E        +  where [<glean_parser.lint.GlinterNit object at 0x7f91aefae710>, <glean_parser.lint.GlinterNit object at 0x7f91aefae6a0>, <gl... <glean_parser.lint.GlinterNit object at 0x7f91aefaef28>, <glean_parser.lint.GlinterNit object at 0x7f91aefae9e8>, ...] = <function lint_metrics at 0x7f91af379378>(DictWrapper([('fog', DictWrapper([('initialization', <glean_parser.metrics.Timespan object at 0x7f91aef9a710>), ('fail...metrics.Counter object at 0x7f91aefae5f8>), ('uri_count', <glean_parser.metrics.Counter object at 0x7f91aefaec50>)]))]), {'allow_reserved': False, 'custom_is_expired': <function get_parser_options.<locals>.<lambda> at 0x7f91aefc47b8>, 'custom_validate_expires': <function get_parser_options.<locals>.<lambda> at 0x7f91aefc4950>})
[task 2022-01-14T16:50:24.714Z]  0:10.16 E        +    where <function lint_metrics at 0x7f91af379378> = lint.lint_metrics
[task 2022-01-14T16:50:24.714Z]  0:10.16 E        +    and   DictWrapper([('fog', DictWrapper([('initialization', <glean_parser.metrics.Timespan object at 0x7f91aef9a710>), ('fail...metrics.Counter object at 0x7f91aefae5f8>), ('uri_count', <glean_parser.metrics.Counter object at 0x7f91aefaec50>)]))]) = <glean_parser.util.keep_value.<locals>.ValueKeepingGenerator object at 0x7f91aef9af60>.value
[task 2022-01-14T16:50:24.714Z]  0:10.16

Comment 10

[Chris H-C :chutten]

Whoops. Forgot that test_no_metrics_expired doesn't use the same "give me all the yamls" logic as mozbuild. Should've caught that when I was reviewing wlach's original patch.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a056148a890a
Add tags to all Firefox-on-Glean metrics r=janerik

Comment 12

[Marian-Vasile Laza]

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