From 9832c14bdffe38dd4d6c3fc7f4a9bebbff59df9b Mon Sep 17 00:00:00 2001 From: Anurag Agarwal Date: Thu, 7 Apr 2022 21:15:06 +0530 Subject: [PATCH] Apache httpasyncclient 5.x (#5697) * Copies code for httpasyncclient-4.1 and creates instrumentation for 5.0 * Makes import changes for http client 5 * Decorate request channel and changes type in tests * Corrects test cases * Corrects most of the test cases * Forces http1 protocol to pass the test cases * Merge supported libraries for async client Co-authored-by: Mateusz Rzeszutek * Remove http.sceme and http.target attr from test Co-authored-by: Mateusz Rzeszutek * Removes not needed null check for status code * Replaces slf4j loggers with JUL * Inlined flavor extraction in attributes getter * Uses parameter placeholders for logging * Uses success endpoint to test flavor * Merges httpasyncclient and httpclient modules * Merges http client 5 modules * Update java-8 compatible changes Co-authored-by: Mateusz Rzeszutek * Change instrumentation name Co-authored-by: Mateusz Rzeszutek * Adds missing import statement * Rename packages * Java 8 * Reverts adding 5.0+ support from supporting libraries * Deleted hanging module * Uses seconds instead of ms in http test * Merges both classic and async client implementations * Moves http client all test cases to java tests * Uses abstract apache test class and moves boilerplate * Uses connection and read timeouts from ApacheHttpClientTest * Refactors remaining classes, shifts logic to HttpUtils * Renames HttpUtils to ApacheHttpClientUtils * Corrects failing code style error * Corrects build errors * Renames package to have http client version * Corrects package name * Uses instrumenter as static import * Inline utility methods Co-authored-by: Mateusz Rzeszutek Co-authored-by: Trask Stalnaker --- .../ApacheHttpAsyncClientInstrumentation.java | 264 ++++++++++++++++++ .../ApacheHttpClientHttpAttributesGetter.java | 51 ++-- .../v5_0/ApacheHttpClientInstrumentation.java | 2 +- ...ApacheHttpClientInstrumentationModule.java | 6 +- .../ApacheHttpClientNetAttributesGetter.java | 13 +- .../v5_0/ApacheHttpClientSingletons.java | 8 +- .../v5_0/HttpHeaderSetter.java | 10 +- .../test/groovy/ApacheHttpClientTest.groovy | 225 --------------- .../v5_0/AbstractApacheHttpClientTest.java | 118 ++++++++ .../v5_0/ApacheHttpAsyncClientTest.java | 128 +++++++++ .../v5_0/ApacheHttpClientTest.java | 214 ++++++++++++++ .../junit/http/AbstractHttpClientTest.java | 6 +- 12 files changed, 780 insertions(+), 265 deletions(-) create mode 100644 instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java delete mode 100644 instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy create mode 100644 instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/AbstractApacheHttpClientTest.java create mode 100644 instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientTest.java create mode 100644 instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientTest.java diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java new file mode 100644 index 000000000000..7c7833652f9d --- /dev/null +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java @@ -0,0 +1,264 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.apachehttpclient.v5_0; + +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.instrumentation.apachehttpclient.v5_0.ApacheHttpClientSingletons.instrumenter; +import static io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge.currentContext; +import static java.util.logging.Level.FINE; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import java.io.IOException; +import java.util.logging.Logger; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import org.apache.hc.core5.concurrent.FutureCallback; +import org.apache.hc.core5.http.EntityDetails; +import org.apache.hc.core5.http.HttpException; +import org.apache.hc.core5.http.HttpRequest; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.nio.AsyncRequestProducer; +import org.apache.hc.core5.http.nio.DataStreamChannel; +import org.apache.hc.core5.http.nio.RequestChannel; +import org.apache.hc.core5.http.protocol.HttpContext; +import org.apache.hc.core5.http.protocol.HttpCoreContext; + +class ApacheHttpAsyncClientInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher classLoaderOptimization() { + return hasClassesNamed("org.apache.hc.client5.http.async.HttpAsyncClient"); + } + + @Override + public ElementMatcher typeMatcher() { + return implementsInterface(named("org.apache.hc.client5.http.async.HttpAsyncClient")); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + isMethod() + .and(named("execute")) + .and(takesArguments(5)) + .and(takesArgument(0, named("org.apache.hc.core5.http.nio.AsyncRequestProducer"))) + .and(takesArgument(1, named("org.apache.hc.core5.http.nio.AsyncResponseConsumer"))) + .and(takesArgument(2, named("org.apache.hc.core5.http.nio.HandlerFactory"))) + .and(takesArgument(3, named("org.apache.hc.core5.http.protocol.HttpContext"))) + .and(takesArgument(4, named("org.apache.hc.core5.concurrent.FutureCallback"))), + this.getClass().getName() + "$ClientAdvice"); + } + + @SuppressWarnings("unused") + public static class ClientAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void methodEnter( + @Advice.Argument(value = 0, readOnly = false) AsyncRequestProducer requestProducer, + @Advice.Argument(3) HttpContext httpContext, + @Advice.Argument(value = 4, readOnly = false) FutureCallback futureCallback) { + + Context parentContext = currentContext(); + + WrappedFutureCallback wrappedFutureCallback = + new WrappedFutureCallback<>(parentContext, httpContext, futureCallback); + requestProducer = + new DelegatingRequestProducer(parentContext, requestProducer, wrappedFutureCallback); + futureCallback = wrappedFutureCallback; + } + } + + public static class DelegatingRequestProducer implements AsyncRequestProducer { + private final Context parentContext; + private final AsyncRequestProducer delegate; + private final WrappedFutureCallback wrappedFutureCallback; + + public DelegatingRequestProducer( + Context parentContext, + AsyncRequestProducer delegate, + WrappedFutureCallback wrappedFutureCallback) { + this.parentContext = parentContext; + this.delegate = delegate; + this.wrappedFutureCallback = wrappedFutureCallback; + } + + @Override + public void failed(Exception ex) { + delegate.failed(ex); + } + + @Override + public void sendRequest(RequestChannel channel, HttpContext context) + throws HttpException, IOException { + DelegatingRequestChannel requestChannel = + new DelegatingRequestChannel(channel, parentContext, wrappedFutureCallback); + delegate.sendRequest(requestChannel, context); + } + + @Override + public boolean isRepeatable() { + return delegate.isRepeatable(); + } + + @Override + public int available() { + return delegate.available(); + } + + @Override + public void produce(DataStreamChannel channel) throws IOException { + delegate.produce(channel); + } + + @Override + public void releaseResources() { + delegate.releaseResources(); + } + } + + public static class DelegatingRequestChannel implements RequestChannel { + private final RequestChannel delegate; + private final Context parentContext; + private final WrappedFutureCallback wrappedFutureCallback; + + public DelegatingRequestChannel( + RequestChannel requestChannel, + Context parentContext, + WrappedFutureCallback wrappedFutureCallback) { + this.delegate = requestChannel; + this.parentContext = parentContext; + this.wrappedFutureCallback = wrappedFutureCallback; + } + + @Override + public void sendRequest(HttpRequest request, EntityDetails entityDetails, HttpContext context) + throws HttpException, IOException { + if (instrumenter().shouldStart(parentContext, request)) { + wrappedFutureCallback.context = instrumenter().start(parentContext, request); + wrappedFutureCallback.httpRequest = request; + } + + delegate.sendRequest(request, entityDetails, context); + } + } + + public static class WrappedFutureCallback implements FutureCallback { + + private static final Logger logger = Logger.getLogger(WrappedFutureCallback.class.getName()); + + private final Context parentContext; + private final HttpContext httpContext; + private final FutureCallback delegate; + + private volatile Context context; + private volatile HttpRequest httpRequest; + + public WrappedFutureCallback( + Context parentContext, HttpContext httpContext, FutureCallback delegate) { + this.parentContext = parentContext; + this.httpContext = httpContext; + // Note: this can be null in real life, so we have to handle this carefully + this.delegate = delegate; + } + + @Override + public void completed(T result) { + if (context == null) { + // this is unexpected + logger.log(FINE, "context was never set"); + completeDelegate(result); + return; + } + + instrumenter().end(context, httpRequest, getResponse(httpContext), null); + + if (parentContext == null) { + completeDelegate(result); + return; + } + + try (Scope ignored = parentContext.makeCurrent()) { + completeDelegate(result); + } + } + + @Override + public void failed(Exception ex) { + if (context == null) { + // this is unexpected + logger.log(FINE, "context was never set"); + failDelegate(ex); + return; + } + + // end span before calling delegate + instrumenter().end(context, httpRequest, getResponse(httpContext), ex); + + if (parentContext == null) { + failDelegate(ex); + return; + } + + try (Scope ignored = parentContext.makeCurrent()) { + failDelegate(ex); + } + } + + @Override + public void cancelled() { + if (context == null) { + // this is unexpected + logger.log(FINE, "context was never set"); + cancelDelegate(); + return; + } + + // TODO (trask) add "canceled" span attribute + // end span before calling delegate + instrumenter().end(context, httpRequest, getResponse(httpContext), null); + + if (parentContext == null) { + cancelDelegate(); + return; + } + + try (Scope ignored = parentContext.makeCurrent()) { + cancelDelegate(); + } + } + + private void completeDelegate(T result) { + if (delegate != null) { + delegate.completed(result); + } + } + + private void failDelegate(Exception ex) { + if (delegate != null) { + delegate.failed(ex); + } + } + + private void cancelDelegate() { + if (delegate != null) { + delegate.cancelled(); + } + } + + private static HttpResponse getResponse(HttpContext context) { + return (HttpResponse) context.getAttribute(HttpCoreContext.HTTP_RESPONSE); + } + } +} diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHttpAttributesGetter.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHttpAttributesGetter.java index 9d17cf002b28..d85b396ff5a5 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHttpAttributesGetter.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientHttpAttributesGetter.java @@ -13,25 +13,25 @@ import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; -import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.MessageHeaders; import org.apache.hc.core5.http.ProtocolVersion; import org.apache.hc.core5.net.URIAuthority; final class ApacheHttpClientHttpAttributesGetter - implements HttpClientAttributesGetter { - + implements HttpClientAttributesGetter { private static final Logger logger = Logger.getLogger(ApacheHttpClientHttpAttributesGetter.class.getName()); @Override - public String method(ClassicHttpRequest request) { + public String method(HttpRequest request) { return request.getMethod(); } @Override - public String url(ClassicHttpRequest request) { + public String url(HttpRequest request) { // similar to org.apache.hc.core5.http.message.BasicHttpRequest.getUri() // not calling getUri() to avoid unnecessary conversion StringBuilder url = new StringBuilder(); @@ -64,34 +64,34 @@ public String url(ClassicHttpRequest request) { } @Override - public List requestHeader(ClassicHttpRequest request, String name) { - return headersToList(request.getHeaders(name)); + public List requestHeader(HttpRequest request, String name) { + return getHeader(request, name); } @Override @Nullable - public Long requestContentLength(ClassicHttpRequest request, @Nullable HttpResponse response) { + public Long requestContentLength(HttpRequest request, @Nullable HttpResponse response) { return null; } @Override @Nullable public Long requestContentLengthUncompressed( - ClassicHttpRequest request, @Nullable HttpResponse response) { + HttpRequest request, @Nullable HttpResponse response) { return null; } @Override - public Integer statusCode(ClassicHttpRequest request, HttpResponse response) { + public Integer statusCode(HttpRequest request, HttpResponse response) { return response.getCode(); } @Override @Nullable - public String flavor(ClassicHttpRequest request, @Nullable HttpResponse response) { - ProtocolVersion protocolVersion = request.getVersion(); + public String flavor(HttpRequest request, @Nullable HttpResponse response) { + ProtocolVersion protocolVersion = getVersion(request, response); if (protocolVersion == null) { - return SemanticAttributes.HttpFlavorValues.HTTP_1_1; + return null; } String protocol = protocolVersion.getProtocol(); if (!protocol.equals("HTTP")) { @@ -114,20 +114,31 @@ public String flavor(ClassicHttpRequest request, @Nullable HttpResponse response @Override @Nullable - public Long responseContentLength(ClassicHttpRequest request, HttpResponse response) { + public Long responseContentLength(HttpRequest request, HttpResponse response) { return null; } @Override @Nullable - public Long responseContentLengthUncompressed(ClassicHttpRequest request, HttpResponse response) { + public Long responseContentLengthUncompressed(HttpRequest request, HttpResponse response) { return null; } @Override - public List responseHeader( - ClassicHttpRequest request, HttpResponse response, String name) { - return headersToList(response.getHeaders(name)); + public List responseHeader(HttpRequest request, HttpResponse response, String name) { + return getHeader(response, name); + } + + private static ProtocolVersion getVersion(HttpRequest request, @Nullable HttpResponse response) { + ProtocolVersion protocolVersion = request.getVersion(); + if (protocolVersion == null && response != null) { + protocolVersion = response.getVersion(); + } + return protocolVersion; + } + + private static List getHeader(MessageHeaders messageHeaders, String name) { + return headersToList(messageHeaders.getHeaders(name)); } // minimize memory overhead by not using streams @@ -136,8 +147,8 @@ private static List headersToList(Header[] headers) { return Collections.emptyList(); } List headersList = new ArrayList<>(headers.length); - for (int i = 0; i < headers.length; ++i) { - headersList.add(headers[i].getValue()); + for (Header header : headers) { + headersList.add(header.getValue()); } return headersList; } diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java index e372d429f51a..4241551f3cf4 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java @@ -27,7 +27,7 @@ import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.io.HttpClientResponseHandler; -public class ApacheHttpClientInstrumentation implements TypeInstrumentation { +class ApacheHttpClientInstrumentation implements TypeInstrumentation { @Override public ElementMatcher classLoaderOptimization() { return hasClassesNamed("org.apache.hc.client5.http.classic.HttpClient"); diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentationModule.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentationModule.java index a454220bbd33..0e478a395f1c 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentationModule.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentationModule.java @@ -5,11 +5,10 @@ package io.opentelemetry.javaagent.instrumentation.apachehttpclient.v5_0; -import static java.util.Collections.singletonList; - import com.google.auto.service.AutoService; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import java.util.Arrays; import java.util.List; @AutoService(InstrumentationModule.class) @@ -21,6 +20,7 @@ public ApacheHttpClientInstrumentationModule() { @Override public List typeInstrumentations() { - return singletonList(new ApacheHttpClientInstrumentation()); + return Arrays.asList( + new ApacheHttpClientInstrumentation(), new ApacheHttpAsyncClientInstrumentation()); } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientNetAttributesGetter.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientNetAttributesGetter.java index 46ec18390f7e..e0eaa8c01aa0 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientNetAttributesGetter.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientNetAttributesGetter.java @@ -11,28 +11,27 @@ import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.util.logging.Logger; import javax.annotation.Nullable; -import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpResponse; final class ApacheHttpClientNetAttributesGetter - implements NetClientAttributesGetter { - + implements NetClientAttributesGetter { private static final Logger logger = Logger.getLogger(ApacheHttpClientNetAttributesGetter.class.getName()); @Override - public String transport(ClassicHttpRequest request, @Nullable HttpResponse response) { + public String transport(HttpRequest request, @Nullable HttpResponse response) { return SemanticAttributes.NetTransportValues.IP_TCP; } @Override @Nullable - public String peerName(ClassicHttpRequest request, @Nullable HttpResponse response) { + public String peerName(HttpRequest request, @Nullable HttpResponse response) { return request.getAuthority().getHostName(); } @Override - public Integer peerPort(ClassicHttpRequest request, @Nullable HttpResponse response) { + public Integer peerPort(HttpRequest request, @Nullable HttpResponse response) { int port = request.getAuthority().getPort(); if (port != -1) { return port; @@ -54,7 +53,7 @@ public Integer peerPort(ClassicHttpRequest request, @Nullable HttpResponse respo @Override @Nullable - public String peerIp(ClassicHttpRequest request, @Nullable HttpResponse response) { + public String peerIp(HttpRequest request, @Nullable HttpResponse response) { return null; } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientSingletons.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientSingletons.java index 1da811376683..5e3f39d34d91 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientSingletons.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientSingletons.java @@ -13,13 +13,13 @@ import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor; import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor; import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesExtractor; -import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpResponse; public final class ApacheHttpClientSingletons { private static final String INSTRUMENTATION_NAME = "io.opentelemetry.apache-httpclient-5.0"; - private static final Instrumenter INSTRUMENTER; + private static final Instrumenter INSTRUMENTER; static { ApacheHttpClientHttpAttributesGetter httpAttributesGetter = @@ -28,7 +28,7 @@ public final class ApacheHttpClientSingletons { new ApacheHttpClientNetAttributesGetter(); INSTRUMENTER = - Instrumenter.builder( + Instrumenter.builder( GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, HttpSpanNameExtractor.create(httpAttributesGetter)) @@ -40,7 +40,7 @@ public final class ApacheHttpClientSingletons { .newClientInstrumenter(HttpHeaderSetter.INSTANCE); } - public static Instrumenter instrumenter() { + public static Instrumenter instrumenter() { return INSTRUMENTER; } diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/HttpHeaderSetter.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/HttpHeaderSetter.java index 18e1f961687e..292d6642dc03 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/HttpHeaderSetter.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/HttpHeaderSetter.java @@ -6,13 +6,17 @@ package io.opentelemetry.javaagent.instrumentation.apachehttpclient.v5_0; import io.opentelemetry.context.propagation.TextMapSetter; -import org.apache.hc.core5.http.ClassicHttpRequest; +import javax.annotation.Nullable; +import org.apache.hc.core5.http.HttpRequest; -enum HttpHeaderSetter implements TextMapSetter { +enum HttpHeaderSetter implements TextMapSetter { INSTANCE; @Override - public void set(ClassicHttpRequest carrier, String key, String value) { + public void set(@Nullable HttpRequest carrier, String key, String value) { + if (carrier == null) { + return; + } carrier.setHeader(key, value); } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy deleted file mode 100644 index 0f118529a8ca..000000000000 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/groovy/ApacheHttpClientTest.groovy +++ /dev/null @@ -1,225 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -import io.opentelemetry.api.common.AttributeKey -import io.opentelemetry.instrumentation.test.AgentTestTrait -import io.opentelemetry.instrumentation.test.base.HttpClientTest -import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest -import io.opentelemetry.semconv.trace.attributes.SemanticAttributes -import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase -import org.apache.hc.client5.http.config.RequestConfig -import org.apache.hc.client5.http.impl.classic.CloseableHttpClient -import org.apache.hc.client5.http.impl.classic.HttpClientBuilder -import org.apache.hc.client5.http.impl.classic.HttpClients -import org.apache.hc.core5.http.ClassicHttpRequest -import org.apache.hc.core5.http.ClassicHttpResponse -import org.apache.hc.core5.http.HttpHost -import org.apache.hc.core5.http.HttpRequest -import org.apache.hc.core5.http.message.BasicClassicHttpRequest -import org.apache.hc.core5.http.message.BasicHeader -import org.apache.hc.core5.http.protocol.BasicHttpContext -import spock.lang.AutoCleanup -import spock.lang.Shared - -import java.util.concurrent.TimeUnit -import java.util.function.Consumer - -abstract class ApacheHttpClientTest extends HttpClientTest implements AgentTestTrait { - @Shared - @AutoCleanup - CloseableHttpClient client - - def setupSpec() { - HttpClientBuilder builder = HttpClients.custom() - builder.setDefaultRequestConfig(RequestConfig.custom() - .setConnectTimeout(CONNECT_TIMEOUT_MS, TimeUnit.MILLISECONDS) - .build()) - - client = builder.build() - } - - @Override - String userAgent() { - return "apachehttpclient" - } - - @Override - T buildRequest(String method, URI uri, Map headers) { - def request = createRequest(method, uri) - request.addHeader("user-agent", userAgent()) - headers.entrySet().each { - request.setHeader(new BasicHeader(it.key, it.value)) - } - return request - } - - @Override - Set> httpAttributes(URI uri) { - Set> extra = [ - SemanticAttributes.HTTP_SCHEME, - SemanticAttributes.HTTP_TARGET - ] - super.httpAttributes(uri) + extra - } - - // compilation fails with @Override annotation on this method (groovy quirk?) - int sendRequest(T request, String method, URI uri, Map headers) { - def response = executeRequest(request, uri) - response.close() // Make sure the connection is closed. - return response.code - } - - // compilation fails with @Override annotation on this method (groovy quirk?) - void sendRequestWithCallback(T request, String method, URI uri, Map headers, AbstractHttpClientTest.RequestResult requestResult) { - try { - executeRequestWithCallback(request, uri) { - it.close() // Make sure the connection is closed. - requestResult.complete(it.code) - } - } catch (Throwable throwable) { - requestResult.complete(throwable) - } - } - - abstract T createRequest(String method, URI uri) - - abstract ClassicHttpResponse executeRequest(T request, URI uri) - - abstract void executeRequestWithCallback(T request, URI uri, Consumer callback) - - static String fullPathFromURI(URI uri) { - StringBuilder builder = new StringBuilder() - if (uri.getPath() != null) { - builder.append(uri.getPath()) - } - - if (uri.getQuery() != null) { - builder.append('?') - builder.append(uri.getQuery()) - } - - if (uri.getFragment() != null) { - builder.append('#') - builder.append(uri.getFragment()) - } - return builder.toString() - } -} - -class ApacheClientHostRequest extends ApacheHttpClientTest { - @Override - ClassicHttpRequest createRequest(String method, URI uri) { - // also testing with an absolute path below - return new BasicClassicHttpRequest(method, fullPathFromURI(uri)) - } - - @Override - ClassicHttpResponse executeRequest(ClassicHttpRequest request, URI uri) { - return client.execute(new HttpHost(uri.getScheme(), uri.getHost(), uri.getPort()), request) - } - - @Override - void executeRequestWithCallback(ClassicHttpRequest request, URI uri, Consumer callback) { - client.execute(new HttpHost(uri.getScheme(), uri.getHost(), uri.getPort()), request) { - callback.accept(it) - } - } -} - -class ApacheClientHostAbsoluteUriRequest extends ApacheHttpClientTest { - @Override - ClassicHttpRequest createRequest(String method, URI uri) { - return new BasicClassicHttpRequest(method, uri.toString()) - } - - @Override - ClassicHttpResponse executeRequest(ClassicHttpRequest request, URI uri) { - return client.execute(new HttpHost(uri.getScheme(), uri.getHost(), uri.getPort()), request) - } - - @Override - void executeRequestWithCallback(ClassicHttpRequest request, URI uri, Consumer callback) { - client.execute(new HttpHost(uri.getScheme(), uri.getHost(), uri.getPort()), request) { - callback.accept(it) - } - } -} - -class ApacheClientHostRequestContext extends ApacheHttpClientTest { - @Override - ClassicHttpRequest createRequest(String method, URI uri) { - // also testing with an absolute path below - return new BasicClassicHttpRequest(method, fullPathFromURI(uri)) - } - - @Override - ClassicHttpResponse executeRequest(ClassicHttpRequest request, URI uri) { - return client.execute(new HttpHost(uri.getScheme(), uri.getHost(), uri.getPort()), request, new BasicHttpContext()) - } - - @Override - void executeRequestWithCallback(ClassicHttpRequest request, URI uri, Consumer callback) { - client.execute(new HttpHost(uri.getScheme(), uri.getHost(), uri.getPort()), request, new BasicHttpContext()) { - callback.accept(it) - } - } -} - -class ApacheClientHostAbsoluteUriRequestContext extends ApacheHttpClientTest { - @Override - ClassicHttpRequest createRequest(String method, URI uri) { - return new BasicClassicHttpRequest(method, uri.toString()) - } - - @Override - ClassicHttpResponse executeRequest(ClassicHttpRequest request, URI uri) { - return client.execute(new HttpHost(uri.getScheme(), uri.getHost(), uri.getPort()), request, new BasicHttpContext()) - } - - @Override - void executeRequestWithCallback(ClassicHttpRequest request, URI uri, Consumer callback) { - client.execute(new HttpHost(uri.getScheme(), uri.getHost(), uri.getPort()), request, new BasicHttpContext()) { - callback.accept(it) - } - } -} - -class ApacheClientUriRequest extends ApacheHttpClientTest { - @Override - ClassicHttpRequest createRequest(String method, URI uri) { - return new HttpUriRequestBase(method, uri) - } - - @Override - ClassicHttpResponse executeRequest(ClassicHttpRequest request, URI uri) { - return client.execute(request) - } - - @Override - void executeRequestWithCallback(ClassicHttpRequest request, URI uri, Consumer callback) { - client.execute(request) { - callback.accept(it) - } - } -} - -class ApacheClientUriRequestContext extends ApacheHttpClientTest { - @Override - ClassicHttpRequest createRequest(String method, URI uri) { - return new HttpUriRequestBase(method, uri) - } - - @Override - ClassicHttpResponse executeRequest(ClassicHttpRequest request, URI uri) { - return client.execute(request, new BasicHttpContext()) - } - - @Override - void executeRequestWithCallback(ClassicHttpRequest request, URI uri, Consumer callback) { - client.execute(request, new BasicHttpContext()) { - callback.accept(it) - } - } -} diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/AbstractApacheHttpClientTest.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/AbstractApacheHttpClientTest.java new file mode 100644 index 000000000000..ccd5990305ab --- /dev/null +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/AbstractApacheHttpClientTest.java @@ -0,0 +1,118 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.apachehttpclient.v5_0; + +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; +import java.net.URI; +import java.time.Duration; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.HttpRequest; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.message.BasicHeader; +import org.apache.hc.core5.http.protocol.BasicHttpContext; +import org.apache.hc.core5.http.protocol.HttpContext; +import org.apache.hc.core5.util.Timeout; + +abstract class AbstractApacheHttpClientTest + extends AbstractHttpClientTest { + + @Override + protected String userAgent() { + return "apachehttpclient"; + } + + @Override + protected void configure(HttpClientTestOptions options) { + options.setUserAgent(userAgent()); + options.enableTestReadTimeout(); + options.setHttpAttributes(this::getHttpAttributes); + } + + protected Set> getHttpAttributes(URI endpoint) { + Set> attributes = new HashSet<>(); + attributes.add(SemanticAttributes.NET_PEER_NAME); + attributes.add(SemanticAttributes.NET_PEER_PORT); + attributes.add(SemanticAttributes.HTTP_URL); + attributes.add(SemanticAttributes.HTTP_METHOD); + if (endpoint.toString().contains("/success")) { + attributes.add(SemanticAttributes.HTTP_FLAVOR); + } + attributes.add(SemanticAttributes.HTTP_USER_AGENT); + return attributes; + } + + @Override + protected T buildRequest(String method, URI uri, Map headers) { + T request = createRequest(method, uri); + request.addHeader("user-agent", userAgent()); + headers.forEach((key, value) -> request.setHeader(new BasicHeader(key, value))); + return request; + } + + @Override + protected int sendRequest(T request, String method, URI uri, Map headers) + throws Exception { + return getResponseCode(executeRequest(request, uri)); + } + + @Override + protected void sendRequestWithCallback( + T request, String method, URI uri, Map headers, RequestResult requestResult) { + try { + executeRequestWithCallback(request, uri, requestResult); + } catch (Throwable throwable) { + requestResult.complete(throwable); + } + } + + protected HttpHost getHost(URI uri) { + return new HttpHost(uri.getScheme(), uri.getHost(), uri.getPort()); + } + + protected HttpContext getContext() { + return new BasicHttpContext(); + } + + protected static String fullPathFromUri(URI uri) { + StringBuilder builder = new StringBuilder(); + if (uri.getPath() != null) { + builder.append(uri.getPath()); + } + + if (uri.getQuery() != null) { + builder.append('?'); + builder.append(uri.getQuery()); + } + + if (uri.getFragment() != null) { + builder.append('#'); + builder.append(uri.getFragment()); + } + return builder.toString(); + } + + abstract T createRequest(String method, URI uri); + + abstract HttpResponse executeRequest(T request, URI uri) throws Exception; + + abstract void executeRequestWithCallback(T request, URI uri, RequestResult requestResult) + throws Exception; + + private static int getResponseCode(HttpResponse response) { + return response.getCode(); + } + + static Timeout getTimeout(Duration duration) { + return Timeout.of(duration.toMillis(), TimeUnit.MILLISECONDS); + } +} diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientTest.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientTest.java new file mode 100644 index 000000000000..f3e1d067a07f --- /dev/null +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientTest.java @@ -0,0 +1,128 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.apachehttpclient.v5_0; + +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import java.net.URI; +import java.util.Map; +import java.util.concurrent.CancellationException; +import org.apache.hc.client5.http.async.methods.SimpleHttpRequest; +import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; +import org.apache.hc.client5.http.config.RequestConfig; +import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient; +import org.apache.hc.client5.http.impl.async.HttpAsyncClients; +import org.apache.hc.core5.concurrent.FutureCallback; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http2.HttpVersionPolicy; +import org.apache.hc.core5.io.CloseMode; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.extension.RegisterExtension; + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class ApacheHttpAsyncClientTest { + @RegisterExtension + static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent(); + + private final CloseableHttpAsyncClient client = createClient(); + + @BeforeAll + void setUp() { + client.start(); + } + + @AfterAll + void tearDown() { + client.close(CloseMode.GRACEFUL); + } + + private static CloseableHttpAsyncClient createClient() { + return HttpAsyncClients.custom().setVersionPolicy(HttpVersionPolicy.FORCE_HTTP_1).build(); + } + + @Nested + class ApacheClientUriRequestTest extends AbstractTest { + @Override + SimpleHttpRequest createRequest(String method, URI uri) { + return new SimpleHttpRequest(method, uri); + } + } + + @Nested + class ApacheClientHostRequestTest extends AbstractTest { + @Override + SimpleHttpRequest createRequest(String method, URI uri) { + return new SimpleHttpRequest(method, getHost(uri), fullPathFromUri(uri)); + } + } + + @Nested + class ApacheClientHostAbsoluteUriRequestTest extends AbstractTest { + @Override + SimpleHttpRequest createRequest(String method, URI uri) { + return new SimpleHttpRequest(method, URI.create(uri.toString())); + } + } + + abstract class AbstractTest extends AbstractApacheHttpClientTest { + @Override + protected SimpleHttpRequest buildRequest(String method, URI uri, Map headers) { + SimpleHttpRequest httpRequest = super.buildRequest(method, uri, headers); + RequestConfig.Builder configBuilder = RequestConfig.custom(); + configBuilder.setConnectTimeout(getTimeout(connectTimeout())); + if (uri.toString().contains("/read-timeout")) { + configBuilder.setResponseTimeout(getTimeout(readTimeout())); + } + RequestConfig requestConfig = configBuilder.build(); + httpRequest.setConfig(requestConfig); + return httpRequest; + } + + @Override + HttpResponse executeRequest(SimpleHttpRequest request, URI uri) throws Exception { + return client.execute(request, null).get(); + } + + @Override + void executeRequestWithCallback(SimpleHttpRequest request, URI uri, RequestResult result) { + client.execute(request, new ResponseCallback(result)); + } + + @Override + protected void configure(HttpClientTestOptions options) { + super.configure(options); + options.setResponseCodeOnRedirectError(302); + } + } + + private static class ResponseCallback implements FutureCallback { + private final AbstractHttpClientTest.RequestResult requestResult; + + public ResponseCallback(AbstractHttpClientTest.RequestResult requestResult) { + this.requestResult = requestResult; + } + + @Override + public void completed(SimpleHttpResponse response) { + requestResult.complete(response.getCode()); + } + + @Override + public void failed(Exception ex) { + requestResult.complete(ex); + } + + @Override + public void cancelled() { + requestResult.complete(new CancellationException()); + } + } +} diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientTest.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientTest.java new file mode 100644 index 000000000000..d418792ab442 --- /dev/null +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientTest.java @@ -0,0 +1,214 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.apachehttpclient.v5_0; + +import static io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest.CONNECTION_TIMEOUT; +import static io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest.READ_TIMEOUT; + +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension; +import java.io.IOException; +import java.net.URI; +import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase; +import org.apache.hc.client5.http.config.RequestConfig; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.io.HttpClientResponseHandler; +import org.apache.hc.core5.http.message.BasicClassicHttpRequest; +import org.apache.hc.core5.io.CloseMode; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.extension.RegisterExtension; + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class ApacheHttpClientTest { + @RegisterExtension + static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent(); + + private final CloseableHttpClient client = createClient(); + private final CloseableHttpClient clientWithReadTimeout = createClientWithReadTimeout(); + + private static RequestConfig requestConfig() { + return RequestConfig.custom() + .setConnectTimeout(AbstractApacheHttpClientTest.getTimeout(CONNECTION_TIMEOUT)) + .build(); + } + + private static RequestConfig requestConfigWithReadTimeout() { + return RequestConfig.copy(requestConfig()) + .setResponseTimeout(AbstractApacheHttpClientTest.getTimeout(READ_TIMEOUT)) + .build(); + } + + private static CloseableHttpClient createClient() { + return HttpClients.custom().setDefaultRequestConfig(requestConfig()).build(); + } + + private static CloseableHttpClient createClientWithReadTimeout() { + return HttpClients.custom().setDefaultRequestConfig(requestConfigWithReadTimeout()).build(); + } + + @AfterAll + void tearDown() { + client.close(CloseMode.GRACEFUL); + clientWithReadTimeout.close(CloseMode.GRACEFUL); + } + + private CloseableHttpClient getClient(URI uri) { + if (uri.toString().contains("/read-timeout")) { + return clientWithReadTimeout; + } + return client; + } + + @Nested + class ApacheClientHostRequestTest extends AbstractTest { + @Override + ClassicHttpRequest createRequest(String method, URI uri) { + // also testing with an absolute path below + return new BasicClassicHttpRequest(method, fullPathFromUri(uri)); + } + + @Override + ClassicHttpResponse doExecuteRequest(ClassicHttpRequest request, URI uri) throws Exception { + return getClient(uri).execute(getHost(uri), request); + } + + @Override + void executeRequestWithCallback(ClassicHttpRequest request, URI uri, RequestResult result) + throws Exception { + getClient(uri).execute(getHost(uri), request, new ResponseHandler(result)); + } + } + + @Nested + class ApacheClientHostAbsoluteUriRequestTest extends AbstractTest { + @Override + ClassicHttpRequest createRequest(String method, URI uri) { + return new BasicClassicHttpRequest(method, uri.toString()); + } + + @Override + ClassicHttpResponse doExecuteRequest(ClassicHttpRequest request, URI uri) throws Exception { + return getClient(uri).execute(getHost(uri), request); + } + + @Override + void executeRequestWithCallback(ClassicHttpRequest request, URI uri, RequestResult result) + throws Exception { + getClient(uri).execute(getHost(uri), request, new ResponseHandler(result)); + } + } + + @Nested + class ApacheClientHostRequestContextTest extends AbstractTest { + @Override + ClassicHttpRequest createRequest(String method, URI uri) { + // also testing with an absolute path below + return new BasicClassicHttpRequest(method, fullPathFromUri(uri)); + } + + @Override + ClassicHttpResponse doExecuteRequest(ClassicHttpRequest request, URI uri) throws Exception { + return getClient(uri).execute(getHost(uri), request, getContext()); + } + + @Override + void executeRequestWithCallback(ClassicHttpRequest request, URI uri, RequestResult result) + throws Exception { + getClient(uri).execute(getHost(uri), request, getContext(), new ResponseHandler(result)); + } + } + + @Nested + class ApacheClientHostAbsoluteUriRequestContextTest extends AbstractTest { + @Override + ClassicHttpRequest createRequest(String method, URI uri) { + return new BasicClassicHttpRequest(method, uri.toString()); + } + + @Override + ClassicHttpResponse doExecuteRequest(ClassicHttpRequest request, URI uri) throws Exception { + return getClient(uri).execute(getHost(uri), request, getContext()); + } + + @Override + void executeRequestWithCallback(ClassicHttpRequest request, URI uri, RequestResult result) + throws Exception { + getClient(uri).execute(getHost(uri), request, getContext(), new ResponseHandler(result)); + } + } + + @Nested + class ApacheClientUriRequestTest extends AbstractTest { + @Override + ClassicHttpRequest createRequest(String method, URI uri) { + return new HttpUriRequestBase(method, uri); + } + + @Override + ClassicHttpResponse doExecuteRequest(ClassicHttpRequest request, URI uri) throws Exception { + return getClient(uri).execute(request); + } + + @Override + void executeRequestWithCallback(ClassicHttpRequest request, URI uri, RequestResult result) + throws Exception { + getClient(uri).execute(request, new ResponseHandler(result)); + } + } + + @Nested + class ApacheClientUriRequestContextTest extends AbstractTest { + @Override + ClassicHttpRequest createRequest(String method, URI uri) { + return new HttpUriRequestBase(method, uri); + } + + @Override + ClassicHttpResponse doExecuteRequest(ClassicHttpRequest request, URI uri) throws Exception { + return getClient(uri).execute(request, getContext()); + } + + @Override + void executeRequestWithCallback(ClassicHttpRequest request, URI uri, RequestResult result) + throws Exception { + getClient(uri).execute(request, getContext(), new ResponseHandler(result)); + } + } + + abstract static class AbstractTest extends AbstractApacheHttpClientTest { + @Override + final HttpResponse executeRequest(ClassicHttpRequest request, URI uri) throws Exception { + ClassicHttpResponse httpResponse = doExecuteRequest(request, uri); + httpResponse.close(); + return httpResponse; + } + + abstract ClassicHttpResponse doExecuteRequest(ClassicHttpRequest request, URI uri) + throws Exception; + } + + private static class ResponseHandler implements HttpClientResponseHandler { + private final AbstractHttpClientTest.RequestResult requestResult; + + public ResponseHandler(AbstractHttpClientTest.RequestResult requestResult) { + this.requestResult = requestResult; + } + + @Override + public Void handleResponse(ClassicHttpResponse response) throws IOException { + response.close(); + requestResult.complete(response.getCode()); + return null; + } + } +} diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java index 8cd0a589bf50..823e6e59cb98 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java @@ -48,6 +48,8 @@ @TestInstance(TestInstance.Lifecycle.PER_CLASS) public abstract class AbstractHttpClientTest { + public static final Duration CONNECTION_TIMEOUT = Duration.ofSeconds(5); + public static final Duration READ_TIMEOUT = Duration.ofSeconds(2); static final String BASIC_AUTH_KEY = "custom-authorization-header"; static final String BASIC_AUTH_VAL = "plain text auth token"; @@ -126,11 +128,11 @@ protected void sendRequestWithCallback( /** Returns the connection timeout that should be used when setting up tested clients. */ protected final Duration connectTimeout() { - return Duration.ofSeconds(5); + return CONNECTION_TIMEOUT; } protected final Duration readTimeout() { - return Duration.ofSeconds(2); + return READ_TIMEOUT; } private InstrumentationTestRunner testing;