Page MenuHomePhabricator

Update refinery PageviewDefinition to account for link previews in apps.
Closed, ResolvedPublic

Description

The pageview definition needs to be updated to account for link previews with which the users will engage in our app.

Relevant source files:
https://github.com/wikimedia/analytics-refinery-source/blob/master/refinery-core/src/main/java/org/wikimedia/analytics/refinery/core/PageviewDefinition.java#L155

https://github.com/wikimedia/analytics-refinery-source/blob/master/refinery-hive/src/main/java/org/wikimedia/analytics/refinery/hive/IsAppPageviewUDF.java

In PageviewDefinition.java add a new method isAppPreview(). Create a new class IsAppPreviewUDF.java.

Event Timeline

Dbrant created this task.Aug 28 2015, 4:57 PM
Dbrant raised the priority of this task from to Normal.
Dbrant updated the task description. (Show Details)
Dbrant moved this task to Tracking on the Wikipedia-Android-App-Backlog board.
Dbrant added subscribers: Dbrant, bearND, Milimetric.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 28 2015, 4:57 PM
bearND updated the task description. (Show Details)Aug 28 2015, 7:48 PM
bearND set Security to None.

Change 234937 had a related patch set uploaded (by BearND):
Add isAppPreview to pageview definition

https://gerrit.wikimedia.org/r/234937

Change 234937 abandoned by BearND:
Add isAppPreview to pageview definition

Reason:
will resubmit

https://gerrit.wikimedia.org/r/234937

Change 234938 had a related patch set uploaded (by BearND):
Add isAppPreview to pageview definition

https://gerrit.wikimedia.org/r/234938

Added iOS project since they might want to update the preview definition as well. Not sure how the iOS preview request differs from the Android one.

bearND added a subscriber: Fjalapeno.

The discussion on Gerrit has introduced some new options.
We want to be more explicit about page preview events:
The apps can add an X-Analytics header to the request with a specific value, e.g. "preview".
Eventually, this should use EventLogging instead since for the iOS case (and probably Android later, too) we might prefetch several preview entries without a user necessarily looking at them all. I'm going to let @Dbrant and @Fjalapeno continue this discussion and any related patches.

Nuria added a subscriber: Nuria.EditedSep 2 2015, 6:02 PM

EventLogging does not need to play part here, we do not need to send events that validate to an schema to count pageviews. We can count pageviews easily with plain requests tagged with an additional header. Using X-analytics seems best as it is already used and parsed by current infrastructure.

Edit: I think @bearND was talking about the new "scalable events system" rather than current EL incarnation.

@bearND, yes, X-Analytics for now. If you sent preview=1 in the header, we could easily distinguish mobile app pageviews from mobile app page previews that way. The patch you already have in Gerrit would need some small changes, to take the x_analytics_map as a parameter and inspect it for preview=1. But we can help with any of that. We don't want you to be blocked on this at all, so please ping me on IRC if you submit new code and are waiting on me for review or if I'm not responding to anything here.

bearND added a comment.Sep 2 2015, 9:43 PM

@Milimetric I think we could use some serious help from your team with how to use the X-Analytics header instead in the UDF code. It's not passed in and I don't know who to hook it up without spending too much time on it.

Nuria added a comment.Sep 2 2015, 10:19 PM

Please ping us on irc as needed, on analytics channel (wikimedia-analytics).
Do you have permits to stats1002 so you can test your code?

Change 234938 abandoned by Nuria:
Add isAppPreview to pageview definition

Reason:
Changes WIP by analytics here: https://gerrit.wikimedia.org/r/#/c/237274

https://gerrit.wikimedia.org/r/234938

JMinor added a subscriber: JMinor.Feb 2 2016, 8:19 PM

Removing iOS from this ticket, as we are not implementing a custom link preview view.