doctrine / dbal

Doctrine Database Abstraction Layer
https://www.doctrine-project.org/projects/dbal.html
MIT License
9.47k stars 1.34k forks source link

Allow to change the logger in different contexts #5366

Closed raziel057 closed 2 years ago

raziel057 commented 2 years ago

Feature Request

Q A
New Feature yes
RFC no

Hello,

Following the recommended way (https://github.com/doctrine/orm/issues/9562) to replace the the deprecated setSQLLogger() method by using the setMiddleware method in a Symfony Command where I want to print specifically a part of generated SQL, I replaced my code as shown bellow:

$debugStack = new StoreSQLLogger($this->em->getConnection()->getDatabasePlatform(), [
    StoreSQLLogger::QUERY_TYPE_INSERT,
    StoreSQLLogger::QUERY_TYPE_UPDATE
]);

$this->em->getConnection()->getConfiguration()->setSQLLogger($debugStack);

Replaced with something like this (simple example):

$this->em->getConnection()->getConfiguration()->setMiddlewares([new Middleware(new class extends AbstractLogger {
    public function log($level, $message, array $context = []): void
    {
        print_r(func_get_args());
    }
})]);

But I can see that doctrine never pass in my log method as the provided middleware is created using Symfony\Bridge\Monolog\Logger when the Connection (Doctrine\DBAL\Logging\Connection) is created. Replacing the logger at runtime seems not so straightforward as the middlewares are implemented by decorating the Driver/Connection used by Doctrine.

Before it was possible to just replace the SQLLogger for such need, or to set it to null in methods that manipulates heavy amount of entities to avoid memory issues and improve performances. This topic is discussed/needed on different tickets / blogs:

https://github.com/symfony/symfony/issues/45974 https://stackoverflow.com/questions/35192173/how-to-disable-query-logging-in-console-while-load-doctrine-fixtures https://www.wanadev.fr/77-5-astuces-pour-mieux-utiliser-doctrine-2/ https://grafikart.fr/forum/34894

Could you please consider to allow replacing the logger in the connection decorator created by the middleware after the connection is already created?

Thanks

morozov commented 2 years ago

I don't believe this feature is desired in the DBAL. Being able to change a dependency at runtime is a bad design on its own since it's effectively a shared mutable state: multiple parties that depend on the connection can change the logger without the others being aware of that.

If such logic needs to be implemented in a given application, it should be implemented in the logger, not in the DBAL/middleware.

derrabus commented 2 years ago
use Psr\Log\AbstractLogger;
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerAwareTrait;
use Psr\Log\NullLogger;
use Stringable;

final class ProxyLogger extends AbstractLogger implements LoggerAwareInterface
{
    use LoggerAwareTrait;

    public function __construct()
    {
        $this->logger = new NullLogger();
    }

    public function log($level, string|Stringable $message, array $context = []): void
    {
        $this->logger->log($level, $message, $context);
    }
}

Inject this logger into the middleware and keep a reference to it. Whenever you want to change the logger of the middleware, call setLogger() on the ProxyLogger instance instead.

raziel057 commented 2 years ago

@derrabus Thanks, I'm going to check.

greg0ire commented 2 years ago

Before it was possible to just replace the SQLLogger for such need, or to set it to null in methods that manipulates heavy amount of entities to avoid memory issues and improve performances.

It was advised to set it to null because it stored the queries in a property:

https://github.com/doctrine/dbal/blob/8fa8adbdc83cecdf81e108404c853fccdbf40606/src/Logging/DebugStack.php#L56-L61

The new logger is a PSR logger, so I don't think you should have this issue, unless maybe you are using the fingers_crossed strategy or something similar.

morozov commented 2 years ago

Closing as the suggested approach should solve the problem. There's no plan to implement the feature as requested.

pmishev commented 2 years ago
use Psr\Log\AbstractLogger;
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerAwareTrait;
use Psr\Log\NullLogger;
use Stringable;

final class ProxyLogger extends AbstractLogger implements LoggerAwareInterface
{
    use LoggerAwareTrait;

    public function __construct()
    {
        $this->logger = new NullLogger();
    }

    public function log($level, string|Stringable $message, array $context = []): void
    {
        $this->logger->log($level, $message, $context);
    }
}

Inject this logger into the middleware and keep a reference to it. Whenever you want to change the logger of the middleware, call setLogger() on the ProxyLogger instance instead.

@derrabus, how would you wire this in Symfony? I tried:

    App\Log\ProxyLogger:
        calls:
            - setLogger: ['@logger']
        tags:
            - { name: monolog.logger, channel: doctrine }

    doctrine.dbal.logging_middleware:
        class: Doctrine\DBAL\Logging\Middleware
        abstract: true
        arguments:
            - '@App\Log\ProxyLogger'
        tags:
            - { name: monolog.logger, channel: doctrine }

However, while it works, it doesn't respect the doctrine->dbal->logging settings. and logs always, even if:

doctrine:
    dbal:
        logging: false
derrabus commented 2 years ago

My approach was meant as a framework-agnostic example. I'm not familiar with how DoctrineBundle implemented logging middlewares. Maybe you want to ask over at https://github.com/doctrine/DoctrineBundle

github-actions[bot] commented 2 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.