-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Issue #18711: Telemetry for credit card autofill #20909
Conversation
750611b
to
dbff02e
Compare
This pull request has conflicts when rebasing. Could you fix it @rocketsroger? 🙏 |
Request for data collection review formAll questions are mandatory. You must receive review from a data steward peer on your responses to these questions before shipping new data collection.
|
Looping in @Dexterp37 for another set of Glean eyes. As I guided Roger through this it might be good to have someone not familiar yet review this briefly. |
app/metrics.yaml
Outdated
send_in_pings: | ||
- metrics |
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.
Is this intentional? Events are usually sent in the events ping. Note that you can also leave this property out for Glean to send the data automatically in the correct ping.
app/metrics.yaml
Outdated
send_in_pings: | ||
- metrics |
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.
As above: this applies to all the other event metrics added to this file.
app/metrics.yaml
Outdated
send_in_pings: | ||
- metrics |
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.
Note that this property is not required, unless you specify a custom ping. This applies to the other definitions as well.
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.
Understood. I'll remove the send_in_pings
property. Thanks!
This pull request has conflicts when rebasing. Could you fix it @rocketsroger? 🙏 |
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.
I am wondering how we decided which of these are metrics counters vs events? Is there a link to a list somewhere where I can double check?
app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt
Show resolved
Hide resolved
app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt
Outdated
Show resolved
Hide resolved
Kimberly provided the initial specs, see this spreadsheet, Roger reviewed that and I also gave it a look. |
118ddf7
to
b4ba766
Compare
Data Review
Yes, through the metrics.yaml file and the Glean Dictionary
Yes, through the "Send Usage Data" preference in the application settings
N/A, collection set to end or be renewed by 2022-09-01
Category 2, Interaction data
default-on
No
Yes
No Resultdata-review+ |
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.
Awesome! 👏
For #18711
Pull Request checklist
To download an APK when reviewing a PR: