Page MenuHomePhabricator

Page previews should use mw.user.getPageviewToken()
Closed, ResolvedPublic2 Story Points

Description

Now we have a standard way of accessing page tokens, we should use that everywhere in the schemas we make use of.

Acceptance criteria

  • Replace usage of mw.user.generateRandomSessionId for accessing a page token with mw.user.getPageviewToken
    • pageToken is set here
  • When refactoring, please keep in mind, that in Page previews repo, when we create a pageInteractionToken -> this token has to be unique for each preview. We cannot use the same token if the user dwells over the same link twice. Please DOCUMENT this in page previews codebase (Popups extension) as it's not correctly documented. The goal is to distinguish between two hovers on the same link.

Note: be careful as page previews also has a concept of a linkInteractionToken for which there is no shared/well-defined API.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 28 2018, 5:51 PM

Change 454435 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/Popups@master] Use getPageviewToken api

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

ovasileva triaged this task as Normal priority.Aug 29 2018, 4:02 PM
ovasileva set the point value for this task to 2.Aug 29 2018, 4:36 PM
phuedx added a subscriber: phuedx.EditedAug 29 2018, 5:26 PM

Note: be careful as page previews also has a concept of a linkInteractionToken for which there is no shared/well-defined API.

As I said during today's prioritization ritual:

The link interaction token (the "LIT") is used to identify a unique link interaction (the user dwelling on a link with their mouse) in the EventLogging instrumentation and we use mw.user.generateRandomSessionId to generate them.

In all of the analysis of the data that I've seen thus far, we've combined LITs with a token that uniquely identifies the pageview (the pageToken property of a Popups event). Consequently, LITs only have to be unique per pageview and not, say, globally unique. Perhaps, @Tbayer can confirm this in case I've either missed something or misunderstood.

Given that the entropy of mw.user.generateRandomSessionId was doubled recently, I think we can safely continue relying on it to generate LITs.

However, you should make sure to update the use of mw.user.generateRandomSessionId here.

phuedx updated the task description. (Show Details)Aug 29 2018, 5:28 PM

The link interaction token (the "LIT") is used to identify a unique link interaction (the user dwelling on a link with their mouse) in the EventLogging instrumentation and we use mw.user.generateRandomSessionId to generate them.
In all of the analysis of the data that I've seen thus far, we've combined LITs with a token that uniquely identifies the pageview (the pageToken property of a Popups event). Consequently, LITs only have to be unique per pageview and not, say, globally unique. Perhaps, @Tbayer can confirm this in case I've either missed something or misunderstood.

I'm not sure I understand the use case for the LIT. Every event is its own entry in the EventLogging database. If a globally unique ID is added to every beacon, and that ID is not meant to correlate with anything else, what purpose does it serve?

Note that the EventLogging databases already have a primary key for the storage of events. Every event beacon received and processed by an EventLogging server is given a UUID (see EventCapsule). This is globally unique and follows a format with much stronger guarantees than the random generator we user client-side (UUID is 128bit with both server identify and and datetime embedded, in addition to rand; compared to just 32bit or 80bit pure rand from the client).

Also, if all we need is to differentiate a handful of events beaconed from the same page view, we may want to consider a JavaScript number that increments. Would that suffice?

Also, if all we need is to differentiate a handful of events beaconed from the same page view, we may want to consider a JavaScript number that increments. Would that suffice?

I was thinking that as I wrote my comment above.

Jdlrobson updated the task description. (Show Details)Sep 4 2018, 5:11 PM

LGTM, ready for merge pending commit dependency on T202748.

Change 457988 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/Popups@master] Doc: add linkInteractionToken description

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

Change 454435 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Use getPageviewToken api

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

Change 457988 merged by Jdlrobson:
[mediawiki/extensions/Popups@master] Doc: add linkInteractionToken description

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

Jdlrobson closed this task as Resolved.Sep 6 2018, 11:56 PM
Jdlrobson claimed this task.
Jdlrobson updated the task description. (Show Details)

Documentation is now up to date. This can be resolved.