Closed Bug 1058813 Opened 8 years ago Closed 8 years ago

Add telemetry for custom preferences

Categories

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

All
Android
defect
Not set
normal

Tracking

(firefox33 fixed, firefox34 fixed, firefox35 fixed)

VERIFIED FIXED
Firefox 35
Tracking Status
firefox33 --- fixed
firefox34 --- fixed
firefox35 --- fixed

People

(Reporter: Margaret, Assigned: Margaret)

References

Details

Attachments

(1 file, 1 obsolete file)

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!
Blocks: 1063114
Assignee: nobody → 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!
Attachment #8484648 - Flags: review?(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.
Attachment #8484648 - Flags: review?(liuche) → review+
(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.
How about this? This ignores the old sync code path, but I think that's okay.
Attachment #8484648 - Attachment is obsolete: true
Attachment #8485256 - Flags: review?(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 :)
Attachment #8485256 - Flags: review?(liuche) → review+
https://hg.mozilla.org/mozilla-central/rev/add87ff4a395
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
I'm seeing this in mfinkle's telemetry dashboard.
Status: RESOLVED → VERIFIED
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
Attachment #8485256 - Flags: approval-mozilla-beta?
Attachment #8485256 - Flags: approval-mozilla-aurora?
Attachment #8485256 - Flags: approval-mozilla-beta?
Attachment #8485256 - Flags: approval-mozilla-beta+
Attachment #8485256 - Flags: approval-mozilla-aurora?
Attachment #8485256 - Flags: approval-mozilla-aurora+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.