Introduced a PSR-3 logger adapter #3156
Conversation
|
The coding standards failure is expected due to the non-compliant signature of |
Additionally, deprecated `EchoSQLLogger` as per proposal in doctrine#2911 (comment)
| /** @var LoggerInterface */ | ||
| private $logger; | ||
|
|
||
| public function __construct(LoggerInterface $logger) |
There was a problem hiding this comment.
This requires an addition to composer.json
There was a problem hiding this comment.
It’s not a hard requirement. Do you want to add a suggestion?
There was a problem hiding this comment.
Would need to be a hard requirement if we rely on its signature in any way.
There was a problem hiding this comment.
We rely on this signature in the same way as on signatures of PDO, mysqli and other extensions, all of which are optional.
Looks like we have different expectations from PSR-3 in DBAL. As I see it, it will just enable dumping debug information into an abstract channel as requested in #2911. We're not deprecating SQLLogger which has a more specific interface oriented on query execution, not just messages. Therefore, no need to introduce a hard dependency on PSR-3.
| use Psr\Log\LoggerInterface; | ||
|
|
||
| /** | ||
| * Logs every query as a debug message |
| /** | ||
| * Logs every query as a debug message | ||
| */ | ||
| final class PsrAdapter implements SQLLogger |
There was a problem hiding this comment.
Rather than PsrAdapter, PsrLoggerSqlLogger? PsrSqlLogger?
There was a problem hiding this comment.
I think the "adapter" here is essential since unlike other implementations, this class just converts calls from one API to another. While the SQL part is implied since the DBAL doesn't log anything else than SQL.
| * {@inheritDoc} | ||
| */ | ||
| public function stopQuery() | ||
| { |
There was a problem hiding this comment.
We probably want to log stopQuery as debug too.
There was a problem hiding this comment.
This way we’ll have two almost identical consecutive messages per query. Without precise timestamps it will be just a noise. Want to elaborate?
There was a problem hiding this comment.
Yes, but the start/end are vital to profile queries. Filtering them out would be trivial here, while adding them wouldn't be as easy.
There was a problem hiding this comment.
I think, using a logging interface for profiling is an abuse. SQLLogger suits this need better.
|
Linking this to #719. |
|
@morozov curious, why was this closed? |
|
@ragboyjr because it's not clear how to proceed and the DBAL's logger is not really a logger: #3513 (comment). |
EchoSQLLoggeras per Add SQL file logger #2911 (comment).