Merge pull request #126 from colinmollenhour/fix-priority-filter

Cleanup config usage, cache config for efficiency and stability. Add …
firegento · Apr 12, 2020 · a9f00ab · a9f00ab
2 parents 4793181 + 8d6af82
commit a9f00ab
Show file tree

Hide file tree

Showing 11 changed files with 109 additions and 67 deletions.
diff --git a/README.md b/README.md
@@ -77,6 +77,7 @@ Configure the different loggers in `System > Configuration > Advanced > Firegent
 * Michael Ryvlin
 * Tobias Zander
 * Achim Rosenhagen
+* Lucas van Staden
 
 ### Current Status of Project
 
@@ -92,6 +93,7 @@ Complete, working logger interfaces:
 - Papertrail (UDP)
 - Chromelogger
 - Logstash
+- Airbrake
 
 It is possible to use **Multiple-Targets**!
 

diff --git a/modman b/modman
@@ -4,6 +4,7 @@ src/app/design/adminhtml/default/default/layout/firegento_logger.xml    app/desi
 src/app/design/adminhtml/default/default/template/firegento_logger/     app/design/adminhtml/default/default/template/firegento_logger/
 src/app/design/frontend/base/default/layout/firegento_logger.xml        app/design/frontend/base/default/layout/firegento_logger.xml
 src/app/design/frontend/base/default/template/firegento_logger/         app/design/frontend/base/default/template/firegento_logger/
+src/lib/Airbrake/                                                       lib/Airbrake/
 src/lib/XMPPHP/                                                         lib/XMPPHP/
 src/lib/Graylog2-gelf-php/                                              lib/Graylog2-gelf-php/
 src/lib/rsyslog/                                                        lib/rsyslog

diff --git a/src/app/code/community/FireGento/Logger/Formatter/Advanced.php b/src/app/code/community/FireGento/Logger/Formatter/Advanced.php
@@ -28,6 +28,9 @@
 class FireGento_Logger_Formatter_Advanced extends Zend_Log_Formatter_Simple
 {
 
+    protected $_maxDataLength;
+    protected $_prettyPrint;
+
     /**
      * Class constructor
      *
@@ -43,6 +46,9 @@ public function __construct($format = null)
             $format = self::DEFAULT_FORMAT;
         }
 
+        $this->_maxDataLength = Mage::helper('firegento_logger')->getLoggerConfig('general/max_data_length') ?: 1000;
+        $this->_prettyPrint = Mage::helper('firegento_logger')->getLoggerConfig('general/pretty_print') && defined('JSON_PRETTY_PRINT') ? JSON_PRETTY_PRINT : 0;
+
         parent::__construct($format . PHP_EOL);
     }
 
@@ -56,16 +62,14 @@ public function format($event)
     {
         Mage::helper('firegento_logger')->addEventMetadata($event, '-', TRUE);
 
-        $maxDataLength = Mage::helper('firegento_logger')->getLoggerConfig('general/max_data_length') ?: 1000;
-        $prettyPrint = Mage::helper('firegento_logger')->getLoggerConfig('general/pretty_print') && defined('JSON_PRETTY_PRINT') ? JSON_PRETTY_PRINT : 0;
-        $output = preg_replace_callback('/%(\w+)%/', function ($match) use ($event, $maxDataLength, $prettyPrint) {
+        $output = preg_replace_callback('/%(\w+)%/', function ($match) use ($event) {
             $value = isset($event[$match[1]]) ? $event[$match[1]] : '-';
             if (is_bool($value)) {
                 return $value ? 'TRUE' : 'FALSE';
             } else if (is_scalar($value) || (is_object($value) && method_exists($value, '__toString'))) {
                 return "$value";
             } else if (is_array($value)) {
-                return substr(@json_encode($value, $prettyPrint), 0, $maxDataLength);
+                return substr(@json_encode($value, $this->_prettyPrint), 0, $this->_maxDataLength);
             } else if (is_scalar($value)) {
                 return "$value";
             } else {

diff --git a/src/app/code/community/FireGento/Logger/Helper/Data.php b/src/app/code/community/FireGento/Logger/Helper/Data.php
@@ -30,15 +30,23 @@ class FireGento_Logger_Helper_Data extends Mage_Core_Helper_Abstract
     const XML_PATH_PRIORITY = 'general/priority';
     const XML_PATH_MAX_DAYS = 'db/max_days_to_keep';
 
-    /**
-     * @var null
-     */
-    protected $_targetMap = null;
-
-    /**
-     * @var null
-     */
-    protected $_notificationRules = null;
+    protected $_targets;
+    protected $_targetMap;
+    protected $_notificationRules;
+    protected $_maxBacktraceLines;
+    protected $_maxDataLength;
+    protected $_prettyPrint;
+    protected $_addSessionData;
+    protected $_keysToFilter;
+
+    public function __construct()
+    {
+        $this->_maxBacktraceLines = (int) $this->getLoggerConfig('general/max_backtrace_lines');
+        $this->_maxDataLength = $this->getLoggerConfig('general/max_data_length') ?: 1000;
+        $this->_prettyPrint = $this->getLoggerConfig('general/pretty_print') && defined('JSON_PRETTY_PRINT') ? JSON_PRETTY_PRINT : 0;
+        $this->_addSessionData = $this->getLoggerConfig('general/add_session_data');
+        $this->_keysToFilter = explode("\n", $this->getLoggerConfig('general/filter_request_data'));
+    }
 
     /**
      * Return a random id for this request
@@ -62,9 +70,21 @@ public function getRequestId()
      */
     public function getLoggerConfig($path)
     {
+        // Do not use Mage::getStoreConfig so that logger may work when db config is not loaded
         return (string) Mage::getConfig()->getNode('default/logger/'.$path);
     }
 
+    /**
+     * @return array
+     */
+    public function getAllTargets()
+    {
+        if ($this->_targets === NULL) {
+            $this->_targets = explode(',', $this->getLoggerConfig('general/targets'));
+        }
+        return $this->_targets;
+    }
+
     /**
      * Returns an array of targets mapped or null if there was an error or there is no map.
      * Keys are target codes, values are bool indicating if backtrace is enabled
@@ -76,25 +96,22 @@ public function getMappedTargets($filename)
     {
         if ($this->_targetMap === null) {
             $targetMap = $this->getLoggerConfig('general/target_map');
-            if ($targetMap) {
-                $this->_targetMap = @unserialize($targetMap);
+            if ($targetMap && ($targetMap = @unserialize($targetMap))) {
+                $targets = array();
+                foreach ($targetMap as $map) {
+                    if (@preg_match('/^'.$map['pattern'].'$/', $filename)) {
+                        $targets[$map['target']] = (int) $map['backtrace'];
+                        if ((int) $map['stop_on_match']) {
+                            break;
+                        }
+                    }
+                }
+                $this->_targetMap = $targetMap;
             } else {
                 $this->_targetMap = false;
             }
         }
-        if (! $this->_targetMap) {
-            return null;
-        }
-        $targets = array();
-        foreach ($this->_targetMap as $map) {
-            if (@preg_match('/^'.$map['pattern'].'$/', $filename)) {
-                $targets[$map['target']] = (int) $map['backtrace'];
-                if ((int) $map['stop_on_match']) {
-                    break;
-                }
-            }
-        }
-        return $targets;
+        return $this->_targetMap;
     }
 
     /**
@@ -125,7 +142,7 @@ public function addPriorityFilter(Zend_Log_Writer_Abstract $writer, $configPath
         if ( ! $configPath || ! strlen($priority)) {
             $priority = $this->getLoggerConfig(self::XML_PATH_PRIORITY);
         }
-        if ($priority !== null && $priority != Zend_Log::WARN) {
+        if ($priority !== null) {
             $writer->addFilter(new Zend_Log_Filter_Priority((int) $priority));
         }
     }
@@ -174,16 +191,13 @@ public function addEventMetadata(&$event, $notAvailable = null, $enableBacktrace
             $basePath = dirname(Mage::getBaseDir()).'/'; // 1 level up in case deployed with symlinks from parent directory
             $nextIsFirst = false;                        // Skip backtrace frames until we reach Mage::log(Exception)
             $recordBacktrace = false;
-            $maxBacktraceLines = (int) $this->getLoggerConfig('general/max_backtrace_lines');
-            $maxDataLength = $this->getLoggerConfig('general/max_data_length') ?: 1000;
-            $prettyPrint = $this->getLoggerConfig('general/pretty_print') && defined('JSON_PRETTY_PRINT') ? JSON_PRETTY_PRINT : 0;
 
             $backtraceFrames = array();
             if (version_compare(PHP_VERSION, '5.4.0') < 0) {
                 $debugBacktrace = debug_backtrace();
             } else {
                 $debugBacktrace = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT,
-                    $maxBacktraceLines + 10
+                    $this->_maxBacktraceLines + 10
                 );
             }
             array_shift($debugBacktrace); // FireGento_Logger_Helper_Data::addEventMetadata
@@ -209,7 +223,7 @@ public function addEventMetadata(&$event, $notAvailable = null, $enableBacktrace
                         $event
                             ->setFile(str_replace($basePath, '', $frame['file']))
                             ->setLine($frame['line']);
-                        if ($maxBacktraceLines) {
+                        if ($this->_maxBacktraceLines) {
                             $backtraceFrames = array($frame);
                         } elseif ($nextIsFirst) {
                             break;
@@ -232,7 +246,7 @@ public function addEventMetadata(&$event, $notAvailable = null, $enableBacktrace
                 }
 
                 if ($recordBacktrace) {
-                    if (count($backtraceFrames) >= $maxBacktraceLines) {
+                    if (count($backtraceFrames) >= $this->_maxBacktraceLines) {
                         break;
                     }
 
@@ -278,22 +292,22 @@ public function addEventMetadata(&$event, $notAvailable = null, $enableBacktrace
         // Fetch request data
         $requestData = array();
         if (!empty($_GET)) {
-            $requestData[] = '  GET|'.substr(@json_encode($this->filterSensibleData($_GET), $prettyPrint), 0, $maxDataLength);
+            $requestData[] = '  GET|'.substr(@json_encode($this->filterSensibleData($_GET), $this->_prettyPrint), 0, $this->_maxDataLength);
         }
         if (!empty($_POST)) {
-            $requestData[] = '  POST|'.substr(@json_encode($this->filterSensibleData($_POST), $prettyPrint), 0, $maxDataLength);
+            $requestData[] = '  POST|'.substr(@json_encode($this->filterSensibleData($_POST), $this->_prettyPrint), 0, $this->_maxDataLength);
         }
         if (!empty($_FILES)) {
-            $requestData[] = '  FILES|'.substr(@json_encode($_FILES, $prettyPrint), 0, $maxDataLength);
+            $requestData[] = '  FILES|'.substr(@json_encode($_FILES, $this->_prettyPrint), 0, $this->_maxDataLength);
         }
         if (Mage::registry('raw_post_data')) {
-            $requestData[] = '  RAWPOST|'.substr(Mage::registry('raw_post_data'), 0, $maxDataLength);
+            $requestData[] = '  RAWPOST|'.substr(Mage::registry('raw_post_data'), 0, $this->_maxDataLength);
         }
         $event->setRequestData($requestData ? implode("\n", $requestData) : $notAvailable);
 
         // Add session data if enabled
-        if ($this->getLoggerConfig('general/add_session_data')) {
-            $event->setSessionData(empty($_SESSION) ? $notAvailable : substr(@json_encode($_SESSION, $prettyPrint), 0, $maxDataLength));
+        if ($this->_addSessionData) {
+            $event->setSessionData(empty($_SESSION) ? $notAvailable : substr(@json_encode($_SESSION, $this->_prettyPrint), 0, $this->_maxDataLength));
         }
 
         if (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
@@ -321,9 +335,7 @@ public function addEventMetadata(&$event, $notAvailable = null, $enableBacktrace
     private function filterSensibleData($data)
     {
         if (is_array($data)) {
-            $keysToFilter = explode("\n",
-                $this->getLoggerConfig('general/filter_request_data'));
-            foreach ($keysToFilter as $key) {
+            foreach ($this->_keysToFilter as $key) {
                 $key = trim($key);
                 if ($key !== '') {
                     $subkeys = explode('.', $key);

diff --git a/src/app/code/community/FireGento/Logger/Model/Airbrake.php b/src/app/code/community/FireGento/Logger/Model/Airbrake.php
@@ -35,9 +35,12 @@
 class FireGento_Logger_Model_Airbrake extends Zend_Log_Writer_Abstract
 {
 
+    protected $_apiKey;
+
     public function __construct()
     {
-        $apiKey = Mage::getStoreConfig('logger/airbrake/apikey');
+        $helper = Mage::helper('firegento_logger');
+        $this->_apiKey = $helper->getLoggerConfig('airbrake/apikey');
 
         if ($this->isDisabled()) {
             return;
@@ -58,11 +61,11 @@ public function __construct()
         $projectRoot = explode('/', $_SERVER['PHP_SELF']);
         array_pop($projectRoot);
         $options['projectRoot'] = implode('/', $projectRoot) . '/';
-        $options['host'] = Mage::getStoreConfig('logger/airbrake/host');
-        $options['secure'] = Mage::getStoreConfig('logger/airbrake/secure');
-        $options['environmentName'] = Mage::getStoreConfig('logger/airbrake/environment');
-        $options['timeout'] = Mage::getStoreConfig('logger/airbrake/timeout');
-        $config = new Airbrake\Configuration($apiKey, $options);
+        $options['host'] = $helper->getLoggerConfig('airbrake/host');
+        $options['secure'] = $helper->getLoggerConfig('airbrake/secure');
+        $options['environmentName'] = $helper->getLoggerConfig('airbrake/environment');
+        $options['timeout'] = $helper->getLoggerConfig('airbrake/timeout');
+        $config = new Airbrake\Configuration($this->_apiKey, $options);
         $this->client = new Airbrake\Client($config);
     }
 
@@ -81,8 +84,7 @@ protected function _write($event)
 
     protected function isDisabled()
     {
-        $apiKey = Mage::getStoreConfig('logger/airbrake/apikey');
-        if (strlen(trim($apiKey)) == 0) {
+        if (strlen(trim($this->_apiKey)) == 0) {
             return true;
         }
         return false;

diff --git a/src/app/code/community/FireGento/Logger/Model/Observer.php b/src/app/code/community/FireGento/Logger/Model/Observer.php
@@ -174,7 +174,7 @@ public function initLoggerClient(Varien_Event_Observer $observer)
             $done = TRUE;
 
             // Install logger clients if needed
-            $targets = explode(',', Mage::helper('firegento_logger')->getLoggerConfig('general/targets'));
+            $targets = Mage::helper('firegento_logger')->getAllTargets();
 
             // Allow Sentry to capture all errors, not just Mage::log
             if (in_array('sentry', $targets)) {

diff --git a/src/app/code/community/FireGento/Logger/Model/Queue.php b/src/app/code/community/FireGento/Logger/Model/Queue.php
@@ -64,11 +64,11 @@ public function __construct($filename)
         $helper = Mage::helper('firegento_logger');;
 
         // Only instantiate writers that are needed for this file based on the Filename Filters
-        $targets = explode(',', $helper->getLoggerConfig('general/targets'));
+        $targets = $helper->getAllTargets();
         if ($targets) {
             $logDir = Mage::getBaseDir('var') . DS . 'log' . DS;
             $mappedTargets = $helper->getMappedTargets(substr($filename, strlen($logDir)));
-            if ($mappedTargets === null) { // No filters, enable backtrace for all targets
+            if ($mappedTargets === false) { // No filters, enable backtrace for all targets
                 $mappedTargets = array_fill_keys($targets, true);
             } else {
                 $targets = array_intersect($targets, array_keys($mappedTargets));

diff --git a/src/app/code/community/FireGento/Logger/Model/Rsyslog.php b/src/app/code/community/FireGento/Logger/Model/Rsyslog.php
@@ -162,7 +162,9 @@ public function __construct($filename)
     protected function _write($event)
     {
         $event = Mage::helper('firegento_logger')->getEventObjectFromArray($event);
-
+        if(!$event->getTimestamp()) {
+            $event->setTimestamp(now());
+        }
         $message = $this->buildSysLogMessage($event);
         return $this->publishMessage($message);
     }

diff --git a/src/app/code/community/FireGento/Logger/Model/Sentry.php b/src/app/code/community/FireGento/Logger/Model/Sentry.php
@@ -48,12 +48,10 @@ class FireGento_Logger_Model_Sentry extends FireGento_Logger_Model_Abstract
     ];
 
     protected $_fileName;
-    protected $_priorityFilter;
 
     public function __construct($fileName = NULL)
     {
         $this->_fileName = $fileName ? basename($fileName) : NULL;
-        $this->_priorityFilter = (int)Mage::helper('firegento_logger')->getLoggerConfig('sentry/priority');
     }
 
     /**
@@ -69,16 +67,20 @@ public function getRavenClient()
     /**
      * Create Raven_Client instance
      *
-     * @return void
+     * @return bool
      * @throws Raven_Exception
      */
     public function initRavenClient()
     {
         if (is_null(self::$_ravenClient)) {
-            require_once Mage::getBaseDir('lib') . DS . 'sentry' . DS . 'lib' . DS . 'Raven' . DS . 'Autoloader.php';
-            spl_autoload_register(array('Raven_Autoloader', 'autoload'), true, true);
             $helper             = Mage::helper('firegento_logger');
             $dsn                = $helper->getLoggerConfig('sentry/public_dsn');
+            if ( ! $dsn) {
+                self::$_ravenClient = FALSE;
+                return FALSE;
+            }
+            require_once Mage::getBaseDir('lib') . DS . 'sentry' . DS . 'lib' . DS . 'Raven' . DS . 'Autoloader.php';
+            spl_autoload_register(array('Raven_Autoloader', 'autoload'), true, true);
             $options            = [
                 'trace'       => $this->_enableBacktrace,
                 'curl_method' => $helper->getLoggerConfig('sentry/curl_method'),
@@ -92,6 +94,7 @@ public function initRavenClient()
             $error_handler = new Raven_ErrorHandler(self::$_ravenClient, false);
             $error_handler->registerShutdownFunction();
         }
+        return !!self::$_ravenClient;
     }
 
     /**
@@ -110,7 +113,9 @@ protected function _write($event)
         try {
             Mage::helper('firegento_logger')->addEventMetadata($event, NULL, $this->_enableBacktrace);
 
-            $this->initRavenClient();
+            if ( ! $this->initRavenClient()) {
+                return;
+            }
 
             /**
              * Get message priority
@@ -120,11 +125,6 @@ protected function _write($event)
             }
             $priority = isset($event['priority']) ? $event['priority'] : 3;
 
-            // If priority is high enough, send to Sentry
-            if ($priority > $this->_priorityFilter) {
-                return;
-            }
-
             //
             // Add extra data and tags
             //

diff --git a/src/app/code/community/FireGento/Logger/etc/config.xml b/src/app/code/community/FireGento/Logger/etc/config.xml
@@ -235,6 +235,9 @@ confirmation]]></filter_request_data>
             <advanced>
                 <priority>default</priority>
             </advanced>
+            <json>
+                <priority>default</priority>
+            </json>
             <mailconfig>
                 <priority>default</priority>
             </mailconfig>