Gilles has uploaded a new change for review. https://gerrit.wikimedia.org/r/113344
Change subject: Record detailed network performance of API calls ...................................................................... Record detailed network performance of API calls Also fixes some issues with incorrect values passed to event logging Change-Id: I2cc730727d30fb4f87a4fbe81725d6859690589b Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/211 --- M MultimediaViewer.php M resources/mmv/mmv.performance.js M resources/mmv/provider/mmv.provider.Api.js M resources/mmv/provider/mmv.provider.FileRepoInfo.js M resources/mmv/provider/mmv.provider.GlobalUsage.js M resources/mmv/provider/mmv.provider.ImageInfo.js M resources/mmv/provider/mmv.provider.ImageUsage.js M resources/mmv/provider/mmv.provider.ThumbnailInfo.js M resources/mmv/provider/mmv.provider.UserInfo.js M tests/qunit/provider/mmv.provider.Api.test.js 10 files changed, 196 insertions(+), 64 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MultimediaViewer refs/changes/44/113344/1 diff --git a/MultimediaViewer.php b/MultimediaViewer.php index ca9d0af..6e777bd 100644 --- a/MultimediaViewer.php +++ b/MultimediaViewer.php @@ -518,7 +518,7 @@ $wgResourceModules['schema.MultimediaViewerNetworkPerformance'] = array( 'class' => 'ResourceLoaderSchemaModule', 'schema' => 'MultimediaViewerNetworkPerformance', - 'revision' => 7393226, + 'revision' => 7488625, ); $wgResourceModules['mmv']['dependencies'][] = 'ext.eventLogging'; diff --git a/resources/mmv/mmv.performance.js b/resources/mmv/mmv.performance.js index 9fec186..45b420b 100755 --- a/resources/mmv/mmv.performance.js +++ b/resources/mmv/mmv.performance.js @@ -48,12 +48,7 @@ if ( request.readyState === 4 ) { deferred.resolve( request.response ); - - // The timeout is necessary because if there's an entry in window.performance, - // it hasn't been added yet at this point - setTimeout( function() { - perf.recordEntry( type, total, url, request ); - }, 1000 ); + perf.recordEntryDelayed( type, total, url, request ); } }; @@ -75,6 +70,9 @@ P.recordEntry = function ( type, total, url, request ) { var timingList, timingEntry, i, matches, + age, + xcache, + xvarnish, stats = { type: type, contentHost: window.location.host, userAgent: navigator.userAgent, @@ -117,10 +115,19 @@ } if ( request ) { - stats.XCache = request.getResponseHeader( 'X-Cache' ); - stats.XVarnish = request.getResponseHeader( 'X-Varnish' ); + xcache = request.getResponseHeader( 'X-Cache' ); - if ( stats.XCache && stats.XCache.length ) { + if ( xcache ) { + stats.XCache = xcache; + } + + xvarnish = request.getResponseHeader( 'X-Varnish' ); + + if ( xvarnish ) { + stats.XVarnish = xvarnish; + } + + if ( stats.XCache && stats.XCache.length && stats.XCache ) { varnishXCache = this.parseVarnishXCacheHeader( stats.XCache ); $.each( varnishXCache, function( key, value ) { @@ -129,7 +136,13 @@ } stats.contentLength = parseInt( request.getResponseHeader( 'Content-Length' ), 10 ); - stats.age = parseInt( request.getResponseHeader( 'Age' ), 10 ); + + age = parseInt( request.getResponseHeader( 'Age' ), 10 ); + + if ( !isNaN( age ) ) { + stats.age = age; + } + stats.timestamp = new Date( request.getResponseHeader( 'Date' ) ).getTime() / 1000; stats.status = request.status; } @@ -141,7 +154,7 @@ for ( i = 0; i < timingList.length; i++ ) { timingEntry = timingList[ i ]; if ( timingEntry.initiatorType === 'xmlhttprequest' - && timingEntry.name === url ) { + && decodeURIComponent( timingEntry.name ) === url ) { stats.total = Math.round( timingEntry.duration ); stats.redirect = Math.round( timingEntry.redirectEnd - timingEntry.redirectStart ); stats.dns = Math.round( timingEntry.domainLookupEnd - timingEntry.domainLookupStart ); @@ -161,7 +174,11 @@ // Add connection information if there's any if ( connection ) { if ( connection.bandwidth ) { - stats.bandwidth = Math.round( connection.bandwidth ); + if ( connection.bandwidth === Infinity ) { + stats.bandwidth = -1; + } else { + stats.bandwidth = Math.round( connection.bandwidth ); + } } if ( connection.metered ) { @@ -178,6 +195,25 @@ }; /** + * Records network performance results for a given url + * Will record if enough data is present and it's not a local cache hit + * Will run after a delay to make sure the window.performance entry is present + * @param {string} type the type of request to be measured + * @param {number} total the total load time tracked with a basic technique + * @param {string} url URL of that was measured + * @param {XMLHttpRequest} request HTTP request that just completed + */ + P.recordEntryDelayed = function ( type, total, url, request ) { + var perf = this; + + // The timeout is necessary because if there's an entry in window.performance, + // it hasn't been added yet at this point + setTimeout( function() { + perf.recordEntry( type, total, url, request ); + } ); + }; + + /** * Parses an X-Cache header from Varnish and extracts varnish information * @param {string} header The X-Cache header from the request * @returns {Object} The parsed X-Cache data diff --git a/resources/mmv/provider/mmv.provider.Api.js b/resources/mmv/provider/mmv.provider.Api.js index 3baa841..e40e08d 100644 --- a/resources/mmv/provider/mmv.provider.Api.js +++ b/resources/mmv/provider/mmv.provider.Api.js @@ -143,6 +143,46 @@ return $.Deferred().reject( this.getErrorMessage( data ), data ); }; + /** + * @method + * Reconstructs the URL reached by an API call based on the parameters passed to mw.api + * @param {Object} ajaxParameters AJAX parameters passed to mw.api.get + * @param {Object} ajaxOptions AJAX options passed to mw.api.get + * @returns {string} the URL for the corresponding API HTTP request + */ + Api.prototype.makeURL = function( ajaxParameters, ajaxOptions ) { + if ( !$.isPlainObject( ajaxParameters ) ) { + throw 'ajaxParameters must be an object'; + } + + // We have to reproduce the defaults of mw.api + // Otherwise the parameters object keys won't be in the right order + var url, + defaults = { action: 'query', format: 'json' }, + combinedParameters = $.extend( defaults, ajaxParameters ), + parameters = []; + + if ( ajaxOptions && $.isPlainObject( ajaxOptions ) && ajaxOptions.url ) { + url = ajaxOptions.url + '?'; + } else { + url = location.protocol + '//' + location.host + mw.util.wikiScript( 'api' ) + '?'; + } + + $.each( combinedParameters, function( key, value ) { + var i; + + if ( value instanceof Array ) { + for (i = 0; i < value.length; i++ ) { + parameters.push( key + '[]=' + value[ i ] ); + } + } else { + parameters.push( key + '=' + value ); + } + } ); + + return url + parameters.join( '&' ); + }; + mw.mmv.provider = {}; mw.mmv.provider.Api = Api; }( mediaWiki, jQuery ) ); diff --git a/resources/mmv/provider/mmv.provider.FileRepoInfo.js b/resources/mmv/provider/mmv.provider.FileRepoInfo.js index e256893..3dd4274 100644 --- a/resources/mmv/provider/mmv.provider.FileRepoInfo.js +++ b/resources/mmv/provider/mmv.provider.FileRepoInfo.js @@ -37,16 +37,21 @@ */ FileRepoInfo.prototype.get = function() { var provider = this, - start; - - if ( !this.cache['*'] ) { - start = $.now(); - this.cache['*'] = this.api.get( { + start, + ajax = { action: 'query', meta: 'filerepoinfo', format: 'json' - } ).then( function( data ) { - provider.performance.recordEntry( 'filerepoinfo', $.now() - start ); + }; + + if ( !this.cache['*'] ) { + start = $.now(); + this.cache['*'] = this.api.get( ajax ).then( function( data, xhr ) { + provider.performance.recordEntryDelayed( 'filerepoinfo', + $.now() - start, + provider.makeURL( ajax ), + xhr); + return provider.getQueryField( 'repos', data ); } ).then( function( reposArray ) { var reposHash = {}; diff --git a/resources/mmv/provider/mmv.provider.GlobalUsage.js b/resources/mmv/provider/mmv.provider.GlobalUsage.js index db439d0..67dc979 100644 --- a/resources/mmv/provider/mmv.provider.GlobalUsage.js +++ b/resources/mmv/provider/mmv.provider.GlobalUsage.js @@ -56,7 +56,16 @@ var provider = this, cacheKey = file.getPrefixedDb(), fileUsage, - start; + start, + ajax = { + action: 'query', + prop: 'globalusage', + titles: file.getPrefixedDb(), + guprop: ['url', 'namespace'], + gufilterlocal: 1, + gulimit: this.options.apiLimit, + format: 'json' + }; if ( this.options.doNotUseApi ) { fileUsage = new mw.mmv.model.FileUsage( file, mw.mmv.model.FileUsage.Scope.GLOBAL, @@ -67,16 +76,12 @@ if ( !this.cache[cacheKey] ) { start = $.now(); - this.cache[cacheKey] = this.api.get( { - action: 'query', - prop: 'globalusage', - titles: file.getPrefixedDb(), - guprop: ['url', 'namespace'], - gufilterlocal: 1, - gulimit: this.options.apiLimit, - format: 'json' - } ).then( function( data ) { - provider.performance.recordEntry( 'globalusage', $.now() - start ); + this.cache[cacheKey] = this.api.get( ajax ).then( function( data, xhr ) { + provider.performance.recordEntryDelayed( 'globalusage', + $.now() - start, + provider.makeURL( ajax ), + xhr ); + return provider.getQueryPage( file, data ); } ).then( function( pageData, data ) { var pages; diff --git a/resources/mmv/provider/mmv.provider.ImageInfo.js b/resources/mmv/provider/mmv.provider.ImageInfo.js index 26b5f0d..d524c05 100644 --- a/resources/mmv/provider/mmv.provider.ImageInfo.js +++ b/resources/mmv/provider/mmv.provider.ImageInfo.js @@ -75,11 +75,8 @@ ImageInfo.prototype.get = function( file ) { var provider = this, cacheKey = file.getPrefixedDb(), - start; - - if ( !this.cache[cacheKey] ) { - start = $.now(); - this.cache[cacheKey] = this.api.get( { + start, + ajax = { action: 'query', prop: 'imageinfo', titles: file.getPrefixedDb(), @@ -87,8 +84,16 @@ iiextmetadatafilter: this.iiextmetadatafilter, iiextmetadatalanguage: this.options.language, format: 'json' - } ).then( function( data ) { - provider.performance.recordEntry( 'imageinfo', $.now() - start ); + }; + + if ( !this.cache[cacheKey] ) { + start = $.now(); + this.cache[cacheKey] = this.api.get( ajax ).then( function( data, xhr ) { + provider.performance.recordEntryDelayed( 'imageinfo', + $.now() - start, + provider.makeURL( ajax ), + xhr ); + return provider.getQueryPage( file, data ); } ).then( function( page ) { if ( page.imageinfo && page.imageinfo.length ) { diff --git a/resources/mmv/provider/mmv.provider.ImageUsage.js b/resources/mmv/provider/mmv.provider.ImageUsage.js index 697b078..f39c8d5 100644 --- a/resources/mmv/provider/mmv.provider.ImageUsage.js +++ b/resources/mmv/provider/mmv.provider.ImageUsage.js @@ -50,19 +50,24 @@ ImageUsage.prototype.get = function( file ) { var provider = this, cacheKey = file.getPrefixedDb(), - start; - - if ( !this.cache[cacheKey] ) { - start = $.now(); - this.cache[cacheKey] = this.api.get( { + start, + ajax = { action: 'query', list: 'imageusage', iutitle: file.getPrefixedDb(), iunamespace: this.options.namespaces, iulimit: this.options.apiLimit, format: 'json' - } ).then( function( data ) { - provider.performance.recordEntry( 'imageusage', $.now() - start ); + }; + + if ( !this.cache[cacheKey] ) { + start = $.now(); + this.cache[cacheKey] = this.api.get( ajax ).then( function( data, xhr ) { + provider.performance.recordEntryDelayed( 'imageusage', + $.now() - start, + provider.makeURL( ajax ), + xhr ); + return provider.getQueryField( 'imageusage', data ); } ).then( function( imageusage, data ) { var pages; diff --git a/resources/mmv/provider/mmv.provider.ThumbnailInfo.js b/resources/mmv/provider/mmv.provider.ThumbnailInfo.js index 1c80cb7..58b68d2 100644 --- a/resources/mmv/provider/mmv.provider.ThumbnailInfo.js +++ b/resources/mmv/provider/mmv.provider.ThumbnailInfo.js @@ -45,11 +45,8 @@ ThumbnailInfo.prototype.get = function( file, width, height ) { var provider = this, cacheKey = file.getPrefixedDb() + '|' + ( width || '' ) + '|' + ( height || '' ), - start; - - if ( !this.cache[cacheKey] ) { - start = $.now(); - this.cache[cacheKey] = this.api.get( { + start, + ajax = { action: 'query', prop: 'imageinfo', titles: file.getPrefixedDb(), @@ -57,8 +54,16 @@ iiurlwidth: width, // mw.Api will omit null/undefined parameters iiurlheight: height, format: 'json' - } ).then( function( data ) { - provider.performance.recordEntry( 'thumbnailinfo', $.now() - start ); + }; + + if ( !this.cache[cacheKey] ) { + start = $.now(); + this.cache[cacheKey] = this.api.get( ajax ).then( function( data, xhr ) { + provider.performance.recordEntryDelayed( 'thumbnailinfo', + $.now() - start, + provider.makeURL( ajax ), + xhr ); + return provider.getQueryPage( file, data ); } ).then( function( page ) { if ( page.imageinfo && page.imageinfo[0] ) { diff --git a/resources/mmv/provider/mmv.provider.UserInfo.js b/resources/mmv/provider/mmv.provider.UserInfo.js index 590c110..82540e3 100644 --- a/resources/mmv/provider/mmv.provider.UserInfo.js +++ b/resources/mmv/provider/mmv.provider.UserInfo.js @@ -41,7 +41,13 @@ var provider = this, ajaxOptions = {}, cacheKey = username, - start; + start, + ajax = { + action: 'query', + list: 'users', + ususers: username, + usprop: 'gender' + }; // For local/shared db images the user should be visible via a local API request, // maybe. (In practice we have Wikimedia users who haven't completed the SUL @@ -56,13 +62,12 @@ if ( !this.cache[cacheKey] ) { start = $.now(); - this.cache[cacheKey] = this.api.get( { - action: 'query', - list: 'users', - ususers: username, - usprop: 'gender' - }, ajaxOptions ).then( function( data ) { - provider.performance.recordEntry( 'userinfo', $.now() - start ); + this.cache[cacheKey] = this.api.get( ajax, ajaxOptions ).then( function( data, xhr ) { + provider.performance.recordEntryDelayed( 'userinfo', + $.now() - start, + provider.makeURL( ajax, ajaxOptions ), + xhr ); + return provider.getQueryField( 'users', data ); } ).then( function( users ) { if ( users[0] && users[0].name && users[0].gender ) { diff --git a/tests/qunit/provider/mmv.provider.Api.test.js b/tests/qunit/provider/mmv.provider.Api.test.js index 1602e8a..9285bb2 100644 --- a/tests/qunit/provider/mmv.provider.Api.test.js +++ b/tests/qunit/provider/mmv.provider.Api.test.js @@ -28,7 +28,7 @@ assert.ok( ApiProviderWithNoOptions ); } ); - QUnit.test( 'getErrorMessage test', 2, function ( assert ) { + QUnit.test( 'getErrorMessage', 2, function ( assert ) { var api = { get: function() {} }, apiProvider = new mw.mmv.provider.Api( api ), errorMessage; @@ -47,7 +47,7 @@ assert.strictEqual( apiProvider.getErrorMessage( {} ), 'unknown error', 'missing error message is handled'); } ); - QUnit.test( 'getNormalizedTitle test', 3, function ( assert ) { + QUnit.test( 'getNormalizedTitle', 3, function ( assert ) { var api = { get: function() {} }, apiProvider = new mw.mmv.provider.Api( api ), title = new mw.Title( 'Image:Stuff.jpg' ), @@ -81,7 +81,7 @@ assert.strictEqual( normalizedTitle.getPrefixedDb(), 'File:Stuff.jpg', 'normalization happens' ); } ); - QUnit.test( 'getQueryField test', 3, function ( assert ) { + QUnit.test( 'getQueryField', 3, function ( assert ) { var api = { get: function() {} }, apiProvider = new mw.mmv.provider.Api( api ), data; @@ -116,7 +116,7 @@ } ); } ); - QUnit.test( 'getQueryPage test', 6, function ( assert ) { + QUnit.test( 'getQueryPage', 6, function ( assert ) { var api = { get: function() {} }, apiProvider = new mw.mmv.provider.Api( api ), title = new mw.Title( 'File:Stuff.jpg' ), @@ -174,4 +174,30 @@ QUnit.start(); } ); } ); + + QUnit.test( 'makeURL', 5, function ( assert ) { + var api = { get: function() {} }, + apiProvider = new mw.mmv.provider.Api( api ), + localAPIURL = location.protocol + '//' + location.host + mw.util.wikiScript( 'api' ); + + assert.strictEqual( apiProvider.makeURL( {}, { url: 'foo' } ), + 'foo?action=query&format=json', + 'URLs match when using custom URL'); + + assert.strictEqual( apiProvider.makeURL( {}, {} ), + localAPIURL + '?action=query&format=json', + 'URLs match when passing empty options object'); + + assert.strictEqual( apiProvider.makeURL( { foo: 'bar', baz: 'boz' } ), + localAPIURL + '?action=query&format=json&foo=bar&baz=boz', + 'URLs match with basic parameters'); + + assert.strictEqual( apiProvider.makeURL( { foo: [ 0, 2, 1 ], baz: 'boz' } ), + localAPIURL + '?action=query&format=json&foo[]=0&foo[]=2&foo[]=1&baz=boz', + 'URLs match with array parameter'); + + assert.strictEqual( apiProvider.makeURL( { a: 1, format: 'jsonp', b: 2 } ), + localAPIURL + '?action=query&format=jsonp&a=1&b=2', + 'URLs match with overriding default parameter'); + } ); }( mediaWiki ) ); -- To view, visit https://gerrit.wikimedia.org/r/113344 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2cc730727d30fb4f87a4fbe81725d6859690589b Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/MultimediaViewer Gerrit-Branch: master Gerrit-Owner: Gilles <gdu...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
