EventLogging: Add helper for logging link clicks
Closed, DuplicatePublic
Actions

Assigned To

Authored By

	• Mattflaschen-WMF
	Jul 30 2013, 7:03 PM

Description

GettingStarted has a method logUnlessTimeout that cancels a log attempt after a certain amount of time. Otherwise, it's the same as logEvent. This is used by code like:

logging.logUnlessTimeout( {
	action: 'navbar-return-click',
	funnel: fullTask,
	pageId: cfg.wgArticleId,
	revId : cfg.wgCurRevisionId
}, 500 ).always( function () {
	location.href = $this.attr( 'href' );
} );

evt.preventDefault();

This is the only use of logUnlessTimeout, and other people have expressed interest in the same thing.

We should implement a helper method, like:

function logThenNavigate( $link, eventInstance, timeout )

with timeout being optional and defaulting to 500. It would basically work the same way but handle both the href and timeout. Unlike bug T44815, the navigation is delayed until 'always' or the timeout fires, but the network request still works the same way.

See Also: T44815

Details

Reference: bz52287

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved		• Gilles	T58426 Image link clicks might not get logged
		Duplicate		• Prtksxna	T54287 EventLogging: Add helper for logging link clicks

Event Timeline

• bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 2:05 AM

• bzimport added a project: MediaWiki-extensions-EventLogging.

• bzimport set Reference to bz52287.

• Mattflaschen-WMF created this task.Jul 30 2013, 7:03 PM

[moving from MediaWiki extensions to Analytics product - see bug 61946]

Are you still planning to do this? If not, Growth might.

Change 116260 had a related patch set (by Prtksxna) published:
Add logUnlessTimeout function

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

Commenting in more detail here what we have been talking about on IRC:

Introducing client side delays is not optimal because it produces a slower UX that users might otherwise have but, not only that, it is double-worrisome(?) cause from a performance data standpoint those delays are impossible to detect.

Example: the analytics team just reported recently page timings for all pages around the world. The "delay" introduced by this logging would not be present in any way on those numbers. So we would be presenting data that says, the 50th percentile in the US is 400ms for page loading times. Now, that would be not at all be what the user saw if we introduce arbitrary client side delays. In reality the user is seeing 400ms + delay.

That being said, this is not an easy problem to solve for "full page" reloads and, as ori noted above, the "good" solution is the beacon API.

The preferable option I can see would be storing those events locally and polling as needed to report them. This works well for logging internal page clickthrough. Even for browsers that do not have localStorage (IE8) events can be logged into a in-memory hashmap.

Now, the caveat is that the system does not work so well to log external clicks.

(In reply to nuria from comment #4)

Commenting in more detail here what we have been talking about on IRC:

Introducing client side delays is not optimal because it produces a slower
UX that users might otherwise have but, not only that, it is
double-worrisome(?) cause from a performance data standpoint those delays
are impossible to detect.

This does not introduce a fixed or minimum client-side delay. On the contrary, it sets a maximum possible delay due to logging. With the current EventLogging code, the choices are:

Block the normal link navigation, wait for the logging to finish, then go to the target URL when the logging is done. This works fine in a performant scenario, but if the server hangs, or takes a long time to return, they will not make it to the next page (not sure what happens if the browser itself detects it as a timeout internally).

Don't block normal link navigation (logging might not work if link navigation happens before the logging network request).

Use a compromise solution like this, where it tries to log normally, and if logging is taking longer than expected (500 ms in the code sketched above), do the navigation anyway.

Looking at the patch (which I think is based on prior code at https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FGettingStarted.git/b66689ef823c759067d4f16c8a894758b727db60/resources%2Fext.gettingstarted.logging.js#L64), note that logEvent returns a promise, which is then chained to the dfd deferred.

So in a normal scenario, EventLogging will finish logging (as fast as it is currently), then it will resolve, which will chain to dfd.resolve (which triggers the always callback) and trigger URL navigation immediately after the logging.

In a bad scenario, the code says (if the setTimeout is hit, meaning they still haven't changed pages) "EventLogging still hasn't finished logging the event after 500 ms. Forget it, I'll go to the target URL anyway and not wait for logging to be confirmed complete".

500 ms is a maximum delay/fail-safe if EventLogging is not responding normally, not a minimum delay.

The preferable option I can see would be storing those events locally and
polling as needed to report them. This works well for logging internal page
clickthrough.

By "internal page" do you mean "without leaving the site" or "without leaving the page"?

I agree this is preferable, but browser support is an important issue. Also, if they leave the site, it won't work, but so far we've been concerned with same-site links.

Even for browsers that do not have localStorage (IE8) events
can be logged into a in-memory hashmap.

An in-memory hashmap will only work if they don't leave the page, right? The current logging mechanism already works fine if there is not a full page load due to a link; an enhancement is not necessary for same-page actions.

This does not introduce a fixed or minimum client-side delay. On the contrary, >it sets a maximum possible delay due to logging.

I understand that this change is better than blocking or 'arbitrary' delays.
Let me rephrase: it introduces a delay as every page viewing is been delayed "ad minimum" by the amount of time it takes for the event to be logged (1 round trip). "Ad maximum", as you noted, the delay would be 500ms. Correct?

We only have loading times for content pages (not bits domain) so we cannot provide loading times for bits but working with what we have: have in mind that time to 1st byte for one of our content pages served from cache might very well be over 500ms in some parts of the world at the 50th percentile. Even if bits is 5 times faster the event reporting time still would take ~100ms (I know, totally wild guess here, but I am trying to make the point that adding 100ms to page load time might not justify the data we are gathering).

A plot we made for an unrelated project to illustrate RT times (note percentile 90, RT time is over 1 sec)
https://www.mediawiki.org/wiki/Analytics/Reports/ULSFOImpact#mediaviewer/File:ResponseStart-connectStart-ASIA.png

The preferable option I can see would be storing those events locally and
polling as needed to report them. This works well for logging internal page
clickthrough.

By "internal page" do you mean "without leaving the site" or "without leaving the page"?

w/o leaving the site

I agree this is preferable, but browser support is an important issue. Also, if >they leave the site, it won't work, but so far we've been concerned with same->site links.

If that is the case could we implement a localstorage/polling solution for browsers that support it?

(In reply to nuria from comment #6)

I understand that this change is better than blocking or 'arbitrary' delays.
Let me rephrase: it introduces a delay as every page viewing is been delayed
"ad minimum" by the amount of time it takes for the event to be logged (1
round trip). "Ad maximum", as you noted, the delay would be 500ms. Correct?

Yes, at maximum, the delay would be 500 ms.

Even if bits is 5 times faster the event reporting time still would take
~100ms (I know, totally wild guess here, but I am trying to make the point
that adding 100ms to page load time might not justify the data we are
gathering).

Right, if we go this route, it's a cost-benefit analysis. I agree 500 ms (and even less) has an impact on some users, even if subconscious (http://googleresearch.blogspot.com/2009/06/speed-matters.html).

I agree this is preferable, but browser support is an important issue. Also, if >they leave the site, it won't work, but so far we've been concerned with same->site links.

If that is the case could we implement a localstorage/polling solution for
browsers that support it?

Yes. Actually, it may be able to use jquery.jStorage, which is already in core and has broad browser support (IE6+, Firefox 2+, Safari 4+, Chrome 4+ and Opera 10.5+) by being able to use localStorage, globalStorage, and userData.

For this particular use case, there is also the ping attribute of the <a> element (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-ping), but I think that would require a special server-side handler and I don't know what the browser support is.

The beacon API just made it into chrome, which, as you know, it is made for this very purpose.

https://dvcs.w3.org/hg/webperf/raw-file/tip/specs/Beacon/Overview.html

Change 162194 had a related patch set uploaded by Prtksxna:
Add sendBeacon function

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

Change 116260 abandoned by Prtksxna:
Add logUnlessTimeout function

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

• Prtksxna triaged this task as Medium priority.Dec 3 2014, 10:38 AM

• Prtksxna updated the task description. (Show Details)

• Prtksxna set Security to None.

• Spage unsubscribed.Apr 6 2015, 8:38 PM

AndyRussG subscribed.Aug 21 2015, 1:18 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 21 2015, 1:18 AM