From 49c20ef724cae2681b6671a121f59f68f96d5b32 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Thu, 5 Aug 2021 11:49:34 +0900 Subject: [PATCH] Migrate Ratpack HTTP Client tests to Java. (#3768) * Migrate Ratpack HTTP Client tests to Java. * Close harness --- .../client/RatpackForkedHttpClientTest.groovy | 33 ---- .../client/RatpackHttpClientTest.groovy | 159 ----------------- .../client/RatpackPooledHttpClientTest.groovy | 26 --- .../ratpack/RatpackForkedHttpClientTest.java | 17 ++ .../ratpack/RatpackHttpClientTest.java | 17 ++ .../ratpack/RatpackPooledHttpClientTest.java | 17 ++ .../ratpack-1.4/testing/build.gradle.kts | 1 + .../AbstractRatpackForkedHttpClientTest.java | 35 ++++ .../client/AbstractRatpackHttpClientTest.java | 168 ++++++++++++++++++ .../AbstractRatpackPooledHttpClientTest.java | 26 +++ .../junit/http/AbstractHttpClientTest.java | 89 +++++----- .../junit/http/HttpClientTestOptions.java | 5 +- .../testing/junit/http/SingleConnection.java | 5 +- 13 files changed, 335 insertions(+), 263 deletions(-) delete mode 100644 instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackForkedHttpClientTest.groovy delete mode 100644 instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackHttpClientTest.groovy delete mode 100644 instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackPooledHttpClientTest.groovy create mode 100644 instrumentation/ratpack-1.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackForkedHttpClientTest.java create mode 100644 instrumentation/ratpack-1.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackHttpClientTest.java create mode 100644 instrumentation/ratpack-1.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackPooledHttpClientTest.java create mode 100644 instrumentation/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackForkedHttpClientTest.java create mode 100644 instrumentation/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackHttpClientTest.java create mode 100644 instrumentation/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackPooledHttpClientTest.java diff --git a/instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackForkedHttpClientTest.groovy b/instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackForkedHttpClientTest.groovy deleted file mode 100644 index 78238103c1..0000000000 --- a/instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackForkedHttpClientTest.groovy +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package client - -import java.time.Duration -import ratpack.exec.Promise -import ratpack.http.client.HttpClient - -class RatpackForkedHttpClientTest extends RatpackHttpClientTest { - - @Override - Promise internalSendRequest(HttpClient client, String method, URI uri, Map headers) { - def resp = client.request(uri) { spec -> - // Connect timeout for the whole client was added in 1.5 so we need to add timeout for each request - spec.connectTimeout(Duration.ofSeconds(2)) - if (uri.getPath() == "/read-timeout") { - spec.readTimeout(Duration.ofMillis(READ_TIMEOUT_MS)) - } - spec.method(method) - spec.headers { headersSpec -> - headers.entrySet().each { - headersSpec.add(it.key, it.value) - } - } - } - return resp.fork().map { - it.status.code - } - } -} diff --git a/instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackHttpClientTest.groovy b/instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackHttpClientTest.groovy deleted file mode 100644 index 848b0ceb46..0000000000 --- a/instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackHttpClientTest.groovy +++ /dev/null @@ -1,159 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package client - -import io.netty.channel.ConnectTimeoutException -import io.netty.handler.timeout.ReadTimeoutException -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.instrumentation.testing.junit.http.SingleConnection -import java.time.Duration -import java.util.concurrent.ExecutionException -import java.util.concurrent.TimeoutException -import ratpack.exec.Operation -import ratpack.exec.Promise -import ratpack.func.Action -import ratpack.http.client.HttpClient -import ratpack.http.client.HttpClientSpec -import ratpack.test.exec.ExecHarness -import spock.lang.AutoCleanup -import spock.lang.Shared - -class RatpackHttpClientTest extends HttpClientTest implements AgentTestTrait { - - @AutoCleanup - @Shared - ExecHarness exec = ExecHarness.harness() - - @AutoCleanup - @Shared - def client = buildHttpClient() - - @AutoCleanup - @Shared - def singleConnectionClient = buildHttpClient({spec -> - spec.poolSize(1) - }) - - HttpClient buildHttpClient() { - return buildHttpClient(null) - } - - HttpClient buildHttpClient(Action action) { - HttpClient.of { - // execController method added in 1.9 - if (HttpClientSpec.metaClass.getMetaMethod("execController") != null) { - it.execController(exec.getController()) - } - if (action != null) { - action.execute(it) - } - } - } - - @Override - Void buildRequest(String method, URI uri, Map headers) { - return null - } - - @Override - int sendRequest(Void request, String method, URI uri, Map headers) { - return exec.yield { - internalSendRequest(client, method, uri, headers) - }.valueOrThrow - } - - @Override - void sendRequestWithCallback(Void request, String method, URI uri, Map headers, AbstractHttpClientTest.RequestResult requestResult) { - exec.execute(Operation.of { - internalSendRequest(client, method, uri, headers).result {result -> - requestResult.complete({ result.value }, result.throwable) - } - }) - } - - // overridden in RatpackForkedHttpClientTest - Promise internalSendRequest(HttpClient client, String method, URI uri, Map headers) { - def resp = client.request(uri) { spec -> - // Connect timeout for the whole client was added in 1.5 so we need to add timeout for each request - spec.connectTimeout(Duration.ofSeconds(2)) - if (uri.getPath() == "/read-timeout") { - spec.readTimeout(Duration.ofMillis(READ_TIMEOUT_MS)) - } - spec.method(method) - spec.headers { headersSpec -> - headers.entrySet().each { - headersSpec.add(it.key, it.value) - } - } - } - return resp.map { - it.status.code - } - } - - @Override - SingleConnection createSingleConnection(String host, int port) { - return new SingleConnection() { - @Override - int doRequest(String path, Map headers) throws ExecutionException, InterruptedException, TimeoutException { - def uri = resolveAddress(path) - return exec.yield { - internalSendRequest(singleConnectionClient, "GET", uri, headers) - }.valueOrThrow - } - } - } - - @Override - String expectedClientSpanName(URI uri, String method) { - switch (uri.toString()) { - case "http://localhost:61/": // unopened port - case "https://192.0.2.1/": // non routable address - return "CONNECT" - default: - return super.expectedClientSpanName(uri, method) - } - } - - @Override - Throwable clientSpanError(URI uri, Throwable exception) { - if (uri.toString() == "https://192.0.2.1/") { - return new ConnectTimeoutException("connection timed out: /192.0.2.1:443") - } else if (uri.getPath() == "/read-timeout") { - return new ReadTimeoutException() - } - return exception - } - - @Override - Set> httpAttributes(URI uri) { - switch (uri.toString()) { - case "http://localhost:61/": // unopened port - case "https://192.0.2.1/": // non routable address - return [] - } - return super.httpAttributes(uri) - } - - @Override - boolean testRedirects() { - false - } - - @Override - boolean testReusedRequest() { - // these tests will pass, but they don't really test anything since REQUEST is Void - false - } - - @Override - boolean testReadTimeout() { - true - } -} diff --git a/instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackPooledHttpClientTest.groovy b/instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackPooledHttpClientTest.groovy deleted file mode 100644 index cb2b8babb2..0000000000 --- a/instrumentation/ratpack-1.4/javaagent/src/test/groovy/client/RatpackPooledHttpClientTest.groovy +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package client - -import io.opentelemetry.instrumentation.testing.junit.http.SingleConnection -import ratpack.http.client.HttpClient - -class RatpackPooledHttpClientTest extends RatpackHttpClientTest { - - @Override - HttpClient buildHttpClient() { - return buildHttpClient({spec -> - spec.poolSize(5) - }) - } - - @Override - SingleConnection createSingleConnection(String host, int port) { - // this test is already run for RatpackHttpClientTest - // returning null here to avoid running the same test twice - return null - } -} diff --git a/instrumentation/ratpack-1.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackForkedHttpClientTest.java b/instrumentation/ratpack-1.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackForkedHttpClientTest.java new file mode 100644 index 0000000000..ed453607a2 --- /dev/null +++ b/instrumentation/ratpack-1.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackForkedHttpClientTest.java @@ -0,0 +1,17 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.ratpack; + +import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackForkedHttpClientTest; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension; +import org.junit.jupiter.api.extension.RegisterExtension; + +class RatpackForkedHttpClientTest extends AbstractRatpackForkedHttpClientTest { + + @RegisterExtension + static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent(); +} diff --git a/instrumentation/ratpack-1.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackHttpClientTest.java b/instrumentation/ratpack-1.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackHttpClientTest.java new file mode 100644 index 0000000000..13b941a218 --- /dev/null +++ b/instrumentation/ratpack-1.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackHttpClientTest.java @@ -0,0 +1,17 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.ratpack; + +import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackHttpClientTest; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension; +import org.junit.jupiter.api.extension.RegisterExtension; + +class RatpackHttpClientTest extends AbstractRatpackHttpClientTest { + + @RegisterExtension + static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent(); +} diff --git a/instrumentation/ratpack-1.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackPooledHttpClientTest.java b/instrumentation/ratpack-1.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackPooledHttpClientTest.java new file mode 100644 index 0000000000..1b5863d5b0 --- /dev/null +++ b/instrumentation/ratpack-1.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackPooledHttpClientTest.java @@ -0,0 +1,17 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.ratpack; + +import io.opentelemetry.instrumentation.ratpack.client.AbstractRatpackPooledHttpClientTest; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension; +import org.junit.jupiter.api.extension.RegisterExtension; + +class RatpackPooledHttpClientTest extends AbstractRatpackPooledHttpClientTest { + + @RegisterExtension + static final InstrumentationExtension testing = HttpClientInstrumentationExtension.forAgent(); +} diff --git a/instrumentation/ratpack-1.4/testing/build.gradle.kts b/instrumentation/ratpack-1.4/testing/build.gradle.kts index 2996c87c95..7eb81f3189 100644 --- a/instrumentation/ratpack-1.4/testing/build.gradle.kts +++ b/instrumentation/ratpack-1.4/testing/build.gradle.kts @@ -6,6 +6,7 @@ dependencies { api(project(":testing-common")) api("io.ratpack:ratpack-core:1.4.0") + api("io.ratpack:ratpack-test:1.4.0") implementation("org.codehaus.groovy:groovy-all") implementation("io.opentelemetry:opentelemetry-api") diff --git a/instrumentation/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackForkedHttpClientTest.java b/instrumentation/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackForkedHttpClientTest.java new file mode 100644 index 0000000000..cf3d7d0d89 --- /dev/null +++ b/instrumentation/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackForkedHttpClientTest.java @@ -0,0 +1,35 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.ratpack.client; + +import java.net.URI; +import java.time.Duration; +import java.util.Map; +import ratpack.exec.Promise; +import ratpack.http.client.HttpClient; +import ratpack.http.client.ReceivedResponse; + +public abstract class AbstractRatpackForkedHttpClientTest extends AbstractRatpackHttpClientTest { + @Override + protected final Promise internalSendRequest( + HttpClient client, String method, URI uri, Map headers) { + Promise resp = + client.request( + uri, + spec -> { + // Connect timeout for the whole client was added in 1.5 so we need to add timeout for + // each request + spec.connectTimeout(Duration.ofSeconds(2)); + if (uri.getPath().equals("/read-timeout")) { + spec.readTimeout(readTimeout()); + } + spec.method(method); + spec.headers(headersSpec -> headers.forEach(headersSpec::add)); + }); + + return resp.fork().map(ReceivedResponse::getStatusCode); + } +} diff --git a/instrumentation/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackHttpClientTest.java b/instrumentation/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackHttpClientTest.java new file mode 100644 index 0000000000..7e4e4d63aa --- /dev/null +++ b/instrumentation/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackHttpClientTest.java @@ -0,0 +1,168 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.ratpack.client; + +import io.netty.channel.ConnectTimeoutException; +import io.netty.handler.timeout.ReadTimeoutException; +import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import java.lang.reflect.Method; +import java.net.URI; +import java.time.Duration; +import java.util.Collections; +import java.util.Map; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import ratpack.exec.ExecController; +import ratpack.exec.Operation; +import ratpack.exec.Promise; +import ratpack.func.Action; +import ratpack.http.client.HttpClient; +import ratpack.http.client.HttpClientSpec; +import ratpack.http.client.ReceivedResponse; +import ratpack.test.exec.ExecHarness; + +public abstract class AbstractRatpackHttpClientTest extends AbstractHttpClientTest { + + private final ExecHarness exec = ExecHarness.harness(); + + private HttpClient client; + private HttpClient singleConnectionClient; + + @BeforeAll + void setUpClient() throws Exception { + client = buildHttpClient(); + singleConnectionClient = buildHttpClient(spec -> spec.poolSize(1)); + } + + @AfterAll + void cleanUpClient() { + client.close(); + singleConnectionClient.close(); + exec.close(); + } + + protected HttpClient buildHttpClient() throws Exception { + return buildHttpClient(Action.noop()); + } + + protected HttpClient buildHttpClient(Action action) throws Exception { + return HttpClient.of( + spec -> { + // execController method added in 1.9 + try { + Method execController = + HttpClientSpec.class.getMethod("execController", ExecController.class); + execController.invoke(spec, exec.getController()); + } catch (NoSuchMethodException e) { + // Ignore + } + + action.execute(spec); + }); + } + + @Override + protected final Void buildRequest(String method, URI uri, Map headers) { + return null; + } + + @Override + protected final int sendRequest(Void request, String method, URI uri, Map headers) + throws Exception { + return exec.yield(unused -> internalSendRequest(client, method, uri, headers)) + .getValueOrThrow(); + } + + @Override + protected final void sendRequestWithCallback( + Void request, + String method, + URI uri, + Map headers, + RequestResult requestResult) + throws Exception { + exec.execute( + Operation.of( + () -> + internalSendRequest(client, method, uri, headers) + .result( + result -> + requestResult.complete(result::getValue, result.getThrowable())))); + } + + // overridden in RatpackForkedHttpClientTest + protected Promise internalSendRequest( + HttpClient client, String method, URI uri, Map headers) { + Promise resp = + client.request( + uri, + spec -> { + // Connect timeout for the whole client was added in 1.5 so we need to add timeout for + // each request + spec.connectTimeout(Duration.ofSeconds(2)); + if (uri.getPath().equals("/read-timeout")) { + spec.readTimeout(readTimeout()); + } + spec.method(method); + spec.headers(headersSpec -> headers.forEach(headersSpec::add)); + }); + + return resp.map(ReceivedResponse::getStatusCode); + } + + @Override + protected void configure(HttpClientTestOptions options) { + options.setSingleConnectionFactory( + (host, port) -> + (path, headers) -> { + URI uri = resolveAddress(path); + return exec.yield( + unused -> internalSendRequest(singleConnectionClient, "GET", uri, headers)) + .getValueOrThrow(); + }); + + options.setExpectedClientSpanNameMapper( + (uri, method) -> { + switch (uri.toString()) { + case "http://localhost:61/": // unopened port + case "https://192.0.2.1/": // non routable address + return "CONNECT"; + default: + return HttpClientTestOptions.DEFAULT_EXPECTED_CLIENT_SPAN_NAME_MAPPER.apply( + uri, method); + } + }); + + options.setClientSpanErrorMapper( + (uri, exception) -> { + if (uri.toString().equals("https://192.0.2.1/")) { + return new ConnectTimeoutException("connection timed out: /192.0.2.1:443"); + } else if (uri.getPath().equals("/read-timeout")) { + return ReadTimeoutException.INSTANCE; + } + return exception; + }); + + options.setHttpAttributes( + uri -> { + switch (uri.toString()) { + case "http://localhost:61/": // unopened port + case "https://192.0.2.1/": // non routable address + return Collections.emptySet(); + default: + return HttpClientTestOptions.DEFAULT_HTTP_ATTRIBUTES; + } + }); + + options.disableTestRedirects(); + + // these tests will pass, but they don't really test anything since REQUEST is Void + options.disableTestReusedRequest(); + + options.enableTestReadTimeout(); + } +} diff --git a/instrumentation/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackPooledHttpClientTest.java b/instrumentation/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackPooledHttpClientTest.java new file mode 100644 index 0000000000..5a5003c5bf --- /dev/null +++ b/instrumentation/ratpack-1.4/testing/src/main/java/io/opentelemetry/instrumentation/ratpack/client/AbstractRatpackPooledHttpClientTest.java @@ -0,0 +1,26 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.ratpack.client; + +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import ratpack.http.client.HttpClient; + +public abstract class AbstractRatpackPooledHttpClientTest extends AbstractRatpackHttpClientTest { + + @Override + protected HttpClient buildHttpClient() throws Exception { + return buildHttpClient(spec -> spec.poolSize(5)); + } + + @Override + protected void configure(HttpClientTestOptions options) { + super.configure(options); + + // this test is already run for RatpackHttpClientTest + // returning null here to avoid running the same test twice + options.setSingleConnectionFactory((host, port) -> 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 9f4a78dcb3..89edff6496 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 @@ -115,14 +115,15 @@ public abstract class AbstractHttpClientTest { * dedicated API for invoking synchronously, such as OkHttp's execute method. */ protected abstract int sendRequest( - REQUEST request, String method, URI uri, Map headers); + REQUEST request, String method, URI uri, Map headers) throws Exception; protected void sendRequestWithCallback( REQUEST request, String method, URI uri, Map headers, - RequestResult requestResult) { + RequestResult requestResult) + throws Exception { // Must be implemented if testAsync is true throw new UnsupportedOperationException(); } @@ -210,7 +211,7 @@ public abstract class AbstractHttpClientTest { @ParameterizedTest @ValueSource(strings = {"/success", "/success?with=params"}) - void successfulGetRequest(String path) { + void successfulGetRequest(String path) throws Exception { URI uri = resolveAddress(path); String method = "GET"; int responseCode = doRequest(method, uri); @@ -233,7 +234,7 @@ public abstract class AbstractHttpClientTest { @ParameterizedTest @ValueSource(strings = {"PUT", "POST"}) - void successfulRequestWithParent(String method) { + void successfulRequestWithParent(String method) throws Exception { URI uri = resolveAddress("/success"); int responseCode = testing.runWithSpan("parent", () -> doRequest(method, uri)); @@ -258,7 +259,7 @@ public abstract class AbstractHttpClientTest { } @Test - void successfulRequestWithNotSampledParent() throws InterruptedException { + void successfulRequestWithNotSampledParent() throws Exception { String method = "GET"; URI uri = resolveAddress("/success"); int responseCode = testing.runWithNonRecordingSpan(() -> doRequest(method, uri)); @@ -361,7 +362,7 @@ public abstract class AbstractHttpClientTest { } @Test - void basicRequestWith1Redirect() { + void basicRequestWith1Redirect() throws Exception { // TODO quite a few clients create an extra span for the redirect // This test should handle both types or we should unify how the clients work @@ -390,7 +391,7 @@ public abstract class AbstractHttpClientTest { } @Test - void basicRequestWith2Redirects() { + void basicRequestWith2Redirects() throws Exception { // TODO quite a few clients create an extra span for the redirect // This test should handle both types or we should unify how the clients work @@ -457,7 +458,7 @@ public abstract class AbstractHttpClientTest { } @Test - void redirectToSecuredCopiesAuthHeader() { + void redirectToSecuredCopiesAuthHeader() throws Exception { assumeTrue(options.testRedirects); String method = "GET"; @@ -514,7 +515,7 @@ public abstract class AbstractHttpClientTest { } @Test - void reuseRequest() { + void reuseRequest() throws Exception { assumeTrue(options.testReusedRequest); String method = "GET"; @@ -556,7 +557,7 @@ public abstract class AbstractHttpClientTest { // (so that it propagates the same trace id / span id that it reports to the backend // and the trace is not broken) @Test - void requestWithExistingTracingHeaders() { + void requestWithExistingTracingHeaders() throws Exception { String method = "GET"; URI uri = resolveAddress("/success"); @@ -619,7 +620,7 @@ public abstract class AbstractHttpClientTest { } @Test - void connectionErrorUnopenedPortWithCallback() { + void connectionErrorUnopenedPortWithCallback() throws Exception { assumeTrue(options.testConnectionFailure); assumeTrue(options.testCallback); assumeTrue(options.testErrorWithCallback); @@ -745,7 +746,7 @@ public abstract class AbstractHttpClientTest { matches = ".*IBM J9 VM.*", disabledReason = "IBM JVM has different protocol support for TLS") @Test - void httpsRequest() { + void httpsRequest() throws Exception { assumeTrue(options.testRemoteConnection); assumeTrue(options.testHttps); @@ -796,15 +797,19 @@ public abstract class AbstractHttpClientTest { } catch (InterruptedException e) { throw new AssertionError(e); } - testing.runWithSpan( - "Parent span " + index, - () -> { - Span.current().setAttribute("test.request.id", index); - doRequest( - method, - uri, - Collections.singletonMap("test-request-id", String.valueOf(index))); - }); + try { + testing.runWithSpan( + "Parent span " + index, + () -> { + Span.current().setAttribute("test.request.id", index); + doRequest( + method, + uri, + Collections.singletonMap("test-request-id", String.valueOf(index))); + }); + } catch (Exception e) { + throw new AssertionError(e); + } }; pool.submit(job); } @@ -870,16 +875,20 @@ public abstract class AbstractHttpClientTest { } catch (InterruptedException e) { throw new AssertionError(e); } - testing.runWithSpan( - "Parent span " + index, - () -> { - Span.current().setAttribute("test.request.id", index); - doRequestWithCallback( - method, - uri, - Collections.singletonMap("test-request-id", String.valueOf(index)), - () -> testing.runWithSpan("child", () -> {})); - }); + try { + testing.runWithSpan( + "Parent span " + index, + () -> { + Span.current().setAttribute("test.request.id", index); + doRequestWithCallback( + method, + uri, + Collections.singletonMap("test-request-id", String.valueOf(index)), + () -> testing.runWithSpan("child", () -> {})); + }); + } catch (Exception e) { + throw new AssertionError(e); + } }; pool.submit(job); }); @@ -1007,7 +1016,7 @@ public abstract class AbstractHttpClientTest { // Visible for spock bridge. SpanDataAssert assertClientSpan( SpanDataAssert span, URI uri, String method, Integer responseCode) { - Set> httpClientAttributes = httpAttributes(uri); + Set> httpClientAttributes = options.httpAttributes.apply(uri); return span.hasName(options.expectedClientSpanNameMapper.apply(uri, method)) .hasKind(SpanKind.CLIENT) .hasAttributesSatisfying( @@ -1255,22 +1264,22 @@ public abstract class AbstractHttpClientTest { .toArray(new Consumer[0]); } - private int doRequest(String method, URI uri) { + private int doRequest(String method, URI uri) throws Exception { return doRequest(method, uri, Collections.emptyMap()); } - private int doRequest(String method, URI uri, Map headers) { + private int doRequest(String method, URI uri, Map headers) throws Exception { REQUEST request = buildRequest(method, uri, headers); return sendRequest(request, method, uri, headers); } - private int doReusedRequest(String method, URI uri) { + private int doReusedRequest(String method, URI uri) throws Exception { REQUEST request = buildRequest(method, uri, Collections.emptyMap()); sendRequest(request, method, uri, Collections.emptyMap()); return sendRequest(request, method, uri, Collections.emptyMap()); } - private int doRequestWithExistingTracingHeaders(String method, URI uri) { + private int doRequestWithExistingTracingHeaders(String method, URI uri) throws Exception { Map headers = new HashMap(); for (String field : testing.getOpenTelemetry().getPropagators().getTextMapPropagator().fields()) { @@ -1280,20 +1289,20 @@ public abstract class AbstractHttpClientTest { return sendRequest(request, method, uri, headers); } - private RequestResult doRequestWithCallback(String method, URI uri, Runnable callback) { + private RequestResult doRequestWithCallback(String method, URI uri, Runnable callback) + throws Exception { return doRequestWithCallback(method, uri, Collections.emptyMap(), callback); } private RequestResult doRequestWithCallback( - String method, URI uri, Map headers, Runnable callback) { + String method, URI uri, Map headers, Runnable callback) throws Exception { REQUEST request = buildRequest(method, uri, headers); RequestResult requestResult = new RequestResult(callback); sendRequestWithCallback(request, method, uri, headers, requestResult); return requestResult; } - // Visible for spock bridge. - URI resolveAddress(String path) { + protected URI resolveAddress(String path) { return URI.create("http://localhost:" + server.httpPort() + path); } 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 488e123c7d..dd22074555 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 @@ -26,10 +26,13 @@ public final class HttpClientTestOptions { SemanticAttributes.HTTP_FLAVOR, SemanticAttributes.HTTP_USER_AGENT))); + public static final BiFunction DEFAULT_EXPECTED_CLIENT_SPAN_NAME_MAPPER = + (uri, method) -> method != null ? "HTTP " + method : "HTTP request"; + Function>> httpAttributes = unused -> DEFAULT_HTTP_ATTRIBUTES; BiFunction expectedClientSpanNameMapper = - (uri, method) -> method != null ? "HTTP " + method : "HTTP request"; + DEFAULT_EXPECTED_CLIENT_SPAN_NAME_MAPPER; Integer responseCodeOnRedirectError = null; String userAgent = null; diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/SingleConnection.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/SingleConnection.java index f28a79b89b..e075309c02 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/SingleConnection.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/SingleConnection.java @@ -6,8 +6,6 @@ package io.opentelemetry.instrumentation.testing.junit.http; import java.util.Map; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeoutException; /** * Helper class for http client tests which require a single connection. @@ -24,6 +22,5 @@ import java.util.concurrent.TimeoutException; public interface SingleConnection { String REQUEST_ID_HEADER = "test-request-id"; - int doRequest(String path, Map headers) - throws ExecutionException, InterruptedException, TimeoutException; + int doRequest(String path, Map headers) throws Exception; }