The schema has been updated: https://meta.wikimedia.org/w/index.php?title=Schema%3APopups&type=revision&diff=16159206&oldid=16143868

I went with Tilman's suggestion of using the Fibonacci numbers as the other option was also fairly arbitrary and mostly useful for the search schema.

Change 327122 had a related patch set uploaded (by Bmansurov):
WIP: Add reading depth

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

@bmansurov When you updated the schema, I noticed you added pageVisibility as a property, however, the Discovery team used that API to pause the timer when the page was hidden. Given a hidden page could generate a large number (currently 19) of unusable data points, would it not make more sense to follow Discovery's lead and only log checkin time, using the pageVisibility API to pause the timer? See https://gerrit.wikimedia.org/r/#/c/311844/3/modules/ext.wikimediaEvents.searchSatisfaction.js for Discovery's implementation.

@jhobs I think that's an overkill personally. I'd like to keep it simple. @ovasileva what do you think?

@bmansurov - it depends. Would we be in danger of logging too many data points? What happens if a tab is open for days (months!) in a row?

@ovasileva logging happens at these seconds: 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233, 377, 610, 987, 1597, 2584, 4181, and 6765. So after about 100mins logging stops.

@bmansurov - I think we're good as is then.

@bmansurov @ovasileva The problem with this approach though is that if someone opens a tab to read later, then comes back to it after any significant period of time, our depth measurement is wildly skewed. If they leave it for over 100 minutes then we won't have any useful depth measurement at all. I know I personally open several tabs all the time when reading an article in order to read them after I'm done with my current one (especially on mobile). Pausing a timer when not visible ensures we get accurate logs, fewer irrelevant data points, and it's already been done and field-tested by another team, so it's a simple implementation (basically just copy-paste).

@jhobs, we always have misleading data, even when we pause the timer. For example, if you have a tab open and then have another application on top of it, we would not pause the timer because as far as the browser is concerned the tab is visible, but in fact the user is doing something else. That's why it's tricky. Besides, if you're tab is not visible and you're on the browser window looking at a different tab, we'd actually be capturing some useful information - the user has the tab open, but it's not visible. This can actually help us in determining if users leave tabs open and not read or read.

So yes, it's unlikely that not pausing the timer will cause an unmanageable amount of extra events. However, I agree with all of @jhobs's points at T147314#2874180 . While it's true that measuring how often and how long tabs are not visible might at some point yield useful information, it's orthogonal to the stated purpose of this instrumentation, namely to measure user engagement with the content (see also T148262 ). For that, we want to get the best possible estimate for the time the user has actually spent reading the page, and the Discovery team's approach of pausing the timer seems to be more suited for that. It would be less accurate (and presumably also quite a pain during data analysis) if one keeps counting and has to later decide how to deal with checkins sent while invisible.

@ovasileva given T147314#2876217, would you please update the task description so that I implement the desired solution? Thanks.

@bmansurov - updated description to allow for pausing timer based on visibility

@ovasileva if we pause, we don't have to log the page visibility information, do we?

In T147314#2877190, @bmansurov wrote:

@ovasileva if we pause, we don't have to log the page visibility information, do we?

We discussed this in standup and have clarified that we only need the "checkin" value with the number of seconds.

The schema has been updated: https://meta.wikimedia.org/w/index.php?title=Schema%3APopups&type=revision&diff=16163887&oldid=16159206

Change 327697 had a related patch set uploaded (by Bmansurov):
WIP: Add reading depth

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

Change 327697 abandoned by Phuedx:
WIP: Add reading depth

Reason:
Wrong branch.

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

I think I need to add one more set of test cases. For now, please review the other parts of https://gerrit.wikimedia.org/r/#/c/327122/

Change 327122 merged by jenkins-bot:
Add reading depth

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

Change 330389 had a related patch set uploaded (by Phuedx):
Hygiene: Avoid touching internal state in tests

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

rEPOP60adbd083154: Hygiene: Avoid touching internal state in tests is a minor follow-on change that only touches a test. Moving this to Ready for Signoff per my review of rEPOP4afa1958a054: Add reading depth, which was as follows:

Generally speaking, this LGTM.

I've tested it locally in Chrome (55.0.2883.95), which supports the Page Visibility API, and in IE11 on Windows 10, which doesn't. In Chrome I saw "checkin" events being logged at the appropriate times and saw that they weren't logged if I tabbed away from the page or minimised the browser. In IE11 I saw no such events being logged.

Change 330389 merged by jenkins-bot:
Hygiene: Avoid touching internal state in tests

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

I happened to mention this endeavour to @Krinkle the other day, and he suggested a different method, which seems much simpler and more accurate at the same time:
Instead of sending a ping in predefined intervals, simply use the onbeforeunload event once at the end when the tab is closed. (One limitation of this is that it requires a sendBeacon capable browser, but we do that anyway in this schema.)
This would yield much more accurate estimates of the actual time spent on the page, e.g. in the sense that we don't need to interpolate between Fibonacci numbers or other intervals when calculating average. It would also address the concerns in T155081 (although I'm not sure), this would also limit the number of additional events generated per pageview to 1. And it looks like other people have been using it for the same purpose before, see e.g. https://github.com/jasonzissman/TimeMe.js (which also handles tab visibility in that setup).
Thoughts? The only downside I could imagine right now concerns cases where the browser process ends abnormally in a way that prevents the onbeforeunload event from being sent (crash, power outage, etc.).

@Tilman, so there will be one check in at the end of the session, right? Another downside happens when the user closes a tab when there is no internet connection. This would prevent the browser from sending the information we need.

In T147314#2945070, @bmansurov wrote:

@Tilman, so there will be one check in at the end of the session, right?

Yes, that would be the idea.

Another downside happens when the user closes a tab when there is no internet connection. This would prevent the browser from sending the information we need.

How would the current interval solution fare in that situation (no internet connection when an event is sent)?

How would the current interval solution fare in that situation (no internet connection when an event is sent)?

Since we have multiple checkin points, a drop in connection may affect some of the checkins, hopefully not all. With only one checkin point a drop in connection at the time of checkin will result in total loss of data. I'm not sure if that's a good thing or not, come to think about it.

Minimising the number of events is a worthy (and necessary) goal.

As @bmansurov notes, we risk a complete loss of data if the user's connection is poor when the UA tries to make the asynchronous request. AFAIK sendBeacon makes no guarantees of delivery, only that the request will be sent without blocking and that it'll be scheduled before unload. If we were to send multiple beacons while the page is visible, then a few requests may succeed resulting in incomplete data, which we can mostly guard against with SELECT MAX(time) ...

moving this back to signoff, as a way to compare method implemented here with T155639: Create reading depth schema

@bmansurov, @phuedx - since we won't be implementing a new structure, I think we're ready to close this.