Skip to content
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

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

Closed
tourze opened this issue Nov 9, 2022 · 9 comments

Comments

@tourze
Copy link
Contributor

tourze commented Nov 9, 2022

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
Copy link
Member

/cc @MatTheCat maybe?

@MatTheCat
Copy link
Contributor

MatTheCat commented Nov 14, 2022

Yes I remember it was to avoid memory leaks: #30339

@stof
Copy link
Member

stof commented Nov 14, 2022

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
Copy link
Contributor Author

tourze commented Nov 24, 2022

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
Copy link
Contributor

MatTheCat commented Nov 24, 2022

Your proposal has two issues:

  • it removes the check on the sapi, which means workers will suffer a memory leak
  • Symfony does not enforce any environment name, so you cannot check against one

I don't know much about Swoole, but could you try checking Swoole\Coroutine::getCid() (from swoole/swoole-src#4284 (comment))?

@MatTheCat
Copy link
Contributor

MatTheCat commented Dec 3, 2022

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 🤔

@carsonbot
Copy link

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
Copy link

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

@carsonbot
Copy link

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants