[WebProfilerBundle] add extra data to logs panel #54445
Conversation
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
| $processors = $logger->getProcessors(); | ||
| while ([] !== $logger->getProcessors()) { | ||
| $logger->popProcessor(); | ||
| } |
There was a problem hiding this comment.
Really not sure about this. Adding tag priority like suggested here symfony/monolog-bundle#455 might help
| // Ensure the DebugLogger is the first processor as Monolog add processors in reverse order | ||
| $logger->pushProcessor($this->processor); | ||
| foreach ($processors as $processor) { | ||
| $logger->pushProcessor($processor); | ||
| } |
There was a problem hiding this comment.
For me this would deserve its own PR.. as this one only says "add extra data" and not "changes the order of procesors" :)
There was a problem hiding this comment.
Indeed, that's why I'm not 100% sure about this change. But if processors are left as is, DebugProcessor is the very last to be pushed to the list, and it will not get extra data added by next processors (because of reverse order used by Monolog).
Do you see another way to do so ? Working on CompilerPass looks pretty complex.
|
What's the status of this PR @jon-ht? |
|
Hi @fabpot I didn't take time to push further this PR. I'm still waiting for symfony/monolog-bundle#485 (previously symfony/monolog-bundle#455) to be accepted/merged, this will also resolve #54445 (comment) as pointed out by @smnandre |
|
I don't see how the MonologBundle PR would solve this. The DebugProcessor is added by a configurator (to add it conditionally), and so it won't benefit from the fact that processors registered by the MonologBundle compiler pass can be sorted explicitly or no (the configurator will always be called after the method calls added in the service definition) |
|
@stof Maybe it's not a requirement for my PR, but the changes I've added to I thought that adding some king of tag priority will have an effect on how/when If the diffs here are OK for you, I'm fine. But if not, do you have any idea on how to do this ? |
This is an attempt to add
extradata toLogspanel in profiler.I'm not convinced by this approch but I couldn't find another way to put
DebugProcessoras last item. Doing so ensure that when logs are collected,extraproperty contains all data from previous processors.I think this PR could help:
symfony/monolog-bundle#455symfony/monolog-bundle#485Here's an example with
Symfony\Bridge\Monolog\Processor\WebProcessorandMonolog\Processor\MemoryUsageProcessorenabledTODO