Closed Bug 1568485 Opened 5 months ago Closed 4 months ago

Add geckoView metrics.yaml to the probe-scraper

Categories

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

task

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Dexter, Assigned: mdroettboom)

References

(Blocks 1 open bug)

Details

(Whiteboard: [telemetry:glean-rs:m7] [geckoview:fenix:m8])

Attachments

(1 file)

Blocks: 1566340
Priority: -- → P3
Whiteboard: [telemetry:glean-rs:m7][geckoview]
Blocks: 1566376
Whiteboard: [telemetry:glean-rs:m7][geckoview] → [telemetry:glean-rs:m7] [geckoview:fenix:m8]
Assignee: nobody → alessio.placitelli
Priority: P3 → P1
Assignee: alessio.placitelli → nobody
Priority: P1 → P3

Hey Mike, any chance you could look into this/have the bandwidth to do so? From an initial look, this looks a bit more involved than just adding the URL to the file there.

We really need to add 3 mappings:

  • a-c's engine-gecko-nightly to https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/geckoview/streaming/metrics.yaml
  • a-c's engine-gecko-beta to https://hg.mozilla.org/releases/mozilla-beta/file/tip/toolkit/components/telemetry/geckoview/metrics.yaml (the file is not there yet, the file will get uplifted to Beta on the 2nd of September as per http://whattrainisitnow.com/
  • a-c's engine-gecko (this is release) to https://hg.mozilla.org/releases/mozilla-release/file/tip/toolkit/components/telemetry/geckoview/streaming/metrics.yaml (the file is not there yet)

Both people from the GV team (Eugen?) and the Firefox Telemetry (Chutten?) should be added to the notifications there, in addition to a-c and us.

The tricky part, that might not be relevant given the way probe-scraper currently work, is that we can't tell the mercurial commit hash from the engine-gecko-* git commit hash.

Flags: needinfo?(mdroettboom)

So probe_scraper only needs to get "every metric defined for an app anywhere for all time", not the exact set of metrics for an exact version of an app. So that should mean that we don't need to have a mapping from a-c's engine-gecko git commit hash to m-c's Mercurial hash.

Along with that idea, I don't think we need to take care about which of the branches (nightly, beta or release) Fenix is built with to determine where to look up the metrics. One might argue that we always want to look up in nightly's metrics.yaml for all channels -- that would at least be consistent with how things are done for a-c components now. That probably warrants a little discussion of course, as I only have a very high-level picture of how that works.

An additional wrinkle: I believe probe_scraper only supports git repositories -- it actually walks through the history of the metrics.yaml files to find all metrics ever. It may be possible to use a mercurial-to-git bridge for this, rather than writing a new mercurial backend. I'm researching that now...

Assignee: nobody → mdroettboom
Flags: needinfo?(mdroettboom)
Flags: needinfo?(fbertsch)

(In reply to Michael Droettboom [:mdroettboom] from comment #2)

So probe_scraper only needs to get "every metric defined for an app anywhere for all time", not the exact set of metrics for an exact version of an app. So that should mean that we don't need to have a mapping from a-c's engine-gecko git commit hash to m-c's Mercurial hash.

Ok, that makes me feel a bit better about this.

Along with that idea, I don't think we need to take care about which of the branches (nightly, beta or release) Fenix is built with to determine where to look up the metrics. One might argue that we always want to look up in nightly's metrics.yaml for all channels -- that would at least be consistent with how things are done for a-c components now. That probably warrants a little discussion of course, as I only have a very high-level picture of how that works.

This is a bit tricky: some Gecko metrics, in general, might live in Nightly only or be removed before they reach Release. a-c's engine-gecko-* components thankfully map to different Gecko channels, so we should be safe by defining 3 different repos.

An additional wrinkle: I believe probe_scraper only supports git repositories -- it actually walks through the history of the metrics.yaml files to find all metrics ever. It may be possible to use a mercurial-to-git bridge for this, rather than writing a new mercurial backend. I'm researching that now...

Maybe that's not even needed: Mozilla has a git read-only mirror of mozilla-central, here: https://github.com/mozilla/gecko-dev - the master branch is nightly, beta is beta and release is release.

Perfect. In summary, then, I think nothing too out-of-the-ordinary here.

I'm following along slowly here, but since we can never remove metrics, might we only use nightly since that should contain all probes?

Flags: needinfo?(fbertsch)

:frank: I had the same question, but in a side discussion with :Dexter, it was noted that metrics often land in nightly but then are removed before they hit beta, and it would be nice to not clutter beta and release with unnecessary metrics. It's not significant additional complexity to support different dependencies here, so maybe that's fine...?

Attached file GitHub Pull Request

(In reply to Frank Bertsch [:frank] from comment #5)

I'm following along slowly here, but since we can never remove metrics, might we only use nightly since that should contain all probes?

Hey Frank, what do you mean by "we can never remove metrics"?

When writing Gecko metrics, it can happen that metrics get removed before reaching release or other channels.

Flags: needinfo?(fbertsch)

(In reply to Alessio Placitelli [:Dexter] from comment #8)

Hey Frank, what do you mean by "we can never remove metrics"?
When writing Gecko metrics, it can happen that metrics get removed before reaching release or other channels.

Okay, I take back the "can never remove metrics", the probe-scraper handles that. You all are fine to continue on and add/remove metrics at will, but please don't change their types!

Flags: needinfo?(fbertsch)
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Blocks: 1573475
You need to log in before you can comment on or make changes to this bug.