For #975: Adds telemetry for settings toggles #1896

sblatz · 2019-04-22T18:45:49Z

Pull Request checklist

Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
Tests: This PR includes thorough tests or an explanation of why it does not
Changelog: This PR includes a changelog entry or does not need one
Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

sblatz · 2019-04-22T18:47:43Z

Request for data collection review form

All questions are mandatory. You must receive review from a data steward peer on your responses to these questions before shipping new data collection.

What questions will you answer with this data?

How often users toggle their preference in the settings screen

Why does Mozilla need to answer these questions? Are there benefits for users? Do we need this information to address product or business requirements?

To measure how useful these features are, and how useful the ability to toggle them is

What alternative methods did you consider to answer these questions? Why were they not sufficient?

N/A (These are baseline metrics)

Can current instrumentation answer these questions?

Currently no, as these are some of the first metrics we're recording

List all proposed measurements and indicate the category of data collection for each measurement, using the Firefox data c ollection categories on the found on the Mozilla wiki.

All data is Category 2.

How long will this data be collected?

Until 04/01/2020 (renewed thereafter)

What populations will you measure?

All release, beta, and nightly users with telemetry enabled.

Please provide a general description of how you will analyze this data.

Glean / Amplitude

Where do you intend to share the results of your analysis?

Only on Glean, Amplitude and with mobile teams.

boek · 2019-04-23T21:14:58Z

app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt

        return event.wrapper != null
    }
+
+    // Switch preferences that should have their toggle events sent to Glean
+    private val switchPreferenceTelemetryAllowList = listOf(


I would move this to the PreferenceToggled type under a private failable constructor to keep all of the logic around that type together 👍

boek · 2019-04-23T21:15:10Z

app/metrics.yaml

+preferences:
+  preference_toggled:
+    type: event
+    description: >


The extras need to be documented, you can look for an example here.
https://github.com/mozilla-mobile/fenix/blob/master/app/metrics.yaml#L42

In the documentation also put all the available options

boek · 2019-04-23T21:15:42Z

app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt

+
+        preferenceManager.sharedPreferences.registerOnSharedPreferenceChangeListener { sharedPreferences, key ->
+            context!!.components.analytics.metrics.track(
+                Event.PreferenceToggled(


and then here you could "try" to create the event, and then track it if it succeeds.

sblatz · 2019-04-25T14:36:12Z

Hoping @liuche can data review this. Jeff gave it a thumbs up before he left (after I updated the extras documentation), but I wanted a data review approval documented on the PR to be sure 😄

liuche

data-review+ just one comment about values

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 with settings provided by Fenix
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

liuche · 2019-04-25T18:45:42Z

app/metrics.yaml

+      A user toggled a preference switch in settings
+    extra_keys:
+      preferenceKey:
+        description: "The preference key for the switch preference the user toggled"


Yeah, I was thinking about if this needs all possible values - it would be helpful for whomever is looking at this from a data perspective to know what the values are (on the other hand, these are values that we're including that might just get out of date). Slight preference for adding those values though!

liuche · 2019-04-25T18:46:40Z

app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt

+        context.getString(R.string.pref_key_show_search_suggestions),
+        context.getString(R.string.pref_key_show_visited_sites_bookmarks),
+        context.getString(R.string.pref_key_remote_debugging),
+        context.getString(R.string.pref_key_telemetry),


I guess technically we won't ever receive this telemetry getting toggled off, but not important enough to exclude.

Stale

colintheshots

I guess I'd put a safe call on Context where you call track ().

colintheshots · 2019-04-26T14:50:12Z

app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt

+
+        preferenceManager.sharedPreferences.registerOnSharedPreferenceChangeListener { sharedPreferences, key ->
+            try {
+                context!!.components.analytics.metrics.track(Event.PreferenceToggled


I think this is safe because onCreate comes right after onAttach, but I don't think we should crash if context is null here.

colintheshots · 2019-04-26T14:53:06Z

app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt

@@ -71,6 +74,29 @@ sealed class Event {
    object CustomTabsActionTapped : Event()
    object CustomTabsMenuOpened : Event()

+    data class PreferenceToggled(val preferenceKey: String, val enabled: Boolean, val context: Context) : Event() {
+        private val switchPreferenceTelemetryAllowList = listOf(
+            context.getString(R.string.pref_key_leakcanary),


So every single preference toggle needs to grab all of these keys from resources? I rather wish there was a way to do this only once and remember the set or list, but this will do for now.

sblatz requested a review from a team as a code owner April 22, 2019 18:45

ghost assigned sblatz Apr 22, 2019

ghost added the review label Apr 22, 2019

sblatz force-pushed the telemetry-settings branch from aecbf89 to efad4fd Compare April 22, 2019 18:46

sblatz removed the request for review from a team April 22, 2019 18:47

sblatz force-pushed the telemetry-settings branch from efad4fd to 7e0ea3a Compare April 22, 2019 18:48

sblatz requested a review from boek April 22, 2019 20:24

sblatz force-pushed the telemetry-settings branch from 7e0ea3a to 228e2e4 Compare April 23, 2019 20:05

boek previously requested changes Apr 23, 2019

View reviewed changes

sblatz force-pushed the telemetry-settings branch 2 times, most recently from d531336 to ffc533a Compare April 24, 2019 16:15

sblatz requested review from a team and liuche April 24, 2019 16:15

liuche approved these changes Apr 25, 2019

View reviewed changes

For #975: Adds telemetry for settings toggles

a391297

sblatz force-pushed the telemetry-settings branch from ffc533a to 0aa1154 Compare April 25, 2019 19:40

colintheshots approved these changes Apr 26, 2019

View reviewed changes

For #975: Uses failable constructor

6b00fc5

sblatz force-pushed the telemetry-settings branch from 0aa1154 to 6b00fc5 Compare April 26, 2019 15:36

sblatz merged commit 8928822 into mozilla-mobile:master Apr 26, 2019

ghost removed the review label Apr 26, 2019

sblatz deleted the telemetry-settings branch May 10, 2019 16:49

liuche mentioned this pull request Aug 21, 2020

For #12046 - Renew telemetry probes before migration completion #13958

Merged

4 tasks

mcarare mentioned this pull request Feb 24, 2021

For #18076: Renew telemetry expiring in April. #18143

Merged

3 tasks

eliserichards mentioned this pull request Jun 14, 2021

For #19923: renew telemetry probes #19924

Merged

3 tasks

mdboom mentioned this pull request Aug 26, 2021

For #21025: Set long term business-critical metrics as non-expiring #21038

Merged

3 tasks

eliserichards mentioned this pull request Oct 8, 2021

For #21313: Renew product telemetry expiring in December #21316

Merged

6 tasks

gabrielluong mentioned this pull request Jul 20, 2022

For #26121 - Renew telemetry expiring in 105 #26123

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

For #975: Adds telemetry for settings toggles #1896

For #975: Adds telemetry for settings toggles #1896

sblatz commented Apr 22, 2019 •

edited

Loading

sblatz commented Apr 22, 2019 •

edited

Loading

boek Apr 23, 2019

boek Apr 23, 2019

boek Apr 23, 2019

sblatz commented Apr 25, 2019

liuche left a comment

liuche Apr 25, 2019

liuche Apr 25, 2019

colintheshots left a comment

colintheshots Apr 26, 2019

colintheshots Apr 26, 2019

For #975: Adds telemetry for settings toggles #1896

For #975: Adds telemetry for settings toggles #1896

Conversation

sblatz commented Apr 22, 2019 • edited Loading

Pull Request checklist

sblatz commented Apr 22, 2019 • edited Loading

Request for data collection review form

boek Apr 23, 2019

Choose a reason for hiding this comment

boek Apr 23, 2019

Choose a reason for hiding this comment

boek Apr 23, 2019

Choose a reason for hiding this comment

sblatz commented Apr 25, 2019

liuche left a comment

Choose a reason for hiding this comment

Data Review Form (to be filled by Data Stewards)

liuche Apr 25, 2019

Choose a reason for hiding this comment

liuche Apr 25, 2019

Choose a reason for hiding this comment

colintheshots left a comment

Choose a reason for hiding this comment

colintheshots Apr 26, 2019

Choose a reason for hiding this comment

colintheshots Apr 26, 2019

Choose a reason for hiding this comment

sblatz commented Apr 22, 2019 •

edited

Loading

sblatz commented Apr 22, 2019 •

edited

Loading