diff --git a/instrumentation/armeria-1.3/library/src/test/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaHttpClientTest.java b/instrumentation/armeria-1.3/library/src/test/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaHttpClientTest.java index 30cd13ef8f52..9bbee0a23664 100644 --- a/instrumentation/armeria-1.3/library/src/test/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaHttpClientTest.java +++ b/instrumentation/armeria-1.3/library/src/test/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaHttpClientTest.java @@ -8,6 +8,7 @@ import com.linecorp.armeria.client.WebClientBuilder; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; import org.junit.jupiter.api.extension.RegisterExtension; class ArmeriaHttpClientTest extends AbstractArmeriaHttpClientTest { @@ -21,21 +22,16 @@ protected WebClientBuilder configureClient(WebClientBuilder clientBuilder) { ArmeriaTracing.create(testing.getOpenTelemetry()).newClientDecorator()); } - // library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet @Override - protected boolean testWithClientParent() { - return false; - } + protected void configure(HttpClientTestOptions options) { + super.configure(options); - // Agent users have automatic propagation through executor instrumentation, but library users - // should do manually using Armeria patterns. - @Override - protected boolean testCallbackWithParent() { - return false; - } + // library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet + options.disableTestWithClientParent(); - @Override - protected boolean testErrorWithCallback() { - return false; + // Agent users have automatic propagation through executor instrumentation, but library users + // should do manually using Armeria patterns. + options.disableTestCallbackWithParent(); + options.disableTestErrorWithCallback(); } } diff --git a/instrumentation/armeria-1.3/testing/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpClientTest.java b/instrumentation/armeria-1.3/testing/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpClientTest.java index 8b308bce4373..0f599757511a 100644 --- a/instrumentation/armeria-1.3/testing/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpClientTest.java +++ b/instrumentation/armeria-1.3/testing/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpClientTest.java @@ -14,6 +14,7 @@ import com.linecorp.armeria.common.util.Exceptions; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest; +import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.net.URI; import java.util.HashSet; @@ -70,27 +71,20 @@ protected final void sendRequestWithCallback( requestResult.complete(() -> response.status().code(), throwable)); } - // Not supported yet: https://github.com/line/armeria/issues/2489 @Override - protected final boolean testRedirects() { - return false; - } - - @Override - protected final boolean testReusedRequest() { + protected void configure(HttpClientTestOptions options) { + // Not supported yet: https://github.com/line/armeria/issues/2489 + options.disableTestRedirects(); // armeria requests can't be reused - return false; - } + options.disableTestReusedRequest(); - @Override - protected Set> httpAttributes(URI uri) { Set> extra = new HashSet<>(); extra.add(SemanticAttributes.HTTP_HOST); extra.add(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH); extra.add(SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH); extra.add(SemanticAttributes.HTTP_SCHEME); extra.add(SemanticAttributes.HTTP_TARGET); - extra.addAll(super.httpAttributes(uri)); - return extra; + extra.addAll(HttpClientTestOptions.DEFAULT_HTTP_ATTRIBUTES); + options.setHttpAttributes(unused -> extra); } } diff --git a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy index 9495756ab7f2..5dc1034fbb3f 100644 --- a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy +++ b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy @@ -227,6 +227,7 @@ abstract class HttpClientTest extends InstrumentationSpecification { def setupSpec() { server = new HttpClientTestServer(openTelemetry) server.start() + junitTest.setupOptions() junitTest.setTesting(testRunner(), server) } 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 13698548fc75..095d90be1b70 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 @@ -42,12 +42,15 @@ import java.util.function.Supplier; import java.util.stream.IntStream; import org.checkerframework.checker.nullness.qual.Nullable; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.condition.DisabledIfSystemProperty; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +@TestInstance(TestInstance.Lifecycle.PER_CLASS) public abstract class AbstractHttpClientTest { static final String BASIC_AUTH_KEY = "custom-authorization-header"; static final String BASIC_AUTH_VAL = "plain text auth token"; @@ -136,6 +139,65 @@ protected final Duration readTimeout() { private InstrumentationTestRunner testing; private HttpClientTestServer server; + private final HttpClientTestOptions options = new HttpClientTestOptions(); + + @BeforeAll + void setupOptions() { + // TODO(anuraaga): Have subclasses configure options directly and remove mapping of legacy + // protected methods. + options.setHttpAttributes(this::httpAttributes); + options.setExpectedClientSpanNameMapper(this::expectedClientSpanName); + Integer responseCodeOnError = responseCodeOnRedirectError(); + if (responseCodeOnError != null) { + options.setResponseCodeOnRedirectError(responseCodeOnError); + } + options.setUserAgent(userAgent()); + options.setClientSpanErrorMapper(this::clientSpanError); + options.setSingleConnectionFactory(this::createSingleConnection); + if (!testWithClientParent()) { + options.disableTestWithClientParent(); + } + if (!testRedirects()) { + options.disableTestRedirects(); + } + if (!testCircularRedirects()) { + options.disableTestCircularRedirects(); + } + options.setMaxRedirects(maxRedirects()); + if (!testReusedRequest()) { + options.disableTestReusedRequest(); + } + if (!testConnectionFailure()) { + options.disableTestConnectionFailure(); + } + if (testReadTimeout()) { + options.enableTestReadTimeout(); + } + if (!testRemoteConnection()) { + options.disableTestRemoteConnection(); + } + if (!testHttps()) { + options.disableTestHttps(); + } + if (!testCausality()) { + options.disableTestCausality(); + } + if (!testCausalityWithCallback()) { + options.disableTestCausalityWithCallback(); + } + if (!testCallback()) { + options.disableTestCallback(); + } + if (!testCallbackWithParent()) { + options.disableTestCallbackWithParent(); + } + if (!testErrorWithCallback()) { + options.disableTestErrorWithCallback(); + } + + configure(options); + } + @BeforeEach void verifyExtension() { if (testing == null) { @@ -198,7 +260,7 @@ void successfulRequestWithParent(String method) { @ParameterizedTest @ValueSource(strings = {"PUT", "POST"}) void shouldSuppressNestedClientSpanIfAlreadyUnderParentClientSpan(String method) { - assumeTrue(testWithClientParent()); + assumeTrue(options.testWithClientParent); URI uri = resolveAddress("/success"); int responseCode = runUnderParentClientSpan(() -> doRequest(method, uri)); @@ -219,8 +281,8 @@ void shouldSuppressNestedClientSpanIfAlreadyUnderParentClientSpan(String method) @Test void requestWithCallbackAndParent() throws Throwable { - assumeTrue(testCallback()); - assumeTrue(testCallbackWithParent()); + assumeTrue(options.testCallback); + assumeTrue(options.testCallbackWithParent); String method = "GET"; URI uri = resolveAddress("/success"); @@ -256,7 +318,7 @@ void requestWithCallbackAndParent() throws Throwable { @Test void requestWithCallbackAndNoParent() throws Throwable { - assumeTrue(testCallback()); + assumeTrue(options.testCallback); String method = "GET"; URI uri = resolveAddress("/success"); @@ -289,7 +351,7 @@ void basicRequestWith1Redirect() { // 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 - assumeTrue(testRedirects()); + assumeTrue(options.testRedirects); String method = "GET"; URI uri = resolveAddress("/redirect"); @@ -318,7 +380,7 @@ void basicRequestWith2Redirects() { // 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 - assumeTrue(testRedirects()); + assumeTrue(options.testRedirects); String method = "GET"; URI uri = resolveAddress("/another-redirect"); @@ -345,8 +407,8 @@ void basicRequestWith2Redirects() { @Test void circularRedirects() { - assumeTrue(testRedirects()); - assumeTrue(testCircularRedirects()); + assumeTrue(options.testRedirects); + assumeTrue(options.testCircularRedirects); String method = "GET"; URI uri = resolveAddress("/circular-redirect"); @@ -358,7 +420,7 @@ void circularRedirects() { } else { ex = thrown; } - Throwable clientError = clientSpanError(uri, ex); + Throwable clientError = options.clientSpanErrorMapper.apply(uri, ex); testing.waitAndAssertTraces( trace -> { @@ -369,10 +431,10 @@ void circularRedirects() { List> assertions = new ArrayList<>(); assertions.add( span -> - assertClientSpan(span, uri, method, responseCodeOnRedirectError()) + assertClientSpan(span, uri, method, options.responseCodeOnRedirectError) .hasParentSpanId(SpanId.getInvalid()) .hasEventsSatisfyingExactly(hasException(clientError))); - for (int i = 0; i < maxRedirects(); i++) { + for (int i = 0; i < options.maxRedirects; i++) { assertions.add( span -> assertServerSpan(span).hasParentSpanId(traces.get(0).get(0).getSpanId())); } @@ -382,7 +444,7 @@ void circularRedirects() { @Test void redirectToSecuredCopiesAuthHeader() { - assumeTrue(testRedirects()); + assumeTrue(options.testRedirects); String method = "GET"; URI uri = resolveAddress("/to-secured"); @@ -439,7 +501,7 @@ void errorSpan() { @Test void reuseRequest() { - assumeTrue(testReusedRequest()); + assumeTrue(options.testReusedRequest); String method = "GET"; URI uri = resolveAddress("/success"); @@ -504,7 +566,7 @@ void requestWithExistingTracingHeaders() { @Test void connectionErrorUnopenedPort() { - assumeTrue(testConnectionFailure()); + assumeTrue(options.testConnectionFailure); String method = "GET"; URI uri = URI.create("http://localhost:" + PortUtils.UNUSABLE_PORT + '/'); @@ -517,7 +579,7 @@ void connectionErrorUnopenedPort() { } else { ex = thrown; } - Throwable clientError = clientSpanError(uri, ex); + Throwable clientError = options.clientSpanErrorMapper.apply(uri, ex); testing.waitAndAssertTraces( trace -> { @@ -544,9 +606,9 @@ void connectionErrorUnopenedPort() { @Test void connectionErrorUnopenedPortWithCallback() { - assumeTrue(testConnectionFailure()); - assumeTrue(testCallback()); - assumeTrue(testErrorWithCallback()); + assumeTrue(options.testConnectionFailure); + assumeTrue(options.testCallback); + assumeTrue(options.testErrorWithCallback); String method = "GET"; URI uri = URI.create("http://localhost:" + PortUtils.UNUSABLE_PORT + '/'); @@ -565,7 +627,7 @@ void connectionErrorUnopenedPortWithCallback() { } else { ex = thrown; } - Throwable clientError = clientSpanError(uri, ex); + Throwable clientError = options.clientSpanErrorMapper.apply(uri, ex); testing.waitAndAssertTraces( trace -> { @@ -591,7 +653,7 @@ void connectionErrorUnopenedPortWithCallback() { @Test void connectionErrorNonRoutableAddress() { - assumeTrue(testRemoteConnection()); + assumeTrue(options.testRemoteConnection); String method = "HEAD"; URI uri = URI.create("https://192.0.2.1/"); @@ -604,7 +666,7 @@ void connectionErrorNonRoutableAddress() { } else { ex = thrown; } - Throwable clientError = clientSpanError(uri, ex); + Throwable clientError = options.clientSpanErrorMapper.apply(uri, ex); testing.waitAndAssertTraces( trace -> { @@ -628,7 +690,7 @@ void connectionErrorNonRoutableAddress() { @Test void readTimedOut() { - assumeTrue(testReadTimeout()); + assumeTrue(options.testReadTimeout); String method = "GET"; URI uri = resolveAddress("/read-timeout"); @@ -641,7 +703,7 @@ void readTimedOut() { } else { ex = thrown; } - Throwable clientError = clientSpanError(uri, ex); + Throwable clientError = options.clientSpanErrorMapper.apply(uri, ex); testing.waitAndAssertTraces( trace -> { @@ -670,8 +732,8 @@ void readTimedOut() { disabledReason = "IBM JVM has different protocol support for TLS") @Test void httpsRequest() { - assumeTrue(testRemoteConnection()); - assumeTrue(testHttps()); + assumeTrue(options.testRemoteConnection); + assumeTrue(options.testHttps); String method = "GET"; URI uri = URI.create("https://localhost:" + server.httpsPort() + "/success"); @@ -702,7 +764,7 @@ void httpsRequest() { */ @Test void highConcurrency() { - assumeTrue(testCausality()); + assumeTrue(options.testCausality); int count = 50; String method = "GET"; @@ -772,10 +834,10 @@ void highConcurrency() { @Test void highConcurrencyWithCallback() { - assumeTrue(testCausality()); - assumeTrue(testCausalityWithCallback()); - assumeTrue(testCallback()); - assumeTrue(testCallbackWithParent()); + assumeTrue(options.testCausality); + assumeTrue(options.testCausalityWithCallback); + assumeTrue(options.testCallback); + assumeTrue(options.testCallbackWithParent); int count = 50; String method = "GET"; @@ -854,7 +916,8 @@ void highConcurrencyWithCallback() { */ @Test void highConcurrencyOnSingleConnection() { - SingleConnection singleConnection = createSingleConnection("localhost", server.httpPort()); + SingleConnection singleConnection = + options.singleConnectionFactory.apply("localhost", server.httpPort()); assumeTrue(singleConnection != null); int count = 50; @@ -931,7 +994,7 @@ void highConcurrencyOnSingleConnection() { SpanDataAssert assertClientSpan( SpanDataAssert span, URI uri, String method, Integer responseCode) { Set> httpClientAttributes = httpAttributes(uri); - return span.hasName(expectedClientSpanName(uri, method)) + return span.hasName(options.expectedClientSpanNameMapper.apply(uri, method)) .hasKind(SpanKind.CLIENT) .hasAttributesSatisfying( attrs -> { @@ -995,7 +1058,7 @@ SpanDataAssert assertClientSpan( SemanticAttributes.HttpFlavorValues.HTTP_1_1); } if (httpClientAttributes.contains(SemanticAttributes.HTTP_USER_AGENT)) { - String userAgent = userAgent(); + String userAgent = options.userAgent; if (userAgent != null) { // TODO(anuraaga): Remove after updating to SDK 1.5.0 which adds this into // hasEntrySatisfying. @@ -1157,6 +1220,8 @@ protected boolean testErrorWithCallback() { return true; } + protected void configure(HttpClientTestOptions options) {} + // Workaround until release of // https://github.com/open-telemetry/opentelemetry-java/pull/3409 // in 1.5 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 new file mode 100644 index 000000000000..488e123c7dec --- /dev/null +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java @@ -0,0 +1,161 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.testing.junit.http; + +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; +import java.net.URI; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import java.util.function.BiFunction; +import java.util.function.Function; + +public final class HttpClientTestOptions { + + public static final Set> DEFAULT_HTTP_ATTRIBUTES = + Collections.unmodifiableSet( + new HashSet<>( + Arrays.asList( + SemanticAttributes.HTTP_URL, + SemanticAttributes.HTTP_METHOD, + SemanticAttributes.HTTP_FLAVOR, + SemanticAttributes.HTTP_USER_AGENT))); + + Function>> httpAttributes = unused -> DEFAULT_HTTP_ATTRIBUTES; + + BiFunction expectedClientSpanNameMapper = + (uri, method) -> method != null ? "HTTP " + method : "HTTP request"; + + Integer responseCodeOnRedirectError = null; + String userAgent = null; + + BiFunction clientSpanErrorMapper = (uri, exception) -> exception; + + BiFunction singleConnectionFactory = (host, port) -> null; + + boolean testWithClientParent = true; + boolean testRedirects = true; + boolean testCircularRedirects = true; + int maxRedirects = 2; + boolean testReusedRequest = true; + boolean testConnectionFailure = true; + boolean testReadTimeout = false; + boolean testRemoteConnection = true; + boolean testHttps = true; + boolean testCausality = true; + boolean testCausalityWithCallback = true; + boolean testCallback = true; + boolean testCallbackWithParent = true; + boolean testErrorWithCallback = true; + + HttpClientTestOptions() {} + + public HttpClientTestOptions setHttpAttributes( + Function>> httpAttributes) { + this.httpAttributes = httpAttributes; + return this; + } + + public HttpClientTestOptions setExpectedClientSpanNameMapper( + BiFunction expectedClientSpanNameMapper) { + this.expectedClientSpanNameMapper = expectedClientSpanNameMapper; + return this; + } + + public HttpClientTestOptions setResponseCodeOnRedirectError(int responseCodeOnRedirectError) { + this.responseCodeOnRedirectError = responseCodeOnRedirectError; + return this; + } + + public HttpClientTestOptions setUserAgent(String userAgent) { + this.userAgent = userAgent; + return this; + } + + public HttpClientTestOptions setClientSpanErrorMapper( + BiFunction clientSpanErrorMapper) { + this.clientSpanErrorMapper = clientSpanErrorMapper; + return this; + } + + public HttpClientTestOptions setSingleConnectionFactory( + BiFunction singleConnectionFactory) { + this.singleConnectionFactory = singleConnectionFactory; + return this; + } + + public HttpClientTestOptions setMaxRedirects(int maxRedirects) { + this.maxRedirects = maxRedirects; + return this; + } + + public HttpClientTestOptions disableTestWithClientParent() { + testWithClientParent = false; + return this; + } + + public HttpClientTestOptions disableTestRedirects() { + testRedirects = false; + return this; + } + + public HttpClientTestOptions disableTestCircularRedirects() { + testCircularRedirects = false; + return this; + } + + public HttpClientTestOptions disableTestReusedRequest() { + testReusedRequest = false; + return this; + } + + public HttpClientTestOptions disableTestConnectionFailure() { + testConnectionFailure = false; + return this; + } + + public HttpClientTestOptions enableTestReadTimeout() { + testReadTimeout = true; + return this; + } + + public HttpClientTestOptions disableTestRemoteConnection() { + testRemoteConnection = false; + return this; + } + + public HttpClientTestOptions disableTestHttps() { + testHttps = false; + return this; + } + + public HttpClientTestOptions disableTestCausality() { + testCausality = false; + return this; + } + + public HttpClientTestOptions disableTestCausalityWithCallback() { + testCausalityWithCallback = false; + return this; + } + + public HttpClientTestOptions disableTestCallback() { + testCallback = false; + return this; + } + + public HttpClientTestOptions disableTestCallbackWithParent() { + testCallbackWithParent = false; + return this; + } + + public HttpClientTestOptions disableTestErrorWithCallback() { + testErrorWithCallback = false; + return this; + } +}