Hello Krinkle, Gilles,
I'd like you to do a code review. Please visit
https://gerrit.wikimedia.org/r/394298
to review the following change.
Change subject: ext.NavigationTiming: Allow oversampling based on geography or
user agent
......................................................................
ext.NavigationTiming: Allow oversampling based on geography or user agent
This code allows oversampling based on geography (specifically, country) or
user agent. Configuration is controlled by a wg parameter. The result is a
standard NavTiming event that goes to the same topic as any other, but with a
new parameter "isOversample" that is set to true. We'll need to make
appropriate changes to the script that processes NavTiming events in order to
ensure that these events do not pollute the standard perf timing population.
Still to-do: Need to update the NavTiming schema for EventLogging, and update
this code to use that new schema. Need to make the backend changes.
Bug: T181413
Change-Id: I0908bad313a8566cd0a47e85092bf8cb3a3a9204
---
M NavigationTiming.hooks.php
M extension.json
M modules/ext.navigationTiming.js
M tests/ext.navigationTiming.test.js
4 files changed, 294 insertions(+), 17 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/NavigationTiming
refs/changes/98/394298/1
diff --git a/NavigationTiming.hooks.php b/NavigationTiming.hooks.php
index d29f135..b94cbed 100644
--- a/NavigationTiming.hooks.php
+++ b/NavigationTiming.hooks.php
@@ -6,10 +6,13 @@
}
public static function onResourceLoaderGetConfigVars( &$vars ) {
- global $wgNavigationTimingSamplingFactor,
$wgNavigationTimingFirstPaintAsiaSamplingFactor;
+ global $wgNavigationTimingSamplingFactor,
$wgNavigationTimingFirstPaintAsiaSamplingFactor,
+ $wgNavigationTimingOversampleFactor;
$vars[ 'wgNavigationTimingSamplingFactor' ] =
$wgNavigationTimingSamplingFactor;
$vars[ 'wgNavigationTimingFirstPaintAsiaSamplingFactor' ] =
$wgNavigationTimingFirstPaintAsiaSamplingFactor;
+ $vars[ 'wgNavigationTimingOversampleFactor' ] =
+ $wgNavigationTimingOversampleFactor;
}
public static function onResourceLoaderTestModules( array &$modules,
ResourceLoader &$rl ) {
diff --git a/extension.json b/extension.json
index 787ca0a..21afa21 100644
--- a/extension.json
+++ b/extension.json
@@ -54,7 +54,8 @@
},
"config": {
"NavigationTimingSamplingFactor": false,
- "NavigationTimingFirstPaintAsiaSamplingFactor": false
+ "NavigationTimingFirstPaintAsiaSamplingFactor": false,
+ "NavigationTimingOversampleFactor": false
},
"manifest_version": 1
}
diff --git a/modules/ext.navigationTiming.js b/modules/ext.navigationTiming.js
index 84aa956..d8576e7 100644
--- a/modules/ext.navigationTiming.js
+++ b/modules/ext.navigationTiming.js
@@ -9,7 +9,7 @@
'use strict';
var timing, navigation, mediaWikiLoadEnd, hiddenProp, visibilityEvent,
- loadEL, isInSample,
+ loadEL, isInSample, isInAsiaSample, isInOversample,
visibilityChanged = false,
TYPE_NAVIGATE = 0;
@@ -171,12 +171,13 @@
return timingData;
}
- function emitNavigationTiming() {
+ function emitNavigationTiming( oversample ) {
var event = {
isHttp2: /H2/.test( $.cookie( 'CP' ) ),
isHiDPI: getDevicePixelRatio() > 1,
isAnon: mw.config.get( 'wgUserId' ) === null,
- mediaWikiVersion: mw.config.get( 'wgVersion' )
+ mediaWikiVersion: mw.config.get( 'wgVersion' ),
+ isOversample: oversample || false // TODO:
Needs to be added to schema
},
page = {
pageId: mw.config.get( 'wgArticleId' ),
@@ -214,12 +215,17 @@
}
$.extend( event, getNavTiming() );
+
if ( navigation && navigation.type === TYPE_NAVIGATE &&
!isCompliant() ) {
// Keep track of non-compliant browsers (only on
TYPE_NAVIGATE)
mw.eventLog.logFailure( 'NavigationTiming',
'nonCompliant' );
}
mw.eventLog.logEvent( 'NavigationTiming', event );
+ }
+
+ function emitOversampleNavigationTiming() {
+ emitNavigationTiming( true );
}
function emitSaveTiming() {
@@ -248,6 +254,99 @@
}
}, 10 );
} );
+ }
+
+ /**
+ * Test whether this client is located in a geography that we want to
+ * oversample
+ *
+ * @param {Object} geos Object whose properties are country/region codes
to be
+ * oversampled
+ * @returns {boolean} True if the client is in an oversampled region
+ **/
+ function inGeoOversample( geos ) {
+ // Geo oversample depends on the global Geo, which is created
by the
+ // CentralNotice extension. We don't depend on it, though,
because
+ // it's pretty heavy.
+ if ( !window.Geo ) {
+ return false;
+ }
+
+ var myGeo = Geo.country || Geo.country_code;
+
+ if ( !( myGeo in geos ) ) {
+ return false;
+ }
+
+ if ( !$.isNumeric( geos[ myGeo ] ) || geos[ myGeo ] < 1 ) {
+ return false;
+ }
+
+ return Math.floor( Math.random() * geos[ myGeo ] ) === 0;
+ }
+
+ /**
+ * Test whether this client's user agent is one that we want to
oversample
+ *
+ * @param {Object} userAgents Objects whose properties are User Agent
strings
+ * to be oversampled, with value equal to the
+ * sample frequency
+ * @return {boolean} True if the client should be oversampled
+ **/
+ function inUserAgentOversample( userAgents ) {
+ if ( !navigator.userAgent ) {
+ return false;
+ }
+
+ if ( userAgents.length == 0 ) {
+ return false;
+ }
+
+ var sampleRate = -1;
+
+ for ( var userAgent in userAgents ) {
+ if ( navigator.userAgent.indexOf(userAgent) >= 0 ) {
+ if ( $.isNumeric( userAgents[userAgent] ) &&
userAgents[userAgent] >= 0 ) {
+ if ( sampleRate < 0 ||
userAgents[userAgent] < sampleRate ) {
+ sampleRate =
userAgents[userAgent];
+ }
+ }
+ }
+ }
+
+ return Math.floor( Math.random() * sampleRate ) === 0;
+ }
+
+ /**
+ * Test whether this client should be oversampled for any reason
+ *
+ * @return {boolean} True if the client should be oversampled, else false
+ **/
+ function inOversample() {
+ var geoOversample = false;
+ var uaOversample = false;
+
+ // Controls
+ // Only regular page views, ignore non-compliant data
+ if ( !navigation || navigation.type !== TYPE_NAVIGATE ||
!isCompliant() ) {
+ return false;
+ }
+
+ var oversamples = mw.config.get(
'wgNavigationTimingOversampleFactor' );
+
+ if ( !oversamples ) {
+ return false;
+ }
+
+ if ( 'geo' in oversamples ) {
+ geoOversample = inGeoOversample( oversamples[ 'geo' ] );
+ }
+
+ if ( 'userAgent' in oversamples ) {
+ uaOversample = inUserAgentOversample( oversamples[
'userAgent' ]);
+ }
+
+ return geoOversample || uaOversample;
}
function inAsiaSample() {
@@ -286,7 +385,8 @@
}
function emitAsiaFirstPaint() {
- var navigationEvents, paintEvents, firstPaint,
firstContentfulPaint, fetchStart;
+ var navigationEvents, paintEvents, firstPaint,
firstContentfulPaint,
+ fetchStart, isInSample, isInAsiaSample, isInOversample;
firstPaint = Math.round( chrome.loadTimes().firstPaintTime *
1000 );
fetchStart = timing.fetchStart;
@@ -355,22 +455,31 @@
}
}
- // Only load EventLogging when page load is in the sampling
- // Use a conditional block instead of early return since module.exports
- // must happen unconditionally for unit tests.
- isInSample = inSample();
- if ( isInSample ) {
- // Preload EventLogging and schema modules
- loadEL = mw.loader.using( [ 'schema.NavigationTiming',
'schema.SaveTiming' ] );
- }
-
// Ensure we run after loadEventEnd.
onLoadComplete( function () {
+
+ // If this happens before onLoadComplete, we may end up running
before
+ // some deps have loaded (eg, Geo)
+ isInSample = inSample();
+ isInAsiaSample = inAsiaSample();
+ isInOversample = inOversample();
+
+ // If we're supposed to be sampling this page load (for any
reason),
+ // then load the NavTiming and SaveTiming schemas
+ if ( isInSample || isInAsiaSample || isInOversample ) {
+ loadEL = mw.loader.using( [ 'schema.NavigationTiming',
'schema.SaveTiming' ] );
+ }
+
+
if ( isInSample && !visibilityChanged ) {
loadEL.done( emitNavigationTiming );
}
- if ( inAsiaSample() && !visibilityChanged ) {
+ if ( isInAsiaSample && !visibilityChanged ) {
emitAsiaFirstPaint();
+ }
+ if ( isInOversample && !visibilityChanged ) {
+ // Emit nav timing with oversample param = true
+ loadEL.done( emitOversampleNavigationTiming );
}
mw.hook( 'postEdit' ).add( function () {
mw.loader.using( 'schema.SaveTiming' )
@@ -386,8 +495,12 @@
*/
module.exports = {
emitNavTiming: emitNavigationTiming,
+ emitOversampleNavigationTiming:
emitOversampleNavigationTiming,
emitAsiaFirstPaint: emitAsiaFirstPaint,
inAsiaSample: inAsiaSample,
+ inOversample: inOversample,
+ inGeoOversample: inGeoOversample,
+ inUserAgentOversample: inUserAgentOversample,
reinit: function () {
// performance is recursively read-only and can
only be
// mocked from the top down via
window.performance. The test
diff --git a/tests/ext.navigationTiming.test.js
b/tests/ext.navigationTiming.test.js
index 8a99ad1..3e9520e 100644
--- a/tests/ext.navigationTiming.test.js
+++ b/tests/ext.navigationTiming.test.js
@@ -2,7 +2,11 @@
/* eslint-env qunit */
QUnit.module( 'ext.navigationTiming', QUnit.newMwEnvironment( {
setup: function () {
+ // Ensure the starting value of these paraeters, regardless of
what's
+ // set in LocalSettings.php
mw.config.set(
'wgNavigationTimingFirstPaintAsiaSamplingFactor', 1 );
+ mw.config.set( 'wgNavigationTimingOversampleFactor', {} );
+
// Because stubs can't work on undefined properties and the
presence
// of window.Geo and window.chrome isn't guaranteed
this.Geo = window.Geo;
@@ -17,8 +21,19 @@
teardown: function() {
window.Geo = this.Geo;
window.chrome = this.chrome;
- }
+ },
} ) );
+
+// Helper to stub mw.config.get
+function overrideMWConfigGet( k, v ) {
+ return function( key ) {
+ if ( key == k ) {
+ return v;
+ } else {
+ return mw.config.values[key];
+ }
+ };
+}
// Basic test will ensure no exceptions are thrown and various
// of the core properties are set as expected.
@@ -303,3 +318,148 @@
'Is not in Asia sample'
);
} );
+
+
+QUnit.test( 'Oversample config and activation', function ( assert ) {
+ var navigationTiming, geoStub, configStub, uaStub;
+
+ // sinon isn't able to shim browser built-ins. This forces the value
+ // of navigator.userAgent to be a specific string so that we can use it
+ // in our testing
+ Object.defineProperty( navigator, 'userAgent', {
+ get: function() {
+ return "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_1)
AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.12345.94 Safari/537.36";
+ }
+ });
+
+ // If navigation type is anything other than TYPE_NAVIGATE, the
+ // check for whether to measure will fail.
+ this.sandbox.stub( window, 'performance', {
+ timing: performance.timing,
+ navigation: {
+ type: 0,
+ redirectCount: 0
+ }
+ } );
+
+ // Init the NavTiming extension
+ navigationTiming = require( 'ext.navigationTiming' );
+ navigationTiming.reinit();
+
+ // Make sure that window.Geo represents us correctly.
+ geoStub = this.sandbox.stub( window, 'Geo', {
+ 'country': 'XX'
+ });
+
+ // Test that the inGeoOversample correctly identifies whether or not
+ // to oversample
+ assert.equal( navigationTiming.inGeoOversample( { 'XX': 1 } ), true,
+ 'Geo oversample occurs when window.Geo.country is present in
wgNavigationTimingOversampleFactor' );
+ assert.equal( navigationTiming.inGeoOversample( { 'US': 1 } ), false,
+ 'Geo oversample does not occur when country is not in config' );
+ assert.equal( navigationTiming.inGeoOversample( {} ), false,
+ 'Geo oversample does not occur when geo is an empty object' );
+
+ // Test that inUserAgentOversample correctly identifies whether or not
+ // to oversample
+ assert.equal( navigationTiming.inUserAgentOversample( { "Chrome": 1 }),
true,
+ "Generic Chrome user agent is identified and oversampled" );
+ assert.equal( navigationTiming.inUserAgentOversample( {
"Chrome/62.0.12345.94": 1 }), true,
+ "Chrome with very specific UA is oversampled" );
+ assert.equal( navigationTiming.inUserAgentOversample( {
+ "Chrome": 1,
+ "AppleWebKit": 999999999999999999999999 // I acknowledge this
will fail eventually.
+ }), true, "Most likely oversample rate is the one used" );
+ assert.equal( navigationTiming.inUserAgentOversample( { "FakeBrowser":
1 }), false,
+ "Non-matching user agent is not sampled" );
+
+ // Ensure that the oversample wrapper correctly identifies whether or
not
+ // to oversample
+ assert.equal( navigationTiming.inOversample(), false,
+ 'No oversample occurs when the oversample var is an empty
object' );
+ configStub = this.sandbox.stub( mw.config, 'get',
+ overrideMWConfigGet( 'wgNavigationTimingOversampleFactor', {
+ 'geo': {
+ 'XX': 1
+ }
+ }));
+ assert.equal( navigationTiming.inOversample(), true,
+ 'Oversample occurs when the client geo is in the mw.config
var');
+ configStub.restore();
+ configStub = this.sandbox.stub( mw.config, 'get',
+ overrideMWConfigGet( 'wgNavigationTimingOversampleFactor', {
+ 'geo': {
+ 'US': 1
+ }
+ }));
+ assert.equal( navigationTiming.inOversample(), false,
+ 'Oversample does not occur when the client geo is not in
config');
+ configStub.restore();
+ configStub = this.sandbox.stub( mw.config, 'get',
+ overrideMWConfigGet( 'wgNavigationTimingOversampleFactor', {
+ 'userAgent': {
+ 'Chrome': 1
+ }
+ }));
+ assert.equal( navigationTiming.inOversample(), true,
+ 'Oversample occurs when the client userAgent is in the
mw.config var');
+ configStub.restore();
+ configStub = this.sandbox.stub( mw.config, 'get',
+ overrideMWConfigGet( 'wgNavigationTimingOversampleFactor', {
+ 'userAgent': {
+ 'Chrome': 1
+ },
+ 'geo': {
+ 'XX': 1
+ }
+ }));
+ assert.equal( navigationTiming.inOversample(), true,
+ 'Oversamples when both UA and Geo should be sampled');
+ configStub.restore();
+ configStub = this.sandbox.stub( mw.config, 'get',
+ overrideMWConfigGet( 'wgNavigationTimingOversampleFactor', {
+ 'userAgent': {
+ 'Chrome': 1
+ },
+ 'geo': {
+ 'US': 1
+ }
+ }));
+ assert.equal( navigationTiming.inOversample(), true,
+ 'Oversamples when geo misses but UA hits');
+ configStub.restore();
+ configStub = this.sandbox.stub( mw.config, 'get',
+ overrideMWConfigGet( 'wgNavigationTimingOversampleFactor', {
+ 'userAgent': {
+ 'This Is Not A Real Browser': 1
+ },
+ 'geo': {
+ 'XX': 1
+ }
+ }));
+ assert.equal( navigationTiming.inOversample(), true,
+ 'Oversamples when ua misses but geo hits');
+ configStub.restore();
+} );
+
+QUnit.test ( 'emitOversampleNavigationTiming tests', function ( assert ) {
+ var navigationTiming, geoStub, configStub, logEventStub, logFailureStub;
+
+ navigationTiming = require( 'ext.navigationTiming' );
+ navigationTiming.reinit();
+
+ logEventStub = this.sandbox.stub( mw.eventLog, 'logEvent' );
+ logFailureStub = this.sandbox.stub( mw.eventLog, 'logFailure' );
+
+ navigationTiming.emitNavTiming();
+ assert.equal(logEventStub.args[0][1].isOversample, false,
+ 'Calling emitNavTiming emits an event with isOversample =
false');
+ logEventStub.reset();
+ navigationTiming.emitOversampleNavigationTiming();
+ assert.equal(logEventStub.called, true,
+ 'Calling emitOversampleNavigationTiming triggers logEvent');
+ assert.equal(logFailureStub.called, false,
+ 'Calling emitOversampleNavigationTiming does not trigger
logFailure');
+ assert.equal(logEventStub.args[0][1].isOversample, true,
+ 'The event emitted has the isOversample flag set to true');
+} );
--
To view, visit https://gerrit.wikimedia.org/r/394298
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0908bad313a8566cd0a47e85092bf8cb3a3a9204
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/NavigationTiming
Gerrit-Branch: master
Gerrit-Owner: Imarlier <[email protected]>
Gerrit-Reviewer: Gilles <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
