Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Bridge\Monolog][FrameworkBundle] Add & wire a DebugProcessor#20416
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
6f1e17e to194d36aComparenicolas-grekas commentedNov 4, 2016
Green on CI.
|
| 'context' => $record['context'], | ||
| 'channel' => isset($record['channel']) ? $record['channel'] : '', | ||
| ); | ||
| switch ($record['level']) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This switch looks unneeded. What about using a simpleif?
if (in_array($record['level'],array(Logger::ERROR, Logger::CRITICAL, Logger::ALERT, Logger::EMERGENCY))) { ++$this->errorCount;}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Longer line, one array allocation, one function call: not my preference...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
OK then ... but according to 3v4l.org, theif code generates 7 opcodes and theswitch code generates 11 opcodes (although it's true that not all opcodes are equal!)
5271fb6 to3f53985Compare3f53985 to7572a53Comparefabpot commentedNov 6, 2016
Thank you@nicolas-grekas. |
…ocessor (nicolas-grekas)This PR was merged into the 3.2-dev branch.Discussion----------[Bridge\Monolog][FrameworkBundle] Add & wire a DebugProcessor| Q | A| ------------- | ---| Branch? | master| Bug fix? | yes| New feature? | yes| BC breaks? | no| Deprecations? | yes| Tests pass? | yes| Fixed tickets |#20370| License | MIT| Doc PR | -As identified in#20370, collecting the log records for the profiler should happen before any processor.The only way to do so is by registering a processor to do exactly that.Since the last added processor is called first, this one is wired in a compiler pass that is called after other processors are registered.The DebugHandler class being now useless, it is deprecated.If this approach is accepted, I'll send a PR on monolog bundle & silex to leverage it.Commits-------7572a53 [Bridge\Monolog][FrameworkBundle] Add & wire a DebugProcessor
fabpot commentedNov 6, 2016
@nicolas-grekas Can you make the PRs on the bundle and Silex? Thanks. |
…le in MonologServiceProvider (nicolas-grekas)This PR was merged into the 2.0.x-dev branch.Discussion----------Use DebugProcessor instead of DebugHandler when available in MonologServiceProviderRelated tosymfony/symfony#20416Commits-------213741a Use DebugProcessor instead of DebugHandler when available in MonologServiceProvider
…sorPass in FrameworkBundle (nicolas-grekas)This PR was merged into the 2.x branch.Discussion----------Deprecate DebugHandlerPass in favor of AddDebugLogProcessorPass in FrameworkBundleRelated tosymfony/symfony#20416Commits-------a55b0a0 Deprecate DebugHandlerPass in favor of AddDebugLogProcessorPass in FrameworkBundle
stof commentedJan 2, 2017
@nicolas-grekas there is an issue here though: if no handler is handling the record, processors will not be called by Composer |
nicolas-grekas commentedJan 2, 2017 via email
What about making DebugProcessor a null handler then? Or adding aNullHandler if that's too hacky :) |
sroze commentedSep 27, 2017
Not very useful for integration tests of processors 🤔 |
This PR was merged into the 4.x branch.Discussion----------Remove support for the `DebugHandler`| Q | A| ------------- | ---| Branch? | 3.x| Bug fix? | no| New feature? | no| Deprecations? | no| Issues | -| License | MITThe `DebugHandler` was [removed way back in Symfony 4.0](symfony/symfony#20416). Since the bundle only supports >=6.4, this is no longer needed.This was an oversight in#526.Commits-------902cd7f Remove support for the `DebugHandler`
As identified in#20370, collecting the log records for the profiler should happen before any processor.
The only way to do so is by registering a processor to do exactly that.
Since the last added processor is called first, this one is wired in a compiler pass that is called after other processors are registered.
The DebugHandler class being now useless, it is deprecated.
If this approach is accepted, I'll send a PR on monolog bundle & silex to leverage it.