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

Fix the decoration of the HTTP client when tracing is enabled #786

Merged
merged 2 commits into from
Dec 4, 2023

Conversation

ste93cry
Copy link
Collaborator

The distributed tracing functionality for the HTTP client has been broken for a long time: first, the decoration of the raw HTTP client does not happen correctly, leading to a double presence of our instrumentation in the decoration chain. Second, but not least, because our instrumentation runs before we have the complete URL of the request, the reported span includes incomplete and/or misleading information.

This PR is an attempt to fix #700 based on @simonberger's contribution in #704. Starting from Symfony 6.3, the framework decoration strategy was revisited and this made the fix quite simple. However, on older versions of the framework, each built-in decorator of the HTTP client (e.g. the RetryableHttpClient), references in the constructor the .inner service of the client being decorated, making impossible to inject our own client at a given position because no real decoration chain exists. For this reason, I had to hardcode and decorate the specific services created by the framework in FrameworkExtension, rather than the raw HTTP client.

@simonberger
Copy link

It is working correctly in my tests on Symfony 5.4. Doing it in the compile stage makes detection easier, good job. I guess I did not make it to work there, because I forgot to re-set the decoration priority when switching the decoration class.
Approving from my side.

@cleptric cleptric enabled auto-merge (squash) December 4, 2023 10:51
@cleptric cleptric merged commit 76b46d2 into master Dec 4, 2023
28 checks passed
@cleptric cleptric deleted the fix-http-client-decoration branch December 4, 2023 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TraceableHttpClientForV6 twice in symfony/http-client decoration chain
3 participants