1568589 - Add GeckoView metrics to the 'metrics.yaml' file in mozilla-central

Status: RESOLVED WONTFIX

(Data Platform and Tools :: Glean: SDK, task, P3)

Description

[Alessio Placitelli [:Dexter]]

This spreadsheet defines a set of histograms that need to be added to the metrics.yaml.

Documentation about the format of this file is available here.

Comment 1

[Chris Peterson [:cpeterson]]

Adding the [geckoview:fenix:m8] whiteboard tag because the GV team would like to start reporting Gecko metrics from Fenix in Q3.

Comment 2

[Alessio Placitelli [:Dexter]]

Hey Chris,

I think we should be ready to add the first set of metrics to the metrics.yaml file (in addition to bug 1566353). This should be done by the parties requesting the data-review (assuming that's needed). Do you have an idea of who should do this?

Comment 3

[Chris Peterson [:cpeterson]]

(In reply to Alessio Placitelli [:Dexter] - PTO until 26/July from comment #2)

I think we should be ready to add the first set of metrics to the metrics.yaml file (in addition to bug 1566353). This should be done by the parties requesting the data-review (assuming that's needed). Do you have an idea of who should do this?

Probably the engineers who selected the [initial metrics]https://docs.google.com/spreadsheets/d/1EZGVGmybvF1sH-XrsXMlplK7fWQfDniIeqXrkfDQrlc/edit?pli=1#gid=0) to be reported: Kats for graphics and bdekoz for perf.

Comment 4

[Chris Peterson [:cpeterson]]

Jessie: since Kats is on paternity leave, is there another graphics engineer that is familiar with the graphics telemetry probes that could add them to the new Glean metrics.yaml file?

Comment 5

[Alessio Placitelli [:Dexter]]

For the record, we have docs for adding telemetry ;)

Comment 6

[Benjamin De Kosnik [:bdekoz]]

Hey, I am adding the pef probes. Thanks for the docs Alessio.

Since a lot of the GFX and Perf Probes are the same, I can add any shared GFX/Perf probes in the first pass.

Comment 8

[Jamie Nicol [:jnicol]]

I'm working on adding the gfx probes. Think I'm nearly done but I have a few questions:

• Is there a way I can test my changes? Even just a lint to make sure I haven't made a typo or am missing a required field?
• How do I convert a gecko release number to a date for the expires field?
• What do I put for data_reviews field?
• Some of the probes in Histograms.json don't have a bug number, what do I put for their bugs field?
• I'm using custom_distribution quite a lot, but the docs say "Note: Custom distributions are currently only allowed for GeckoView metrics (the gecko_datapoint parameter is present)." What does this mean?

And what is the preferred system for naming the probes?
The checkerboard example in the docs has gfx.content.checkerboard: duration, which makes sense. But I'd like to put some (eg CONTENT_FRAME_TIME) directly under gfx.content, and COMPOSITE_TIME would be simply gfx: composite_time? So should I put gfx:, gfx.content:, and gfx.content.checkerboard: all as top level items? Or am I mean to nest them under eachother somehow?

Comment 9

[Alessio Placitelli [:Dexter]]

(In reply to Jamie Nicol [:jnicol] from comment #8)

I'm working on adding the gfx probes. Think I'm nearly done but I have a few questions:

Great! Thanks for doing that! Is there a bug for that already filed?

• Is there a way I can test my changes? Even just a lint to make sure I haven't made a typo or am missing a required field?

This is a bit quirky. There's two things you could do:

1. For linting, you could use the glean_parser, i.e. glean_parser translate -o output_dir -f kotlin metrics.yaml. It will attempt to generate Kotlin code and error out/provide tips if something is fishy in the metrics.yaml file. We know that's quirky, we filed bug 1566355 for improving this.
2. For testing this in Fenix, is a bit more complicated. You should build Gecko, then use dependency substitution in GV for making engine-gecko pick it up, then publish locally, then make Fenix pick it up.

I would recommend going with one metric first, to get acquainted. Please do flag me for review (even if you're only drafting this right now), I'm happy to help and mentor this first batch of metrics! :)

• How do I convert a gecko release number to a date for the expires field?

Glean SDK only understands dates. I'm afraid you'll need to do the conversion manually by looking at our release calendar.

• What do I put for data_reviews field?

In the bug that you're using for developing this, you should request a data collection review, as stated in our docs. You can add a link to the data review response in that field.

• Some of the probes in Histograms.json don't have a bug number, what do I put for their bugs field?

The new bug that you have filed for adding these metrics is ok.

• I'm using custom_distribution quite a lot, but the docs say "Note: Custom distributions are currently only allowed for GeckoView metrics (the gecko_datapoint parameter is present)." What does this mean?

The Glean SDK is used outside of GeckoView as well: this states that the CustomDistribution metric type is only available for exfiltrating Gecko metrics. It can't be used by other products using the Glean SDK.

And what is the preferred system for naming the probes?

Great question! We have some guidelines here.

The checkerboard example in the docs has gfx.content.checkerboard: duration, which makes sense. But I'd like to put some (eg CONTENT_FRAME_TIME) directly under gfx.content, and COMPOSITE_TIME would be simply gfx: composite_time? So should I put gfx:, gfx.content:, and gfx.content.checkerboard: all as top level items? Or am I mean to nest them under eachother somehow?

To your specific example, I think the layout you suggest would make sense.

gfx.content.checkerboard:
  duration:
    ...

gfx.content:
  frame_time:
    ...

gfx:
  composite_time:
    ...

In general, we are trying to encourage meaningful groupings to help navigate the yaml files. Nesting under each other is not supported. I'd advise to pick the most specific category name that makes sense in your case, as you just did! Favour category names that allow for logical grouping of metrics. The thing we'd like to discourage is:

histograms:
  content_frame_time:
    ...

  composite_time:
    ...

Comment 10

[Jamie Nicol [:jnicol]]

Thanks Alessio, that's very helpful!

Great! Thanks for doing that! Is there a bug for that already filed?

Nope. To clarify I'm referering to the graphics-related probes in the spreadsheet in comment 0. Would you like me to file a bug (blocking this one) for those specifically?

Comment 11

[Alessio Placitelli [:Dexter]]

(In reply to Jamie Nicol [:jnicol] from comment #10)

Thanks Alessio, that's very helpful!

Great! Thanks for doing that! Is there a bug for that already filed?

Nope. To clarify I'm referering to the graphics-related probes in the spreadsheet in comment 0. Would you like me to file a bug (blocking this one) for those specifically?

Yes please! I think that's required for the data-review anyway :)

Comment 12

[Alessio Placitelli [:Dexter]]

Closing this as adding metrics is being tracked in other bugs and being owned by other teams.