-
-
Notifications
You must be signed in to change notification settings - Fork 449
[Core] add PsrLogger and support RFC 5424 log level #5144
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: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR adds PSR-3 LoggerInterface support to OpenMage by introducing Mage_Core_Helper_PsrLogger and extending Mage::log() to accept RFC 5424 numeric log levels (0-7) and PSR-3 string level names. This enables better integration with modern PHP logging libraries and tools while maintaining backward compatibility with existing code.
Key Changes
- New
Mage_Core_Helper_PsrLoggerclass provides PSR-3 LoggerInterface implementation wrappingMage::log() - Enhanced
Mage::log()to automatically convert RFC 5424 numeric levels (0-7, used by PHP's LOG_* constants) to corresponding Monolog levels - Added support for PSR-3 string level names ('debug', 'info', 'warning', etc.) via
Level::fromName()
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| app/code/core/Mage/Core/Helper/PsrLogger.php | New PSR-3 LoggerInterface implementation that wraps Mage::log(), enabling integration with PSR-3 compatible libraries |
| app/Mage.php | Extended log level handling to support RFC 5424 numeric levels and PSR-3 string level names, with proper conversion to Monolog level values |
| class Mage_Core_Helper_PsrLogger extends Mage_Core_Helper_Abstract implements LoggerInterface | ||
| { | ||
| use LoggerTrait; | ||
|
|
||
| public function log($level, string|\Stringable $message, array $context = []): void | ||
| { | ||
| Mage::log((string) $message, $level, null, false, $context); | ||
| } | ||
| } |
Copilot
AI
Dec 9, 2025
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 new Mage_Core_Helper_PsrLogger class lacks test coverage. Since other helpers in the same directory have unit tests (e.g., tests/unit/Mage/Core/Helper/DataTest.php), this class should also have a corresponding test file. Consider adding tests/unit/Mage/Core/Helper/PsrLoggerTest.php to verify:
- The
log()method correctly callsMage::log()with the expected parameters - PSR-3 log levels (debug, info, warning, error, etc.) are properly passed through
- The
$contextarray is correctly forwarded - String and Stringable messages are handled correctly
|
|
Not an "accessibility bypass" like SonarQube thinks |
|
Note: there is a PSR-Handler
There a serveral ideas to support mor handlers (and possible replace firegento/logger), Any help is welcome. :) |
|
it is listed there, at the bottom of the handler in the "Wrappers / Special Handlers" section:
But this PR is a different direction:
|
That's what got me thinking first, i added firegento/logger to another project recently. |
|
Only from reviewing the code ... i think changing log level to monolog values coud cause problems. Updated PR to use RFC5424 levels. |

Description (*)
Log_*constants.Mage_Core_Helper_PsrLoggeras way to implementLoggerInterfaceusing Mage Log functionRelated Pull Requests
Zend_Logwithmonolog#5126 (comment)Fixed Issues (if relevant)
Manual testing scenarios (*)
LOG_DEBUGonMage::logQuestions or comments
Contribution checklist (*)