#6622 Logger refactoring: turned Piwik's log writers into Monolog Handlers

7 files changed, 55 insertions, 101 deletions

diff --git a/core/Log.php b/core/Log.php
index 44e64278fb..3f73f5dc24 100644
--- a/core/Log.php
+++ b/core/Log.php
@@ -8,6 +8,8 @@
 
 namespace Piwik;
 
+use Monolog\Handler\AbstractHandler;
+use Monolog\Handler\HandlerInterface;
 use Monolog\Logger;
 use Piwik\Container\StaticContainer;
 use Piwik\Db;
@@ -137,12 +139,12 @@ class Log extends Singleton
     private $processors = array();
 
     /**
-     * The array of callbacks executed when logging a message. Each callback writes a log
+     * The array of handlers called when logging a message. Each handler writes a log
      * message to a logging backend.
      *
-     * @var callable[]
+     * @var HandlerInterface[]
      */
-    private $writers = array();
+    private $handlers = array();
 
     public static function getInstance()
     {
@@ -161,13 +163,13 @@ class Log extends Singleton
     }
 
     /**
-     * @param callable[] $writers
+     * @param HandlerInterface[] $handlers
      * @param int $logLevel
      * @param callable[] $processors
      */
-    public function __construct(array $writers, $logLevel, array $processors)
+    public function __construct(array $handlers, $logLevel, array $processors)
     {
-        $this->writers = $writers;
+        $this->handlers = $handlers;
         $this->currentLogLevel = $logLevel;
         $this->processors = $processors;
     }
@@ -235,9 +237,21 @@ class Log extends Singleton
         self::logMessage(self::VERBOSE, $message, array_slice(func_get_args(), 1));
     }
 
+    /**
+     * @param int $logLevel
+     * @deprecated We should remove the log level from this class entirely
+     */
     public function setLogLevel($logLevel)
     {
         $this->currentLogLevel = $logLevel;
+
+        // Apply the log level to all handlers
+        $logLevel = self::getMonologLevel($logLevel);
+        foreach ($this->handlers as $handler) {
+            if ($handler instanceof AbstractHandler) {
+                $handler->setLevel($logLevel);
+            }
+        }
     }
 
     public function getLogLevel()
@@ -247,10 +261,6 @@ class Log extends Singleton
 
     private function doLog($level, $message, $parameters = array())
     {
-        if (!$this->shouldLoggerLog($level)) {
-            return;
-        }
-
         // Create a record similar to Monolog to ease future transition
         $record = array(
             'message'    => $message,
@@ -266,8 +276,10 @@ class Log extends Singleton
             $record = $processor($record);
         }
 
-        foreach ($this->writers as $writer) {
-            call_user_func($writer, $record, $this);
+        foreach ($this->handlers as $handler) {
+            if ($handler->isHandling($record)) {
+                $handler->handle($record);
+            }
         }
     }
 
@@ -276,11 +288,6 @@ class Log extends Singleton
         self::getInstance()->doLog($level, $message, $parameters);
     }
 
-    private function shouldLoggerLog($level)
-    {
-        return $level <= $this->currentLogLevel;
-    }
-
     private function getStringLevel($level)
     {
         static $levelToName = array(
@@ -294,7 +301,7 @@ class Log extends Singleton
         return $levelToName[$level];
     }
 
-    private function getMonologLevel($level)
+    public static function getMonologLevel($level)
     {
         switch ($level) {
             case self::ERROR:
diff --git a/core/Log/Backend/DatabaseBackend.php b/core/Log/Backend/DatabaseBackend.php
index 0dc707df78..8b25c3afa0 100644
--- a/core/Log/Backend/DatabaseBackend.php
+++ b/core/Log/Backend/DatabaseBackend.php
@@ -8,6 +8,7 @@
 
 namespace Piwik\Log\Backend;
 
+use Monolog\Handler\AbstractProcessingHandler;
 use Piwik\Common;
 use Piwik\Db;
 use Piwik\Log;
@@ -15,20 +16,16 @@ use Piwik\Log;
 /**
  * Writes log to database.
  */
-class DatabaseBackend extends Backend
+class DatabaseBackend extends AbstractProcessingHandler
 {
-    public function __invoke(array $record, Log $logger)
+    protected function write(array $record)
     {
-        $message = $this->formatMessage($record, $logger);
-
-        if (empty($message)) {
-            return;
-        }
+        $record = $record['formatted'];
 
         $sql = "INSERT INTO " . Common::prefixTable('logger_message')
             . " (tag, timestamp, level, message)"
             . " VALUES (?, ?, ?, ?)";
 
-        Db::query($sql, array($record['extra']['class'], $record['time']->format('Y-m-d H:i:s'), $record['level_name'], (string) $message));
+        Db::query($sql, array($record['extra']['class'], $record['time']->format('Y-m-d H:i:s'), $record['level_name'], (string) $record['message']));
     }
 }
diff --git a/core/Log/Backend/FileBackend.php b/core/Log/Backend/FileBackend.php
index 2ffcea1e41..2a8a117ebe 100644
--- a/core/Log/Backend/FileBackend.php
+++ b/core/Log/Backend/FileBackend.php
@@ -8,14 +8,15 @@
 
 namespace Piwik\Log\Backend;
 
+use Monolog\Handler\AbstractProcessingHandler;
+use Monolog\Logger;
 use Piwik\Filechecks;
 use Piwik\Log;
-use Piwik\Log\Formatter\Formatter;
 
 /**
  * Writes log to file.
  */
-class FileBackend extends Backend
+class FileBackend extends AbstractProcessingHandler
 {
     /**
      * Path to the file to log to.
@@ -24,17 +25,16 @@ class FileBackend extends Backend
      */
     private $logToFilePath;
 
-    public function __construct(Formatter $formatter, $logToFilePath)
+    public function __construct($logToFilePath, $level = Logger::DEBUG)
     {
         $this->logToFilePath = $logToFilePath;
 
-        parent::__construct($formatter);
+        parent::__construct($level);
     }
 
-    public function __invoke(array $record, Log $logger)
+    protected function write(array $record)
     {
-        $message = $this->formatMessage($record, $logger);
-        $message = str_replace("\n", "\n  ", $message) . "\n";
+        $message = str_replace("\n", "\n  ", $record['formatted']['message']) . "\n";
 
         if (!@file_put_contents($this->logToFilePath, $message, FILE_APPEND)
             && !defined('PIWIK_TEST_MODE')
diff --git a/core/Log/Backend/StdErrBackend.php b/core/Log/Backend/StdErrBackend.php
index 32443322f6..a173823dba 100644
--- a/core/Log/Backend/StdErrBackend.php
+++ b/core/Log/Backend/StdErrBackend.php
@@ -8,34 +8,36 @@
 
 namespace Piwik\Log\Backend;
 
+use Monolog\Formatter\FormatterInterface;
+use Monolog\Handler\AbstractProcessingHandler;
 use Monolog\Logger;
 use Piwik\Log;
-use Piwik\Log\Formatter\Formatter;
 
 /**
  * Writes log to stderr.
  */
-class StdErrBackend extends Backend
+class StdErrBackend extends AbstractProcessingHandler
 {
     /**
      * @var bool
      */
     private $isLoggingToStdOut;
 
-    public function __construct(Formatter $formatter, $isLoggingToStdOut)
+    public function __construct(FormatterInterface $formatter, $isLoggingToStdOut)
     {
         $this->isLoggingToStdOut = $isLoggingToStdOut;
 
-        parent::__construct($formatter);
+        // Log level is hardcoded for this one
+        $level = Logger::ERROR;
+
+        parent::__construct($level);
+
+        $this->setFormatter($formatter);
     }
 
-    public function __invoke(array $record, Log $logger)
+    protected function write(array $record)
     {
-        if ($record['level'] < Logger::ERROR) {
-            return;
-        }
-
-        $message = $this->formatMessage($record, $logger) . "\n";
+        $message = $record['formatted']['message'] . "\n";
 
         // Do not log on stderr during tests (prevent display of errors in CI output)
         if (! defined('PIWIK_TEST_MODE')) {
diff --git a/core/Log/Backend/StdOutBackend.php b/core/Log/Backend/StdOutBackend.php
index fd0a89c5ec..0b704a12c8 100644
--- a/core/Log/Backend/StdOutBackend.php
+++ b/core/Log/Backend/StdOutBackend.php
@@ -8,15 +8,16 @@
 
 namespace Piwik\Log\Backend;
 
+use Monolog\Handler\AbstractProcessingHandler;
 use Piwik\Log;
 
 /**
  * Writes log to stdout.
  */
-class StdOutBackend extends Backend
+class StdOutBackend extends AbstractProcessingHandler
 {
-    public function __invoke(array $record, Log $logger)
+    protected function write(array $record)
     {
-        echo $this->formatMessage($record, $logger) . "\n";
+        echo $record['formatted']['message'] . "\n";
     }
 }
diff --git a/core/Log/Formatter/ErrorHtmlFormatter.php b/core/Log/Formatter/ErrorHtmlFormatter.php
index 6b4bbe3ba3..619e1e0630 100644
--- a/core/Log/Formatter/ErrorHtmlFormatter.php
+++ b/core/Log/Formatter/ErrorHtmlFormatter.php
@@ -40,7 +40,7 @@ class ErrorHtmlFormatter extends Formatter
         Common::sendHeader('Content-Type: text/html; charset=utf-8');
 
         $html = '';
-        $html .= "\n<div style='word-wrap: break-word; border: 3px solid red; padding:4px; width:70%; background-color:#FFFF96;'>
+        $html .= "<div style='word-wrap: break-word; border: 3px solid red; padding:4px; width:70%; background-color:#FFFF96;'>
         <strong>There is an error. Please report the message (Piwik " . (class_exists('Piwik\Version') ? Version::VERSION : '') . ")
         and full backtrace in the <a href='?module=Proxy&action=redirect&url=http://forum.piwik.org' target='_blank'>Piwik forums</a> (please do a Search first as it might have been reported already!).<br /><br/>
         ";
diff --git a/core/Log/LoggerFactory.php b/core/Log/LoggerFactory.php
index 0141187780..f9f437d468 100644
--- a/core/Log/LoggerFactory.php
+++ b/core/Log/LoggerFactory.php
@@ -9,7 +9,6 @@
 namespace Piwik\Log;
 
 use Interop\Container\ContainerInterface;
-use Piwik\Common;
 use Piwik\Config;
 use Piwik\Log;
 use Piwik\Log\Backend\StdErrBackend;
@@ -25,31 +24,13 @@ class LoggerFactory
     {
         $logConfig = Config::getInstance()->log;
 
-        $logLevel = self::getLogLevel($logConfig, $container);
+        $logLevel = $container->get('log.level.piwik');
         $writers = self::getLogWriters($logConfig, $container);
         $processors = $container->get('log.processors');
 
         return new Log($writers, $logLevel, $processors);
     }
 
-    private static function getLogLevel($logConfig, ContainerInterface $container)
-    {
-        if (self::isLoggingDisabled($logConfig)) {
-            return Log::NONE;
-        }
-
-        if ($container->has('old_config.log.log_level')) {
-            $configLogLevel = self::getLogLevelFromStringName($container->get('old_config.log.log_level'));
-
-            // sanity check
-            if ($configLogLevel >= Log::NONE && $configLogLevel <= Log::VERBOSE) {
-                return $configLogLevel;
-            }
-        }
-
-        return Log::WARN;
-    }
-
     private static function getLogWriters($logConfig, ContainerInterface $container)
     {
         $writerNames = @$logConfig[Log::LOG_WRITERS_CONFIG_OPTION];
@@ -83,40 +64,6 @@ class LoggerFactory
         return $writers;
     }
 
-    private static function isLoggingDisabled($logConfig)
-    {
-        if (!empty($logConfig['log_only_when_cli']) && !Common::isPhpCliMode()) {
-            return true;
-        }
-
-        if (!empty($logConfig['log_only_when_debug_parameter']) && !isset($_REQUEST['debug'])) {
-            return true;
-        }
-
-        return false;
-    }
-
-    private static function getLogLevelFromStringName($name)
-    {
-        $name = strtoupper($name);
-        switch ($name) {
-            case 'NONE':
-                return Log::NONE;
-            case 'ERROR':
-                return Log::ERROR;
-            case 'WARN':
-                return Log::WARN;
-            case 'INFO':
-                return Log::INFO;
-            case 'DEBUG':
-                return Log::DEBUG;
-            case 'VERBOSE':
-                return Log::VERBOSE;
-            default:
-                return -1;
-        }
-    }
-
     private static function getAvailableWriters()
     {
         $writers = array();