SQL logger should not be replaced

[Mairu]

Currently the previously set logger is overwritten by the bundle in the compiler pass.

        $doctrine = $container->getParameter('doctrine.connections');

        foreach ($doctrine as $connectionId) {
            $container
                ->getDefinition(sprintf('%s.configuration', $connectionId))
                ->addMethodCall('setSQLLogger', [new Reference('app_insights_php.doctrine.logger.dependency')]);
        }

As there is already a logger chain defined where the logger can be added to (at least for me in symfony 4.4 with doctrine 2.7) the logger should be added to that chain instead of replacing it.

        $container->getDefinition('doctrine.dbal.logger.chain')
            ->addMethodCall('addLogger', [new Reference('app_insights_php.doctrine.logger.dependency')]);

[norberttech]

Good point, how would you suggest to detect logger chain in service container in order to add logger into it?

[Mairu]

$conterainer->getDefiniton throws a Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException if the requested service does not exist, so it can be handled with a simple try-catch-block

[norberttech]

ok, and there is no way that chain logger will be registered under different id than doctrine.dbal.logger.chain ?

[Mairu]

Well currently it is defined in the doctrine bundle.

https://github.com/doctrine/DoctrineBundle/blob/2.2.x/Resources/config/dbal.xml#L26

But it seems they want to remove it, but first only deprecated it. So I would assume it will be replaced / renamed / removed in 3.0.0 of the bundle.

There is an issue doctrine/DoctrineBundle#1280

[norberttech]

right, it seems that LoggerChain::addLogger() method is deprecated and actually removed from DBAL 3.0
doctrine/dbal@06dc97e#diff-ede4922d6cee6c8394a3d85443d4a4e4b0faf43fd34190fb1d82193116f4d928

[Mairu]

Well so to use it and write it compatible even a check if addLogger (still) exists would need to be added to the compiler pass.

I don't know if you would want to add this. Or wait for doctrine/DoctrineBundle#1280, which then only would work with newer versions.

I can also create a PR if you want.

[norberttech]

Right, you would need to check if the addLogger method exists first, but what will you do if it wont exist?