From 3d2676786fd9bbfcca88dc98044048bab924c26a Mon Sep 17 00:00:00 2001 From: Alexey Shishkov Date: Mon, 18 May 2020 14:56:49 +0300 Subject: [PATCH] Bugfixes and refactoring (#38) * Fixed passing additional data (tags, extra, etc) to exception handler Added passing user data * Updated tests for use global Yii object * Fixed passing extra data * Removed old levels * Fixed setting ip if ip is null * Updated e2e tests to test logging user id and ip * Sentry init moved to constructor * Updated tests for configureScope() check * Added note about additional context * Added some notes * Fixed typo * Updated changelog * Fixed version * Context configuring moved to usage * Restored old methods * Changed version * Restored tests for deprecated method * Bootstrap setting moved to root according with deprecation in codeception 3 * Added scripts to composer for run tests * Added messages for e2e test command * Remove return type for bc * Changed release date * Changed release date --- .travis.yml | 2 +- CHANGELOG.md | 11 +++ README.md | 26 +++++ codeception.yml | 2 +- composer.json | 6 +- src/SentryTarget.php | 145 ++++++++++++++++++---------- tests/commands/SentryController.php | 62 ++++++++---- tests/models/User.php | 64 ++++++++++++ tests/sentry-fill | 53 ++++++++++ tests/unit.suite.yml | 3 + tests/yii | 36 ------- 11 files changed, 303 insertions(+), 107 deletions(-) create mode 100644 tests/models/User.php create mode 100644 tests/sentry-fill delete mode 100644 tests/yii diff --git a/.travis.yml b/.travis.yml index bfa5b43..4fdf53e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,4 +16,4 @@ install: script: - composer exec codecept build -v - - composer exec codecept run -v + - composer test -v diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ecadce..4dc4d72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Change log +## 1.5.0-beta - 2020-05-18 +### Fixed +* Fix message level (debug, info, warning, error) translating to sentry. +* Fix message scope. For now every message has own scope and not affect to other. +* Fix adding additional data (extra context, user data) for exception messages. +### Changed +* Sentry init will be invoking at application start, and not before log export started. +### Added +* Log user ID and IP, if available. +* Added ability to add own context data for messages scope. + ## 1.4.2 - 2020-01-21 ### Fixed * Array export fix if text not contains message key. diff --git a/README.md b/README.md index 321d3ef..98c5f23 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,32 @@ Example: More about tags see https://docs.sentry.io/learn/context/#tagging-events +### Additional context + +You can add additional context (such as user information, fingerprint, etc) by calling `\Sentry\configureScope()` before logging. +For example in main configuration on `beforeAction` event (real place will dependant on your project): +```php +return [ + // ... + 'on beforeAction' => function (\yii\base\ActionEvent $event) { + /** @var \yii\web\User $user */ + $user = Yii::$app->has('user', true) ? Yii::$app->get('user', false) : null; + if ($user && ($identity = $user->getIdentity(false))) { + \Sentry\configureScope(function (\Sentry\State\Scope $scope) use ($identity) { + $scope->setUser([ + // User ID and IP will be added by logger automatically + 'username' => $identity->username, + 'email' => $identity->email, + ], true); // Don't forget to set second param of setUser to true for merging data + }); + } + + return $event->isValid; + }, + // ... +]; +``` + ## Log levels Yii2 log levels converts to Sentry levels: diff --git a/codeception.yml b/codeception.yml index 66ceb22..9468ed4 100644 --- a/codeception.yml +++ b/codeception.yml @@ -1,4 +1,5 @@ actor: Tester +bootstrap: bootstrap.php paths: tests: tests log: tests/_output @@ -6,7 +7,6 @@ paths: support: tests/_support envs: tests/_envs settings: - bootstrap: bootstrap.php colors: true memory_limit: 1024M extensions: diff --git a/composer.json b/composer.json index c98f51a..e96ed57 100644 --- a/composer.json +++ b/composer.json @@ -31,5 +31,9 @@ "type": "composer", "url": "https://asset-packagist.org" } - ] + ], + "scripts": { + "test": "codecept run", + "e2e": "php tests/sentry-fill" + } } diff --git a/src/SentryTarget.php b/src/SentryTarget.php index b7db819..2bd9a77 100644 --- a/src/SentryTarget.php +++ b/src/SentryTarget.php @@ -6,9 +6,15 @@ namespace notamedia\sentry; +use Sentry\Severity; +use Sentry\State\Scope; +use Throwable; +use Yii; use yii\helpers\ArrayHelper; use yii\log\Logger; use yii\log\Target; +use yii\web\Request; +use yii\web\User; /** * SentryTarget records log messages in a Sentry. @@ -33,19 +39,15 @@ class SentryTarget extends Target * @var callable Callback function that can modify extra's array */ public $extraCallback; - /** - * @var \Sentry - */ - protected $client; /** - * @inheritdoc + * @inheritDoc */ - public function collect($messages, $final) + public function __construct($config = []) { - \Sentry\init(array_merge(['dsn' => $this->dsn], $this->clientOptions)); + parent::__construct($config); - parent::collect($messages, $final); + \Sentry\init(array_merge(['dsn' => $this->dsn], $this->clientOptions)); } /** @@ -62,69 +64,82 @@ protected function getContextMessage() public function export() { foreach ($this->messages as $message) { - list($text, $level, $category, $timestamp, $traces) = $message; + [$text, $level, $category] = $message; $data = [ - 'level' => static::getLevelName($level), 'message' => '', - 'timestamp' => $timestamp, - 'tags' => ['category' => $category] + 'tags' => ['category' => $category], + 'extra' => [], + 'userData' => [], ]; - if ($text instanceof \Throwable || $text instanceof \Exception) { - $data = $this->runExtraCallback($text, $data); - \Sentry\captureException($text, $data); - continue; - } elseif (is_array($text)) { - if (isset($text['msg'])) { - $data['message'] = $text['msg']; - unset($text['msg']); - } + $request = Yii::$app->getRequest(); + if ($request instanceof Request && $request->getUserIP()) { + $data['userData']['ip_address'] = $request->getUserIP(); + } - if (isset($text['tags'])) { - $data['tags'] = ArrayHelper::merge($data['tags'], $text['tags']); - \Sentry\configureScope(function (\Sentry\State\Scope $scope) use ($data): void { - foreach ($data['tags'] as $key => $value) { - $scope->setTag($key, $value); - } - }); - unset($text['tags']); + try { + /** @var User $user */ + $user = Yii::$app->has('user', true) ? Yii::$app->get('user', false) : null; + if ($user && ($identity = $user->getIdentity(false))) { + $data['userData']['id'] = $identity->getId(); + } + } catch (Throwable $e) {} + + \Sentry\withScope(function (Scope $scope) use ($text, $level, $data) { + if (is_array($text)) { + if (isset($text['msg'])) { + $data['message'] = $text['msg']; + unset($text['msg']); + } + + if (isset($text['tags'])) { + $data['tags'] = ArrayHelper::merge($data['tags'], $text['tags']); + unset($text['tags']); + } + + $data['extra'] = $text; + } else { + $data['message'] = (string) $text; } - $data['extra'] = $text; - - if (!empty($data['extra'])) { - \Sentry\configureScope(function (\Sentry\State\Scope $scope) use ($data): void { - foreach ($data['extra'] as $key => $value) { - $scope->setExtra((string)$key, $value); - } - }); + if ($this->context) { + $data['extra']['context'] = parent::getContextMessage(); } - - } else { - $data['message'] = $text; - } - if ($this->context) { - $data['extra']['context'] = parent::getContextMessage(); - } + $data = $this->runExtraCallback($text, $data); - $data = $this->runExtraCallback($text, $data); - \Sentry\captureMessage($data['message']); + $scope->setUser($data['userData'], true); + foreach ($data['extra'] as $key => $value) { + $scope->setExtra((string) $key, $value); + } + foreach ($data['tags'] as $key => $value) { + if ($value) { + $scope->setTag($key, $value); + } + } + + if ($text instanceof Throwable) { + \Sentry\captureException($text); + } else { + \Sentry\captureMessage($data['message'], $this->getLogLevel($level)); + } + }); } } /** * Calls the extra callback if it exists * - * @param $text - * @param $data + * @param mixed $text + * @param array $data + * * @return array */ public function runExtraCallback($text, $data) { if (is_callable($this->extraCallback)) { - $data['extra'] = call_user_func($this->extraCallback, $text, isset($data['extra']) ? $data['extra'] : []); + $data['extra'] = call_user_func($this->extraCallback, $text, $data['extra'] ?? []); } return $data; @@ -133,7 +148,10 @@ public function runExtraCallback($text, $data) /** * Returns the text display of the specified level for the Sentry. * - * @param integer $level The message level, e.g. [[LEVEL_ERROR]], [[LEVEL_WARNING]]. + * @deprecated Deprecated from 1.5, will remove in 2.0 + * + * @param int $level The message level, e.g. [[LEVEL_ERROR]], [[LEVEL_WARNING]]. + * * @return string */ public static function getLevelName($level) @@ -147,6 +165,31 @@ public static function getLevelName($level) Logger::LEVEL_PROFILE_END => 'debug', ]; - return isset($levels[$level]) ? $levels[$level] : 'error'; + return $levels[$level] ?? 'error'; + } + + /** + * Translates Yii2 log levels to Sentry Severity. + * + * @param int $level + * + * @return Severity + */ + protected function getLogLevel($level): Severity + { + switch ($level) { + case Logger::LEVEL_PROFILE: + case Logger::LEVEL_PROFILE_BEGIN: + case Logger::LEVEL_PROFILE_END: + case Logger::LEVEL_TRACE: + return Severity::debug(); + case Logger::LEVEL_WARNING: + return Severity::warning(); + case Logger::LEVEL_ERROR: + return Severity::error(); + case Logger::LEVEL_INFO: + default: + return Severity::info(); + } } } diff --git a/tests/commands/SentryController.php b/tests/commands/SentryController.php index ac8b656..af14ab3 100644 --- a/tests/commands/SentryController.php +++ b/tests/commands/SentryController.php @@ -2,6 +2,9 @@ namespace tests\commands; +use RuntimeException; +use tests\models\User; +use Yii; use yii\console\Controller; use yii\log\Logger; @@ -13,16 +16,29 @@ class SentryController extends Controller public function actionFill() { /* @var $logger \yii\log\Logger */ - $logger = \Yii::createObject(Logger::class); - \Yii::setLogger($logger); - \Yii::$app->log->setLogger(\Yii::getLogger()); + $logger = Yii::createObject(Logger::class); + Yii::setLogger($logger); + Yii::$app->log->setLogger(Yii::getLogger()); foreach ($this->logsProvider() as $log) { - \Yii::getLogger()->log($log['message'], $log['level'], $log['category']); - unset($log); - } + Yii::$app->user->logout(); + if (isset($log['user'])) { + Yii::$app->user->login($log['user']); + } + $_SERVER['REMOTE_ADDR'] = $log['ip'] ?? null; + + \Sentry\configureScope(function (\Sentry\State\Scope $scope) use ($log) { + $scope->setUser([ + // configureScope modify global scope, so we revert changes from previous log message + 'username' => isset($log['user']) ? $log['user']->username : null, + 'email' => isset($log['user']) ? $log['user']->email : null, + ], true); + }); - \Yii::getLogger()->flush(); + Yii::getLogger()->log($log['message'], $log['level'], $log['category']); + // We need to final flush logs for change ip and user on fly + Yii::getLogger()->flush(true); + } } protected function logsProvider() @@ -31,40 +47,52 @@ protected function logsProvider() [ 'level' => Logger::LEVEL_ERROR, 'message' => [ - 'msg' => new \RuntimeException('Connection error', 999, new \Exception), + 'msg' => new RuntimeException('Connection error', 999, new \Exception), 'extra' => 'Hello, World!', - 'tags' => ['db-name' => 'bulling'] + 'tags' => ['db-name' => 'bulling'], ], - 'category' => 'dbms' + 'category' => 'dbms', ], [ 'level' => Logger::LEVEL_ERROR, - 'message' => new \RuntimeException('Oops... This is exception.', 999, new \Exception), - 'category' => 'exceptions' + 'message' => new RuntimeException('Oops... This is exception.', 999, new \Exception), + 'category' => 'exceptions', + 'user' => new User([ + 'id' => 47, + 'username' => 'Agent 47', + 'email' => '47@agency.com', + ]), + 'ip' => '127.0.0.42', ], [ 'level' => Logger::LEVEL_INFO, 'message' => [ 'msg' => 'Message from bulling service', 'extra' => 'Hello, World!', - 'tags' => ['currency' => 'RUB'] + 'tags' => ['currency' => 'RUB'], ], - 'category' => 'monitoring' + 'category' => 'monitoring', + 'user' => new User([ + 'id' => 543, + 'username' => 'John', + 'email' => 'hello@example.com', + ]), + 'ip' => '2607:f0d0:1002:51::4', ], [ 'level' => Logger::LEVEL_WARNING, 'message' => 'Invalid request', - 'category' => 'UI' + 'category' => 'UI', ], [ 'level' => null, 'message' => [1, 2, 3], - 'category' => null + 'category' => null, ], [ 'level' => '', 'message' => ['one' => 'value 1', 'two' => 'value 2'], - 'category' => null + 'category' => null, ], ]; } diff --git a/tests/models/User.php b/tests/models/User.php new file mode 100644 index 0000000..78fdf96 --- /dev/null +++ b/tests/models/User.php @@ -0,0 +1,64 @@ + $id]); + } + + /** + * {@inheritdoc} + */ + public static function findIdentityByAccessToken($token, $type = null) + { + return new self(); + } + + /** + * {@inheritdoc} + */ + public function getId() + { + return $this->id; + } + + /** + * {@inheritdoc} + */ + public function getAuthKey() + { + return '123'; + } + + /** + * {@inheritdoc} + */ + public function validateAuthKey($authKey) + { + return true; + } +} diff --git a/tests/sentry-fill b/tests/sentry-fill new file mode 100644 index 0000000..414fe47 --- /dev/null +++ b/tests/sentry-fill @@ -0,0 +1,53 @@ +#!/usr/bin/env php + 'sentry-tests', + 'basePath' => Yii::getAlias('@tests'), + 'runtimePath' => Yii::getAlias('@tests/_output'), + 'controllerNamespace' => 'tests\commands', + 'bootstrap' => ['log'], + 'params' => $params, + 'components' => [ + 'request' => [ + 'cookieValidationKey' => '123', + ], + 'log' => [ + 'targets' => [ + [ + 'class' => \yii\log\FileTarget::class, + ], + [ + 'class' => \notamedia\sentry\SentryTarget::class, + 'dsn' => $params['sentryDsn'], + ], + ], + ], + 'user' => [ + 'class' => \yii\web\User::class, + 'identityClass' => \tests\models\User::class, + 'enableSession' => false, + ], + ], +]); + +$application->runAction('sentry/fill'); + +echo 'All messages sent.'; diff --git a/tests/unit.suite.yml b/tests/unit.suite.yml index 6e6076b..9550715 100644 --- a/tests/unit.suite.yml +++ b/tests/unit.suite.yml @@ -1,5 +1,8 @@ class_name: UnitTester modules: enabled: + - Yii2: + configFile: 'tests/unit/config.php' + part: init - Asserts - \Helper\Unit diff --git a/tests/yii b/tests/yii deleted file mode 100644 index 4184bd4..0000000 --- a/tests/yii +++ /dev/null @@ -1,36 +0,0 @@ -#!/usr/bin/env php - 'sentry-tests', - 'basePath' => \Yii::getAlias('@tests'), - 'runtimePath' => \Yii::getAlias('@tests/_output'), - 'enableCoreCommands' => false, - 'controllerNamespace' => 'tests\commands', - 'bootstrap' => ['log'], - 'params' => $params, - 'components' => [ - 'log' => [ - 'targets' => [ - [ - 'class' => \yii\log\FileTarget::class - ], - [ - 'class' => \notamedia\sentry\SentryTarget::class, - 'dsn' => $params['sentryDsn'] - ] - ] - ] - ] -]); - -$exitCode = $application->run(); -exit($exitCode);