symfony / symfony

The Symfony PHP framework
https://symfony.com
MIT License
29.81k stars 9.47k forks source link

AddDebugLogProcessorPass is not working correctly in long run runtime and debug environment #48176

Closed tourze closed 1 year ago

tourze commented 2 years ago

Symfony version(s) affected

6.1.*

Description

I start to develop a new project using symfony and swoole. Symfony & Swoole is really fast, but when running in APP_ENV=dev environment, I cannnot see any log in profiler bar. After some reseach, I found that the problem is \Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddDebugLogProcessorPass.

This classs will remove all debug logger when symfony is under CLI. So swoole or any other same framework like workerman/reactphp is not working well.

How to reproduce

  1. Install a new Symfony project
  2. composer install runtime/swoole
  3. Ensure the APP_ENV=dev
  4. Start your application from runtime/swoole
  5. Visit home page, or log something, open profiler bar and check.

Possible Solution

I dont know if we can detect the running environment while kernel compile. Maybe we can add a new env variable, just like $_ENV['REMOTE_DEBUG_LOGGER'] (default: false) to replace the check in \Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddDebugLogProcessorPass::configureLogger.

Additional Context

No response

nicolas-grekas commented 2 years ago

/cc @MatTheCat maybe?

MatTheCat commented 2 years ago

Yes I remember it was to avoid memory leaks: https://github.com/symfony/symfony/pull/30339

stof commented 2 years ago

this is because in a non-swoole setup, the profiler does not collect anything in the CLI (as web requests are not handled there) so this avoids memory leaks for long-running commands.

We would need a way to disable that SAPI check for swoole.

tourze commented 1 year ago

How about make a patch like:

     public static function configureLogger(mixed $logger)
     {
-        if (\is_object($logger) && method_exists($logger, 'removeDebugLogger') && \in_array(\PHP_SAPI, ['cli', 'phpdbg'], true)) {
+        if (\is_object($logger) && method_exists($logger, 'removeDebugLogger') && $_ENV['APP_ENV'] === 'prod') {
             $logger->removeDebugLogger();
         }
     }

I patch it locally, and works fine now ( in dev and test environment).

MatTheCat commented 1 year ago

Your proposal has two issues:

I don't know much about Swoole, but could you try checking Swoole\Coroutine::getCid() (from https://github.com/swoole/swoole-src/issues/4284#issuecomment-871283010)?

MatTheCat commented 1 year ago

After some testing we could indeed check if Swoole\Coroutine exists and its getCid method returns -1.

But seeing that Roadrunner also leverages the cli SAPI makes me wonder if it really is Symfony’s responsibility to determine whether collecting logs or not :thinking:

carsonbot commented 1 year ago

Hey, thanks for your report! There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

carsonbot commented 1 year ago

Hello? This issue is about to be closed if nobody replies.

carsonbot commented 1 year ago

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!