Skip to content

Conversation

@mdboom
Copy link
Contributor

@mdboom mdboom commented Mar 1, 2021

Technically this is a backward-incompatible change. However, there is nothing in the wild sending a rate metric 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:

For glean changes:

  • Update templates/include/glean/CHANGELOG.md

@auto-assign auto-assign bot requested a review from mreid-moz March 1, 2021 21:14
@jklukas
Copy link
Contributor

jklukas commented Mar 1, 2021

Pulling down generated-schemas locally, I was able to confirm no rates or labeled_rates exist in the wild:

› find schemas -name "*.bq" | xargs grep '"rate"' | wc
       0       0       0

› find schemas -name "*.bq" | xargs grep '"labeled_rate"' | wc
       0       0       0

Compare this to searching for "counter":

› find schemas -name "*.bq" | xargs grep '"counter"' | wc
      76     228    6251

@jklukas jklukas requested review from acmiyaguchi and fbertsch and removed request for mreid-moz March 1, 2021 21:21
Copy link
Contributor

@fbertsch fbertsch left a 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?

@mdboom
Copy link
Contributor Author

mdboom commented Mar 2, 2021

Aside, are we implementing labeled_rate?

No, there are a bunch of labeled_X metric types from the original design doc that have been in here from the beginning that aren't implemented. They should probably be removed, but maybe as a follow-on bug?

@mdboom mdboom merged commit 04043f1 into mozilla-services:master Mar 2, 2021
dataops-pipeline-schemas added a commit that referenced this pull request Mar 8, 2021
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
acmiyaguchi pushed a commit that referenced this pull request Mar 23, 2021
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)
bytesized pushed a commit to bytesized/mozilla-pipeline-schemas that referenced this pull request Aug 23, 2022
)

* Fix mozilla-services#660: Add the rate metric type

* Fix validation tests

* Add minimum

* Update labeled_rate
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