From ab9c688e7a4fb19ef0930ca1a8b0b20789d21151 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Fri, 27 Aug 2021 11:16:59 +0200 Subject: [PATCH] Test nested CLIENT span suppression in library instrumentations (#3970) --- ...ntHostAbsoluteUriRequestContextTest.groovy | 6 ------ ...cheClientHostAbsoluteUriRequestTest.groovy | 6 ------ .../ApacheClientHostRequestContextTest.groovy | 6 ------ .../v4_3/ApacheClientHostRequestTest.groovy | 6 ------ .../ApacheClientUriRequestContextTest.groovy | 6 ------ .../v4_3/ApacheClientUriRequestTest.groovy | 6 ------ .../v9_2/JettyHttpClient9LibraryTest.groovy | 7 ------- .../okhttp/v3_0/OkHttp3Test.groovy | 7 ------- .../SpringWebInstrumentationTest.groovy | 6 ------ .../junit/http/AbstractHttpClientTest.java | 19 ++++--------------- 10 files changed, 4 insertions(+), 71 deletions(-) diff --git a/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostAbsoluteUriRequestContextTest.groovy b/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostAbsoluteUriRequestContextTest.groovy index baf9bbe9d9..2a0e851f65 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostAbsoluteUriRequestContextTest.groovy +++ b/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostAbsoluteUriRequestContextTest.groovy @@ -19,10 +19,4 @@ class ApacheClientHostAbsoluteUriRequestContextTest extends AbstractApacheClient .build() return builder.build() } - - // library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet - @Override - boolean testWithClientParent() { - false - } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostAbsoluteUriRequestTest.groovy b/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostAbsoluteUriRequestTest.groovy index 64c6d588a5..d3522603d0 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostAbsoluteUriRequestTest.groovy +++ b/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostAbsoluteUriRequestTest.groovy @@ -19,10 +19,4 @@ class ApacheClientHostAbsoluteUriRequestTest extends AbstractApacheClientHostAbs .build() return builder.build() } - - // library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet - @Override - boolean testWithClientParent() { - false - } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostRequestContextTest.groovy b/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostRequestContextTest.groovy index 7b50ccf311..20a9af73fc 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostRequestContextTest.groovy +++ b/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostRequestContextTest.groovy @@ -19,10 +19,4 @@ class ApacheClientHostRequestContextTest extends AbstractApacheClientHostRequest .build() return builder.build() } - - // library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet - @Override - boolean testWithClientParent() { - false - } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostRequestTest.groovy b/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostRequestTest.groovy index 35820cf14a..076f482ac8 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostRequestTest.groovy +++ b/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostRequestTest.groovy @@ -19,10 +19,4 @@ class ApacheClientHostRequestTest extends AbstractApacheClientHostRequestTest im .build() return builder.build() } - - // library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet - @Override - boolean testWithClientParent() { - false - } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientUriRequestContextTest.groovy b/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientUriRequestContextTest.groovy index 48e21255c2..4713b34300 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientUriRequestContextTest.groovy +++ b/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientUriRequestContextTest.groovy @@ -19,10 +19,4 @@ class ApacheClientUriRequestContextTest extends AbstractApacheClientUriRequestCo .build() return builder.build() } - - // library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet - @Override - boolean testWithClientParent() { - false - } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientUriRequestTest.groovy b/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientUriRequestTest.groovy index 488983d6a9..9cd38014a4 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientUriRequestTest.groovy +++ b/instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientUriRequestTest.groovy @@ -19,10 +19,4 @@ class ApacheClientUriRequestTest extends AbstractApacheClientUriRequestTest impl .build() return builder.build() } - - // library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet - @Override - boolean testWithClientParent() { - false - } } diff --git a/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/test/groovy/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/JettyHttpClient9LibraryTest.groovy b/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/test/groovy/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/JettyHttpClient9LibraryTest.groovy index 091fb9a02a..0926834161 100644 --- a/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/test/groovy/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/JettyHttpClient9LibraryTest.groovy +++ b/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/test/groovy/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/JettyHttpClient9LibraryTest.groovy @@ -12,13 +12,6 @@ import org.eclipse.jetty.util.ssl.SslContextFactory class JettyHttpClient9LibraryTest extends AbstractJettyClient9Test implements LibraryTestTrait { - - @Override - boolean testWithClientParent() { - //The client parent test does not work well in the context of library only tests. - false - } - @Override HttpClient createStandardClient() { JettyClientTracingBuilder jettyClientTracingBuilder = new JettyClientTracingBuilder(getOpenTelemetry()) diff --git a/instrumentation/okhttp/okhttp-3.0/library/src/test/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttp3Test.groovy b/instrumentation/okhttp/okhttp-3.0/library/src/test/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttp3Test.groovy index e4eb099742..783de6d3d1 100644 --- a/instrumentation/okhttp/okhttp-3.0/library/src/test/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttp3Test.groovy +++ b/instrumentation/okhttp/okhttp-3.0/library/src/test/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttp3Test.groovy @@ -16,11 +16,4 @@ class OkHttp3Test extends AbstractOkHttp3Test implements LibraryTestTrait { Call.Factory createCallFactory(OkHttpClient.Builder clientBuilder) { return OkHttpTracing.create(getOpenTelemetry()).newCallFactory(clientBuilder.build()) } - - // library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet - @Override - boolean testWithClientParent() { - false - } - } diff --git a/instrumentation/spring/spring-web-3.1/library/src/test/groovy/SpringWebInstrumentationTest.groovy b/instrumentation/spring/spring-web-3.1/library/src/test/groovy/SpringWebInstrumentationTest.groovy index 3e4383bee6..4c4f10f442 100644 --- a/instrumentation/spring/spring-web-3.1/library/src/test/groovy/SpringWebInstrumentationTest.groovy +++ b/instrumentation/spring/spring-web-3.1/library/src/test/groovy/SpringWebInstrumentationTest.groovy @@ -63,12 +63,6 @@ class SpringWebInstrumentationTest extends HttpClientTest> im false } - // library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet - @Override - boolean testWithClientParent() { - false - } - @Override Set> httpAttributes(URI uri) { def attributes = super.httpAttributes(uri) 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 dc38db8fe8..749402bcb2 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 @@ -12,7 +12,6 @@ import static org.junit.jupiter.api.Assumptions.assumeTrue; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanKind; -import io.opentelemetry.extension.annotations.WithSpan; import io.opentelemetry.instrumentation.test.utils.PortUtils; import io.opentelemetry.instrumentation.testing.InstrumentationTestRunner; import io.opentelemetry.sdk.testing.assertj.SpanDataAssert; @@ -29,7 +28,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -257,11 +255,13 @@ public abstract class AbstractHttpClientTest { @ParameterizedTest @ValueSource(strings = {"PUT", "POST"}) - void shouldSuppressNestedClientSpanIfAlreadyUnderParentClientSpan(String method) { + void shouldSuppressNestedClientSpanIfAlreadyUnderParentClientSpan(String method) + throws Exception { assumeTrue(options.testWithClientParent); URI uri = resolveAddress("/success"); - int responseCode = runUnderParentClientSpan(() -> doRequest(method, uri)); + int responseCode = + testing.runWithClientSpan("parent-client-span", () -> doRequest(method, uri)); assertThat(responseCode).isEqualTo(200); @@ -1137,15 +1137,4 @@ public abstract class AbstractHttpClientTest { this.testing = testing; this.server = server; } - - // Must create span within agent using annotation until - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/1726 - @WithSpan(value = "parent-client-span", kind = SpanKind.CLIENT) - private static T runUnderParentClientSpan(Callable r) { - try { - return r.call(); - } catch (Throwable t) { - throw new AssertionError(t); - } - } }