Skip to content

Conversation

HypeMC
Copy link
Contributor

@HypeMC HypeMC commented Sep 18, 2025

Description

Currently, the constructor only accepts integers as the log level, e.g. by using the Monolog\Logger constants:

services:
  Sentry\SentryBundle\Monolog\LogsHandler:
    arguments:
      - !php/const Monolog\Logger::INFO

This, however, isn't very flexible, especially since the Monolog constants are deprecated.
This PR expands the accepted types to also allow strings or the Monolog Level enum, like other handlers:

services:
  Sentry\SentryBundle\Monolog\LogsHandler:
    arguments:
      - 'info'
# or using PSR constants
services:
  Sentry\SentryBundle\Monolog\LogsHandler:
    arguments:
      - !php/const Psr\Log\LogLevel::INFO
// or via PHP config
$container->services()
    ->set(\Sentry\SentryBundle\Monolog\LogsHandler::class)
    ->args([\Monolog\Level::Info]);

<code><![CDATA[isMasterRequest]]></code>
</UndefinedMethod>
</file>
<file src="src/Monolog/LogsHandler.php">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be a bug in Psalm when using const on enums: https://psalm.dev/r/a6f9298b82

Copy link
Contributor

@Litarnus Litarnus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution, it makes a lot of sense to allow string levels!

*/
public function __construct($level = MonologLogger::DEBUG, bool $bubble = true)
{
$level = MonologLogger::toMonologLevel($level);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will throw if an invalid level is passed. Please default to Info in such a case like getSentryLogLevelFromMonologLevel does. Also this would benefit from a test case.

Copy link
Contributor Author

@HypeMC HypeMC Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HypeMC HypeMC force-pushed the improve-logshandler branch from 1e072fc to de332aa Compare September 22, 2025 18:44
Copy link
Contributor

@Litarnus Litarnus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you!

@Litarnus Litarnus merged commit 927ea3e into getsentry:master Sep 24, 2025
40 checks passed
@HypeMC HypeMC deleted the improve-logshandler branch September 24, 2025 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants