-
-
Notifications
You must be signed in to change notification settings - Fork 464
ref(logs): allow different log level types #1992
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| } | ||
|
|
||
| public static function logLevelDataProvider(): iterable | ||
| public static function monologLegacyLevelDataProvider(): iterable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests are maybe a bit too verbose and confusing, not sure
| return $record['level'] >= $this->logLevel->value; | ||
| } else { | ||
| return $record['level'] >= $this->logLevel; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enum-to-int comparison fails in Monolog 3
High Severity
In Monolog 3, $record['level'] returns a \Monolog\Level enum object, not an integer. The comparisons on lines 59 and 61 attempt to compare this enum directly with integer values ($this->logLevel->value or $this->logLevel), which throws a TypeError in PHP 8.1+ since backed enums cannot be compared with integers using >=. The BreadcrumbHandler in the same codebase demonstrates the correct pattern: checking if ($level instanceof Level) and extracting $level->value before comparison.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The analysis seems to be incorrect: https://github.com/Seldaek/monolog/blob/main/src/Monolog/LogRecord.php#L86
level is returning $this->level->value, which is the integer defined in the enum
This PR extends the possible log level to properly handle the filtering of log messages.
Since sentry has fewer log levels than monolog, it wasn't possible to filter based on monolog levels because we would convert it losing some levels. With the changes in this PR, it will be possible to filter based on monolog levels and the severity will only be converted when sending to sentry.