feat(mc): Add telemetry for Pocket #2911
Conversation
| @@ -5,6 +5,7 @@ The Activity Stream system add-on sends various types of pings to the backend (H | |||
| - an `event` ping that records specific data about individual user interactions while interacting with Activity Stream | |||
| - a `performance` ping that records specific performance related events | |||
| - an `undesired` ping that records data about bad app states and missing data | |||
| - an `impresion_stats` ping that records data about Pocket impressions and user interactions | |||
docs/v2-system-addon/data_events.md
Outdated
| @@ -264,3 +265,46 @@ perf: { | |||
| "visibility_event_rcvd_ts": 2, | |||
| } | |||
| ``` | |||
|
|
|||
| ## Pocket pings | |||
There was a problem hiding this comment.
Would be better to write Top Story pings or even more general Section/Card pings, but happy to fix it up later.
| "tiles": [{"id": 10000, "pos": 0}], | ||
|
|
||
| // A 0-based index to record which tile in the "tiles" list that the user just interacted with. | ||
| "click|block|pocket": 0 |
There was a problem hiding this comment.
Given that the "click" ping includes both the tiles ID and the client_id, could this measurement ever be mapped to a URL visit?
There was a problem hiding this comment.
Technically yes. If that's a nope, we'll have to take out the client_id from this ping.
There was a problem hiding this comment.
Just to be clear, this id is currently generated by Pocket for each recommended story, and A-S team does not have access to the mapping of "id" -> "URL".
There was a problem hiding this comment.
Whether or not the URL-equivalent is attached to a user identifier, this is Category 3 data so it will need a full review.
I suggest reaching out to @bsmedberg since he also has context on the similar data collection that was done as part of the old tiles (Content Services).
There was a problem hiding this comment.
That makes sense. Thanks for pointing this out! @fmarier
|
This PR is blocked by a data collection review tracked by this bug on bugzilla |
|
@csadilek I've modified this PR to allow us to empty out user specific GUIDs in the impression ping. Could you take another look at it? |
ncloudioj
force-pushed
the
gh2898-pocket-telemetry
branch
2 times, most recently
from
a77f381
to
8fa19c1
Compare
|
@ncloudioj lgtm! We still need to come up with a way to dedupe events or better to avoid sending duplicates, but that's for another day. |
|
Hi @fmarier, So all the user specific information has been emptied out from the events ping, and I've received the go-ahead call for that change on my end. https://github.com/mozilla/activity-stream/pull/2911/files#diff-f9d20d6688eef2754298456e5d5b836b Wanting to confirm with you here before I merge this patch. Does it make sense for you? Thanks, |
There is at least one more meeting on that topic before a conclusion is reached. Rebecca Weiss is now handling this from a data stewardship point view. |
|
Thanks for the update! @fmarier Since this patch just added the telemetry APIs for Top Stories, but not sending anything out. I'll just merge it for now so that we can move forward. |
ncloudioj
force-pushed
the
gh2898-pocket-telemetry
branch
from
8fa19c1
to
e2be8b1
Compare
This closes #2898. r? @k88hudson
@fmarier Could you take a look at the new pings defined in
data_events.md?@csadilek This is lite literally a direct port from the Test Pilot Pocket telemetry. Wanna take a quick look at it, please?