For #11118: Add missing telemetry #11446

sblatz · 2020-06-10T20:03:24Z

This is mostly an extension of #11211, just tying up a couple loose ends on data we missed that's important. Also we now have more confidence that this format works, so we're adding a couple more.

Pull Request checklist

Tests: This PR includes thorough tests or an explanation of why it does not
Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

After merge

Milestone: Make sure issues finished by this pull request are added to the milestone of the version currently in development.

To download an APK when reviewing a PR:

click on Show All Checks,
click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
click on the "Fenix - assemble" task, then click "Run Artifacts".
the APK links should be on the left side of the screen, named for each CPU architecture

sblatz · 2020-06-10T20:06:01Z

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?

What settings preferences do users have?
What % of our users have open_links_in_app enabled?
What % of our users have chosen each theme option?

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

It is crucial we know what types of user segments we have so we can prioritize certain features. It is also important for helping us drive decisions around resourcing. This data will help us better slice our user population & figure out retention metrics.

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

We currently have preferenceToggled events for most of these. Unfortunately these queries are incredibly expensive to run and can often be error prone. Because of this, we are approaching it with a simpler metrics ping.

Can current instrumentation answer these questions?

No, see above.

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 09/01/2020

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.

codecov-commenter · 2020-06-10T20:17:12Z

Codecov Report

Merging #11446 into master will increase coverage by 0.03%.
The diff coverage is 53.19%.

@@             Coverage Diff              @@
##             master   #11446      +/-   ##
============================================
+ Coverage     20.86%   20.90%   +0.03%     
- Complexity      678      680       +2     
============================================
  Files           372      372              
  Lines         14861    14884      +23     
  Branches       2000     2006       +6     
============================================
+ Hits           3101     3111      +10     
- Misses        11478    11484       +6     
- Partials        282      289       +7

Impacted Files	Coverage Δ	Complexity Δ
...la/fenix/components/metrics/GleanMetricsService.kt	`14.42% <53.19%> (+1.48%)`	`4.00 <1.00> (+1.00)`
.../src/main/java/org/mozilla/fenix/utils/Settings.kt	`68.60% <0.00%> (ø)`	`33.00% <0.00%> (+1.00%)`
...ug/java/org/mozilla/fenix/DebugFenixApplication.kt	`0.00% <0.00%> (ø)`	`0.00% <0.00%> (ø%)`
...g/mozilla/fenix/crashes/CrashReporterController.kt	`95.45% <0.00%> (+0.21%)`	`6.00% <0.00%> (ø%)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31edbc9...9e462ac. Read the comment docs.

liuche

A few questions about the documentation here, but otherwise looks fine. Just want to double check that all the new things have been documented.

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 in a public, complete, and accurate way?

Yes, 2 new items documented, r+ pending the other items that look new?

Is there a control mechanism that allows the user to turn the data collection on and off?

Yes, data collection controlled by Fenix data controls

If the request is for permanent data collection, is there someone who will monitor the data over time?

Expiry set

Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Type 2, user interaction w/ settings

Is the data collection request for default-on or default-off?

Default on

Does the instrumentation include the addition of any new identifiers

No, user interactions only

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 · 2020-06-11T17:52:12Z

app/metrics.yaml

+  open_links_in_app:
+    type: string_list
+    description: >
+      Whether or not the user has the open links in apps feature enabled.


Why not a boolean? If there are other options, please list all of them.

Okay, I see why now - can you list both true/false as the possible values, that will make it a little more clear. (Optionally explain why this needs to be string - it's in-code, but not here. OTOH if someone is analyzing this data, they might not care about why boolean vs string).

Okay pt 3, I see we don't do this for other booleans either. I think this should be pretty clear, so just ignore me 😂

liuche · 2020-06-11T17:54:32Z

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

+        installationPing.checkAndSend()
+    }
+
+    private fun setPreferenceMetrics() {
        // We purposefully make all of our preferences the string_list format to make data analysis


Ah, I see why this isn't a boolean now.

liuche · 2020-06-11T18:03:43Z

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

+                    "top"
+                }
+
+            toolbarPosition.set(listOf(toolbarPositionSelection))


Is this a new addition? If so, please document

Nope! Part of the initial telemetry I forgot.

liuche · 2020-06-11T18:05:03Z

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

@@ -603,6 +649,8 @@ class GleanMetricsService(private val context: Context) : MetricsService {
            showSearchShortcuts.set(context.settings().shouldShowSearchShortcuts.toStringList())
            openLinksInAPrivateTab.set(context.settings().openLinksInAPrivateTab.toStringList())
            searchSuggestionsPrivate.set(context.settings().shouldShowSearchSuggestionsInPrivate.toStringList())
+            showVoiceSearch.set(context.settings().shouldShowVoiceSearch.toStringList())


Are these new additions to the pref-toggle-event or prefs-metrics ping? If so please document. Or was it one of the missing pings?

Nope! Part of the initial telemetry I forgot.

sblatz force-pushed the add-ALL-the-telemetry branch from d87c092 to 6094f91 Compare June 10, 2020 20:03

sblatz added the needs:data-review PR is awaiting a data review label Jun 10, 2020

sblatz force-pushed the add-ALL-the-telemetry branch from 6094f91 to e462615 Compare June 10, 2020 20:04

sblatz requested a review from liuche June 10, 2020 20:06

sblatz force-pushed the add-ALL-the-telemetry branch from e462615 to 1759958 Compare June 10, 2020 20:18

For mozilla-mobile#11118: Add missing telemetry

9e462ac

sblatz force-pushed the add-ALL-the-telemetry branch from 1759958 to 9e462ac Compare June 10, 2020 20:18

ekager mentioned this pull request Jun 11, 2020

For #10173: login duplicates and save #11208

Merged

4 tasks

liuche approved these changes Jun 11, 2020

View reviewed changes

liuche removed the needs:data-review PR is awaiting a data review label Jun 11, 2020

sblatz merged commit d1cd877 into mozilla-mobile:master Jun 12, 2020

Dexterp37 mentioned this pull request Jun 30, 2020

[Telemetry] string_list metric type is being misused #12131

Closed

liuche mentioned this pull request Oct 6, 2020

Issue #14142 - Telemetry renewal to 08-01-2021 #15713

Merged

3 tasks

eliserichards mentioned this pull request Jul 26, 2021

For #20516: Renew product telemetry probes expiring in August #20517

Merged

3 tasks

eliserichards mentioned this pull request Jan 28, 2022

For #22870: Renew/remove metrics set to expire in February #23453

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

For #11118: Add missing telemetry #11446

For #11118: Add missing telemetry #11446

sblatz commented Jun 10, 2020 •

edited

Loading

sblatz commented Jun 10, 2020

codecov-commenter commented Jun 10, 2020 •

edited

Loading

liuche left a comment

liuche Jun 11, 2020

liuche Jun 11, 2020

liuche Jun 11, 2020

liuche Jun 11, 2020

liuche Jun 11, 2020

sblatz Jun 12, 2020

liuche Jun 11, 2020

sblatz Jun 12, 2020

For #11118: Add missing telemetry #11446

For #11118: Add missing telemetry #11446

Conversation

sblatz commented Jun 10, 2020 • edited Loading

Pull Request checklist

After merge

To download an APK when reviewing a PR:

sblatz commented Jun 10, 2020

Request for data collection review form

codecov-commenter commented Jun 10, 2020 • edited Loading

Codecov Report

liuche left a comment

Choose a reason for hiding this comment

Data Review Form (to be filled by Data Stewards)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sblatz commented Jun 10, 2020 •

edited

Loading

codecov-commenter commented Jun 10, 2020 •

edited

Loading