-
Notifications
You must be signed in to change notification settings - Fork 1.3k
For #1190: Adds telemetry for FxA login #2745
Conversation
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.
Until 04/01/2020 (renewed thereafter)
|
3b8a67a
to
ca63bbd
Compare
@@ -187,6 +187,7 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse | |||
|
|||
private fun getClickListenerForSignIn(): OnPreferenceClickListener { | |||
return OnPreferenceClickListener { | |||
requireComponents.analytics.metrics.track(Event.SyncOpened) |
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.
You're tracking this metric twice
app/metrics.yaml
Outdated
@@ -732,4 +744,83 @@ error_page: | |||
- https://github.com/mozilla-mobile/fenix/pull/2491#issuecomment-492414486 | |||
notification_emails: | |||
- fenix-core@mozilla.com | |||
expires: "2020-03-01" | |||
|
|||
sync: |
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 think we'll want to split this up into a account.auth flow and an account.settings flow. because
opened:
type: event
description: >
A user opened the sync page
This event is really means opened the page to start the sign in flow to firefox account.
@@ -71,6 +76,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat(), CoroutineScope { | |||
|
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.
Do we also want telemetry when a user lands on this Fragment?
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.
A couple of small nits about the structure. + Missing an event
ca63bbd
to
4191f5b
Compare
Resolved your concerns, @boek! |
4191f5b
to
33d9254
Compare
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.
Data Review Form (to be filled by Data Stewards)
-
Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?
Yes, in metrics.yaml -
Is there a control mechanism that allows the user to turn the data collection on and off? (Note, for data collection not needed for security purposes, Mozilla provides such a control mechanism) Provide details as to the control mechanism available.
Yes, Fenix data controls -
If the request is for permanent data collection, is there someone who will monitor the data over time?**
No, @vesta0 will monitor and it has expiry -
Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under? **
Type 2, interaction sync authentication / account UI -
Is the data collection request for default-on or default-off?
Default on -
Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)?
No -
Is the data collection covered by the existing Firefox privacy notice?
Yes -
Does there need to be a check-in in the future to determine whether to renew the data? (Yes/No) (If yes, set a todo reminder or file a bug if appropriate)**
No, has expiry -
Does the data collection use a third-party collection tool? If yes, escalate to legal.
No
33d9254
to
83e30d9
Compare
83e30d9
to
dfd5bf1
Compare
Pull Request checklist