-
Notifications
You must be signed in to change notification settings - Fork 873
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 Apache HttpClient host + absolute uri #3694
Conversation
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.
Do we need for 4.3 also?
What is this 4.3? 😉 |
I took a slightly different approach using instanceof, but since the tests are the same may easily have the same bug |
...o/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientRequest.java
Outdated
Show resolved
Hide resolved
...ain/java/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientRequest.java
Outdated
Show resolved
Hide resolved
...in/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientTest.groovy
Outdated
Show resolved
Hide resolved
@open-telemetry/java-instrumentation-approvers apologies, i got mixed up a bit between branches and apache httpclient spaghetti, and a couple of changes from this PR leaked into #3692, which is why a couple diffs in this PR are a bit weird |
// TODO(trask) substitute a non-resolving host and port to make sure that the host and port | ||
// from this uri are not used (currently that causes redirect tests to fail | ||
// because Apache HttpClient 5 appears to resolve relative redirects against this uri | ||
// instead of against the host, which is different from Apache HttpClient 4 behavior) |
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 is captured in #3697 now
Noticed this gap while fixing #3692