Skip to content

Update to latest metrics_index list on main#551

Merged
relud merged 2 commits into
mainfrom
fog-update/update-metrics-index-main
Mar 2, 2023
Merged

Update to latest metrics_index list on main#551
relud merged 2 commits into
mainfrom
fog-update/update-metrics-index-main

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

This (automated) patch updates the list from metrics_index.py.

For reviewers:


The source code of this automation bot lives in https://github.com/mozilla/probe-scraper/tree/main/fog-updater.

@chutten
Copy link
Copy Markdown
Contributor

chutten commented Jan 30, 2023

@badboy badboy self-assigned this Feb 6, 2023
@chutten chutten force-pushed the fog-update/update-metrics-index-main branch from 4f28882 to 730e83c Compare February 21, 2023 13:52
@chutten chutten requested a review from travis79 as a code owner February 21, 2023 13:52
@chutten
Copy link
Copy Markdown
Contributor

chutten commented Feb 21, 2023

I think these should be fine but I'd appreciate one last approval from @relud before we hit the big green button. Moving and removing files shouldn't be as fraught as moving and removing deps, but it may be more complicated than moving or removing individual metrics definitions.

Comment thread repositories.yaml
- netwerk/metrics.yaml
- netwerk/protocol/http/metrics.yaml
- toolkit/components/cookiebanners/metrics.yaml
- toolkit/components/crashes/metrics.yaml
Copy link
Copy Markdown
Member

@travis79 travis79 Feb 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure what happens with apps that rely on this dependency regarding the table-generation, so hearing from @relud would make me feel a lot more comfortable about how this affects that and what problems we might encounter.

My guess is that it should work, since Desktop is still declaring this metrics.yaml file, just in the application definition instead of in this dependency, and the Android apps that consume gecko should still have these metrics definitions from the metrics.yaml file in libcrash. So it should amount to just moving things around, but I don't know if something about how this is generated for storage is affected by this or not.

Copy link
Copy Markdown
Contributor

@relud relud Feb 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, due to push mode the results of historical scans will remain in place from metrics|ping|tag files that are no longer referenced in repositories.yaml, so removing these from gecko should be fine.

But, adding these files to an application that depends on gecko would cause duplicated metric identifiers.

After doing some work to fix duplicate metric checks, I was able to produce the following email message in testing:

New Email
    From: telemetry-alerts@mozilla.com
    To: crash-reporting-wg@mozilla.org,chutten@mozilla.com,stability@mozilla.org,glean-team@mozilla.com
    Subject: Glean: Duplicate metric identifiers detected
    Body:
Glean has detected duplicated metric identifiers coming from the product 'firefox-desktop'.

- 'crash.process_type' defined more than once in firefox-desktop, gecko
- 'crash.startup' defined more than once in firefox-desktop, gecko
- 'crash.time' defined more than once in firefox-desktop, gecko
- 'crash.uptime' defined more than once in firefox-desktop, gecko

What to do about this:

1. File a bug to track your investigation. You can just copy this email into the bug Description to get you started.
2. Reply-All to this email to let the list know that you are investigating. Include the bug number so we can help out.
3. Rename the most recently added metric to be more specific. See [1]
4. Make sure a Glean team member reviews any patches. Care needs to be taken that the resolution of this problem is schema-compatible.

If you have any problems, please ask for help on the #glean Slack channel. We'll give you a hand.

What this is:

We have a system called probe-scraper [2] that scrapes the metric information from all Mozilla products using the Glean SDK. All the scraped data is available on the probeinfo service [3]. The scraped definition is used to build things such as the probe-dictionary [4] and other data tools. It detected that one metric that was recently added has an identifier collision with some metric that already existed in the application namespace. So it sent this email out, encouraging you to fix the problem.

What happens if you don't fix this:

The metrics will compete to send their data in pings, making the data unreliable at best.

You can do this!

Your Friendly, Neighborhood Glean Team

[1] - https://mozilla.github.io/glean/book/user/adding-new-metrics.html#naming-things
[2] - https://github.com/mozilla/probe-scraper
[3] - https://probeinfo.telemetry.mozilla.org/
[4] - https://telemetry.mozilla.org/probe-dictionary/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@relud Does it matter that this is firefox_desktop and gecko which don't use push-mode?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they use update mode, which is essentially push mode with daily batches, so they would still be impacted.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is basically the same issue Jan-Erik ran into with this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1815432

Copy link
Copy Markdown
Contributor

@relud relud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r-wc

Comment thread repositories.yaml
Comment thread repositories.yaml
- netwerk/metrics.yaml
- netwerk/protocol/http/metrics.yaml
- toolkit/components/cookiebanners/metrics.yaml
- toolkit/components/crashes/metrics.yaml
Copy link
Copy Markdown
Contributor

@relud relud Feb 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, due to push mode the results of historical scans will remain in place from metrics|ping|tag files that are no longer referenced in repositories.yaml, so removing these from gecko should be fine.

But, adding these files to an application that depends on gecko would cause duplicated metric identifiers.

After doing some work to fix duplicate metric checks, I was able to produce the following email message in testing:

New Email
    From: telemetry-alerts@mozilla.com
    To: crash-reporting-wg@mozilla.org,chutten@mozilla.com,stability@mozilla.org,glean-team@mozilla.com
    Subject: Glean: Duplicate metric identifiers detected
    Body:
Glean has detected duplicated metric identifiers coming from the product 'firefox-desktop'.

- 'crash.process_type' defined more than once in firefox-desktop, gecko
- 'crash.startup' defined more than once in firefox-desktop, gecko
- 'crash.time' defined more than once in firefox-desktop, gecko
- 'crash.uptime' defined more than once in firefox-desktop, gecko

What to do about this:

1. File a bug to track your investigation. You can just copy this email into the bug Description to get you started.
2. Reply-All to this email to let the list know that you are investigating. Include the bug number so we can help out.
3. Rename the most recently added metric to be more specific. See [1]
4. Make sure a Glean team member reviews any patches. Care needs to be taken that the resolution of this problem is schema-compatible.

If you have any problems, please ask for help on the #glean Slack channel. We'll give you a hand.

What this is:

We have a system called probe-scraper [2] that scrapes the metric information from all Mozilla products using the Glean SDK. All the scraped data is available on the probeinfo service [3]. The scraped definition is used to build things such as the probe-dictionary [4] and other data tools. It detected that one metric that was recently added has an identifier collision with some metric that already existed in the application namespace. So it sent this email out, encouraging you to fix the problem.

What happens if you don't fix this:

The metrics will compete to send their data in pings, making the data unreliable at best.

You can do this!

Your Friendly, Neighborhood Glean Team

[1] - https://mozilla.github.io/glean/book/user/adding-new-metrics.html#naming-things
[2] - https://github.com/mozilla/probe-scraper
[3] - https://probeinfo.telemetry.mozilla.org/
[4] - https://telemetry.mozilla.org/probe-dictionary/

@chutten chutten requested a review from relud February 21, 2023 18:06
chutten added a commit to chutten/probe-scraper that referenced this pull request Feb 27, 2023
Leave crash-related things alone (See mozilla#551).

And while we're here, remove the note about the now-defunct `pine`.
chutten added a commit that referenced this pull request Feb 27, 2023
Leave crash-related things alone (See #551).

And while we're here, remove the note about the now-defunct `pine`.
@relud
Copy link
Copy Markdown
Contributor

relud commented Mar 2, 2023

per https://bugzilla.mozilla.org/show_bug.cgi?id=1815432#c3 i'm going to merge this and run probe scraper in non-update mode to effectively move crash ping from gecko to firefox-desktop, deleting the (empty) glean crash ping tables for all other impacted apps.

@relud relud merged commit e43dab8 into main Mar 2, 2023
@relud relud deleted the fog-update/update-metrics-index-main branch March 2, 2023 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants