981028 - Add telemetry probes for Top Sites

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Deb Richardson [:dria] (plz NEEDINFO)]

I'd like to know how/if users currently interact with our existing Top Sites implementation, including, if possible:

1) How many users see the top sites panel?
2) How often do users see the top sites panel?
3) When and how often do users see the top sites panel? (On first run? After opening a new tab? After closing all open tabs?)
4) How often do users tap on top sites thumbnails?
5) How many users have pinned a thumbnail on the Top Sites panel?
6) How many users have pinned multiple thumbnails on the Top Sites panel?
7) How many users have edited a top site?
8) How many users have permanently removed/deleted a thumbnail on the Top Sites panel?
9) How many users have changed their default home panel?
10) What built-in panels do users have set to default?

We can add more Hub-related probes later.

If we can't or don't want to do some of these for whatever reason, please comment so we can discuss it.  This list is very much off the top of my head.

Comment 1

[Deb Richardson [:dria] (plz NEEDINFO)]

Needinfo'ing ibarlow in case he has some additional ideas.

Comment 2

[Ian Barlow (:ibarlow)]

The main questions I have are pretty simple:

1. To what extent are people using Top Sites (thumbnails and/or list items) to get to where they want to go?
2. For users who do use the Top Sites panel as a starting point, how many are simply letting frecency do the work for them in choosing what to display, vs how many are overriding it with their own pinned sites?

It looks like some combination of Deb's questions may be able to piece together answers to these.

Comment 3

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

I think some of these probes are being added in bug 968308 and bug 977196. Let's make sure we don't add more than we need to answer the questions.

I don't believe we are adding any probes for the context menu actions on Home panels (or Top Sites specifically) so we should consider adding some to answer the "pinned" and "edited" questions.

Comment 4

[:Margaret Leibovic]

We should get an assignee here. Sola?

Comment 5

[Sola Ogunsakin [:sola]]

I'll take it

Comment 6

[Sola Ogunsakin [:sola]]

Attachment: Part 1: Probes for pinning & editing a top site item

Comment 7

[Sola Ogunsakin [:sola]]

Attachment: Part 2: Sessions for preference pages

Comment 8

[Sola Ogunsakin [:sola]]

Attachment: Part 3: Probe for setting a default panel

Comment 9

[Sola Ogunsakin [:sola]]

(In reply to Deb Richardson [:dria] from comment #0)
Status:
> 1) How many users see the top sites panel?
answered by bug 968308
> 2) How often do users see the top sites panel?
answered by bug 968308
> 3) When and how often do users see the top sites panel? (On first run? After
> opening a new tab? After closing all open tabs?)
"how do users see the top sites panel?" unanswered. not sure if our current setup can answer this.
> 4) How often do users tap on top sites thumbnails?
answered by bug 977196
> 5) How many users have pinned a thumbnail on the Top Sites panel?
answered by patch#1
> 6) How many users have pinned multiple thumbnails on the Top Sites panel?
answered by patch#1
> 7) How many users have edited a top site?
answered by patch#1
> 8) How many users have permanently removed/deleted a thumbnail on the Top
> Sites panel?
unanswered. do you mean unpinning a site?
> 9) How many users have changed their default home panel?
answered by patch#3
> 10) What built-in panels do users have set to default?
answered by patch#3

Comment 10

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

Comment on attachment 8400915 [details] [diff] [review]
Part 1: Probes for pinning & editing a top site item

In my efforts to keep Events generic and Sessions specific, I think just using "edit.1" and "pin.1" might be enough. Those events will have a ""homepanel.1:<panel_id>" Session, right?

I think that might be enough for us to analyze. Thoughts?

Comment 11

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

(In reply to Sola Ogunsakin [:sola] from comment #9)

> > 9) How many users have changed their default home panel?
> answered by patch#3
> > 10) What built-in panels do users have set to default?
> answered by patch#3

I wonder if we could find this using other means of analysis. When the Home panel is opened, we start a "home.1" Session. The first ""homepanel.1:<panel_id>" Session inside the "home.1" Session is the default panel (I think).

Is this enough to determine #9 and #10. If so, drop patch #3. NI'ing Josh to help answer this one.

Since patch #2 is not used to answer any of the questions, I'd rather hold off on using it.

Comment 12

[Chenxia Liu [:liuche]]

Comment on attachment 8400915 [details] [diff] [review]
Part 1: Probes for pinning & editing a top site item

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

::: mobile/android/base/TelemetryContract.java
@@ +15,5 @@
>       * Telemetry.sendUIEvent() as the "action" parameter.
>       */
> +    public interface Event {
> +        // Fired when a user pins a top site.
> +        public static final String TOP_SITE_PIN = "topsite.pin.1";

Nit: TOP_SITES* seems more consistent with the rest of our code base.

::: mobile/android/base/home/TopSitesPanel.java
@@ +339,5 @@
>                      BrowserDB.pinSite(context.getContentResolver(), url, title, position);
>                  }
>              });
>  
> +            // Record UI Telemetry event for pin.

Doesn't seem necessary.

@@ +363,5 @@
>              // Decode "user-entered" URLs before showing them.
>              mEditPinnedSiteListener.onEditPinnedSite(info.position, decodeUserEnteredUrl(info.url));
> +
> +            // Record UI Telemetry event for the edit.
> +            Telemetry.sendUIEvent(TelemetryContract.Event.TOP_SITE_EDIT);

A single-line Telemetry.sendUIEvent doesn't need a comment because the code is pretty self-documenting.

Comment 13

[Sola Ogunsakin [:sola]]

(In reply to Mark Finkle (:mfinkle) from comment #11)
> I wonder if we could find this using other means of analysis. When the Home
> panel is opened, we start a "home.1" Session. The first
> ""homepanel.1:<panel_id>" Session inside the "home.1" Session is the default
> panel (I think).

bug 909618 makes it so that this is not always case

Comment 14

[Chenxia Liu [:liuche]]

Comment on attachment 8401032 [details] [diff] [review]
Part 2: Sessions for preference pages

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

I don't think this handles pre-ICS devices.

::: mobile/android/base/TelemetryContract.java
@@ +39,5 @@
>          // Started when a user enters a given home panel.
>          // Session name is dynamic, encoded as "homepanel.1:<panel_id>"
>          public static final String HOME_PANEL = "homepanel.1:";
> +
> +        // Started when a user enters a dynamic settings/preference page.

Drop dynamic here.

@@ +41,5 @@
>          public static final String HOME_PANEL = "homepanel.1:";
> +
> +        // Started when a user enters a dynamic settings/preference page.
> +        // Session name is dynamic, encoded as "settings.1:<preference_res_name>"
> +        // examples include:

For example:

@@ +42,5 @@
> +
> +        // Started when a user enters a dynamic settings/preference page.
> +        // Session name is dynamic, encoded as "settings.1:<preference_res_name>"
> +        // examples include:
> +        //  "Display" -> "settings.1:preferences_display"

How about:
Display: "settings.1:preferences_display"

@@ +44,5 @@
> +        // Session name is dynamic, encoded as "settings.1:<preference_res_name>"
> +        // examples include:
> +        //  "Display" -> "settings.1:preferences_display"
> +        //  "Privacy" -> "settings.1:preferences_privacy"
> +        //  "Developer tools" -> "settings.1:preferences_devtools"

Two examples seem plenty, drop the last one.

Comment 15

[Chenxia Liu [:liuche]]

Comment on attachment 8401034 [details] [diff] [review]
Part 3: Probe for setting a default panel

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

::: mobile/android/base/TelemetryContract.java
@@ +20,5 @@
>  
>          // Fired when a user edits a top site.
>          public static final String TOP_SITE_EDIT = "topsite.edit.1";
> +
> +        // Occurs when a home panel is made default

I think we might want to drop the verbose verbage here - it should be clear that these events are "fired", so just describe the event.

"Set default home panel" or something like that.

::: mobile/android/base/preferences/PanelsPreferenceCategory.java
@@ +172,5 @@
>  
>          mConfigEditor.setDefault(id);
>          mConfigEditor.apply();
> +
> +        // Record UI Telemetry event for "set default" action.

No comment needed, this one line of code seems pretty self-explanatory.

Comment 16

[Sola Ogunsakin [:sola]]

(In reply to Chenxia Liu [:liuche] from comment #12)
> ::: mobile/android/base/home/TopSitesPanel.java
> @@ +339,5 @@
> >                      BrowserDB.pinSite(context.getContentResolver(), url, title, position);
> >                  }
> >              });
> >  
> > +            // Record UI Telemetry event for pin.
> 
> Doesn't seem necessary.
> 
> @@ +363,5 @@
> >              // Decode "user-entered" URLs before showing them.
> >              mEditPinnedSiteListener.onEditPinnedSite(info.position, decodeUserEnteredUrl(info.url));
> > +
> > +            // Record UI Telemetry event for the edit.
> > +            Telemetry.sendUIEvent(TelemetryContract.Event.TOP_SITE_EDIT);
> 
> A single-line Telemetry.sendUIEvent doesn't need a comment because the code
> is pretty self-documenting.

Alright, I was following guidelines set in this review https://bugzilla.mozilla.org/show_bug.cgi?id=968308#c11

Comment 17

[Josh Dover [:jdover]]

(In reply to Mark Finkle (:mfinkle) from comment #11)
> (In reply to Sola Ogunsakin [:sola] from comment #9)
> 
> > > 9) How many users have changed their default home panel?
> > answered by patch#3
> > > 10) What built-in panels do users have set to default?
> > answered by patch#3
> 
> I wonder if we could find this using other means of analysis. When the Home
> panel is opened, we start a "home.1" Session. The first
> ""homepanel.1:<panel_id>" Session inside the "home.1" Session is the default
> panel (I think).

We should be able to discern this from the session timestamps. However, once bug 909618
is finished and lands, about:home remembers its last position so the first "homepanel.1:"
session may not be the default panel.

Comment 18

[Sola Ogunsakin [:sola]]

Attachment: bug-981028-pin-edit.patch.patch

Comment 19

[Sola Ogunsakin [:sola]]

Attachment: Part 1: Probes for pinning & editing a top site item

Comment 20

[Sola Ogunsakin [:sola]]

Attachment: Part 2: Sessions for preference pages

-handle pre-ICS devices
-change comments

Comment 21

[Sola Ogunsakin [:sola]]

Attachment: Part 3: Probe for setting a default panel

-change comments

Comment 22

[Sola Ogunsakin [:sola]]

How do we want to define first run. First time the user opens app? first X minutes or hours of usage? etc?. There is no good way to tell when the app has been minimized or closed so the first approach would be more favourable from an implementation standpoint.

Comment 23

[Chenxia Liu [:liuche]]

Comment on attachment 8402033 [details] [diff] [review]
Part 1: Probes for pinning & editing a top site item

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

::: mobile/android/base/TelemetryContract.java
@@ +14,5 @@
>       * Holds event names. Intended for use with
>       * Telemetry.sendUIEvent() as the "action" parameter.
>       */
> +    public interface Event {
> +        // Fired when a user pins a top site.

Let's move away from prefacing everything with "Fired when". How about "Pinning a Top Site"?

We should also track unpinning a top site.

Comment 24

[Sola Ogunsakin [:sola]]

Attachment: Part 1: Probes for pinning & editing a top site item

Comment 25

[Sola Ogunsakin [:sola]]

Attachment: Part 3: Probe for setting a default panel

Comment 26

[Deb Richardson [:dria] (plz NEEDINFO)]

Yeah for simplicity's sake, let's say "first-run" = "when the user first opens the app".  If we need more/different information later, we can figure it out then.  Thanks.

Comment 27

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

(In reply to Sola Ogunsakin [:sola] from comment #22)
> How do we want to define first run. First time the user opens app? first X
> minutes or hours of usage? etc?. There is no good way to tell when the app
> has been minimized or closed so the first approach would be more favourable
> from an implementation standpoint.

Android has onPause / onResume triggers that can tell us when the app is "minimized" and "restored."

Comment 28

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

(In reply to Deb Richardson [:dria] from comment #0)

> 3) When and how often do users see the top sites panel? (On first run? After
> opening a new tab? After closing all open tabs?)

Does "first-run" mean the very first time a user starts Firefox, or does it mean when launching Firefox from the homescreen icon and about:home is opened?

It sounds to me like the latter.

Comment 29

[Sola Ogunsakin [:sola]]

(In reply to Mark Finkle (:mfinkle) from comment #27)
> Android has onPause / onResume triggers that can tell us when the app is
> "minimized" and "restored."

onPause gets called only when the main activity is no longer visible. its called when we launch another activity like settings.

similarly onResume gets called when the main activity become visible again (e.g after closing settings).

Comment 30

[Chenxia Liu [:liuche]]

(In reply to Sola Ogunsakin [:sola] from comment #29)
> (In reply to Mark Finkle (:mfinkle) from comment #27)
> > Android has onPause / onResume triggers that can tell us when the app is
> > "minimized" and "restored."
> 
> onPause gets called only when the main activity is no longer visible. its
> called when we launch another activity like settings.
> 
> similarly onResume gets called when the main activity become visible again
> (e.g after closing settings).

Just to be clear, we need to define when to stop measuring "first run." Waiting until the app is actually killed (onDestroy called) will likely give us a longer session than we need, because apps can be backgrounded without being killed. onStop for BroweserApp gets called when the Settings are opened, because BrowserApp gets backgrounded. I don't see a simple way to track Application-level on* calls.

The question is mostly,

Comment 31

[Chenxia Liu [:liuche]]

Comment on attachment 8402087 [details] [diff] [review]
Part 1: Probes for pinning & editing a top site item

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

Just comments.

::: mobile/android/base/TelemetryContract.java
@@ +14,5 @@
>       * Holds event names. Intended for use with
>       * Telemetry.sendUIEvent() as the "action" parameter.
>       */
> +    public interface Event {
> +        // User pinned a top site.

Sorry for all this back and forth, but here's a rule of thumb - if you're repeating the same thing many times, rethink if it's necessary. We know that all these events are going to be user-initiated events, so it's really not necessary to have that everywhere.

"Top site pinned", maybe?

Comment 32

[Chenxia Liu [:liuche]]

After thinking and talking about this a little, I don't think we need to track the Settings session just yet. If we want to measure setting a default, we should not do that through the settings - do it through the API, in HomeConfig. It's okay if there's not an associated session - at the moment, the only way to set a default is through settings anyways, but we shouldn't hinge tracking that on the Setting UI.

Comment 33

[Sola Ogunsakin [:sola]]

Comment on attachment 8402035 [details] [diff] [review]
Part 2: Sessions for preference pages

dropping settings patch

Comment 34

[Sola Ogunsakin [:sola]]

Attachment: Part 1: Probes for pinning & editing a top site item

-edit comments

Comment 35

[Sola Ogunsakin [:sola]]

Attachment: Part 2: Probe for setting a default panel

Comment 36

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

These look good from my perspective. We can always add more later.

Comment 37

[Chenxia Liu [:liuche]]

Comment on attachment 8403422 [details] [diff] [review]
Part 2: Probe for setting a default panel

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

Looks good, except one last thing - we're not checking if we're setting a fallback default (in the case when the user disables/removes a default). I think we don't need to check that, because I don't think that's particularly useful, but we can add it later.

Comment 38

[Sola Ogunsakin [:sola]]

try: https://tbpl.mozilla.org/?tree=Try&rev=d17fc52ab420

Comment 39

[Sola Ogunsakin [:sola]]

-clearing "checkin-needed" due to potential bug with sessions leaking

Comment 40

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing.

Comment 41

[Sola Ogunsakin [:sola]]

Attachment: Part 1: Probes for pinning & editing a top site item

Comment 42

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/85e45452ebd7
https://hg.mozilla.org/integration/fx-team/rev/b5fbb38d86d7

Comment 43

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/85e45452ebd7
https://hg.mozilla.org/mozilla-central/rev/b5fbb38d86d7

Comment 44

[Flaviu Cos]

The following probes are logged in logcat:

1. Change default home panel:
SendUIEvent: action = setdefault.1 method = null timestamp = 21774933 extras = 7f6d419a-cd6c-4e34-b26f-f68b1b551907
2. Pin site to Top Sites:
SendUIEvent: action = action.1 method = contextmenu timestamp = 21925964 extras = top_sites_pin
SendUIEvent: action = pin.1 method = null timestamp = 21925966 extras = null
3. Unpin site from Top Sites:
SendUIEvent: action = action.1 method = contextmenu timestamp = 21987369 extras = top_sites_unpin
SendUIEvent: action = unpin.1 method = null timestamp = 21987370 extras = null
4. Edit pinned site:
SendUIEvent: action = action.1 method = contextmenu timestamp = 22036460 extras = top_sites_edit
SendUIEvent: action = edit.1 method = null timestamp = 22036461 extras = null

Verified as fixed in builds:
- 32.0a1 (2014-05-19);
- 31.0a2 (2014-05-19);

Device: Google Nexus 4 (Android 4.4.2).