-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Cache][EventDispatcher][HttpClient][HttpKernel][Messenger][Validator][Workflow] Don't enable tracing unless the profiler is enabled #60243
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
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
02cd114
to
4c2f79a
Compare
OK, here is a way: make the added Let's try the current simpler approach first and improve later, if the need arises. |
5781dc7
to
bd0a99f
Compare
The |
src/Symfony/Component/Workflow/DataCollector/WorkflowDataCollector.php
Outdated
Show resolved
Hide resolved
Basically yes, but I'd do it in a more subtle way: I'd add a intermediary closure who would get the profiler using a scoped container, and this container would reference the profiler using |
bd0d686
to
37412d9
Compare
PR updated with the closure I described in my previous comment: I figured out it was actually simpler this way, because it then doesn't need patching data collectors anymore. |
@nicolas-grekas according to #60037 (comment) In devland code (external bundles or app « tracers ») What are the changes required to mimic this? Is it worth an entry in the doc as well ? (Perhaps near data collector section as most likely to be related) |
Autowiring that should be possible with some named alias. I'm not going to be able to update this PR soon but it could be merged and followed up by the alias one. |
Feel free to ping me for doc part :) |
Thank you @nicolas-grekas. |
This PR ensures traceable decorators are enabled only if the profiler is also enabled.
The way it works is by adding a new argument to all traceable decorators:
?\Closure $disabled
. When a closure is given and when the closure returns true, tracing should be skipped. Calling the closure should be done before every decorated method to cope for dynamically enabled/disabled states.Then, this argument is wired using dependency injection so that a closure is passed that returns the current state of the profiler. If the profiler is not instantiated yet (eg because we're on the CLI, or because we're before ProfilerListener enabled it), then a default state is used (disabled on the CLI, enabled on the web). This allows collecting data on the web even if the profiler didn't start yet (as happens when a request comes in, before ProfilerListener is triggered).
Note that I didn't change all
Traceable*
decorators: the remaining ones look inoffensive to me. Of course, this could be wrong and fixed in follow up PRs.