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

Profiler enabled check in DebugHandlerPass doesn't work correctly #37

Closed
mac-cain13 opened this issue Mar 28, 2013 · 7 comments
Closed

Comments

@mac-cain13
Copy link

The DebugHandlerPass checks if the profiler is enabled, but I think that this check is incomplete. I'm running a Symfony 2.2 instance and even in production mode the profiler section is defined in the config, only the profiler.enabled option is set to false to disable the profiler.

I think that the DebugHandlerPass class should check if the enabled is set to true and only then add the DebugHandler. Or is there something I'm missing?

@stof
Copy link
Member

stof commented Mar 28, 2013

There is another issue actually: even if it is configured as disabled by default, it can be enabled later in the request.

@fabpot what would be the right behavior here ?

@Seldaek
Copy link
Member

Seldaek commented Apr 8, 2013

Ping @fabpot?

@fabpot
Copy link
Member

fabpot commented Apr 11, 2013

I think the easiest thing to do is to additionally check for the kernel.debug setting and registering the debug handler only if it is set to true.

@Seldaek
Copy link
Member

Seldaek commented Apr 22, 2013

OK I now just check if the profiler will be disabled by a method call.. Which works if it's disabled even if kernel.debug is true. It means you won't get logs collected if you enable it during the request though, but I hope we can live with that. The other way would be to propagate the state of the profiler to its collectors so we could then enable/disable the logger to prevent leaking.

@mac-cain13
Copy link
Author

Great! Thanks for fixing this one. :)

@stof
Copy link
Member

stof commented Apr 27, 2013

@Seldaek 2.3 can now remove the profiler properly instead of just defaulting to disable it. So this workaround code should probably be removed for 2.3 to allow collecting logs properly when enabling the profiler selectively

@Seldaek
Copy link
Member

Seldaek commented Apr 27, 2013

Alright, reopening so it doesn't get lost.

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

No branches or pull requests

4 participants