Skip to content

Commit

Permalink
Ensure compatibility with latest 5.x Apache HC
Browse files Browse the repository at this point in the history
Tests were failing because overriding the less specific execute method resulted in our code not being called at all in newer versions. The changes here should work with both versions of the code.

Resolves gh-5575
  • Loading branch information
shakuzen committed Oct 9, 2024
1 parent 44de2af commit d0870fe
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import io.micrometer.core.instrument.Tags;
import org.apache.hc.client5.http.HttpRoute;
import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.protocol.HttpContext;

Expand All @@ -31,9 +32,15 @@ static String[] generateTagStringsForRoute(HttpContext context) {
String targetScheme = "UNKNOWN";
String targetHost = "UNKNOWN";
String targetPort = "UNKNOWN";
Object routeAttribute = context.getAttribute("http.route");
if (routeAttribute instanceof HttpRoute) {
HttpHost host = ((HttpRoute) routeAttribute).getTargetHost();
Object route;
if (context instanceof HttpClientContext) {
route = ((HttpClientContext) context).getHttpRoute();
}
else {
route = context.getAttribute("http.route");
}
if (route instanceof HttpRoute) {
HttpHost host = ((HttpRoute) route).getTargetHost();
targetScheme = host.getSchemeName();
targetHost = host.getHostName();
targetPort = String.valueOf(host.getPort());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.hc.core5.http.*;
import org.apache.hc.core5.http.impl.io.HttpRequestExecutor;
import org.apache.hc.core5.http.io.HttpClientConnection;
import org.apache.hc.core5.http.io.HttpResponseInformationCallback;
import org.apache.hc.core5.http.protocol.HttpContext;
import org.apache.hc.core5.util.Timeout;

Expand Down Expand Up @@ -102,17 +103,18 @@ public static Builder builder(MeterRegistry registry) {
}

@Override
public ClassicHttpResponse execute(ClassicHttpRequest request, HttpClientConnection conn, HttpContext context)
public ClassicHttpResponse execute(ClassicHttpRequest request, HttpClientConnection conn,
@Nullable HttpResponseInformationCallback informationCallback, HttpContext localContext)
throws IOException, HttpException {
ObservationOrTimerCompatibleInstrumentation<ApacheHttpClientContext> sample = ObservationOrTimerCompatibleInstrumentation
.start(registry, observationRegistry, () -> new ApacheHttpClientContext(request,
HttpClientContext.adapt(context), uriMapper, exportTagsForRoute), convention,
HttpClientContext.adapt(localContext), uriMapper, exportTagsForRoute), convention,
DefaultApacheHttpClientObservationConvention.INSTANCE);
String statusCodeOrError = "UNKNOWN";
Outcome statusOutcome = Outcome.UNKNOWN;

try {
ClassicHttpResponse response = super.execute(request, conn, context);
ClassicHttpResponse response = super.execute(request, conn, informationCallback, localContext);
sample.setResponse(response);
statusCodeOrError = DefaultApacheHttpClientObservationConvention.INSTANCE.getStatusValue(response, null);
statusOutcome = DefaultApacheHttpClientObservationConvention.INSTANCE.getStatusOutcome(response);
Expand All @@ -130,11 +132,17 @@ public ClassicHttpResponse execute(ClassicHttpRequest request, HttpClientConnect
() -> Tags
.of("method", DefaultApacheHttpClientObservationConvention.INSTANCE.getMethodString(request),
"uri", uriMapper.apply(request), "status", status, "outcome", outcome)
.and(exportTagsForRoute ? HttpContextUtils.generateTagsForRoute(context) : Tags.empty())
.and(exportTagsForRoute ? HttpContextUtils.generateTagsForRoute(localContext) : Tags.empty())
.and(extraTags));
}
}

@Override
public ClassicHttpResponse execute(ClassicHttpRequest request, HttpClientConnection conn, HttpContext context)
throws IOException, HttpException {
return execute(request, conn, null, context);
}

public static class Builder {

private final MeterRegistry registry;
Expand Down

0 comments on commit d0870fe

Please sign in to comment.