From 9448a439839cd4330153ca8f14b2a837af49a751 Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Tue, 17 Dec 2024 17:54:07 +0800 Subject: [PATCH 01/10] add jaxrs client 2.0 testing --- .../groovy/JaxMultithreadedClientTest.groovy | 68 ------ .../src/test/groovy/JaxRsClientTest.groovy | 218 ------------------ .../groovy/ResteasyProxyClientTest.groovy | 119 ---------- .../groovy/ResteasySingleConnection.groovy | 54 ----- .../jaxrsclient/CxfClientTest.java | 53 +++++ .../JaxMultithreadedClientTest.java | 87 +++++++ .../jaxrsclient/JaxRsClientTest.java | 158 +++++++++++++ .../jaxrsclient/JerseyClientTest.java | 33 +++ .../jaxrsclient/ResteasyClientTest.java | 28 +++ .../jaxrsclient/ResteasyProxyClientTest.java | 80 +++++++ .../jaxrsclient/ResteasyProxyResource.java | 45 ++++ .../jaxrsclient/ResteasySingleConnection.java | 65 ++++++ .../junit/http/AbstractHttpClientTest.java | 1 + .../junit/http/HttpClientTestOptions.java | 5 + 14 files changed, 555 insertions(+), 459 deletions(-) delete mode 100644 instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/JaxMultithreadedClientTest.groovy delete mode 100644 instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/JaxRsClientTest.groovy delete mode 100644 instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/ResteasyProxyClientTest.groovy delete mode 100644 instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/ResteasySingleConnection.groovy create mode 100644 instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/CxfClientTest.java create mode 100644 instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java create mode 100644 instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java create mode 100644 instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java create mode 100644 instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java create mode 100644 instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyProxyClientTest.java create mode 100644 instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyProxyResource.java create mode 100644 instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasySingleConnection.java diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/JaxMultithreadedClientTest.groovy b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/JaxMultithreadedClientTest.groovy deleted file mode 100644 index 6aa0ee272dc5..000000000000 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/JaxMultithreadedClientTest.groovy +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification -import io.opentelemetry.testing.internal.armeria.common.HttpResponse -import io.opentelemetry.testing.internal.armeria.common.HttpStatus -import io.opentelemetry.testing.internal.armeria.common.MediaType -import io.opentelemetry.testing.internal.armeria.server.ServerBuilder -import io.opentelemetry.testing.internal.armeria.testing.junit5.server.ServerExtension -import org.glassfish.jersey.client.JerseyClientBuilder -import spock.lang.Shared -import spock.util.concurrent.AsyncConditions - -import javax.ws.rs.client.Client - -class JaxMultithreadedClientTest extends AgentInstrumentationSpecification { - - @Shared - def server = new ServerExtension() { - @Override - protected void configure(ServerBuilder sb) throws Exception { - sb.service("/success") { ctx, req -> - HttpResponse.of(HttpStatus.OK, MediaType.PLAIN_TEXT, "Hello.") - } - } - } - - def setupSpec() { - server.start() - } - - def cleanupSpec() { - server.stop() - } - - def "multiple threads using the same builder works"() { - given: - def conds = new AsyncConditions(10) - def uri = server.httpUri().resolve("/success") - def builder = new JerseyClientBuilder() - - // Start 10 threads and do 50 requests each - when: - (1..10).each { - Thread.start { - boolean hadErrors = (1..50).any { - try { - Client client = builder.build() - client.target(uri).request().get() - } catch (Exception e) { - e.printStackTrace() - return true - } - return false - } - - conds.evaluate { - assert !hadErrors - } - } - } - - then: - conds.await(30) - } -} diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/JaxRsClientTest.groovy b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/JaxRsClientTest.groovy deleted file mode 100644 index b41a8efeea62..000000000000 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/JaxRsClientTest.groovy +++ /dev/null @@ -1,218 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -import io.opentelemetry.instrumentation.test.AgentTestTrait -import io.opentelemetry.instrumentation.test.base.HttpClientTest -import io.opentelemetry.instrumentation.testing.junit.http.HttpClientResult -import io.opentelemetry.instrumentation.testing.junit.http.SingleConnection -import io.opentelemetry.semconv.ServerAttributes -import io.opentelemetry.semconv.ErrorAttributes -import io.opentelemetry.semconv.HttpAttributes -import io.opentelemetry.semconv.NetworkAttributes -import io.opentelemetry.semconv.UrlAttributes -import org.apache.cxf.jaxrs.client.spec.ClientBuilderImpl -import org.glassfish.jersey.client.ClientConfig -import org.glassfish.jersey.client.ClientProperties -import org.glassfish.jersey.client.JerseyClientBuilder -import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder -import spock.lang.Unroll - -import javax.ws.rs.ProcessingException -import javax.ws.rs.client.ClientBuilder -import javax.ws.rs.client.Entity -import javax.ws.rs.client.Invocation -import javax.ws.rs.client.InvocationCallback -import javax.ws.rs.core.MediaType -import javax.ws.rs.core.Response -import java.util.concurrent.TimeUnit - -import static io.opentelemetry.api.trace.SpanKind.CLIENT -import static io.opentelemetry.api.trace.StatusCode.ERROR - -abstract class JaxRsClientTest extends HttpClientTest implements AgentTestTrait { - - boolean testRedirects() { - false - } - - @Override - boolean testNonStandardHttpMethod() { - false - } - - @Override - Invocation.Builder buildRequest(String method, URI uri, Map headers) { - return internalBuildRequest(uri, headers) - } - - @Override - int sendRequest(Invocation.Builder request, String method, URI uri, Map headers) { - try { - def body = BODY_METHODS.contains(method) ? Entity.text("") : null - def response = request.build(method, body).invoke() - // read response body to avoid broken pipe errors on the server side - response.readEntity(String) - try { - response.close() - } catch (IOException ignore) { - } - return response.status - } catch (ProcessingException exception) { - throw exception.getCause() - } - } - - @Override - void sendRequestWithCallback(Invocation.Builder request, String method, URI uri, Map headers, HttpClientResult requestResult) { - def body = BODY_METHODS.contains(method) ? Entity.text("") : null - - request.async().method(method, (Entity) body, new InvocationCallback() { - @Override - void completed(Response response) { - // read response body - response.readEntity(String) - requestResult.complete(response.status) - } - - @Override - void failed(Throwable throwable) { - if (throwable instanceof ProcessingException) { - throwable = throwable.getCause() - } - requestResult.complete(throwable) - } - }) - } - - private Invocation.Builder internalBuildRequest(URI uri, Map headers) { - def client = builder().build() - def service = client.target(uri) - def requestBuilder = service.request(MediaType.TEXT_PLAIN) - headers.each { requestBuilder.header(it.key, it.value) } - return requestBuilder - } - - abstract ClientBuilder builder() - - @Unroll - def "should properly convert HTTP status #statusCode to span error status"() { - given: - def method = "GET" - def uri = resolveAddress(path) - - when: - def actualStatusCode = doRequest(method, uri) - - then: - assert actualStatusCode == statusCode - - assertTraces(1) { - trace(0, 2) { - span(0) { - hasNoParent() - name "$method" - kind CLIENT - status ERROR - attributes { - "$NetworkAttributes.NETWORK_PROTOCOL_VERSION" "1.1" - "$ServerAttributes.SERVER_ADDRESS" uri.host - "$ServerAttributes.SERVER_PORT" uri.port > 0 ? uri.port : { it == null || it == 443 } - "$NetworkAttributes.NETWORK_PEER_ADDRESS" { it == "127.0.0.1" || it == null } - "$UrlAttributes.URL_FULL" "${uri}" - "$HttpAttributes.HTTP_REQUEST_METHOD" method - "$HttpAttributes.HTTP_RESPONSE_STATUS_CODE" statusCode - "$ErrorAttributes.ERROR_TYPE" "$statusCode" - } - } - serverSpan(it, 1, span(0)) - } - } - - where: - path | statusCode - "/client-error" | 400 - "/error" | 500 - } -} - -class JerseyClientTest extends JaxRsClientTest { - - @Override - ClientBuilder builder() { - ClientConfig config = new ClientConfig() - config.property(ClientProperties.CONNECT_TIMEOUT, CONNECT_TIMEOUT_MS) - config.property(ClientProperties.READ_TIMEOUT, READ_TIMEOUT_MS) - return new JerseyClientBuilder().withConfig(config) - } - - @Override - SingleConnection createSingleConnection(String host, int port) { - // Jersey JAX-RS client uses HttpURLConnection internally, which does not support pipelining nor - // waiting for a connection in the pool to become available. Therefore a high concurrency test - // would require manually doing requests one after another which is not meaningful for a high - // concurrency test. - return null - } -} - -class ResteasyClientTest extends JaxRsClientTest { - - @Override - ClientBuilder builder() { - return new ResteasyClientBuilder() - .establishConnectionTimeout(CONNECT_TIMEOUT_MS, TimeUnit.MILLISECONDS) - .socketTimeout(READ_TIMEOUT_MS, TimeUnit.MILLISECONDS) - } - - @Override - SingleConnection createSingleConnection(String host, int port) { - return new ResteasySingleConnection(host, port) - } -} - -class CxfClientTest extends JaxRsClientTest { - - @Override - Throwable clientSpanError(URI uri, Throwable exception) { - switch (uri.toString()) { - case "http://localhost:61/": // unopened port - if (exception.getCause() instanceof ConnectException) { - exception = exception.getCause() - } - break - case "https://192.0.2.1/": // non routable address - if (exception.getCause() != null) { - exception = exception.getCause() - } - } - return exception - } - - @Override - boolean testWithClientParent() { - !Boolean.getBoolean("testLatestDeps") - } - - @Override - boolean testReadTimeout() { - return false - } - - @Override - ClientBuilder builder() { - return new ClientBuilderImpl() - .property("http.connection.timeout", (long) CONNECT_TIMEOUT_MS) - .property("org.apache.cxf.transport.http.forceVersion", "1.1") - } - - @Override - SingleConnection createSingleConnection(String host, int port) { - // CXF JAX-RS client uses HttpURLConnection internally, which does not support pipelining nor - // waiting for a connection in the pool to become available. Therefore a high concurrency test - // would require manually doing requests one after another which is not meaningful for a high - // concurrency test. - return null - } -} diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/ResteasyProxyClientTest.groovy b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/ResteasyProxyClientTest.groovy deleted file mode 100644 index d4a5e4556a2b..000000000000 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/ResteasyProxyClientTest.groovy +++ /dev/null @@ -1,119 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -import io.opentelemetry.instrumentation.test.AgentTestTrait -import io.opentelemetry.instrumentation.test.base.HttpClientTest -import org.apache.http.client.utils.URLEncodedUtils -import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder -import org.jboss.resteasy.specimpl.ResteasyUriBuilder -import spock.lang.AutoCleanup -import spock.lang.Shared - -import javax.ws.rs.GET -import javax.ws.rs.HeaderParam -import javax.ws.rs.POST -import javax.ws.rs.PUT -import javax.ws.rs.Path -import javax.ws.rs.QueryParam -import javax.ws.rs.core.Response -import java.nio.charset.StandardCharsets - -class ResteasyProxyClientTest extends HttpClientTest implements AgentTestTrait { - - @Shared - @AutoCleanup - def client = new ResteasyClientBuilder() - .connectionPoolSize(4) - .build() - - @Override - ResteasyProxyResource buildRequest(String method, URI uri, Map headers) { - return client - .target(new ResteasyUriBuilder().uri(resolveAddress(""))) - .proxy(ResteasyProxyResource) - } - - @Override - int sendRequest(ResteasyProxyResource proxy, String method, URI uri, Map headers) { - def proxyMethodName = "${method}_${uri.path}".toLowerCase() - .replace("/", "") - .replace('-', '_') - - def param = URLEncodedUtils.parse(uri, StandardCharsets.UTF_8.name()) - .stream().findFirst() - .map({ it.value }) - .orElse(null) - - def isTestServer = headers.get("is-test-server") - def requestId = headers.get("test-request-id") - - Response response = proxy."$proxyMethodName"(param, isTestServer, requestId) - response.close() - - return response.status - } - - @Override - boolean testRedirects() { - false - } - - @Override - boolean testConnectionFailure() { - false - } - - @Override - boolean testRemoteConnection() { - false - } - - @Override - boolean testCallback() { - false - } - - @Override - boolean testCapturedHttpHeaders() { - false - } - - @Override - boolean testReadTimeout() { - return false - } - - @Override - boolean testNonStandardHttpMethod() { - false - } -} - -@Path("") -interface ResteasyProxyResource { - @GET - @Path("success") - Response get_success(@QueryParam("with") String param, - @HeaderParam("is-test-server") String isTestServer, - @HeaderParam("test-request-id") String requestId) - - @POST - @Path("success") - Response post_success(@QueryParam("with") String param, - @HeaderParam("is-test-server") String isTestServer, - @HeaderParam("test-request-id") String requestId) - - @PUT - @Path("success") - Response put_success(@QueryParam("with") String param, - @HeaderParam("is-test-server") String isTestServer, - @HeaderParam("test-request-id") String requestId) - - @GET - @Path("error") - Response get_error(@QueryParam("with") String param, - @HeaderParam("is-test-server") String isTestServer, - @HeaderParam("test-request-id") String requestId) -} diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/ResteasySingleConnection.groovy b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/ResteasySingleConnection.groovy deleted file mode 100644 index 11e2a0a4b51e..000000000000 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/groovy/ResteasySingleConnection.groovy +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -import io.opentelemetry.instrumentation.testing.junit.http.SingleConnection -import org.jboss.resteasy.client.jaxrs.ResteasyClient -import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder - -import javax.ws.rs.core.MediaType -import java.util.concurrent.ExecutionException -import java.util.concurrent.TimeUnit -import java.util.concurrent.TimeoutException - -class ResteasySingleConnection implements SingleConnection { - private final ResteasyClient client - private final String host - private final int port - - ResteasySingleConnection(String host, int port) { - this.host = host - this.port = port - this.client = new ResteasyClientBuilder() - .establishConnectionTimeout(5000, TimeUnit.MILLISECONDS) - .connectionPoolSize(1) - .build() - } - - @Override - int doRequest(String path, Map headers) throws ExecutionException, InterruptedException, TimeoutException { - String requestId = Objects.requireNonNull(headers.get(REQUEST_ID_HEADER)) - - URI uri - try { - uri = new URL("http", host, port, path).toURI() - } catch (MalformedURLException e) { - throw new ExecutionException(e) - } - - def requestBuilder = client.target(uri).request(MediaType.TEXT_PLAIN) - headers.each { requestBuilder.header(it.key, it.value) } - - def response = requestBuilder.buildGet().invoke() - response.close() - - String responseId = response.getHeaderString(REQUEST_ID_HEADER) - if (requestId != responseId) { - throw new IllegalStateException( - String.format("Received response with id %s, expected %s", responseId, requestId)) - } - - return response.getStatus() - } -} diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/CxfClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/CxfClientTest.java new file mode 100644 index 000000000000..6c2537b438cb --- /dev/null +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/CxfClientTest.java @@ -0,0 +1,53 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jaxrsclient; + +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import java.net.ConnectException; +import java.net.URI; +import javax.ws.rs.client.ClientBuilder; +import org.apache.cxf.jaxrs.client.spec.ClientBuilderImpl; + +class CxfClientTest extends JaxRsClientTest { + + @Override + public ClientBuilder builder() { + return new ClientBuilderImpl() + .property("http.connection.timeout", (long) CONNECT_TIMEOUT_MS) + .property("org.apache.cxf.transport.http.forceVersion", "1.1"); + } + + Throwable clientSpanError(URI uri, Throwable exception) { + switch (uri.toString()) { + case "http://localhost:61/": // unopened port + if (exception.getCause() instanceof ConnectException) { + exception = exception.getCause(); + } + break; + case "https://192.0.2.1/": // non routable address + if (exception.getCause() != null) { + exception = exception.getCause(); + } + break; + default: + break; + } + return exception; + } + + @Override + protected void configure(HttpClientTestOptions.Builder optionsBuilder) { + super.configure(optionsBuilder); + optionsBuilder.setTestReadTimeout(false); + optionsBuilder.setTestWithClientParent(!Boolean.getBoolean("testLatestDeps")); + optionsBuilder.setClientSpanErrorMapper((uri, exception) -> clientSpanError(uri, exception)); + // CXF JAX-RS client uses HttpURLConnection internally, which does not support pipelining nor + // waiting for a connection in the pool to become available. Therefore a high concurrency test + // would require manually doing requests one after another which is not meaningful for a high + // concurrency test. + optionsBuilder.setSingleConnectionFactory((host, port) -> null); + } +} diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java new file mode 100644 index 000000000000..948134180d33 --- /dev/null +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java @@ -0,0 +1,87 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jaxrsclient; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.testing.internal.armeria.common.HttpResponse; +import io.opentelemetry.testing.internal.armeria.common.HttpStatus; +import io.opentelemetry.testing.internal.armeria.common.MediaType; +import io.opentelemetry.testing.internal.armeria.server.ServerBuilder; +import io.opentelemetry.testing.internal.armeria.testing.junit5.server.ServerExtension; +import java.net.URI; +import java.util.concurrent.CountDownLatch; +import javax.ws.rs.client.Client; +import org.glassfish.jersey.client.JerseyClientBuilder; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.extension.RegisterExtension; + +class JaxMultithreadedClientTest { + + @RegisterExtension + static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); + + public static ServerExtension server = + new ServerExtension() { + @Override + protected void configure(ServerBuilder sb) { + sb.service( + "/success", + (ctx, req) -> HttpResponse.of(HttpStatus.OK, MediaType.PLAIN_TEXT, "Hello.")); + } + }; + + @BeforeAll + static void setUp() { + server.start(); + } + + @AfterAll + static void cleanupSpec() { + server.stop(); + } + + @SuppressWarnings("CatchingUnchecked") + boolean checkUri(JerseyClientBuilder builder, URI uri) { + try { + Client client = builder.build(); + client.target(uri).request().get(); + } catch (Exception e) { + return true; + } + return false; + } + + @DisplayName("multiple threads using the same builder works") + void testMultipleThreads() throws InterruptedException { + URI uri = server.httpUri().resolve("/success"); + JerseyClientBuilder builder = new JerseyClientBuilder(); + + // Start 10 threads and do 50 requests each + CountDownLatch latch = new CountDownLatch(10); + for (int i = 0; i < 10; i++) { + new Thread( + new Runnable() { + @Override + public void run() { + boolean hadErrors = false; + for (int j = 0; j < 50; j++) { + hadErrors = hadErrors || checkUri(builder, uri); + } + assertThat(hadErrors).isFalse(); + latch.countDown(); + } + }) + .start(); + } + + latch.await(); + } +} diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java new file mode 100644 index 000000000000..90e33a999aab --- /dev/null +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java @@ -0,0 +1,158 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jaxrsclient; + +import static io.opentelemetry.api.trace.SpanKind.CLIENT; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies; +import static java.util.Arrays.asList; +import static org.assertj.core.api.Assertions.assertThat; + +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.HttpClientResult; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import io.opentelemetry.sdk.trace.data.StatusData; +import io.opentelemetry.semconv.ErrorAttributes; +import io.opentelemetry.semconv.HttpAttributes; +import io.opentelemetry.semconv.NetworkAttributes; +import io.opentelemetry.semconv.ServerAttributes; +import io.opentelemetry.semconv.UrlAttributes; +import java.net.URI; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; +import javax.ws.rs.ProcessingException; +import javax.ws.rs.client.Client; +import javax.ws.rs.client.ClientBuilder; +import javax.ws.rs.client.Entity; +import javax.ws.rs.client.Invocation; +import javax.ws.rs.client.InvocationCallback; +import javax.ws.rs.client.WebTarget; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +abstract class JaxRsClientTest extends AbstractHttpClientTest { + + @RegisterExtension + static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent(); + + protected static final List BODY_METHODS = asList("POST", "PUT"); + protected static final int CONNECT_TIMEOUT_MS = 5000; + protected static final int READ_TIMEOUT_MS = 2000; + + static Stream preparedPathStream() { + return Stream.of(Arguments.of("/client-error", 400), Arguments.of("/error", 500)); + } + + @Override + public Invocation.Builder buildRequest(String method, URI uri, Map headers) { + return internalBuildRequest(uri, headers); + } + + abstract ClientBuilder builder(); + + @Override + protected void configure(HttpClientTestOptions.Builder optionsBuilder) { + super.configure(optionsBuilder); + optionsBuilder.setTestRedirects(false); + optionsBuilder.setTestNonStandardHttpMethod(false); + } + + private Invocation.Builder internalBuildRequest(URI uri, Map headers) { + Client client = builder().build(); + WebTarget service = client.target(uri); + Invocation.Builder requestBuilder = service.request(MediaType.TEXT_PLAIN); + for (Map.Entry entry : headers.entrySet()) { + requestBuilder.header(entry.getKey(), entry.getValue()); + } + return requestBuilder; + } + + @Override + public int sendRequest( + Invocation.Builder request, String method, URI uri, Map headers) { + try { + Entity body = BODY_METHODS.contains(method) ? Entity.text("") : null; + Response response = request.build(method, body).invoke(); + // read response body to avoid broken pipe errors on the server side + response.readEntity(String.class); + response.close(); + return response.getStatus(); + } catch (ProcessingException exception) { + throw exception; + } + } + + @Override + public void sendRequestWithCallback( + Invocation.Builder request, + String method, + URI uri, + Map headers, + HttpClientResult requestResult) { + Entity body = BODY_METHODS.contains(method) ? Entity.text("") : null; + + request + .async() + .method( + method, + body, + new InvocationCallback() { + @Override + public void completed(Response response) { + // read response body + response.readEntity(String.class); + requestResult.complete(response.getStatus()); + } + + @Override + public void failed(Throwable throwable) { + if (throwable instanceof ProcessingException) { + throwable = throwable.getCause(); + } + requestResult.complete(throwable); + } + }); + } + + @ParameterizedTest + @MethodSource("preparedPathStream") + void testError(String path, int statusCode) throws Throwable { + String method = "GET"; + URI uri = resolveAddress(path); + int actualStatusCode = + sendRequest( + buildRequest(method, uri, Collections.emptyMap()), method, uri, Collections.emptyMap()); + assertThat(actualStatusCode).isEqualTo(statusCode); + + testing.waitAndAssertTraces( + trace -> + trace.hasSpansSatisfyingExactly( + span -> + span.hasName(method) + .hasKind(CLIENT) + .hasStatus(StatusData.error()) + .hasAttributesSatisfying( + equalTo(NetworkAttributes.NETWORK_PROTOCOL_VERSION, "1.1"), + equalTo(ServerAttributes.SERVER_ADDRESS, uri.getHost()), + satisfies(ServerAttributes.SERVER_PORT, val -> val.isIn(null, 443)), + satisfies( + NetworkAttributes.NETWORK_PEER_ADDRESS, + val -> val.isIn(null, "127.0.0.1")), + equalTo(UrlAttributes.URL_FULL, uri.getHost()), + equalTo(HttpAttributes.HTTP_REQUEST_METHOD, method), + equalTo(HttpAttributes.HTTP_RESPONSE_STATUS_CODE, statusCode), + equalTo(ErrorAttributes.ERROR_TYPE, "$statusCode")), + span -> span.hasParent(trace.getSpan(0)).hasName(method))); + } +} diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java new file mode 100644 index 000000000000..afc6a9d9b19d --- /dev/null +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java @@ -0,0 +1,33 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jaxrsclient; + +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import javax.ws.rs.client.ClientBuilder; +import org.glassfish.jersey.client.ClientConfig; +import org.glassfish.jersey.client.ClientProperties; +import org.glassfish.jersey.client.JerseyClientBuilder; + +class JerseyClientTest extends JaxRsClientTest { + + @Override + public ClientBuilder builder() { + ClientConfig config = new ClientConfig(); + config.property(ClientProperties.CONNECT_TIMEOUT, CONNECT_TIMEOUT_MS); + config.property(ClientProperties.READ_TIMEOUT, READ_TIMEOUT_MS); + return new JerseyClientBuilder().withConfig(config); + } + + @Override + protected void configure(HttpClientTestOptions.Builder optionsBuilder) { + super.configure(optionsBuilder); + // Jersey JAX-RS client uses HttpURLConnection internally, which does not support pipelining nor + // waiting for a connection in the pool to become available. Therefore a high concurrency test + // would require manually doing requests one after another which is not meaningful for a high + // concurrency test. + optionsBuilder.setSingleConnectionFactory((host, port) -> null); + } +} diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java new file mode 100644 index 000000000000..55db774d9533 --- /dev/null +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java @@ -0,0 +1,28 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jaxrsclient; + +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import java.util.concurrent.TimeUnit; +import javax.ws.rs.client.ClientBuilder; +import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder; + +class ResteasyClientTest extends JaxRsClientTest { + + @Override + ClientBuilder builder() { + return new ResteasyClientBuilder() + .establishConnectionTimeout(CONNECT_TIMEOUT_MS, TimeUnit.MILLISECONDS) + .socketTimeout(READ_TIMEOUT_MS, TimeUnit.MILLISECONDS); + } + + @Override + protected void configure(HttpClientTestOptions.Builder optionsBuilder) { + super.configure(optionsBuilder); + optionsBuilder.setSingleConnectionFactory( + (host, port) -> new ResteasySingleConnection(host, port)); + } +} diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyProxyClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyProxyClientTest.java new file mode 100644 index 000000000000..8ac5d557a4cd --- /dev/null +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyProxyClientTest.java @@ -0,0 +1,80 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jaxrsclient; + +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.nio.charset.StandardCharsets; +import java.util.Locale; +import java.util.Map; +import javax.ws.rs.core.Response; +import org.apache.http.NameValuePair; +import org.apache.http.client.utils.URLEncodedUtils; +import org.jboss.resteasy.client.jaxrs.ResteasyClient; +import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder; +import org.jboss.resteasy.specimpl.ResteasyUriBuilder; +import org.junit.jupiter.api.extension.RegisterExtension; + +class ResteasyProxyClientTest extends AbstractHttpClientTest { + @RegisterExtension + static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent(); + + static ResteasyClient client = new ResteasyClientBuilder().connectionPoolSize(4).build(); + + @Override + public ResteasyProxyResource buildRequest(String method, URI uri, Map headers) { + return client + .target(new ResteasyUriBuilder().uri(resolveAddress(""))) + .proxy(ResteasyProxyResource.class); + } + + @Override + public int sendRequest( + ResteasyProxyResource proxy, String method, URI uri, Map headers) { + String proxyMethodName = + (method + "_" + uri.getPath()).toLowerCase(Locale.ROOT).replace("/", "").replace('-', '_'); + + String param = + URLEncodedUtils.parse(uri, StandardCharsets.UTF_8.name()).stream() + .findFirst() + .map(NameValuePair::getValue) + .orElse(null); + + String isTestServer = headers.get("is-test-server"); + String requestId = headers.get("test-request-id"); + + Response response; + if (proxyMethodName.equals("get_success")) { + response = proxy.get_success(param, isTestServer, requestId); + } else if (proxyMethodName.equals("post_success")) { + response = proxy.post_success(param, isTestServer, requestId); + } else if (proxyMethodName.equals("put_success")) { + response = proxy.put_success(param, isTestServer, requestId); + } else if (proxyMethodName.equals("get_error")) { + response = proxy.get_error(param, isTestServer, requestId); + } else { + throw new IllegalArgumentException("Unknown method: " + proxyMethodName); + } + response.close(); + return response.getStatus(); + } + + @Override + protected void configure(HttpClientTestOptions.Builder optionsBuilder) { + super.configure(optionsBuilder); + optionsBuilder.setTestCallback(false); + optionsBuilder.setTestConnectionFailure(false); + optionsBuilder.setTestNonStandardHttpMethod(false); + optionsBuilder.setTestReadTimeout(false); + optionsBuilder.setTestRemoteConnection(false); + optionsBuilder.setTestRedirects(false); + optionsBuilder.setTestCaptureHttpHeaders(false); + optionsBuilder.disableTestSpanEndsAfter(); + } +} diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyProxyResource.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyProxyResource.java new file mode 100644 index 000000000000..e356796c88ea --- /dev/null +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyProxyResource.java @@ -0,0 +1,45 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jaxrsclient; + +import javax.ws.rs.GET; +import javax.ws.rs.HeaderParam; +import javax.ws.rs.POST; +import javax.ws.rs.PUT; +import javax.ws.rs.Path; +import javax.ws.rs.QueryParam; +import javax.ws.rs.core.Response; + +@Path("") +interface ResteasyProxyResource { + @GET + @Path("error") + Response get_error( + @QueryParam("with") String param, + @HeaderParam("is-test-server") String isTestServer, + @HeaderParam("test-request-id") String requestId); + + @GET + @Path("success") + Response get_success( + @QueryParam("with") String param, + @HeaderParam("is-test-server") String isTestServer, + @HeaderParam("test-request-id") String requestId); + + @POST + @Path("success") + Response post_success( + @QueryParam("with") String param, + @HeaderParam("is-test-server") String isTestServer, + @HeaderParam("test-request-id") String requestId); + + @PUT + @Path("success") + Response put_success( + @QueryParam("with") String param, + @HeaderParam("is-test-server") String isTestServer, + @HeaderParam("test-request-id") String requestId); +} diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasySingleConnection.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasySingleConnection.java new file mode 100644 index 000000000000..b8e4180b6ff3 --- /dev/null +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasySingleConnection.java @@ -0,0 +1,65 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jaxrsclient; + +import io.opentelemetry.instrumentation.testing.junit.http.SingleConnection; +import java.net.MalformedURLException; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import javax.ws.rs.client.Invocation; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import org.jboss.resteasy.client.jaxrs.ResteasyClient; +import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder; + +public class ResteasySingleConnection implements SingleConnection { + private final ResteasyClient client; + private final String host; + private final int port; + + ResteasySingleConnection(String host, int port) { + this.host = host; + this.port = port; + this.client = + new ResteasyClientBuilder() + .establishConnectionTimeout(5000, TimeUnit.MILLISECONDS) + .connectionPoolSize(1) + .build(); + } + + @Override + public int doRequest(String path, Map headers) throws ExecutionException { + String requestId = Objects.requireNonNull(headers.get(REQUEST_ID_HEADER)); + + URI uri; + try { + uri = new URL("http", host, port, path).toURI(); + } catch (MalformedURLException | URISyntaxException e) { + throw new ExecutionException(e); + } + + Invocation.Builder requestBuilder = client.target(uri).request(MediaType.TEXT_PLAIN); + for (Map.Entry entry : headers.entrySet()) { + requestBuilder.header(entry.getKey(), entry.getValue()); + } + + Response response = requestBuilder.buildGet().invoke(); + response.close(); + + String responseId = response.getHeaderString(REQUEST_ID_HEADER); + if (Objects.equals(requestId, responseId)) { + throw new IllegalStateException( + String.format("Received response with id %s, expected %s", responseId, requestId)); + } + + return response.getStatus(); + } +} 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 73db5eac8c02..b06c933bfd53 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 @@ -510,6 +510,7 @@ void requestWithExistingTracingHeaders() throws Exception { @Test void captureHttpHeaders() throws Exception { + assumeTrue(options.getTestCaptureHttpHeaders()); URI uri = resolveAddress("/success"); String method = "GET"; int responseCode = diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java index 4500176bbf0c..de33011271ac 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java @@ -58,6 +58,8 @@ public abstract class HttpClientTestOptions { abstract HttpClientInstrumentationType getInstrumentationType(); + public abstract boolean getTestCaptureHttpHeaders(); + public boolean isLowLevelInstrumentation() { return getInstrumentationType() == HttpClientInstrumentationType.LOW_LEVEL; } @@ -131,9 +133,12 @@ default Builder withDefaults() { .setTestCallbackWithParent(true) .setTestErrorWithCallback(true) .setTestNonStandardHttpMethod(true) + .setTestCaptureHttpHeaders(true) .setHttpProtocolVersion(uri -> "1.1"); } + Builder setTestCaptureHttpHeaders(boolean value); + Builder setHttpAttributes(Function>> value); Builder setResponseCodeOnRedirectError(Integer value); From 05f1efdd3e804b8cb19fc63e9ba93d4a254b5e83 Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Tue, 17 Dec 2024 20:05:09 +0800 Subject: [PATCH 02/10] fix ut --- .../jaxrsclient/CxfClientTest.java | 5 -- .../JaxMultithreadedClientTest.java | 2 +- .../jaxrsclient/JaxRsClientTest.java | 74 +++---------------- .../jaxrsclient/JerseyClientTest.java | 11 --- .../jaxrsclient/ResteasyClientTest.java | 3 +- .../jaxrsclient/ResteasySingleConnection.java | 2 +- 6 files changed, 15 insertions(+), 82 deletions(-) diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/CxfClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/CxfClientTest.java index 6c2537b438cb..a07ddcee0d70 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/CxfClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/CxfClientTest.java @@ -44,10 +44,5 @@ protected void configure(HttpClientTestOptions.Builder optionsBuilder) { optionsBuilder.setTestReadTimeout(false); optionsBuilder.setTestWithClientParent(!Boolean.getBoolean("testLatestDeps")); optionsBuilder.setClientSpanErrorMapper((uri, exception) -> clientSpanError(uri, exception)); - // CXF JAX-RS client uses HttpURLConnection internally, which does not support pipelining nor - // waiting for a connection in the pool to become available. Therefore a high concurrency test - // would require manually doing requests one after another which is not meaningful for a high - // concurrency test. - optionsBuilder.setSingleConnectionFactory((host, port) -> null); } } diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java index 948134180d33..4c3780388f44 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java @@ -28,7 +28,7 @@ class JaxMultithreadedClientTest { @RegisterExtension static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); - public static ServerExtension server = + static ServerExtension server = new ServerExtension() { @Override protected void configure(ServerBuilder sb) { diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java index 90e33a999aab..8f94541c5a4c 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java @@ -5,28 +5,16 @@ package io.opentelemetry.javaagent.instrumentation.jaxrsclient; -import static io.opentelemetry.api.trace.SpanKind.CLIENT; -import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; -import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies; import static java.util.Arrays.asList; -import static org.assertj.core.api.Assertions.assertThat; 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.HttpClientResult; import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; -import io.opentelemetry.sdk.trace.data.StatusData; -import io.opentelemetry.semconv.ErrorAttributes; -import io.opentelemetry.semconv.HttpAttributes; -import io.opentelemetry.semconv.NetworkAttributes; -import io.opentelemetry.semconv.ServerAttributes; -import io.opentelemetry.semconv.UrlAttributes; import java.net.URI; -import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.stream.Stream; import javax.ws.rs.ProcessingException; import javax.ws.rs.client.Client; import javax.ws.rs.client.ClientBuilder; @@ -37,9 +25,6 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import org.junit.jupiter.api.extension.RegisterExtension; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; abstract class JaxRsClientTest extends AbstractHttpClientTest { @@ -50,13 +35,15 @@ abstract class JaxRsClientTest extends AbstractHttpClientTest preparedPathStream() { - return Stream.of(Arguments.of("/client-error", 400), Arguments.of("/error", 500)); - } - @Override public Invocation.Builder buildRequest(String method, URI uri, Map headers) { - return internalBuildRequest(uri, headers); + Client client = builder().build(); + WebTarget service = client.target(uri); + Invocation.Builder requestBuilder = service.request(MediaType.TEXT_PLAIN); + for (Map.Entry entry : headers.entrySet()) { + requestBuilder.header(entry.getKey(), entry.getValue()); + } + return requestBuilder; } abstract ClientBuilder builder(); @@ -68,19 +55,10 @@ protected void configure(HttpClientTestOptions.Builder optionsBuilder) { optionsBuilder.setTestNonStandardHttpMethod(false); } - private Invocation.Builder internalBuildRequest(URI uri, Map headers) { - Client client = builder().build(); - WebTarget service = client.target(uri); - Invocation.Builder requestBuilder = service.request(MediaType.TEXT_PLAIN); - for (Map.Entry entry : headers.entrySet()) { - requestBuilder.header(entry.getKey(), entry.getValue()); - } - return requestBuilder; - } - @Override public int sendRequest( - Invocation.Builder request, String method, URI uri, Map headers) { + Invocation.Builder request, String method, URI uri, Map headers) + throws Exception { try { Entity body = BODY_METHODS.contains(method) ? Entity.text("") : null; Response response = request.build(method, body).invoke(); @@ -89,6 +67,9 @@ public int sendRequest( response.close(); return response.getStatus(); } catch (ProcessingException exception) { + if (exception.getCause() instanceof Exception) { + throw (Exception) exception.getCause(); + } throw exception; } } @@ -124,35 +105,4 @@ public void failed(Throwable throwable) { } }); } - - @ParameterizedTest - @MethodSource("preparedPathStream") - void testError(String path, int statusCode) throws Throwable { - String method = "GET"; - URI uri = resolveAddress(path); - int actualStatusCode = - sendRequest( - buildRequest(method, uri, Collections.emptyMap()), method, uri, Collections.emptyMap()); - assertThat(actualStatusCode).isEqualTo(statusCode); - - testing.waitAndAssertTraces( - trace -> - trace.hasSpansSatisfyingExactly( - span -> - span.hasName(method) - .hasKind(CLIENT) - .hasStatus(StatusData.error()) - .hasAttributesSatisfying( - equalTo(NetworkAttributes.NETWORK_PROTOCOL_VERSION, "1.1"), - equalTo(ServerAttributes.SERVER_ADDRESS, uri.getHost()), - satisfies(ServerAttributes.SERVER_PORT, val -> val.isIn(null, 443)), - satisfies( - NetworkAttributes.NETWORK_PEER_ADDRESS, - val -> val.isIn(null, "127.0.0.1")), - equalTo(UrlAttributes.URL_FULL, uri.getHost()), - equalTo(HttpAttributes.HTTP_REQUEST_METHOD, method), - equalTo(HttpAttributes.HTTP_RESPONSE_STATUS_CODE, statusCode), - equalTo(ErrorAttributes.ERROR_TYPE, "$statusCode")), - span -> span.hasParent(trace.getSpan(0)).hasName(method))); - } } diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java index afc6a9d9b19d..411f33b5d171 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java @@ -5,7 +5,6 @@ package io.opentelemetry.javaagent.instrumentation.jaxrsclient; -import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; import javax.ws.rs.client.ClientBuilder; import org.glassfish.jersey.client.ClientConfig; import org.glassfish.jersey.client.ClientProperties; @@ -20,14 +19,4 @@ public ClientBuilder builder() { config.property(ClientProperties.READ_TIMEOUT, READ_TIMEOUT_MS); return new JerseyClientBuilder().withConfig(config); } - - @Override - protected void configure(HttpClientTestOptions.Builder optionsBuilder) { - super.configure(optionsBuilder); - // Jersey JAX-RS client uses HttpURLConnection internally, which does not support pipelining nor - // waiting for a connection in the pool to become available. Therefore a high concurrency test - // would require manually doing requests one after another which is not meaningful for a high - // concurrency test. - optionsBuilder.setSingleConnectionFactory((host, port) -> null); - } } diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java index 55db774d9533..efe62a06f256 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java @@ -22,7 +22,6 @@ ClientBuilder builder() { @Override protected void configure(HttpClientTestOptions.Builder optionsBuilder) { super.configure(optionsBuilder); - optionsBuilder.setSingleConnectionFactory( - (host, port) -> new ResteasySingleConnection(host, port)); + optionsBuilder.setSingleConnectionFactory(ResteasySingleConnection::new); } } diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasySingleConnection.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasySingleConnection.java index b8e4180b6ff3..60169e1c65d1 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasySingleConnection.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasySingleConnection.java @@ -20,7 +20,7 @@ import org.jboss.resteasy.client.jaxrs.ResteasyClient; import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder; -public class ResteasySingleConnection implements SingleConnection { +class ResteasySingleConnection implements SingleConnection { private final ResteasyClient client; private final String host; private final int port; From d9531b23ee32c9864a432f3a9b9bd1606d969be2 Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Tue, 17 Dec 2024 20:13:49 +0800 Subject: [PATCH 03/10] update --- .../testing/junit/http/HttpClientTestOptions.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java index de33011271ac..2601ce2f49cf 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java @@ -58,8 +58,6 @@ public abstract class HttpClientTestOptions { abstract HttpClientInstrumentationType getInstrumentationType(); - public abstract boolean getTestCaptureHttpHeaders(); - public boolean isLowLevelInstrumentation() { return getInstrumentationType() == HttpClientInstrumentationType.LOW_LEVEL; } @@ -91,6 +89,8 @@ public boolean isLowLevelInstrumentation() { public abstract boolean getTestNonStandardHttpMethod(); + public abstract boolean getTestCaptureHttpHeaders(); + public abstract Function getHttpProtocolVersion(); @Nullable From 8035fb37b7d299c94219fbb0687bdc2c41a0515a Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Wed, 18 Dec 2024 14:14:29 +0800 Subject: [PATCH 04/10] refine ut --- .../instrumentation/jaxrsclient/JaxMultithreadedClientTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java index 4c3780388f44..990d6e0fd354 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java @@ -44,7 +44,7 @@ static void setUp() { } @AfterAll - static void cleanupSpec() { + static void cleanUp() { server.stop(); } From 044bc96a575ff785130c68cad6d526c382056ca5 Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Wed, 18 Dec 2024 17:39:38 +0800 Subject: [PATCH 05/10] add test client error --- .../jaxrsclient/JaxRsClientTest.java | 3 ++- .../junit/http/AbstractHttpClientTest.java | 20 +++++++++++++++++++ .../junit/http/HttpClientTestOptions.java | 5 +++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java index 8f94541c5a4c..152e994f5de4 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java @@ -33,7 +33,7 @@ abstract class JaxRsClientTest extends AbstractHttpClientTest BODY_METHODS = asList("POST", "PUT"); protected static final int CONNECT_TIMEOUT_MS = 5000; - protected static final int READ_TIMEOUT_MS = 2000; + protected static final int READ_TIMEOUT_MS = 3000; @Override public Invocation.Builder buildRequest(String method, URI uri, Map headers) { @@ -52,6 +52,7 @@ public Invocation.Builder buildRequest(String method, URI uri, Map + trace.hasSpansSatisfyingExactly( + span -> + assertClientSpan(span, uri, method, 400, null) + .hasNoParent() + .hasStatus(StatusData.error()), + span -> assertServerSpan(span).hasParent(trace.getSpan(0)))); + } + // Visible for spock bridge. SpanDataAssert assertClientSpan( SpanDataAssert span, diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java index 2601ce2f49cf..f92e3ff9e902 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java @@ -89,6 +89,8 @@ public boolean isLowLevelInstrumentation() { public abstract boolean getTestNonStandardHttpMethod(); + public abstract boolean getTestClientError(); + public abstract boolean getTestCaptureHttpHeaders(); public abstract Function getHttpProtocolVersion(); @@ -134,6 +136,7 @@ default Builder withDefaults() { .setTestErrorWithCallback(true) .setTestNonStandardHttpMethod(true) .setTestCaptureHttpHeaders(true) + .setTestClientError(false) .setHttpProtocolVersion(uri -> "1.1"); } @@ -179,6 +182,8 @@ default Builder withDefaults() { Builder setTestNonStandardHttpMethod(boolean value); + Builder setTestClientError(boolean value); + Builder setHttpProtocolVersion(Function value); @CanIgnoreReturnValue From 63fdefb53f65560bb4f23063ad20cfc57255e1d8 Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Thu, 19 Dec 2024 13:09:41 +0800 Subject: [PATCH 06/10] Revert "add test client error" This reverts commit 044bc96a575ff785130c68cad6d526c382056ca5. --- .../jaxrsclient/JaxRsClientTest.java | 3 +-- .../junit/http/AbstractHttpClientTest.java | 20 ------------------- .../junit/http/HttpClientTestOptions.java | 5 ----- 3 files changed, 1 insertion(+), 27 deletions(-) diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java index 152e994f5de4..8f94541c5a4c 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java @@ -33,7 +33,7 @@ abstract class JaxRsClientTest extends AbstractHttpClientTest BODY_METHODS = asList("POST", "PUT"); protected static final int CONNECT_TIMEOUT_MS = 5000; - protected static final int READ_TIMEOUT_MS = 3000; + protected static final int READ_TIMEOUT_MS = 2000; @Override public Invocation.Builder buildRequest(String method, URI uri, Map headers) { @@ -52,7 +52,6 @@ public Invocation.Builder buildRequest(String method, URI uri, Map - trace.hasSpansSatisfyingExactly( - span -> - assertClientSpan(span, uri, method, 400, null) - .hasNoParent() - .hasStatus(StatusData.error()), - span -> assertServerSpan(span).hasParent(trace.getSpan(0)))); - } - // Visible for spock bridge. SpanDataAssert assertClientSpan( SpanDataAssert span, diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java index f92e3ff9e902..2601ce2f49cf 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java @@ -89,8 +89,6 @@ public boolean isLowLevelInstrumentation() { public abstract boolean getTestNonStandardHttpMethod(); - public abstract boolean getTestClientError(); - public abstract boolean getTestCaptureHttpHeaders(); public abstract Function getHttpProtocolVersion(); @@ -136,7 +134,6 @@ default Builder withDefaults() { .setTestErrorWithCallback(true) .setTestNonStandardHttpMethod(true) .setTestCaptureHttpHeaders(true) - .setTestClientError(false) .setHttpProtocolVersion(uri -> "1.1"); } @@ -182,8 +179,6 @@ default Builder withDefaults() { Builder setTestNonStandardHttpMethod(boolean value); - Builder setTestClientError(boolean value); - Builder setHttpProtocolVersion(Function value); @CanIgnoreReturnValue From 2fa992f6b8b5592708008df49299203a29244527 Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Thu, 19 Dec 2024 13:12:19 +0800 Subject: [PATCH 07/10] refine timeout --- .../javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java index 8f94541c5a4c..ac8c16fd7ddf 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java @@ -33,7 +33,7 @@ abstract class JaxRsClientTest extends AbstractHttpClientTest BODY_METHODS = asList("POST", "PUT"); protected static final int CONNECT_TIMEOUT_MS = 5000; - protected static final int READ_TIMEOUT_MS = 2000; + protected static final int READ_TIMEOUT_MS = 3000; @Override public Invocation.Builder buildRequest(String method, URI uri, Map headers) { From 40a5a76cbcaeef0167f81a8b146cbe906a6bc415 Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Mon, 23 Dec 2024 14:01:20 +0800 Subject: [PATCH 08/10] Apply suggestions from code review Co-authored-by: Jay DeLuca --- .../instrumentation/jaxrsclient/JaxMultithreadedClientTest.java | 2 +- .../javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java index 990d6e0fd354..6ed84b0f53fa 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java @@ -82,6 +82,6 @@ public void run() { .start(); } - latch.await(); + latch.await(10, TimeUnit.SECONDS); } } diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java index ac8c16fd7ddf..663ee2de00b3 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java @@ -26,7 +26,7 @@ import javax.ws.rs.core.Response; import org.junit.jupiter.api.extension.RegisterExtension; -abstract class JaxRsClientTest extends AbstractHttpClientTest { +abstract class AbstractJaxRsClientTest extends AbstractHttpClientTest { @RegisterExtension static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent(); From 9dc98bd4e8c864a62e0ecf461fa994a12875342f Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Mon, 23 Dec 2024 14:05:59 +0800 Subject: [PATCH 09/10] refine ut --- .../{JaxRsClientTest.java => AbstractJaxRsClientTest.java} | 0 .../javaagent/instrumentation/jaxrsclient/CxfClientTest.java | 2 +- .../instrumentation/jaxrsclient/JaxMultithreadedClientTest.java | 1 + .../javaagent/instrumentation/jaxrsclient/JerseyClientTest.java | 2 +- .../instrumentation/jaxrsclient/ResteasyClientTest.java | 2 +- 5 files changed, 4 insertions(+), 3 deletions(-) rename instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/{JaxRsClientTest.java => AbstractJaxRsClientTest.java} (100%) diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/AbstractJaxRsClientTest.java similarity index 100% rename from instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxRsClientTest.java rename to instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/AbstractJaxRsClientTest.java diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/CxfClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/CxfClientTest.java index a07ddcee0d70..f7a2fea09c87 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/CxfClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/CxfClientTest.java @@ -11,7 +11,7 @@ import javax.ws.rs.client.ClientBuilder; import org.apache.cxf.jaxrs.client.spec.ClientBuilderImpl; -class CxfClientTest extends JaxRsClientTest { +class CxfClientTest extends AbstractJaxRsClientTest { @Override public ClientBuilder builder() { diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java index 6ed84b0f53fa..a81f5f797a57 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JaxMultithreadedClientTest.java @@ -16,6 +16,7 @@ import io.opentelemetry.testing.internal.armeria.testing.junit5.server.ServerExtension; import java.net.URI; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import javax.ws.rs.client.Client; import org.glassfish.jersey.client.JerseyClientBuilder; import org.junit.jupiter.api.AfterAll; diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java index 411f33b5d171..aca721968e9f 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java @@ -10,7 +10,7 @@ import org.glassfish.jersey.client.ClientProperties; import org.glassfish.jersey.client.JerseyClientBuilder; -class JerseyClientTest extends JaxRsClientTest { +class JerseyClientTest extends AbstractJaxRsClientTest { @Override public ClientBuilder builder() { diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java index efe62a06f256..b2a2c570b776 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java @@ -10,7 +10,7 @@ import javax.ws.rs.client.ClientBuilder; import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder; -class ResteasyClientTest extends JaxRsClientTest { +class ResteasyClientTest extends AbstractJaxRsClientTest { @Override ClientBuilder builder() { From 5b878d6d6660b81732a24b479d053c6592fa95fe Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 31 Dec 2024 10:13:06 +0200 Subject: [PATCH 10/10] use timeouts from the base http client test, set read timeout only for the read timeout endpoint, restore comment about why the single connection concurrently test is disabled --- .../jaxrsclient/AbstractJaxRsClientTest.java | 6 ++---- .../jaxrsclient/CxfClientTest.java | 13 ++++++++---- .../jaxrsclient/JerseyClientTest.java | 20 ++++++++++++++++--- .../jaxrsclient/ResteasyClientTest.java | 13 ++++++++---- 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/AbstractJaxRsClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/AbstractJaxRsClientTest.java index 663ee2de00b3..fb3985103d0b 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/AbstractJaxRsClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/AbstractJaxRsClientTest.java @@ -32,12 +32,10 @@ abstract class AbstractJaxRsClientTest extends AbstractHttpClientTest BODY_METHODS = asList("POST", "PUT"); - protected static final int CONNECT_TIMEOUT_MS = 5000; - protected static final int READ_TIMEOUT_MS = 3000; @Override public Invocation.Builder buildRequest(String method, URI uri, Map headers) { - Client client = builder().build(); + Client client = builder(uri).build(); WebTarget service = client.target(uri); Invocation.Builder requestBuilder = service.request(MediaType.TEXT_PLAIN); for (Map.Entry entry : headers.entrySet()) { @@ -46,7 +44,7 @@ public Invocation.Builder buildRequest(String method, URI uri, Map clientSpanError(uri, exception)); + optionsBuilder.setClientSpanErrorMapper(CxfClientTest::clientSpanError); + // CXF JAX-RS client uses HttpURLConnection internally, which does not support pipelining nor + // waiting for a connection in the pool to become available. Therefore, a high concurrency test + // would require manually doing requests one after another which is not meaningful for a high + // concurrency test. + optionsBuilder.setSingleConnectionFactory((host, port) -> null); } } diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java index aca721968e9f..9330375423bd 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/JerseyClientTest.java @@ -5,6 +5,8 @@ package io.opentelemetry.javaagent.instrumentation.jaxrsclient; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import java.net.URI; import javax.ws.rs.client.ClientBuilder; import org.glassfish.jersey.client.ClientConfig; import org.glassfish.jersey.client.ClientProperties; @@ -13,10 +15,22 @@ class JerseyClientTest extends AbstractJaxRsClientTest { @Override - public ClientBuilder builder() { + public ClientBuilder builder(URI uri) { ClientConfig config = new ClientConfig(); - config.property(ClientProperties.CONNECT_TIMEOUT, CONNECT_TIMEOUT_MS); - config.property(ClientProperties.READ_TIMEOUT, READ_TIMEOUT_MS); + config.property(ClientProperties.CONNECT_TIMEOUT, (int) CONNECTION_TIMEOUT.toMillis()); + if (uri.toString().contains("/read-timeout")) { + config.property(ClientProperties.READ_TIMEOUT, (int) READ_TIMEOUT.toMillis()); + } return new JerseyClientBuilder().withConfig(config); } + + @Override + protected void configure(HttpClientTestOptions.Builder optionsBuilder) { + super.configure(optionsBuilder); + // Jersey JAX-RS client uses HttpURLConnection internally, which does not support pipelining nor + // waiting for a connection in the pool to become available. Therefore, a high concurrency test + // would require manually doing requests one after another which is not meaningful for a high + // concurrency test. + optionsBuilder.setSingleConnectionFactory((host, port) -> null); + } } diff --git a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java index b2a2c570b776..422844ee5655 100644 --- a/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java +++ b/instrumentation/jaxrs-client/jaxrs-client-2.0-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/jaxrsclient/ResteasyClientTest.java @@ -6,6 +6,7 @@ package io.opentelemetry.javaagent.instrumentation.jaxrsclient; import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import java.net.URI; import java.util.concurrent.TimeUnit; import javax.ws.rs.client.ClientBuilder; import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder; @@ -13,10 +14,14 @@ class ResteasyClientTest extends AbstractJaxRsClientTest { @Override - ClientBuilder builder() { - return new ResteasyClientBuilder() - .establishConnectionTimeout(CONNECT_TIMEOUT_MS, TimeUnit.MILLISECONDS) - .socketTimeout(READ_TIMEOUT_MS, TimeUnit.MILLISECONDS); + ClientBuilder builder(URI uri) { + ResteasyClientBuilder builder = + new ResteasyClientBuilder() + .establishConnectionTimeout(CONNECTION_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); + if (uri.toString().contains("/read-timeout")) { + builder.socketTimeout(READ_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); + } + return builder; } @Override