From ed9026377636777fefe482f2c02ac41c5a5ee410 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Tue, 18 Nov 2014 22:42:33 -0800 Subject: [PATCH] objectcache: Use Psr\Log\LoggerInterface instead of wfDebug ObjectCache::newFromParams() will handle a 'loggroup' parameter specially, getting an instance of \Psr\Log\LoggerInterface to pass to the BagsOStuff. BagOStuff implementations can use $this->logger which will be an implementation of \Psr\Log\LoggerInterface. As this is set in BagOStuff::__construct(), all subclasses must now call the parent constructor. The goal of this is to make the logging in BagOStuff non-MediaWiki specific, in the hopes of separating it out into a separate library in the future. Change-Id: I8a8e278e6f028814499d8457d6d5341d03eabc7a --- includes/DefaultSettings.php | 8 ++--- includes/objectcache/BagOStuff.php | 32 +++++++++++++++++-- includes/objectcache/HashBagOStuff.php | 3 +- includes/objectcache/MemcachedBagOStuff.php | 2 +- includes/objectcache/MemcachedClient.php | 14 ++++++-- .../objectcache/MemcachedPeclBagOStuff.php | 11 ++++--- .../objectcache/MemcachedPhpBagOStuff.php | 1 + includes/objectcache/MultiWriteBagOStuff.php | 1 + includes/objectcache/ObjectCache.php | 7 ++++ includes/objectcache/RedisBagOStuff.php | 3 +- includes/objectcache/SqlBagOStuff.php | 24 +++++++------- 11 files changed, 79 insertions(+), 27 deletions(-) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 83870c20b6..0b37b972bb 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -2111,17 +2111,17 @@ $wgLanguageConverterCacheType = CACHE_ANYTHING; */ $wgObjectCaches = array( CACHE_NONE => array( 'class' => 'EmptyBagOStuff' ), - CACHE_DB => array( 'class' => 'SqlBagOStuff' ), + CACHE_DB => array( 'class' => 'SqlBagOStuff', 'loggroup' => 'SQLBagOStuff' ), CACHE_ANYTHING => array( 'factory' => 'ObjectCache::newAnything' ), CACHE_ACCEL => array( 'factory' => 'ObjectCache::newAccelerator' ), - CACHE_MEMCACHED => array( 'factory' => 'ObjectCache::newMemcached' ), + CACHE_MEMCACHED => array( 'factory' => 'ObjectCache::newMemcached', 'loggroup' => 'memcached' ), 'apc' => array( 'class' => 'APCBagOStuff' ), 'xcache' => array( 'class' => 'XCacheBagOStuff' ), 'wincache' => array( 'class' => 'WinCacheBagOStuff' ), - 'memcached-php' => array( 'class' => 'MemcachedPhpBagOStuff' ), - 'memcached-pecl' => array( 'class' => 'MemcachedPeclBagOStuff' ), + 'memcached-php' => array( 'class' => 'MemcachedPhpBagOStuff', 'loggroup' => 'memcached' ), + 'memcached-pecl' => array( 'class' => 'MemcachedPeclBagOStuff', 'loggroup' => 'memcached' ), 'hash' => array( 'class' => 'HashBagOStuff' ), ); diff --git a/includes/objectcache/BagOStuff.php b/includes/objectcache/BagOStuff.php index 85b0b62087..ae49564422 100644 --- a/includes/objectcache/BagOStuff.php +++ b/includes/objectcache/BagOStuff.php @@ -28,6 +28,10 @@ * @defgroup Cache Cache */ +use Psr\Log\LoggerAwareInterface; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; + /** * interface is intended to be more or less compatible with * the PHP memcached client. @@ -40,17 +44,38 @@ * * @ingroup Cache */ -abstract class BagOStuff { +abstract class BagOStuff implements LoggerAwareInterface { private $debugMode = false; protected $lastError = self::ERR_NONE; + /** + * @var LoggerInterface + */ + protected $logger; + /** Possible values for getLastError() */ const ERR_NONE = 0; // no error const ERR_NO_RESPONSE = 1; // no response const ERR_UNREACHABLE = 2; // can't connect const ERR_UNEXPECTED = 3; // response gave some error + public function __construct( array $params = array() ) { + if ( isset( $params['logger'] ) ) { + $this->setLogger( $params['logger'] ); + } else { + $this->setLogger( new NullLogger() ); + } + } + + /** + * @param LoggerInterface $logger + * @return null + */ + public function setLogger( LoggerInterface $logger ) { + $this->logger = $logger; + } + /** * @param bool $bool */ @@ -358,8 +383,9 @@ abstract class BagOStuff { */ public function debug( $text ) { if ( $this->debugMode ) { - $class = get_class( $this ); - wfDebug( "$class debug: $text\n" ); + $this->logger->debug( "{class} debug: $text", array( + 'class' => get_class( $this ), + ) ); } } diff --git a/includes/objectcache/HashBagOStuff.php b/includes/objectcache/HashBagOStuff.php index 06a08655e9..6da1882a59 100644 --- a/includes/objectcache/HashBagOStuff.php +++ b/includes/objectcache/HashBagOStuff.php @@ -31,7 +31,8 @@ class HashBagOStuff extends BagOStuff { /** @var array */ protected $bag; - function __construct() { + function __construct( $params = array() ) { + parent::__construct( $params ); $this->bag = array(); } diff --git a/includes/objectcache/MemcachedBagOStuff.php b/includes/objectcache/MemcachedBagOStuff.php index 0e133a8813..6a98a8a63a 100644 --- a/includes/objectcache/MemcachedBagOStuff.php +++ b/includes/objectcache/MemcachedBagOStuff.php @@ -174,6 +174,6 @@ class MemcachedBagOStuff extends BagOStuff { * @param string $text */ protected function debugLog( $text ) { - wfDebugLog( 'memcached', $text ); + $this->logger->debug( $text ); } } diff --git a/includes/objectcache/MemcachedClient.php b/includes/objectcache/MemcachedClient.php index 1e04d45240..bc4a00b271 100644 --- a/includes/objectcache/MemcachedClient.php +++ b/includes/objectcache/MemcachedClient.php @@ -64,6 +64,9 @@ * @version 0.1.2 */ +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; + // {{{ requirements // }}} @@ -233,6 +236,11 @@ class MWMemcached { */ public $_connect_attempts; + /** + * @var LoggerInterface + */ + private $_logger; + // }}} // }}} // {{{ methods @@ -263,6 +271,8 @@ class MWMemcached { $this->_connect_timeout = isset( $args['connect_timeout'] ) ? $args['connect_timeout'] : 0.1; $this->_connect_attempts = 2; + + $this->_logger = isset( $args['logger'] ) ? $args['logger'] : new NullLogger(); } // }}} @@ -1104,14 +1114,14 @@ class MWMemcached { * @param string $text */ function _debugprint( $text ) { - wfDebugLog( 'memcached', $text ); + $this->_logger->debug( $text ); } /** * @param string $text */ function _error_log( $text ) { - wfDebugLog( 'memcached-serious', "Memcached error: $text" ); + $this->_logger->error( "Memcached error: $text" ); } /** diff --git a/includes/objectcache/MemcachedPeclBagOStuff.php b/includes/objectcache/MemcachedPeclBagOStuff.php index 7c0a6456aa..4ead417362 100644 --- a/includes/objectcache/MemcachedPeclBagOStuff.php +++ b/includes/objectcache/MemcachedPeclBagOStuff.php @@ -46,6 +46,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff { * @throws MWException */ function __construct( $params ) { + parent::__construct( $params ); $params = $this->applyDefaultParams( $params ); if ( $params['persistent'] ) { @@ -54,7 +55,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff { // We can only reuse a pool ID if we keep the config consistent. $this->client = new Memcached( md5( serialize( $params ) ) ); if ( count( $this->client->getServerList() ) ) { - wfDebug( __METHOD__ . ": persistent Memcached object already loaded.\n" ); + $this->logger->debug( __METHOD__ . ": persistent Memcached object already loaded." ); return; // already initialized; don't add duplicate servers } } else { @@ -223,14 +224,16 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff { break; default: $msg = $this->client->getResultMessage(); + $logCtx = array(); if ( $key !== false ) { $server = $this->client->getServerByKey( $key ); - $serverName = "{$server['host']}:{$server['port']}"; - $msg = "Memcached error for key \"$key\" on server \"$serverName\": $msg"; + $logCtx['memcached-server'] = "{$server['host']}:{$server['port']}"; + $logCtx['memcached-key'] = $key; + $msg = "Memcached error for key \"{memcached-key}\" on server \"{memcached-server}\": $msg"; } else { $msg = "Memcached error: $msg"; } - wfDebugLog( 'memcached-serious', $msg ); + $this->logger->error( $msg, $logCtx ); $this->setLastError( BagOStuff::ERR_UNEXPECTED ); } return $result; diff --git a/includes/objectcache/MemcachedPhpBagOStuff.php b/includes/objectcache/MemcachedPhpBagOStuff.php index 939a7153ab..6fba61ba0f 100644 --- a/includes/objectcache/MemcachedPhpBagOStuff.php +++ b/includes/objectcache/MemcachedPhpBagOStuff.php @@ -42,6 +42,7 @@ class MemcachedPhpBagOStuff extends MemcachedBagOStuff { * @param array $params */ function __construct( $params ) { + parent::__construct( $params ); $params = $this->applyDefaultParams( $params ); $this->client = new MemCachedClientforWiki( $params ); diff --git a/includes/objectcache/MultiWriteBagOStuff.php b/includes/objectcache/MultiWriteBagOStuff.php index 04ed89478c..dbaccd62bc 100644 --- a/includes/objectcache/MultiWriteBagOStuff.php +++ b/includes/objectcache/MultiWriteBagOStuff.php @@ -43,6 +43,7 @@ class MultiWriteBagOStuff extends BagOStuff { * @throws MWException */ public function __construct( $params ) { + parent::__construct( $params ); if ( !isset( $params['caches'] ) ) { throw new MWException( __METHOD__ . ': the caches parameter is required' ); } diff --git a/includes/objectcache/ObjectCache.php b/includes/objectcache/ObjectCache.php index 62856f907f..1f2c9c046c 100644 --- a/includes/objectcache/ObjectCache.php +++ b/includes/objectcache/ObjectCache.php @@ -81,6 +81,13 @@ class ObjectCache { * @return BagOStuff */ static function newFromParams( $params ) { + if ( isset( $params['loggroup'] ) ) { + $params['logger'] = MWLoggerFactory::getInstance( $params['loggroup'] ); + } else { + // For backwards-compatability with custom parameters, lets not + // have all logging suddenly disappear + $params['logger'] = MWLoggerFactory::getInstance( 'objectcache' ); + } if ( isset( $params['factory'] ) ) { return call_user_func( $params['factory'], $params ); } elseif ( isset( $params['class'] ) ) { diff --git a/includes/objectcache/RedisBagOStuff.php b/includes/objectcache/RedisBagOStuff.php index 6836f74872..fef71553a8 100644 --- a/includes/objectcache/RedisBagOStuff.php +++ b/includes/objectcache/RedisBagOStuff.php @@ -56,6 +56,7 @@ class RedisBagOStuff extends BagOStuff { * @param array $params */ function __construct( $params ) { + parent::__construct( $params ); $redisConf = array( 'serializer' => 'none' ); // manage that in this class foreach ( array( 'connectTimeout', 'persistent', 'password' ) as $opt ) { if ( isset( $params[$opt] ) ) { @@ -363,7 +364,7 @@ class RedisBagOStuff extends BagOStuff { * @param string $msg */ protected function logError( $msg ) { - wfDebugLog( 'redis', "Redis error: $msg" ); + $this->logger->error( "Redis error: $msg" ); } /** diff --git a/includes/objectcache/SqlBagOStuff.php b/includes/objectcache/SqlBagOStuff.php index 7524240895..2a92beeeac 100644 --- a/includes/objectcache/SqlBagOStuff.php +++ b/includes/objectcache/SqlBagOStuff.php @@ -88,6 +88,7 @@ class SqlBagOStuff extends BagOStuff { * @param array $params */ public function __construct( $params ) { + parent::__construct( $params ); if ( isset( $params['servers'] ) ) { $this->serverInfos = $params['servers']; $this->numServers = count( $this->serverInfos ); @@ -138,12 +139,12 @@ class SqlBagOStuff extends BagOStuff { # If server connection info was given, use that if ( $this->serverInfos ) { if ( $wgDebugDBTransactions ) { - wfDebug( "Using provided serverInfo for SqlBagOStuff\n" ); + $this->logger->debug( "Using provided serverInfo for SqlBagOStuff" ); } $info = $this->serverInfos[$serverIndex]; $type = isset( $info['type'] ) ? $info['type'] : 'mysql'; $host = isset( $info['host'] ) ? $info['host'] : '[unknown]'; - wfDebug( __CLASS__ . ": connecting to $host\n" ); + $this->logger->debug( __CLASS__ . ": connecting to $host" ); $db = DatabaseBase::factory( $type, $info ); $db->clearFlag( DBO_TRX ); } else { @@ -161,7 +162,7 @@ class SqlBagOStuff extends BagOStuff { } } if ( $wgDebugDBTransactions ) { - wfDebug( sprintf( "Connection %s will be used for SqlBagOStuff\n", $db ) ); + $this->logger->debug( sprintf( "Connection %s will be used for SqlBagOStuff", $db ) ); } $this->conns[$serverIndex] = $db; } @@ -702,13 +703,13 @@ class SqlBagOStuff extends BagOStuff { if ( $exception instanceof DBConnectionError ) { $this->markServerDown( $exception, $serverIndex ); } - wfDebugLog( 'SQLBagOStuff', "DBError: {$exception->getMessage()}" ); + $this->logger->error( "DBError: {$exception->getMessage()}" ); if ( $exception instanceof DBConnectionError ) { $this->setLastError( BagOStuff::ERR_UNREACHABLE ); - wfDebug( __METHOD__ . ": ignoring connection error\n" ); + $this->logger->debug( __METHOD__ . ": ignoring connection error" ); } else { $this->setLastError( BagOStuff::ERR_UNEXPECTED ); - wfDebug( __METHOD__ . ": ignoring query error\n" ); + $this->logger->debug( __METHOD__ . ": ignoring query error" ); } } @@ -728,13 +729,14 @@ class SqlBagOStuff extends BagOStuff { } catch ( DBError $e ) { } } - wfDebugLog( 'SQLBagOStuff', "DBError: {$exception->getMessage()}" ); + + $this->logger->error( "DBError: {$exception->getMessage()}" ); if ( $exception instanceof DBConnectionError ) { $this->setLastError( BagOStuff::ERR_UNREACHABLE ); - wfDebug( __METHOD__ . ": ignoring connection error\n" ); + $this->logger->debug( __METHOD__ . ": ignoring connection error" ); } else { $this->setLastError( BagOStuff::ERR_UNEXPECTED ); - wfDebug( __METHOD__ . ": ignoring query error\n" ); + $this->logger->debug( __METHOD__ . ": ignoring query error" ); } } @@ -750,12 +752,12 @@ class SqlBagOStuff extends BagOStuff { unset( $this->connFailureTimes[$serverIndex] ); unset( $this->connFailureErrors[$serverIndex] ); } else { - wfDebug( __METHOD__ . ": Server #$serverIndex already down\n" ); + $this->logger->debug( __METHOD__ . ": Server #$serverIndex already down" ); return; } } $now = time(); - wfDebug( __METHOD__ . ": Server #$serverIndex down until " . ( $now + 60 ) . "\n" ); + $this->logger->info( __METHOD__ . ": Server #$serverIndex down until " . ( $now + 60 ) ); $this->connFailureTimes[$serverIndex] = $now; $this->connFailureErrors[$serverIndex] = $exception; } -- 2.20.1