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
Apachehttpclient v5 #3419
Apachehttpclient v5 #3419
Conversation
👋 @videnkz Thanks a lot for your contribution! It may take some time before we review a PR, so even if you don’t see activity for some time, it does not mean that we have forgotten about it. Every once in a while we go through a process of prioritization, after which we are focussing on the tasks that were planned for the upcoming milestone. The prioritization status is typically reflected through the PR labels. It could be pending triage, a candidate for a future milestone, or have a target milestone set to it. |
…lientInstrumentationTest 5
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.
Marvellous work! Just a few smaller comments, I think after those we are ready to merge.
...n/src/main/java/co/elastic/apm/agent/httpclient/v5/BaseApacheHttpClient5Instrumentation.java
Outdated
Show resolved
Hide resolved
...lugin/src/main/java/co/elastic/apm/agent/httpclient/v5/ApacheHttpClient5Instrumentation.java
Outdated
Show resolved
Hide resolved
...lugin/src/main/java/co/elastic/apm/agent/httpclient/v5/ApacheHttpClient5Instrumentation.java
Outdated
Show resolved
Hide resolved
.../src/main/java/co/elastic/apm/agent/httpclient/v5/ApacheHttpAsyncClient5Instrumentation.java
Outdated
Show resolved
Hide resolved
...rc/main/java/co/elastic/apm/agent/httpclient/common/AbstractApacheHttpAsyncClientAdvice.java
Outdated
Show resolved
Hide resolved
...gin/src/main/java/co/elastic/apm/agent/httpclient/v5/helper/ApacheHttpAsyncClientHelper.java
Outdated
Show resolved
Hide resolved
apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient-common/pom.xml
Outdated
Show resolved
Hide resolved
...ttemplate-plugin/src/test/java/co/elastic/apm/agent/resttemplate/FeignClientIntegration.java
Outdated
Show resolved
Hide resolved
...ttemplate-plugin/src/test/java/co/elastic/apm/agent/resttemplate/FeignClientIntegration.java
Outdated
Show resolved
Hide resolved
…syncRequestProducerWrapper in case when delegate is not null. Added tests that breaks behavior when asyncClientHelper.recycle(this); called multiple times when delegate is already null in ASyncRequestProductWrapper
@elasticmachine run elasticsearch-ci/docs |
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.
LGTM. I mostly have minor comments and I think only adding the try/finally
when wrapping is really needed (or at least double-check if changing that is not relevant).
...t5-plugin/src/main/java/co/elastic/apm/agent/httpclient/v5/helper/FutureCallbackWrapper.java
Outdated
Show resolved
Hide resolved
...t5-plugin/src/main/java/co/elastic/apm/agent/httpclient/v5/helper/FutureCallbackWrapper.java
Outdated
Show resolved
Hide resolved
...t5-plugin/src/main/java/co/elastic/apm/agent/httpclient/v5/helper/FutureCallbackWrapper.java
Outdated
Show resolved
Hide resolved
...c/test/java/co/elastic/apm/agent/httpclient/v5/ApacheHttpAsyncClientInstrumentationTest.java
Outdated
Show resolved
Hide resolved
...c/test/java/co/elastic/apm/agent/httpclient/v5/ApacheHttpAsyncClientInstrumentationTest.java
Outdated
Show resolved
Hide resolved
...c/test/java/co/elastic/apm/agent/httpclient/v5/ApacheHttpAsyncClientInstrumentationTest.java
Outdated
Show resolved
Hide resolved
@elasticmachine run elasticsearch-ci/docs |
@elasticmachine run elasticsearch-ci/docs |
What does this PR do?
finally closes #2954
Checklist
Added an API method or config option? Document in which version this will be introducedI have made corresponding changes to the documentationFeignClient
requests