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.

Sign up for GitHub

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

Update Apache HttpClient5 to 5.4.1 #1453

Merged
merged 1 commit into from
Feb 19, 2025
Merged

Update Apache HttpClient5 to 5.4.1 #1453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions instrumentation/httpclient5/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@
<main.basedir>${project.basedir}/../..</main.basedir>

<!-- To obey the integration tests, we need to use the version >= 5.2 -->
<!-- TODO: pinned due to timeouts on 5.4.x on brave.httpclient5.ITTracingCachingH2ClientBuilder.cacheControl -->
<httpclient5.version>5.3.1</httpclient5.version>
<httpclient5.version>5.4.1</httpclient5.version>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codefromthecrypt there was regression introduced into HttpCore5 5.3.3 [1] (that is used by HttpClient 5.4.2) and fixed by [2] (upcoming HttpCore5 5.3.4, I tested against the latest snapshots and things looks good). So I have pinned HttpClient5 to 5.4.1 (and we could update to 5.4.3 once released, no code changes are anticipated),

[1] apache/httpcomponents-core@98e23cb
[2] apache/httpcomponents-core@7274811

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good research and sounds fine

</properties>

<dependencies>
Expand Down
4 changes: 2 additions & 2 deletions instrumentation/httpclient5/src/main/java/brave/httpclient5/HttpClientUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ static boolean isLocalCached(HttpContext context, Span span) {
if (!HAS_CLIENT_CACHE_SUPPORT) {
return false;
}
boolean cacheHit = CacheResponseStatus.CACHE_HIT == context.getAttribute(
HttpCacheContext.CACHE_RESPONSE_STATUS);
final HttpCacheContext cacheContext = HttpCacheContext.cast(context);
final boolean cacheHit = CacheResponseStatus.CACHE_HIT == cacheContext.getCacheResponseStatus();
if (cacheHit) {
span.tag("http.cache_hit", "");
}
Expand Down
7 changes: 5 additions & 2 deletions instrumentation/httpclient5/src/main/java/brave/httpclient5/TracingHttpAsyncClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.io.IOException;
import java.util.concurrent.Future;
import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient;
import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.core5.concurrent.FutureCallback;
import org.apache.hc.core5.function.Supplier;
import org.apache.hc.core5.http.HttpHost;
Expand Down Expand Up @@ -58,15 +59,17 @@ protected <T> Future<T> doExecute(
HandlerFactory<AsyncPushConsumer> pushHandlerFactory, HttpContext context,
FutureCallback<T> callback) {
TraceContext invocationContext = currentTraceContext.get();

final HttpClientContext clientContext = HttpClientContext.castOrCreate(context);
if (invocationContext != null) {
context.setAttribute(TraceContext.class.getName(), invocationContext);
clientContext.setAttribute(TraceContext.class.getName(), invocationContext);
}

if (callback != null && invocationContext != null) {
callback = new TraceContextFutureCallback<>(callback, currentTraceContext, invocationContext);
}

return delegate.execute(target, requestProducer, responseConsumer, pushHandlerFactory, context,
return delegate.execute(target, requestProducer, responseConsumer, pushHandlerFactory, clientContext,
callback);
}

Expand Down
3 changes: 3 additions & 0 deletions ...entation/httpclient5/src/test/java/brave/httpclient5/ITTracingCachingH2ClientBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
import brave.propagation.SamplingFlags;
import brave.propagation.TraceContext;
import java.io.IOException;
import java.time.ZonedDateTime;
import java.util.Arrays;
import okhttp3.mockwebserver.MockResponse;
import org.apache.hc.client5.http.impl.async.H2AsyncClientBuilder;
import org.apache.hc.client5.http.impl.cache.CachingH2AsyncClientBuilder;
import org.apache.hc.client5.http.utils.DateUtils;
import org.junit.jupiter.api.Test;

import static brave.Span.Kind.CLIENT;
Expand All @@ -33,6 +35,7 @@ protected H2AsyncClientBuilder newClientBuilder() {
server.enqueue(new MockResponse()
.addHeader("Content-Type", "text/plain")
.addHeader("Cache-Control", "max-age=600, stale-while-revalidate=1200")
.addHeader("Date", DateUtils.FORMATTER_RFC1123.format(ZonedDateTime.now()))
.setBody("Hello"));

TraceContext parent = newTraceContext(SamplingFlags.SAMPLED);
Expand Down
3 changes: 3 additions & 0 deletions ...n/httpclient5/src/test/java/brave/httpclient5/ITTracingCachingHttpAsyncClientBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
import brave.propagation.SamplingFlags;
import brave.propagation.TraceContext;
import java.io.IOException;
import java.time.ZonedDateTime;
import java.util.Arrays;
import okhttp3.mockwebserver.MockResponse;
import org.apache.hc.client5.http.impl.async.HttpAsyncClientBuilder;
import org.apache.hc.client5.http.impl.cache.CachingHttpAsyncClientBuilder;
import org.apache.hc.client5.http.utils.DateUtils;
import org.junit.jupiter.api.Test;

import static brave.Span.Kind.CLIENT;
Expand All @@ -32,6 +34,7 @@ protected HttpAsyncClientBuilder newClientBuilder() {
server.enqueue(new MockResponse()
.addHeader("Content-Type", "text/plain")
.addHeader("Cache-Control", "max-age=600, stale-while-revalidate=1200")
.addHeader("Date", DateUtils.FORMATTER_RFC1123.format(ZonedDateTime.now()))
.setBody("Hello"));

TraceContext parent = newTraceContext(SamplingFlags.SAMPLED);
Expand Down
3 changes: 3 additions & 0 deletions ...tation/httpclient5/src/test/java/brave/httpclient5/ITTracingCachingHttpClientBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
import brave.propagation.SamplingFlags;
import brave.propagation.TraceContext;
import java.io.IOException;
import java.time.ZonedDateTime;
import java.util.Arrays;
import okhttp3.mockwebserver.MockResponse;
import org.apache.hc.client5.http.impl.cache.CachingHttpClientBuilder;
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
import org.apache.hc.client5.http.utils.DateUtils;
import org.junit.jupiter.api.Test;

import static brave.Span.Kind.CLIENT;
Expand All @@ -32,6 +34,7 @@ protected CloseableHttpClient newClient(int port) {
server.enqueue(new MockResponse()
.addHeader("Content-Type", "text/plain")
.addHeader("Cache-Control", "max-age=600, stale-while-revalidate=1200")
.addHeader("Date", DateUtils.FORMATTER_RFC1123.format(ZonedDateTime.now()))
.setBody("Hello"));

TraceContext parent = newTraceContext(SamplingFlags.SAMPLED);
Expand Down