From 56b70b61afc69139e28ef501947184897eaa64c3 Mon Sep 17 00:00:00 2001 From: Bryan Davis Date: Tue, 13 Jan 2015 16:54:18 -0700 Subject: [PATCH] Replace MWLogger with MWLoggerFactory Time wounds all heels. During the code review for the PSR-3 logging introduction, several people asked me why we needed a wrapper for Psr\Log\LoggerInterface if the point was to use the standard. At the time I was convinced that it would be better to introduce the dependency via a wrapper class so that we could use the wrapper to patch over any deficiencies that we might find in the PSR-3 API. After going on to work on a project to disentangle other MediaWiki components from internal project dependencies I have suddenly and clearly seen the error of my ways. We still need a logger factory as PSR-3 does not specify a standard mechanism for creating Psr\Log\LoggerInterface instances. My solution is to convert MWLogger into MWLoggerFactory to retain a static factory interface for creating PSR-3 loggers but remove the MWLogger wrapper class itself in favor of direct exposure of Psr\Log\LoggerInterface to the MediaWiki consumer classes. Change-Id: Ie47467657dcf341991ada00827dca5e8eff95438 --- RELEASE-NOTES-1.25 | 2 +- autoload.php | 2 +- docs/mwlogger.txt | 51 ++-- includes/DefaultSettings.php | 8 +- includes/GlobalFunctions.php | 10 +- includes/debug/logger/Factory.php | 115 +++++++++ includes/debug/logger/Logger.php | 234 ------------------ includes/debug/logger/NullSpi.php | 9 +- includes/debug/logger/Spi.php | 13 +- includes/debug/logger/legacy/Logger.php | 2 +- includes/debug/logger/legacy/Spi.php | 7 +- .../debug/logger/monolog/SamplingHandler.php | 2 +- includes/debug/logger/monolog/Spi.php | 11 +- 13 files changed, 176 insertions(+), 290 deletions(-) create mode 100644 includes/debug/logger/Factory.php delete mode 100644 includes/debug/logger/Logger.php diff --git a/RELEASE-NOTES-1.25 b/RELEASE-NOTES-1.25 index 2e937e277a..f0c00f34b2 100644 --- a/RELEASE-NOTES-1.25 +++ b/RELEASE-NOTES-1.25 @@ -93,7 +93,7 @@ production. * The following libraries are now required: ** psr/log This library provides the interfaces set by the PSR-3 standard (http://www.php-fig.org/psr/psr-3/) - which are used by MediaWiki interally by the MWLogger class. + which are used by MediaWiki internally via the MWLoggerFactory class. See the structured logging RfC (https://www.mediawiki.org/wiki/Requests_for_comment/Structured_logging) for more background information. ** cssjanus/cssjanus diff --git a/autoload.php b/autoload.php index 46c8b01cc8..c035439520 100644 --- a/autoload.php +++ b/autoload.php @@ -689,7 +689,7 @@ $wgAutoloadLocalClasses = array( 'MWFunction' => __DIR__ . '/includes/utils/MWFunction.php', 'MWHookException' => __DIR__ . '/includes/Hooks.php', 'MWHttpRequest' => __DIR__ . '/includes/HttpFunctions.php', - 'MWLogger' => __DIR__ . '/includes/debug/logger/Logger.php', + 'MWLoggerFactory' => __DIR__ . '/includes/debug/logger/Factory.php', 'MWLoggerLegacyLogger' => __DIR__ . '/includes/debug/logger/legacy/Logger.php', 'MWLoggerLegacySpi' => __DIR__ . '/includes/debug/logger/legacy/Spi.php', 'MWLoggerMonologHandler' => __DIR__ . '/includes/debug/logger/monolog/Handler.php', diff --git a/docs/mwlogger.txt b/docs/mwlogger.txt index aab95992f0..ecc3626db0 100644 --- a/docs/mwlogger.txt +++ b/docs/mwlogger.txt @@ -1,28 +1,30 @@ -MWLogger implements a PSR-3 [0] compatible message logging system. +MWLoggerFactory implements a PSR-3 [0] compatible message logging system. -The MWLogger class is actually a thin wrapper around any PSR-3 LoggerInterface -implementation. Named MWLogger instances can be obtained from the -MWLogger::getInstance() static method. MWLogger expects a class implementing -the MWLoggerSpi interface to act as a factory for new MWLogger instances. +Named Psr\Log\LoggerInterface instances can be obtained from the +MWLoggerFactory::getInstance() static method. MWLoggerFactory expects a class +implementing the MWLoggerSpi interface to act as a factory for new +Psr\Log\LoggerInterface instances. -The "Spi" in MWLoggerSpi stands for "service provider interface". An SPI is -a API intended to be implemented or extended by a third party. This software +The "Spi" in MWLoggerSpi stands for "service provider interface". A SPI is +an API intended to be implemented or extended by a third party. This software design pattern is intended to enable framework extension and replaceable -components. It is specifically used in the MWLogger service to allow alternate -PSR-3 logging implementations to be easily integrated with MediaWiki. +components. It is specifically used in the MWLoggerFactory service to allow +alternate PSR-3 logging implementations to be easily integrated with +MediaWiki. -The MWLogger::getInstance() static method is the means by which most code -acquires an MWLogger instance. This in turn delegates creation of MWLogger -instances to a class implementing the MWLoggerSpi service provider interface. +The MWLoggerFactory::getInstance() static method is the means by which most +code acquires a Psr\Log\LoggerInterface instance. This in turn delegates +creation of Psr\Log\LoggerInterface instances to a class implementing the +MWLoggerSpi service provider interface. The service provider interface allows the backend logging library to be implemented in multiple ways. The $wgMWLoggerDefaultSpi global provides the classname of the default MWLoggerSpi implementation to be loaded at runtime. This can either be the name of a class implementing the MWLoggerSpi with a zero argument constructor or a callable that will return an MWLoggerSpi -instance. Alternately the MWLogger::registerProvider method can be called -to inject an MWLoggerSpi instance into MWLogger and bypass the use of this -configuration variable. +instance. Alternately the MWLoggerFactory::registerProvider method can be +called to inject an MWLoggerSpi instance into MWLoggerFactory and bypass the +use of this configuration variable. The MWLoggerLegacySpi class implements a service provider to generate MWLoggerLegacyLogger instances. The MWLoggerLegacyLogger class implements the @@ -33,18 +35,17 @@ DefaultSettings.php. It's usage should be transparent for users who are not ready or do not wish to switch to a alternate logging platform. The MWLoggerMonologSpi class implements a service provider to generate -MWLogger instances that use the Monolog [1] logging library. See the PHP docs -(or source) for MWLoggerMonologSpi for details on the configuration of this -provider. The default configuration installs a null handler that will silently -discard all logging events. The documentation provided by the class describes -a more feature rich logging configuration. +Psr\Log\LoggerInterface instances that use the Monolog [1] logging library. +See the PHP docs (or source) for MWLoggerMonologSpi for details on the +configuration of this provider. The default configuration installs a null +handler that will silently discard all logging events. The documentation +provided by the class describes a more feature rich logging configuration. == Classes == -; MWLogger -: PSR-3 compatible logger that wraps any \Psr\Log\LoggerInterface - implementation +; MWLoggerFactory +: Factory for Psr\Log\LoggerInterface loggers ; MWLoggerSpi -: Service provider interface for MWLogger factories +: Service provider interface for MWLoggerFactory ; MWLoggerNullSpi : MWLoggerSpi for creating instances that discard all log events ; MWLoggerLegacySpi @@ -64,7 +65,7 @@ a more feature rich logging configuration. == Globals == ; $wgMWLoggerDefaultSpi : Specification for creating the default service provider interface to use - with MWLogger + with MWLoggerFactory [0]: https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-3-logger-interface.md [1]: https://github.com/Seldaek/monolog diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 66f695c8d3..0b0c7d0d18 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -5289,16 +5289,16 @@ $wgDebugDumpSqlLength = 500; $wgDebugLogGroups = array(); /** - * Default service provider for creating MWLogger instances. + * Default service provider for creating Psr\Log\LoggerInterface instances. * * The value should be an array suitable for use with * ObjectFactory::getObjectFromSpec(). The created object is expected to * implement the MWLoggerSpi interface. See ObjectFactory for additional * details. * - * Alternately the MWLogger::registerProvider method can be called to inject - * an MWLoggerSpi instance into MWLogger and bypass the use of this - * configuration variable entirely. + * Alternately the MWLoggerFactory::registerProvider method can be called to + * inject an MWLoggerSpi instance into MWLoggerFactory and bypass the use of + * this configuration variable entirely. * * @since 1.25 * @var array $wgMWLoggerDefaultSpi diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php index e21ae117d7..2025e170e9 100644 --- a/includes/GlobalFunctions.php +++ b/includes/GlobalFunctions.php @@ -1078,7 +1078,7 @@ function wfDebug( $text, $dest = 'all', array $context = array() ) { $context['prefix'] = $wgDebugLogPrefix; } - $logger = MWLogger::getInstance( 'wfDebug' ); + $logger = MWLoggerFactory::getInstance( 'wfDebug' ); $logger->debug( $text, $context ); } @@ -1182,7 +1182,7 @@ function wfDebugLog( MWDebug::debugMsg( "[{$logGroup}] {$text}\n" ); } - $logger = MWLogger::getInstance( $logGroup ); + $logger = MWLoggerFactory::getInstance( $logGroup ); $context['private'] = ( $dest === 'private' ); $logger->info( $text, $context ); } @@ -1196,7 +1196,7 @@ function wfDebugLog( * @param array $context Additional logging context data */ function wfLogDBError( $text, array $context = array() ) { - $logger = MWLogger::getInstance( 'wfLogDBError' ); + $logger = MWLoggerFactory::getInstance( 'wfLogDBError' ); $logger->error( trim( $text ), $context ); } @@ -1259,7 +1259,7 @@ function wfLogWarning( $msg, $callerOffset = 1, $level = E_USER_WARNING ) { */ function wfErrorLog( $text, $file, array $context = array() ) { wfDeprecated( __METHOD__, '1.25' ); - $logger = MWLogger::getInstance( 'wfErrorLog' ); + $logger = MWLoggerFactory::getInstance( 'wfErrorLog' ); $context['destination'] = $file; $logger->info( trim( $text ), $context ); } @@ -1334,7 +1334,7 @@ function wfLogProfilingData() { $ctx['output'] = $profiler->getOutput(); - $log = MWLogger::getInstance( 'profileoutput' ); + $log = MWLoggerFactory::getInstance( 'profileoutput' ); $log->info( "Elapsed: {elapsed}; URL: <{url}>\n{output}", $ctx ); } diff --git a/includes/debug/logger/Factory.php b/includes/debug/logger/Factory.php new file mode 100644 index 0000000000..2660b92adf --- /dev/null +++ b/includes/debug/logger/Factory.php @@ -0,0 +1,115 @@ + + * @copyright © 2014 Bryan Davis and Wikimedia Foundation. + */ +class MWLoggerFactory { + + /** + * Service provider. + * @var MWLoggerSpi $spi + */ + private static $spi; + + + /** + * Register a service provider to create new \Psr\Log\LoggerInterface + * instances. + * + * @param MWLoggerSpi $provider Provider to register + */ + public static function registerProvider( MWLoggerSpi $provider ) { + self::$spi = $provider; + } + + + /** + * Get the registered service provider. + * + * If called before any service provider has been registered, it will + * attempt to use the $wgMWLoggerDefaultSpi global to bootstrap + * MWLoggerSpi registration. $wgMWLoggerDefaultSpi is expected to be an + * array usable by ObjectFactory::getObjectFromSpec() to create a class. + * + * @return MWLoggerSpi + * @see registerProvider() + * @see ObjectFactory::getObjectFromSpec() + */ + public static function getProvider() { + if ( self::$spi === null ) { + global $wgMWLoggerDefaultSpi; + $provider = ObjectFactory::getObjectFromSpec( + $wgMWLoggerDefaultSpi + ); + self::registerProvider( $provider ); + } + return self::$spi; + } + + + /** + * Get a named logger instance from the currently configured logger factory. + * + * @param string $channel Logger channel (name) + * @return \Psr\Log\LoggerInterface + */ + public static function getInstance( $channel ) { + if ( !interface_exists( '\Psr\Log\LoggerInterface' ) ) { + $message = <<PSR-3 logging library to be present. This library is not embedded directly in MediaWiki's git repository and must be installed separately by the end user. + +Please see mediawiki.org for help on installing the required components. +TXT; + echo $message; + trigger_error( $message, E_USER_ERROR ); + die( 1 ); + } + + return self::getProvider()->getLogger( $channel ); + } + + + /** + * Construction of utility class is not allowed. + */ + private function __construct() { + // no-op + } +} diff --git a/includes/debug/logger/Logger.php b/includes/debug/logger/Logger.php deleted file mode 100644 index 960faefd92..0000000000 --- a/includes/debug/logger/Logger.php +++ /dev/null @@ -1,234 +0,0 @@ -PSR-3 logging library to be present. This library is not embedded directly in MediaWiki's git repository and must be installed separately by the end user. - -Please see mediawiki.org for help on installing the required components. -TXT; - echo $message; - trigger_error( $message, E_USER_ERROR ); - die( 1 ); -} - -/** - * PSR-3 logging service. - * - * This class provides a service interface for logging system events. The - * MWLogger class itself is intended to be a thin wrapper around another PSR-3 - * compliant logging library. Creation of MWLogger instances is managed via - * the MWLogger::getInstance() static method which in turn delegates to the - * currently registered service provider. - * - * A service provider is any class implementing the MWLoggerSpi interface. - * There are two possible methods of registering a service provider. The - * MWLogger::registerProvider() static method can be called at any time to - * change the service provider. If MWLogger::getInstance() is called before - * any service provider has been registered, it will attempt to use the - * $wgMWLoggerDefaultSpi global to bootstrap MWLoggerSpi registration. - * $wgMWLoggerDefaultSpi is expected to be an array usable by - * ObjectFactory::getObjectFromSpec() to create a class. - * - * @see MWLoggerSpi - * @since 1.25 - * @author Bryan Davis - * @copyright © 2014 Bryan Davis and Wikimedia Foundation. - */ -class MWLogger implements \Psr\Log\LoggerInterface { - - /** - * Service provider. - * @var MWLoggerSpi $spi - */ - protected static $spi; - - - /** - * Wrapped PSR-3 logger instance. - * - * @var \Psr\Log\LoggerInterface $delegate - */ - protected $delegate; - - - /** - * @param \Psr\Log\LoggerInterface $logger - */ - public function __construct( \Psr\Log\LoggerInterface $logger ) { - $this->delegate = $logger; - } - - - /** - * Logs with an arbitrary level. - * - * @param string|int $level - * @param string $message - * @param array $context - */ - public function log( $level, $message, array $context = array() ) { - $this->delegate->log( $level, $message, $context ); - } - - - /** - * System is unusable. - * - * @param string $message - * @param array $context - */ - public function emergency( $message, array $context = array() ) { - $this->log( \Psr\Log\LogLevel::EMERGENCY, $message, $context ); - } - - - /** - * Action must be taken immediately. - * - * Example: Entire website down, database unavailable, etc. This should - * trigger the SMS alerts and wake you up. - * - * @param string $message - * @param array $context - */ - public function alert( $message, array $context = array() ) { - $this->log( \Psr\Log\LogLevel::ALERT, $message, $context ); - } - - - /** - * Critical conditions. - * - * Example: Application component unavailable, unexpected exception. - * - * @param string $message - * @param array $context - */ - public function critical( $message, array $context = array( ) ) { - $this->log( \Psr\Log\LogLevel::CRITICAL, $message, $context ); - } - - - /** - * Runtime errors that do not require immediate action but should typically - * be logged and monitored. - * - * @param string $message - * @param array $context - */ - public function error( $message, array $context = array( ) ) { - $this->log( \Psr\Log\LogLevel::ERROR, $message, $context ); - } - - - /** - * Exceptional occurrences that are not errors. - * - * Example: Use of deprecated APIs, poor use of an API, undesirable things - * that are not necessarily wrong. - * - * @param string $message - * @param array $context - */ - public function warning( $message, array $context = array() ) { - $this->log( \Psr\Log\LogLevel::WARNING, $message, $context ); - } - - - /** - * Normal but significant events. - * - * @param string $message - * @param array $context - */ - public function notice( $message, array $context = array() ) { - $this->log( \Psr\Log\LogLevel::NOTICE, $message, $context ); - } - - - /** - * Interesting events. - * - * Example: User logs in, SQL logs. - * - * @param string $message - * @param array $context - */ - public function info( $message, array $context = array() ) { - $this->log( \Psr\Log\LogLevel::INFO, $message, $context ); - } - - - /** - * Detailed debug information. - * - * @param string $message - * @param array $context - */ - public function debug( $message, array $context = array() ) { - $this->log( \Psr\Log\LogLevel::DEBUG, $message, $context ); - } - - - /** - * Register a service provider to create new MWLogger instances. - * - * @param MWLoggerSpi $provider Provider to register - */ - public static function registerProvider( MWLoggerSpi $provider ) { - self::$spi = $provider; - } - - - /** - * Get the registered service provider. - * - * If called before any service provider has been registered, it will - * attempt to use the $wgMWLoggerDefaultSpi global to bootstrap - * MWLoggerSpi registration. $wgMWLoggerDefaultSpi is expected to be an - * array usable by ObjectFactory::getObjectFromSpec() to create a class. - * - * @return MWLoggerSpi - * @see registerProvider() - * @see ObjectFactory::getObjectFromSpec() - */ - public static function getProvider() { - if ( self::$spi === null ) { - global $wgMWLoggerDefaultSpi; - $provider = ObjectFactory::getObjectFromSpec( - $wgMWLoggerDefaultSpi - ); - self::registerProvider( $provider ); - } - return self::$spi; - } - - /** - * Get a named logger instance from the currently configured logger factory. - * - * @param string $channel Logger channel (name) - * @return MWLogger - */ - public static function getInstance( $channel ) { - return self::getProvider()->getLogger( $channel ); - } - -} diff --git a/includes/debug/logger/NullSpi.php b/includes/debug/logger/NullSpi.php index f725b64953..617842cc8f 100644 --- a/includes/debug/logger/NullSpi.php +++ b/includes/debug/logger/NullSpi.php @@ -18,9 +18,10 @@ * @file */ + /** - * MWLogger service provider that creates \Psr\Log\NullLogger instances. - * A NullLogger silently discards all log events sent to it. + * MWLoggerFactory service provider that creates \Psr\Log\NullLogger + * instances. A NullLogger silently discards all log events sent to it. * * Usage: * @code @@ -29,7 +30,7 @@ * ); * @endcode * - * @see MWLogger + * @see MWLoggerFactory * @since 1.25 * @author Bryan Davis * @copyright © 2014 Bryan Davis and Wikimedia Foundation. @@ -51,7 +52,7 @@ class MWLoggerNullSpi implements MWLoggerSpi { * Get a logger instance. * * @param string $channel Logging channel - * @return MWLogger Logger instance + * @return \Psr\Log\NullLogger Logger instance */ public function getLogger( $channel ) { return $this->singleton; diff --git a/includes/debug/logger/Spi.php b/includes/debug/logger/Spi.php index cd4af9c706..cd9f17fc98 100644 --- a/includes/debug/logger/Spi.php +++ b/includes/debug/logger/Spi.php @@ -19,14 +19,15 @@ */ /** - * Service provider interface for MWLogger implementation libraries. + * Service provider interface for \Psr\Log\LoggerInterface implementation + * libraries. * * MediaWiki can be configured to use a class implementing this interface to - * create new MWLogger instances via either the $wgMWLoggerDefaultSpi global - * variable or code that constructs an instance and registeres it via the - * MWLogger::registerProvider() static method. + * create new \Psr\Log\LoggerInterface instances via either the + * $wgMWLoggerDefaultSpi global variable or code that constructs an instance + * and registers it via the MWLoggerFactory::registerProvider() static method. * - * @see MWLogger + * @see MWLoggerFactory * @since 1.25 * @author Bryan Davis * @copyright © 2014 Bryan Davis and Wikimedia Foundation. @@ -37,7 +38,7 @@ interface MWLoggerSpi { * Get a logger instance. * * @param string $channel Logging channel - * @return MWLogger Logger instance + * @return \Psr\Log\LoggerInterface Logger instance */ public function getLogger( $channel ); diff --git a/includes/debug/logger/legacy/Logger.php b/includes/debug/logger/legacy/Logger.php index 0737770fb8..be46c27e63 100644 --- a/includes/debug/logger/legacy/Logger.php +++ b/includes/debug/logger/legacy/Logger.php @@ -31,7 +31,7 @@ * See documentation in DefaultSettings.php for detailed explanations of each * variable. * - * @see MWLogger + * @see MWLoggerFactory * @since 1.25 * @author Bryan Davis * @copyright © 2014 Bryan Davis and Wikimedia Foundation. diff --git a/includes/debug/logger/legacy/Spi.php b/includes/debug/logger/legacy/Spi.php index b8813aa587..79d4f24a73 100644 --- a/includes/debug/logger/legacy/Spi.php +++ b/includes/debug/logger/legacy/Spi.php @@ -19,7 +19,8 @@ */ /** - * MWLogger service provider that creates MWLoggerLegacyLogger instances. + * MWLoggerFactory service provider that creates MWLoggerLegacyLogger + * instances. * * Usage: * @code @@ -28,7 +29,7 @@ * ); * @endcode * - * @see MWLogger + * @see MWLoggerFactory * @since 1.25 * @author Bryan Davis * @copyright © 2014 Bryan Davis and Wikimedia Foundation. @@ -45,7 +46,7 @@ class MWLoggerLegacySpi implements MWLoggerSpi { * Get a logger instance. * * @param string $channel Logging channel - * @return MWLogger Logger instance + * @return \Psr\Log\LoggerInterface Logger instance */ public function getLogger( $channel ) { if ( !isset( $this->singletons[$channel] ) ) { diff --git a/includes/debug/logger/monolog/SamplingHandler.php b/includes/debug/logger/monolog/SamplingHandler.php index 6f6aa727ef..a9f83b055f 100644 --- a/includes/debug/logger/monolog/SamplingHandler.php +++ b/includes/debug/logger/monolog/SamplingHandler.php @@ -40,7 +40,7 @@ use Monolog\Formatter\FormatterInterface; * 'class' => 'MWLoggerMonologSamplingHandler', * 'args' => array( * function() { - * return MWLogger::getProvider()->getHandler( 'some-handler'); + * return MWLoggerFactory::getProvider()->getHandler( 'some-handler'); * }, * 2, // emit logs with a 1:2 chance * ), diff --git a/includes/debug/logger/monolog/Spi.php b/includes/debug/logger/monolog/Spi.php index 121dbe4522..68acf1df26 100644 --- a/includes/debug/logger/monolog/Spi.php +++ b/includes/debug/logger/monolog/Spi.php @@ -19,7 +19,8 @@ */ /** - * MWLogger service provider that creates loggers implemented by Monolog. + * MWLoggerFactory service provider that creates loggers implemented by + * Monolog. * * Configured using an array of configuration data with the keys 'loggers', * 'processors', 'handlers' and 'formatters'. @@ -29,8 +30,8 @@ * section. * * Configuration will most typically be provided in the $wgMWLoggerDefaultSpi - * global configuration variable used by MWLogger to construct its default SPI - * provider: + * global configuration variable used by MWLoggerFactory to construct its + * default SPI provider: * @code * $wgMWLoggerDefaultSpi = array( * 'class' => 'MWLoggerMonologSpi', @@ -152,7 +153,7 @@ class MWLoggerMonologSpi implements MWLoggerSpi { * name will return the cached instance. * * @param string $channel Logging channel - * @return MWLogger Logger instance + * @return \Psr\Log\LoggerInterface Logger instance */ public function getLogger( $channel ) { if ( !isset( $this->singletons['loggers'][$channel] ) ) { @@ -163,7 +164,7 @@ class MWLoggerMonologSpi implements MWLoggerSpi { $this->config['loggers']['@default']; $monolog = $this->createLogger( $channel, $spec ); - $this->singletons['loggers'][$channel] = new MWLogger( $monolog ); + $this->singletons['loggers'][$channel] = $monolog; } return $this->singletons['loggers'][$channel]; -- 2.20.1