-
Notifications
You must be signed in to change notification settings - Fork 97
Fix #660: Add the rate metric type #661
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Pulling down generated-schemas locally, I was able to confirm no rates or labeled_rates exist in the wild: Compare this to searching for "counter": |
fbertsch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, we are actually generating rate and labeled_rate cols if they appear in probe-info-service. However, I have confirmed that no apps have ever reported a rate or labeled_rate type, so we should be good to go.
Aside, are we implementing labeled_rate?
No, there are a bunch of |
c2e18f4 2021-03-23 18:04:10 +0000 Bug 1697602 Remove AET schemas (#664) 5f21300 2021-03-17 19:04:16 +0000 Bug 1677567 - Add rallyId to pioneer metadata schemas (#665) 731721a 2021-03-09 10:03:38 -0800 Bug 1696074 - Add schema for uninstall-deletion ping in pioneer-core (#663) 793fc04 2021-03-08 09:55:50 -0500 Bug 1695795: Remove unused metric types from the Glean schema (#662) 04043f1 2021-03-02 08:30:04 -0500 Fix #660: Add the rate metric type (#661) 0a24af0 2021-02-24 10:43:58 -0500 [Bug 1694254] Add environment.settings.attribution.dltoken field c2dae92 2021-02-22 18:30:19 +0000 Bug 1693305 - Clone citp-news-disinfo study (#658) f3a1464 2021-02-17 16:05:26 -0600 Bug 1688698 - Add 'source' to TopSites schemas in Contextual Services (#657) 5904f8c 2021-02-12 16:29:41 -0600 Bug 1688698 - Add initial schemas for Contextual Services (#656)
) * Fix mozilla-services#660: Add the rate metric type * Fix validation tests * Add minimum * Update labeled_rate
Technically this is a backward-incompatible change. However, there is nothing in the wild sending a
ratemetric type yet, so I believe this is ok (and should be even when we get to the schema generator part of this). But please check my assumption, @frank and @acmiyaguchi .Checklist for reviewer:
.circleci/config.yml) will cause environment variables (particularly credentials) to be exposed in test logsintegrationCI test by pushing this revision as discussed in the README and review the report posted in the comments.For glean changes:
templates/include/glean/CHANGELOG.md