1058813 - Add telemetry for custom preferences

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Settings and Preferences, defect)

Description

[:Margaret Leibovic]

Follow-up to bug 996753. That bug only adds probes for toggling built-in android preference widgets, but we'll need to add our own logic to add probes for custom widgets.

Some of this already exists for things like home panels and search engines, but we should probe all the things!

Comment 1

[:Margaret Leibovic]

Auditing our preferences and telemetry probes, I found that the only things not being caught by our action/settings and edit/settings probes are the sync preference, home panel management, and search engine management.

Since we already have existing probes for home panels/search engines, it looks like we just need to add a probe to SyncPreference here and we'll have full telemetry coverage in settings!

Comment 2

[:Margaret Leibovic]

Attachment: Add telemetry probe for clicking sync preference

Comment 3

[Chenxia Liu [:liuche]]

Comment on attachment 8484648 [details] [diff] [review]
Add telemetry probe for clicking sync preference

Review of attachment 8484648 [details] [diff] [review]:
-----------------------------------------------------------------

This is fine, but maybe we would want to consider splitting this probe into "launching sync settings" vs "setting up sync".

::: mobile/android/base/preferences/SyncPreference.java
@@ +52,5 @@
>      }
>  
>      @Override
>      protected void onClick() {
> +        Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, Method.SETTINGS, "preferences_sync");

I wonder if we should have a separate telemetry event for "setting up sync from preferences" - it's probably a tiny number because we have so many more entry points, but the logic is right here.

Comment 4

[:Margaret Leibovic]

(In reply to Chenxia Liu [:liuche] from comment #3)
> Comment on attachment 8484648 [details] [diff] [review]
> Add telemetry probe for clicking sync preference
> 
> Review of attachment 8484648 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This is fine, but maybe we would want to consider splitting this probe into
> "launching sync settings" vs "setting up sync".
> 
> ::: mobile/android/base/preferences/SyncPreference.java
> @@ +52,5 @@
> >      }
> >  
> >      @Override
> >      protected void onClick() {
> > +        Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, Method.SETTINGS, "preferences_sync");
> 
> I wonder if we should have a separate telemetry event for "setting up sync
> from preferences" - it's probably a tiny number because we have so many more
> entry points, but the logic is right here.

Good call, I didn't think about this. I also didn't think about adding probes for the sync settings page itself, but since that's implemented somewhere totally different, I'll punt on that.

Comment 5

[:Margaret Leibovic]

Attachment: (v2) Add telemetry probe for clicking sync preference

How about this? This ignores the old sync code path, but I think that's okay.

Comment 6

[Chenxia Liu [:liuche]]

Comment on attachment 8485256 [details] [diff] [review]
(v2) Add telemetry probe for clicking sync preference

Review of attachment 8485256 [details] [diff] [review]:
-----------------------------------------------------------------

Great, more info :)

Comment 7

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/add87ff4a395

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/add87ff4a395

Comment 9

[:Margaret Leibovic]

I'm seeing this in mfinkle's telemetry dashboard.

Comment 10

[:Margaret Leibovic]

Comment on attachment 8485256 [details] [diff] [review]
(v2) Add telemetry probe for clicking sync preference

Approval Request Comment
[Feature/regressing bug #]: bug 996753
[User impact if declined]: we have less insight into what users do
[Describe test coverage new/current, TBPL]: no tests, verified in Nightly
[Risks and why]: very low risk, just adds telemetry probe
[String/UUID change made/needed]: none

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8a86bb3f2c50
https://hg.mozilla.org/releases/mozilla-beta/rev/340bddec5bf5