1063128 - UI Telemetry for Settings records empty preference name

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Chenxia Liu [:liuche]]

Under edit.1 > Setttings events, some of the preferences do not have a string extra of the preference name that is being changed or accessed. This may be due to navigating between Preferences screens or something else, but in any case, the empty pref name is not useful information.

Comment 1

[:Margaret Leibovic]

I found that this is happening for the link-like preferences we have that don't have keys. However, we do have keys for some of them, and those are showing up (e.g. android.not_a_preference.geo.learn_more), so I think we should just make sure we have keys for every preference, and that will help the probe catch all the data.

Comment 2

[:Margaret Leibovic]

Attachment: Make sure all preferences have keys

I went through all the preferences, and I believe this catches them all.

I went with the not_a_preference pattern we use elsewhere.

Comment 3

[Chenxia Liu [:liuche]]

Comment on attachment 8484636 [details] [diff] [review]
Make sure all preferences have keys

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

Since we don't care about the preference "value" of these "prefs", you should also set android:persistent to be false - that way, Android won't create a SharedPreference key-value for each of these fake preferences.

::: mobile/android/base/resources/xml/preferences_devtools.xml
@@ +15,5 @@
>         <CheckBoxPreference android:key="devtools.debugger.remote-enabled"
>                             android:title="@string/pref_developer_remotedebugging" />
>  
> +       <org.mozilla.gecko.preferences.AlignRightLinkPreference android:key="android.not_a_preference.remote_debugging.link"
> +                                                               android:title="@string/pref_learn_more"

Also add android:persistent="false".

Comment 4

[:Margaret Leibovic]

(In reply to Chenxia Liu [:liuche] from comment #3)
> Comment on attachment 8484636 [details] [diff] [review]
> Make sure all preferences have keys
> 
> Review of attachment 8484636 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Since we don't care about the preference "value" of these "prefs", you
> should also set android:persistent to be false - that way, Android won't
> create a SharedPreference key-value for each of these fake preferences.
> 
> ::: mobile/android/base/resources/xml/preferences_devtools.xml
> @@ +15,5 @@
> >         <CheckBoxPreference android:key="devtools.debugger.remote-enabled"
> >                             android:title="@string/pref_developer_remotedebugging" />
> >  
> > +       <org.mozilla.gecko.preferences.AlignRightLinkPreference android:key="android.not_a_preference.remote_debugging.link"
> > +                                                               android:title="@string/pref_learn_more"
> 
> Also add android:persistent="false".

I also added this to the existing links that didn't have it.

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

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/40de5e023fda

Comment 6

[:Margaret Leibovic]

I'm seeing at least one of these new keys in mfinkle's telemetry dashboard, but I'm also still seeing empty edit.1 > settings events. AIUI, these are "daily" events, which means the data comes from the day before mfinkle runs his script to update it. Is this correct? Could it just be that people haven't updated their builds? It's weird that there are so many more of these empty actions than the other actions.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to :Margaret Leibovic from comment #6)
> I'm seeing at least one of these new keys in mfinkle's telemetry dashboard,
> but I'm also still seeing empty edit.1 > settings events. AIUI, these are
> "daily" events, which means the data comes from the day before mfinkle runs
> his script to update it. Is this correct?

Correct

> Could it just be that people
> haven't updated their builds? It's weird that there are so many more of
> these empty actions than the other actions.

People don't update Nightly/Aurora as often as we'd like. It should start to trickle off, but you can also filter the data using BuildID.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Mark Finkle (:mfinkle) from comment #7)

> > Could it just be that people
> > haven't updated their builds? It's weird that there are so many more of
> > these empty actions than the other actions.
> 
> People don't update Nightly/Aurora as often as we'd like. It should start to
> trickle off, but you can also filter the data using BuildID.

<too many to list>

I guess you can't filter by BuildID right now.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

We could filter by Version, but We'd need to wait for 35a2 (aurora) to be released to see a difference.

Comment 10

[:Margaret Leibovic]

Attachment: Make sure all preferences have keys (patch that landed)

Approval Request Comment
[Feature/regressing bug #]: bug 996753
[User impact if declined]: we receive some empty telemetry settings events
[Describe test coverage new/current, TBPL]: no tests, landed on Nightly 9/8
[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/7b2de1eb9c74
https://hg.mozilla.org/releases/mozilla-beta/rev/e981cc82a3e5