jenkins-bot has submitted this change and it was merged.
Change subject: Make logging less noisy in debug mode
......................................................................
Make logging less noisy in debug mode
Suppress logger console output if the logger is disabled.
Change-Id: I5f45bb7fc68f33880f8a4d1737f0055335fe5071
---
M resources/mmv/logging/mmv.logging.ActionLogger.js
M resources/mmv/logging/mmv.logging.AttributionLogger.js
M resources/mmv/logging/mmv.logging.DimensionLogger.js
M resources/mmv/logging/mmv.logging.DurationLogger.js
M resources/mmv/logging/mmv.logging.Logger.js
M resources/mmv/logging/mmv.logging.PerformanceLogger.js
M tests/qunit/mmv/logging/mmv.logging.ActionLogger.test.js
7 files changed, 52 insertions(+), 14 deletions(-)
Approvals:
Gilles: Looks good to me, approved
jenkins-bot: Verified
diff --git a/resources/mmv/logging/mmv.logging.ActionLogger.js
b/resources/mmv/logging/mmv.logging.ActionLogger.js
index 7d55dd0..6bfb13d 100644
--- a/resources/mmv/logging/mmv.logging.ActionLogger.js
+++ b/resources/mmv/logging/mmv.logging.ActionLogger.js
@@ -130,7 +130,9 @@
var actionText = this.logActions[action] || action,
self = this;
- mw.log( actionText );
+ if ( this.isEnabled( action ) ) {
+ mw.log( actionText );
+ }
if ( forceEventLog || self.isInSample( action ) ) {
return this.loadDependencies().then( function () {
@@ -169,6 +171,18 @@
return Math.floor( Math.random() * factor ) === 0;
};
+ /**
+ * Returns whether logging this event is enabled. This is intended for
console logging, which
+ * (in debug mode) should be done even if the request is not being
sampled, as long as logging
+ * is enabled for some sample.
+ * @param {string} action The key representing the action
+ * @returns {boolean} True if this logging is enabled
+ */
+ L.isEnabled = function ( action ) {
+ var factor = this.getActionFactor( action );
+ return $.isNumeric( factor ) && factor >= 1;
+ };
+
mw.mmv.logging.ActionLogger = ActionLogger;
mw.mmv.actionLogger = new ActionLogger();
}( mediaWiki, jQuery, OO ) );
diff --git a/resources/mmv/logging/mmv.logging.AttributionLogger.js
b/resources/mmv/logging/mmv.logging.AttributionLogger.js
index 330ef30..44d4773 100644
--- a/resources/mmv/logging/mmv.logging.AttributionLogger.js
+++ b/resources/mmv/logging/mmv.logging.AttributionLogger.js
@@ -56,9 +56,12 @@
loggedIn: !mw.user.isAnon(),
samplingFactor: this.samplingFactor
};
- mw.log( 'author: ' + ( data.authorPresent ? 'present' :
'absent' ) +
- ', source: ' + ( data.sourcePresent ? 'present' :
'absent' ) +
- ', license: ' + ( data.licensePresent ? 'present' :
'absent' ) );
+
+ if ( this.isEnabled() ) {
+ mw.log( 'author: ' + ( data.authorPresent ? 'present' :
'absent' ) +
+ ', source: ' + ( data.sourcePresent ? 'present'
: 'absent' ) +
+ ', license: ' + ( data.licensePresent ?
'present' : 'absent' ) );
+ }
this.log( data );
};
diff --git a/resources/mmv/logging/mmv.logging.DimensionLogger.js
b/resources/mmv/logging/mmv.logging.DimensionLogger.js
index ca18f0c..1bcad32 100644
--- a/resources/mmv/logging/mmv.logging.DimensionLogger.js
+++ b/resources/mmv/logging/mmv.logging.DimensionLogger.js
@@ -66,7 +66,10 @@
context: context,
samplingFactor: this.samplingFactor
};
- mw.log( 'mw.mmw.logger.DimensionLogger', data );
+
+ if ( this.isEnabled() ) {
+ mw.log( 'mw.mmw.logger.DimensionLogger', data );
+ }
this.log( data );
};
diff --git a/resources/mmv/logging/mmv.logging.DurationLogger.js
b/resources/mmv/logging/mmv.logging.DurationLogger.js
index 99728fb..591048e 100644
--- a/resources/mmv/logging/mmv.logging.DurationLogger.js
+++ b/resources/mmv/logging/mmv.logging.DurationLogger.js
@@ -136,7 +136,9 @@
} );
}
- mw.log( 'mw.mmw.logger.DurationLogger', e );
+ if ( this.isEnabled() ) {
+ mw.log( 'mw.mmw.logger.DurationLogger', e );
+ }
this.log( e );
diff --git a/resources/mmv/logging/mmv.logging.Logger.js
b/resources/mmv/logging/mmv.logging.Logger.js
index af36f5e..fffe72c 100644
--- a/resources/mmv/logging/mmv.logging.Logger.js
+++ b/resources/mmv/logging/mmv.logging.Logger.js
@@ -103,6 +103,16 @@
};
/**
+ * Returns whether logging this event is enabled. This is intended for
console logging, which
+ * (in debug mode) should be done even if the request is not being
sampled, as long as logging
+ * is enabled for some sample.
+ * @returns {boolean} True if this logging is enabled
+ */
+ L.isEnabled = function () {
+ return $.isNumeric( this.samplingFactor ) &&
this.samplingFactor >= 1;
+ };
+
+ /**
* True if the schema has a country field. Broken out in a separate
function so it's easy to mock.
* @returns {boolean}
*/
diff --git a/resources/mmv/logging/mmv.logging.PerformanceLogger.js
b/resources/mmv/logging/mmv.logging.PerformanceLogger.js
index 62c9d83..1d5d310 100644
--- a/resources/mmv/logging/mmv.logging.PerformanceLogger.js
+++ b/resources/mmv/logging/mmv.logging.PerformanceLogger.js
@@ -258,7 +258,7 @@
stats.request = Math.round(
timingEntry.responseStart - timingEntry.requestStart );
stats.response = Math.round(
timingEntry.responseEnd - timingEntry.responseStart );
stats.cache = Math.round(
timingEntry.domainLookupStart - timingEntry.fetchStart );
- } else if ( performance.getEntriesByType( 'resource'
).length === 150 ) {
+ } else if ( performance.getEntriesByType( 'resource'
).length === 150 && this.isEnabled() ) {
// browser stops logging after 150 entries
mw.log( 'performance buffer full, results are
probably incorrect' );
}
@@ -416,7 +416,9 @@
* @inheritdoc
*/
PL.log = function ( data ) {
- mw.log( 'mw.mmv.logging.PerformanceLogger', data );
+ if ( this.isEnabled() ) {
+ mw.log( 'mw.mmv.logging.PerformanceLogger', data );
+ }
return mw.mmv.logging.Logger.prototype.log.call( this, data );
};
diff --git a/tests/qunit/mmv/logging/mmv.logging.ActionLogger.test.js
b/tests/qunit/mmv/logging/mmv.logging.ActionLogger.test.js
index 2ebbd6a..509a1fb 100644
--- a/tests/qunit/mmv/logging/mmv.logging.ActionLogger.test.js
+++ b/tests/qunit/mmv/logging/mmv.logging.ActionLogger.test.js
@@ -21,18 +21,22 @@
logger.log( unknownAction );
- assert.strictEqual( mw.log.getCall( 0 ).args[ 0 ],
unknownAction , 'Log message defaults to unknown key' );
- assert.strictEqual( fakeEventLog.logEvent.callCount, 1, 'event
log has been recorded' );
+ assert.strictEqual( mw.log.lastCall.args[ 0 ], unknownAction ,
'Log message defaults to unknown key' );
+ assert.ok( fakeEventLog.logEvent.called, 'event log has been
recorded' );
+ mw.log.reset();
+ fakeEventLog.logEvent.reset();
logger.log( action1key );
- assert.strictEqual( mw.log.getCall( 1 ).args[ 0 ],
action1value, 'Log message is translated to its text' );
- assert.strictEqual( fakeEventLog.logEvent.callCount, 2, 'event
log has been recorded' );
+ assert.strictEqual( mw.log.lastCall.args[ 0 ], action1value,
'Log message is translated to its text' );
+ assert.ok( fakeEventLog.logEvent.called, 'event log has been
recorded' );
+ mw.log.reset();
+ fakeEventLog.logEvent.reset();
logger.samplingFactorMap = { 'default': 0 };
logger.log( action1key, true );
- assert.strictEqual( mw.log.getCall( 2 ).args[ 0 ],
action1value, 'Log message is translated to its text' );
- assert.strictEqual( fakeEventLog.logEvent.callCount, 3, 'event
log has been recorded' );
+ assert.ok( !mw.log.called, 'No logging when disabled' );
+ assert.ok( fakeEventLog.logEvent.called, 'event log has been
recorded' );
} );
}( mediaWiki, jQuery ) );
--
To view, visit https://gerrit.wikimedia.org/r/191810
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I5f45bb7fc68f33880f8a4d1737f0055335fe5071
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/MultimediaViewer
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <[email protected]>
Gerrit-Reviewer: Gilles <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
