Adds log-level configuration to PSR logger #23
Conversation
|
StyleCI don't seem to be running on this repo? |
src/BugsnagLogger.php
Outdated
| @@ -17,6 +18,13 @@ class BugsnagLogger extends AbstractLogger | |||
| protected $client; | |||
|
|
|||
| /** | |||
| * The minimum level required to notify bugsnag. | |||
There was a problem hiding this comment.
There should be a note here to indicate that under the threshold, log messages are converted to breadcrumbs.
src/BugsnagLogger.php
Outdated
| @@ -26,6 +34,8 @@ class BugsnagLogger extends AbstractLogger | |||
| public function __construct(Client $client) | |||
| { | |||
| $this->client = $client; | |||
| $logNotifyLevel = $this->client->getConfig()->getLogLevel(); | |||
There was a problem hiding this comment.
This seems like a roundabout way to set the log level by default. Loading it from the config prevents users from configuring the logger directly or separately from their initial Bugsnag configuration. In addition, it is potentially confusing for users who are using bugsnag-php without the logger, and may be conflated with setErrorReportingLevel for example. A setter would be preferable, with usage like:
$bugsnag = Bugsnag\Client::make('API KEY');
$logger = new Bugsnag\PsrLogger\BugsnagLogger($bugsnag);
// Set minimum log-level for sending a report to Bugsnag
$logger->setLogReportingLevel(\Psr\Log\LogLevel::ERROR);
// Add breadcrumbs for low-severity log messages
$logger->notice("Reticulating splines");
// Log an exception to Bugsnag
$logger->error(new Exception("Invalid configuration at runtime"));The existing constructor could be preserved and we can have new option for log level solely where needed, such as in config/bugsnag.php and an env variable for Laravel. The documentation would be something like:
Log reporting level
The log reporting level is the minimum log level (as specified by
\Psr\Log\LogLevel) required to generate an error report to send to Bugsnag. All log messages below the selected level are recorded as breadcrumbs on subsequent reports.You can set the Log reporting level by setting the
BUGSNAG_LOG_REPORTING_LEVELenvironment variable or by settinglog_reporting_levelwithinconfig/bugsnag.php:"log_reporting_level" => "error"By default, this is set to
notice
|
Closing in favour of #27 |
|
Re-opening as primary logLevel PR |
* Add example of simpler usage, update test * chore: Run the linter * Added notifyLevel info to readme
src/BugsnagLogger.php
Outdated
| public function setNotifyLevel($notifyLevel) | ||
| { | ||
| if (!in_array($notifyLevel, $this->getLogLevelOrder())) { | ||
| throw new TypeError("notifyLevel must be a valid Psr\Log\LogLevel value"); |
There was a problem hiding this comment.
We should be careful with hard failures, crashing the whole library. I think in some other places, we raise warnings, such as if the http layer fails.
There was a problem hiding this comment.
Also, TypeError doesn't exist in PHP 5, so can't be used here.
There was a problem hiding this comment.
I was going to say we should log the error and ignore the invalid input, but then we are within the logger 🤔 😄
This adds 3 new configuration options obtained through the Bugsnag-php config object for:
This requires an update of the PHP notifier and a dependency update before it can be merged.