[HttpClient] Added TraceableHttpClient and WebProfiler panel #33015
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@tyx: Can you please re-add the header information? This is a requirement. |
|
fabbot has some interesting things to say :) |
fabpot
requested changes
Aug 7, 2019
| @@ -310,6 +312,10 @@ public function load(array $configs, ContainerBuilder $container) | ||
| $container->removeDefinition('console.command.messenger_failed_messages_remove'); | ||
| } | ||
|
|
||
| if ($this->httpClientConfigEnabled = $this->isConfigEnabled($container, $config['http_client'])) { | ||
| $this->registerHttpClientConfiguration($config['http_client'], $container, $loader); | ||
| } |
There was a problem hiding this comment.
Can you move it back to the same position it was before? Or is the move important?
There was a problem hiding this comment.
It is important because profiler should be process after this one to use $this->httpClientConfigEnabled
|
|
||
| <services> | ||
| <service id="data_collector.http_client" class="Symfony\Component\HttpClient\DataCollector\HttpClientDataCollector"> | ||
| <tag name="data_collector" template="@WebProfiler/Collector/http_client.html.twig" id="http_client" priority="240" /> |
There was a problem hiding this comment.
Not sure, was already here 😛
94eb7ea to
37b7f3f
Compare
|
Many failures on travis seem unrelated : https://travis-ci.org/symfony/symfony/jobs/568907222#L5407 Except one https://travis-ci.org/symfony/symfony/jobs/568907222#L5422 but I guess it's a normal failure as it does not use the right |
357b689 to
a523a5e
Compare
nicolas-grekas
approved these changes
Aug 23, 2019
Co-authored-by: Jérémy Romey <jeremy@free-agent.fr> Co-authored-by: Timothée Barray <tim@amicalement-web.net>
a523a5e to
5164001
Compare
|
Thank you @tyx, @IonBazan and @jeremyFreeAgent! |
nicolas-grekas
added a commit
that referenced
this pull request
Aug 23, 2019
… panel (jeremyFreeAgent) This PR was merged into the 4.4 branch. Discussion ---------- [HttpClient] Added TraceableHttpClient and WebProfiler panel | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes <!-- please update src/**/CHANGELOG.md files --> | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Replace #30494 I added : - tests - move debug services declaration in dedicated `http_client_debug.xml` file - rename stuff to follow messenger data collector stuff - add CompilerPass to allow bundle to trace their own http client I didn't add all @nicolas-grekas requests on UI profiler. I will continue to make more PR after this one. IMO everything looks fine to make a first merge except one strange behavior that I am not sure to get : When making a sub request : - we still have the http_client parent data. (like messenger, but currently I did not see anything in code that could avoid that, so different topic I guess). - The data collected are already "converted" to VarDumper data, so I have errors when trying to do all the unset stuff in the TraceableHttpClient. Is it for this reason, some collector use `lateCollect` ? Should we also move to lateCollect in the `HttpClientDataCollector` ? But I'm still new on this subject but glad to help, so feel free to request more changes ! Commits ------- 5164001 [HttpClient] Added TraceableHttpClient and WebProfiler panel
|
See #33311 for possible next steps. |
This was referenced Nov 12, 2019
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace #30494
I added :
http_client_debug.xmlfileI didn't add all @nicolas-grekas requests on UI profiler. I will continue to make more PR after this one.
IMO everything looks fine to make a first merge except one strange behavior that I am not sure to get :
When making a sub request :
Is it for this reason, some collector use
lateCollect? Should we also move to lateCollect in theHttpClientDataCollector?But I'm still new on this subject but glad to help, so feel free to request more changes !