Bugfixes and refactoring #38
Conversation
CookiesEater
commented
Apr 13, 2020
- Removed SentryTarget::$client (not used)
- Changed visibility for SentryTarget::runExtraCallback
- Added logging user data (ip and user id)
- For exception logging added additional data (tags, extra, user data)
- For now every log message used own scope
- Fixed message level (debug, info, warning, error) translating to sentry
- Old method SentryTarget::getLevelName removed
Added passing user data
There was a problem hiding this comment.
Хорошая работа, надеюсь, ничего из старого функционала не сломали :)
Есть несколько предложений. Они не связаны с данным ПР, но без них совсем плохо жить.
- Нужно добиться того, чтобы в ПР тоже запускались проверки TravisCI.
- Давайте сделаем команды
composer testиcomposer e2e. Первая для запуска Кодсепшена (в .travis.yml вызовы поправить надо будет), вторая — для запуска отправки тестовых логов в Сентри. - Добавьте вывод состояния в команду, отправляющую тестовые логи в Сентри. Сейчас непонятно, что происходит — то ли она отправила логи, то ли ничего не произошло из-за того, что sentryDsn в параметрах не был указан.
README.md
Outdated
|
|
||
| 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): | ||
| ``` |
There was a problem hiding this comment.
Нужно добавить ПХП-стилизацию, читать код неудобно без подсветки синтаксиса.
README.md
Outdated
| @@ -101,3 +101,29 @@ Yii2 log levels converts to Sentry levels: | |||
| \yii\log\Logger::LEVEL_PROFILE_BEGIN => 'debug', | |||
| \yii\log\Logger::LEVEL_PROFILE_END => 'debug', | |||
| ``` | |||
|
|
|||
| ## Additional context | |||
There was a problem hiding this comment.
Информация этой главы описывает способы конфигурации логера. Значит эта глава должна быть подглавой Usage.
There was a problem hiding this comment.
Перенёс в Usage главу
src/SentryTarget.php
Outdated
| * @return array | ||
| */ | ||
| public function runExtraCallback($text, $data) | ||
| protected function runExtraCallback($text, array $extra): array |
There was a problem hiding this comment.
Нельзя менять уровень видимости у методов, которые могли бы отнаследованы в таргетах пользователей, решивших расширить нашу библиотеку. Нужно либо вернуть предыдущий уровень видимости, либо выпускать мажорный релиз вер. 2. Я за первый вариант.
There was a problem hiding this comment.
Кроме того, нельзя удалять метод getLevelName (по тем же самым соображениям). Но его можно пометить как @deprecated.
There was a problem hiding this comment.
Вернул сигнатуру как была ранее, и вернул метод getLevelName() как deprecated