From 356223226eae5b279afa302590172258aa87109c Mon Sep 17 00:00:00 2001 From: Eric Khun Date: Thu, 12 Dec 2019 02:53:04 +0000 Subject: [PATCH 1/6] only write logs according the verbosity level --- src/BuffLog/BuffLog.php | 45 +++++++++++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/src/BuffLog/BuffLog.php b/src/BuffLog/BuffLog.php index bb35a61..9b471d5 100644 --- a/src/BuffLog/BuffLog.php +++ b/src/BuffLog/BuffLog.php @@ -9,8 +9,14 @@ class BuffLog { protected static $instance; private static $logger = null; + // we output every logs starting this level + // It can be changed with setting an environment + private static $verbosityLevel = Logger::NOTICE; + + // we can use strtolower(Logger::getLevels()) instead private static $logOutputMethods = ['debug', 'info', 'notice', 'warning', 'error', 'critical']; - private static $extraAllowedMethods = ['getName', 'pushHandler', 'setHandlers', 'getHandlers', 'pushProcessor', 'getProcessors']; + + private static $extraAllowedMethods = ['getName', 'pushHandler', 'setHandlers', 'getHandlers', 'pushProcessor', 'getProcessors', 'getLevels']; /** * Method to return the Monolog instance @@ -47,12 +53,9 @@ public static function __callStatic($methodName, $args) if (in_array($methodName, self::$logOutputMethods)) { - // @TODO: need to make sure we "output" only the correct level of log - // old version looked like: - // self::setVerbosity(); - // if (self::$currentVerbosity > Logger::WARNING) { - // return; - // } + if (! self::shouldWriteLogs($methodName)) { + return; + } self::enrichLog(); } @@ -60,7 +63,6 @@ public static function __callStatic($methodName, $args) return call_user_func_array(array(self::getLogger(), $methodName), $args); } else { - error_log("BuffLog::$methodName() is not supported yet. Add it to the BuffLog whitelist to allow it"); } } else { @@ -68,6 +70,33 @@ public static function __callStatic($methodName, $args) } } + private static function setVerbosityLevel() + { + $logLevelFromEnv = getenv("LOG_LEVEL"); + $monologLevels = self::getLogger()->getLevels(); + if ($logLevelFromEnv) { + // only if the level exists, we change the verbosity level + if (key_exists($logLevelFromEnv, $monologLevels)) { + self::$verbosityLevel = $monologLevels[$logLevelFromEnv]; + } else { + error_log("LOG_LEVEL {$logLevelFromEnv} is not defined, use one of: " . implode(', ', array_keys($monologLevels))); + } + } + } + + + private static function shouldWriteLogs($methodName) + { + // Optimization: we could move this at the singleton initialization + self::setVerbosityLevel(); + + if ( self::getLogger()->getLevels()[strtoupper($methodName)] >= self::$verbosityLevel) { + return true; + } + + return false; + } + private static function enrichLog() { // This should probably implemented as a Monolog Processor From c14b8104cb28cb155aff77d0f71bb3303b7fe001 Mon Sep 17 00:00:00 2001 From: Eric Khun Date: Thu, 12 Dec 2019 03:58:12 +0000 Subject: [PATCH 2/6] move LOG_LEVEL definittion to class member --- composer.json | 2 +- src/BuffLog/BuffLog.php | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/composer.json b/composer.json index e433618..cd617ba 100644 --- a/composer.json +++ b/composer.json @@ -1,7 +1,7 @@ { "name": "bufferapp/php-bufflog", "description": "PHP log libraries for Buffer services", - "version": "0.1.1", + "version": "0.1.2", "require": { "php": "^7.1", "monolog/monolog": "^1.20", diff --git a/src/BuffLog/BuffLog.php b/src/BuffLog/BuffLog.php index 9b471d5..e9d4652 100644 --- a/src/BuffLog/BuffLog.php +++ b/src/BuffLog/BuffLog.php @@ -9,14 +9,16 @@ class BuffLog { protected static $instance; private static $logger = null; - // we output every logs starting this level - // It can be changed with setting an environment + // default verbosity starting at this level private static $verbosityLevel = Logger::NOTICE; + // verbosity can be changed with setting this env var + public static $logLevelEnvVar = "LOG_LEVEL"; + // we can use strtolower(Logger::getLevels()) instead - private static $logOutputMethods = ['debug', 'info', 'notice', 'warning', 'error', 'critical']; + private static $logOutputMethods = ['debug', 'info', 'notice', 'warning', 'error', 'critical']; - private static $extraAllowedMethods = ['getName', 'pushHandler', 'setHandlers', 'getHandlers', 'pushProcessor', 'getProcessors', 'getLevels']; + private static $extraAllowedMethods = ['getName', 'pushHandler', 'setHandlers', 'getHandlers', 'pushProcessor', 'getProcessors', 'getLevels']; /** * Method to return the Monolog instance @@ -72,7 +74,7 @@ public static function __callStatic($methodName, $args) private static function setVerbosityLevel() { - $logLevelFromEnv = getenv("LOG_LEVEL"); + $logLevelFromEnv = getenv(self::$logLevelEnvVar); $monologLevels = self::getLogger()->getLevels(); if ($logLevelFromEnv) { // only if the level exists, we change the verbosity level From c8727eaccf1d095ce5a9f8081f1e868dbbb9370d Mon Sep 17 00:00:00 2001 From: Eric Khun Date: Thu, 12 Dec 2019 07:48:29 +0000 Subject: [PATCH 3/6] use StreamHandler parameter to output verbosity --- src/BuffLog/BuffLog.php | 45 ++++++++++++----------------------------- 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/src/BuffLog/BuffLog.php b/src/BuffLog/BuffLog.php index e9d4652..44256c5 100644 --- a/src/BuffLog/BuffLog.php +++ b/src/BuffLog/BuffLog.php @@ -40,7 +40,19 @@ protected static function configureInstance() // define the logger name. This will make it easier for developers // to read and friendlier to identify where come the logs at a glance $logger = new Logger('php-bufflog'); - $handler = new StreamHandler('php://stdout'); + + $logLevelFromEnv = getenv("LOG_LEVEL"); + $monologLevels = $logger->getLevels(); + if ($logLevelFromEnv) { + // only if the level exists, we change the verbosity level + if (key_exists($logLevelFromEnv, $monologLevels)) { + self::$verbosityLevel = $monologLevels[$logLevelFromEnv]; + } else { + error_log("LOG_LEVEL {$logLevelFromEnv} is not defined, use one of: " . implode(', ', array_keys($monologLevels))); + } + } + + $handler = new StreamHandler('php://stdout', self::$verbosityLevel); $handler->setFormatter( new \Monolog\Formatter\JsonFormatter() ); $logger->pushHandler($handler); self::$instance = $logger; @@ -55,10 +67,6 @@ public static function __callStatic($methodName, $args) if (in_array($methodName, self::$logOutputMethods)) { - if (! self::shouldWriteLogs($methodName)) { - return; - } - self::enrichLog(); } // Where the magic happen. We "proxy" functions name with arguments to the Monolog instance @@ -72,33 +80,6 @@ public static function __callStatic($methodName, $args) } } - private static function setVerbosityLevel() - { - $logLevelFromEnv = getenv(self::$logLevelEnvVar); - $monologLevels = self::getLogger()->getLevels(); - if ($logLevelFromEnv) { - // only if the level exists, we change the verbosity level - if (key_exists($logLevelFromEnv, $monologLevels)) { - self::$verbosityLevel = $monologLevels[$logLevelFromEnv]; - } else { - error_log("LOG_LEVEL {$logLevelFromEnv} is not defined, use one of: " . implode(', ', array_keys($monologLevels))); - } - } - } - - - private static function shouldWriteLogs($methodName) - { - // Optimization: we could move this at the singleton initialization - self::setVerbosityLevel(); - - if ( self::getLogger()->getLevels()[strtoupper($methodName)] >= self::$verbosityLevel) { - return true; - } - - return false; - } - private static function enrichLog() { // This should probably implemented as a Monolog Processor From c23b09caf880f411d1185bd0fef2bcce32f91cbc Mon Sep 17 00:00:00 2001 From: Eric Khun Date: Fri, 13 Dec 2019 02:07:15 +0000 Subject: [PATCH 4/6] use member for variable name --- src/BuffLog/BuffLog.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/BuffLog/BuffLog.php b/src/BuffLog/BuffLog.php index 44256c5..509f72e 100644 --- a/src/BuffLog/BuffLog.php +++ b/src/BuffLog/BuffLog.php @@ -41,14 +41,14 @@ protected static function configureInstance() // to read and friendlier to identify where come the logs at a glance $logger = new Logger('php-bufflog'); - $logLevelFromEnv = getenv("LOG_LEVEL"); + $logLevelFromEnv = getenv(self::$logLevelEnvVar); $monologLevels = $logger->getLevels(); if ($logLevelFromEnv) { // only if the level exists, we change the verbosity level if (key_exists($logLevelFromEnv, $monologLevels)) { self::$verbosityLevel = $monologLevels[$logLevelFromEnv]; } else { - error_log("LOG_LEVEL {$logLevelFromEnv} is not defined, use one of: " . implode(', ', array_keys($monologLevels))); + error_log(self::$logLevelEnvVar . "={$logLevelFromEnv} verbosity level does not exists. Please use: " . implode(', ', array_keys($monologLevels))); } } From 33a33e2abc07c7705898fb5a5e6139acf851dfbc Mon Sep 17 00:00:00 2001 From: Eric Khun Date: Fri, 13 Dec 2019 02:07:51 +0000 Subject: [PATCH 5/6] better error message --- src/BuffLog/BuffLog.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BuffLog/BuffLog.php b/src/BuffLog/BuffLog.php index 509f72e..f0ca3e2 100644 --- a/src/BuffLog/BuffLog.php +++ b/src/BuffLog/BuffLog.php @@ -76,7 +76,7 @@ public static function __callStatic($methodName, $args) error_log("BuffLog::$methodName() is not supported yet. Add it to the BuffLog whitelist to allow it"); } } else { - error_log("BuffLog::$methodName() does not exist"); + error_log("BuffLog::$methodName() method does not exist"); } } From 49a9f507a1e1678364b861a95377b6632d5e8a5c Mon Sep 17 00:00:00 2001 From: Eric Khun Date: Fri, 13 Dec 2019 02:10:14 +0000 Subject: [PATCH 6/6] typo --- src/BuffLog/BuffLog.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BuffLog/BuffLog.php b/src/BuffLog/BuffLog.php index f0ca3e2..f6e6171 100644 --- a/src/BuffLog/BuffLog.php +++ b/src/BuffLog/BuffLog.php @@ -96,7 +96,7 @@ private static function enrichLog() // 'profileID' => $user->getProfileID() // ); - // Add traces information to logs to be able correlate with APM + // Add traces information to be able to correlate logs with APM $ddTraceSpan = \DDTrace\GlobalTracer::get()->getActiveSpan(); $record['context']['dd'] = [ "trace_id" => $ddTraceSpan->getTraceId(),