Page MenuHomePhabricator

EventLogging returns a new session ID on each pageview
Open, MediumPublic

Description

EventLogging's main session ID is supposed to persist until a user has been inactive for 30 minutes. However, I've noticed that mw.eventLog.id.getSessionId() returns a different ID on every pageview, no matter how short the interval between them.

I've tested this on Firefox 103.0 and Safari 15.5 (17613.2.7.1.8) and the result is the same.

Event Timeline

Hmm, it looks like the sessionTick values in the cookie persist fine; it's just that the function is returning a separate session ID that is different on every pageview. I'm pretty sure that's not supposed to happen. So maybe sessionTick itself isn't impacted but other schemas that use the session ID are?

Oh wait, there's mw.eventLog.id.getSessionId() and mw.user.sessionID()? And they're different? 😵‍💫

So it seems like mw.user.sessionID() is the "old" session ID which lasts until the browser is closed, and mw.eventLog.id.getSessionId() is the "new" session ID which corresponds. the session tick instrumentation. For a minute, I thought I was just looking at the wrong one, but no, mw.eventLog.id.getSessionId() is the one I want and should be persisting between pages.

Hmm, it looks like:

  • pretty much all the existing streams use mw.user.sessionID (except the new Wikistories streams I was testing, T313633)
  • sessionTick does not seem to be impacted (I looked at the tick events being submitted from one of my browsers and the length seemed to be measured correctly)

So even if this is real, the impact is pretty limited. It should still be fixed, though.

MetricsClient has a bug in its storage helper definition.

core.storage = {
	get: function ( name ) {
		mw.cookie.get( 'el-' + name );
	},
	set: function ( name, value ) {
		mw.cookie.set( 'el-' + name, value );
	},
	unset: function ( name ) {
		mw.cookie.set( 'el-' + name, null );
	}
};

Note the lack of a return in the get method. Means it never thinks it has a sessionid stored...

Change 820701 had a related patch set uploaded (by Phuedx; author: Phuedx):

[mediawiki/extensions/EventLogging@master] storage: Return value retrieved from cookie jar

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

Looking at rEEVL01fb701db1c7: Sampling logic and session/pageview id support, it appears that the bug was introduced at the same time the method was. Fortunately, as @DLynch points out, it's simple enough to fix.

Change 820701 merged by jenkins-bot:

[mediawiki/extensions/EventLogging@master] storage: Return value retrieved from cookie jar

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

mpopov triaged this task as Medium priority.Tue, Aug 16, 5:21 PM
mpopov moved this task from Triage to Tracking on the Product-Analytics board.