Closed Bug 1267837 Opened 8 years ago Closed 8 years ago

Update grid item telemetry to indicate whether an item is pinned

Categories

(Firefox for Android Graveyard :: Metrics, defect)

defect
Not set
normal

Tracking

(firefox47 fixed, firefox48 fixed, firefox49 fixed)

RESOLVED FIXED
Firefox 49
Tracking Status
firefox47 --- fixed
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: Margaret, Assigned: Margaret)

Details

Attachments

(1 file)

We should edit this probe here to understand if an item is pinned or not:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/TopSitesPanel.java#217

The item has a type TopSites.TYPE_PINNED, so this would be straightforward to add.

Should I just add this to the position extra, e.g. "0-pinned"? Or send it as an additional extra in an array?
Flags: needinfo?(mark.finkle)
Flags: needinfo?(bbermes)
Do we care about the position? Given that all of the Extras data is not already an array, I don't like the idea of some of the Extras data being array and some not.

If we intend to create various types of tiles, or split into categories, we might need more context via Arrays. Let's think about how we'd use SQL to access old (non-array) and new (array) data. This might mean we'd switch to arrays for pinned and non-pinned tiles.

If we can't use the data in SQL analyses, then it's not even worth collecting the data. Let's make sure we can use the data effectively.
Flags: needinfo?(mark.finkle)
We can use Presto's regexp_like(field, regex) to break up the Extras data. We could also try using the json_* methods like this:

>select
>  date_trunc('day', submissiondate) as date,
>  action,
>  method,
>  coalesce(json_array_get(extras, 0), json_parse(extras)),
>  count(*) as count
>from android_events_v1
>where
>  submissiondate > current_date - interval '7' day
>  and action = 'loadurl.1' and method = 'griditem'
>group by 1, 2, 3, 4

The coalesce attempts to get the JSON array element, but that will be NULL if it's non array data, which falls back to the second argument, which needs to be of type 'JSON' for Presto. This does seem to return the data we'd want.

If we decide to use a JSON array, we need to decide that index 0 is the position and index 1 is the type, and so on. The indexes end up with special meaning. We could try using a JSON object, like { "p":0, "t":"pinned" } but who wants to deal with that shit?
I like the idea of tracking the pins in general, the rest of how and why to store it a certain way, I'll leave this up to you for now.
Flags: needinfo?(bbermes)
(In reply to :Margaret Leibovic from comment #0)

> Should I just add this to the position extra, e.g. "0-pinned"? Or send it as
> an additional extra in an array?

After thinking about this a bit more, using "0-pinned" seems like the simple way forward. We can use regexp_* functions to split the data however we like.
Comment on attachment 8751342 [details]
MozReview Request: Bug 1267837 - Update grid item telemetry to indicate whether an item is pinned. r=mfinkle

https://reviewboard.mozilla.org/r/51975/#review48793
Attachment #8751342 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/integration/fx-team/rev/e62b981d0cbe620420b0c6fb7af38a07be913afb
Bug 1267837 - Update grid item telemetry to indicate whether an item is pinned. r=mfinkle
Comment on attachment 8751342 [details]
MozReview Request: Bug 1267837 - Update grid item telemetry to indicate whether an item is pinned. r=mfinkle

Approval Request Comment
[Feature/regressing bug #]: none
[User impact if declined]: we can't understand whether people use pinned top sites
[Describe test coverage new/current, TreeHerder]: tested locally
[Risks and why]: low-risk, only a change to a probe (no functional changes)
[String/UUID change made/needed]: none
Attachment #8751342 - Flags: approval-mozilla-beta?
Attachment #8751342 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/e62b981d0cbe
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Comment on attachment 8751342 [details]
MozReview Request: Bug 1267837 - Update grid item telemetry to indicate whether an item is pinned. r=mfinkle

More telemetry data, verified locally, Aurora48+, Beta47+
Attachment #8751342 - Flags: approval-mozilla-beta?
Attachment #8751342 - Flags: approval-mozilla-beta+
Attachment #8751342 - Flags: approval-mozilla-aurora?
Attachment #8751342 - 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.