From f91549e7990eb6a08d54b4f610a5a5441d8fe980 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 5 Mar 2020 09:50:41 -0800 Subject: [PATCH] Simplify some tests by using runUnderTrace (#204) --- .../src/test/groovy/SessionTest.groovy | 36 +++-- .../src/test/groovy/SessionTest.groovy | 36 +++-- .../AkkaExecutorInstrumentationTest.groovy | 16 +-- .../ScalaExecutorInstrumentationTest.groovy | 16 +-- .../groovy/CompletableFutureTest.groovy | 9 +- .../groovy/ExecutorInstrumentationTest.groovy | 22 +-- .../src/test/groovy/Netty41ClientTest.groovy | 9 +- .../test/groovy/OpenTelemetryApiTest.groovy | 11 +- .../test/groovy/test/TwilioClientTest.groovy | 128 ++++++++---------- .../test/log/events/LogEventsTestBase.groovy | 15 +- 10 files changed, 114 insertions(+), 184 deletions(-) diff --git a/instrumentation/hibernate/core-3.3/src/test/groovy/SessionTest.groovy b/instrumentation/hibernate/core-3.3/src/test/groovy/SessionTest.groovy index 914bd4fe45..11b00d37f6 100644 --- a/instrumentation/hibernate/core-3.3/src/test/groovy/SessionTest.groovy +++ b/instrumentation/hibernate/core-3.3/src/test/groovy/SessionTest.groovy @@ -23,6 +23,7 @@ import org.hibernate.ReplicationMode import org.hibernate.Session import spock.lang.Shared +import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace import static io.opentelemetry.trace.Span.Kind.CLIENT import static io.opentelemetry.trace.Span.Kind.INTERNAL @@ -532,28 +533,23 @@ class SessionTest extends AbstractHibernateTest { def "test hibernate overlapping Sessions"() { setup: - def rootSpan = TEST_TRACER.spanBuilder("overlapping Sessions").startSpan() - def scope = TEST_TRACER.withSpan(rootSpan) + runUnderTrace("overlapping Sessions") { + def session1 = sessionFactory.openSession() + session1.beginTransaction() + def session2 = sessionFactory.openStatelessSession() + def session3 = sessionFactory.openSession() - def session1 = sessionFactory.openSession() - session1.beginTransaction() - def session2 = sessionFactory.openStatelessSession() - def session3 = sessionFactory.openSession() - - def value1 = new Value("Value 1") - session1.save(value1) - session2.insert(new Value("Value 2")) - session3.save(new Value("Value 3")) - session1.delete(value1) - - session2.close() - session1.getTransaction().commit() - session1.close() - session3.close() - - rootSpan.end() - scope.close() + def value1 = new Value("Value 1") + session1.save(value1) + session2.insert(new Value("Value 2")) + session3.save(new Value("Value 3")) + session1.delete(value1) + session2.close() + session1.getTransaction().commit() + session1.close() + session3.close() + } expect: assertTraces(1) { diff --git a/instrumentation/hibernate/core-4.0/src/test/groovy/SessionTest.groovy b/instrumentation/hibernate/core-4.0/src/test/groovy/SessionTest.groovy index d15d406b36..8c67c99a8b 100644 --- a/instrumentation/hibernate/core-4.0/src/test/groovy/SessionTest.groovy +++ b/instrumentation/hibernate/core-4.0/src/test/groovy/SessionTest.groovy @@ -23,6 +23,7 @@ import org.hibernate.ReplicationMode import org.hibernate.Session import spock.lang.Shared +import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace import static io.opentelemetry.trace.Span.Kind.CLIENT import static io.opentelemetry.trace.Span.Kind.INTERNAL @@ -460,28 +461,23 @@ class SessionTest extends AbstractHibernateTest { def "test hibernate overlapping Sessions"() { setup: - def rootSpan = TEST_TRACER.spanBuilder("overlapping Sessions").startSpan() - def scope = TEST_TRACER.withSpan(rootSpan) + runUnderTrace("overlapping Sessions") { + def session1 = sessionFactory.openSession() + session1.beginTransaction() + def session2 = sessionFactory.openStatelessSession() + def session3 = sessionFactory.openSession() - def session1 = sessionFactory.openSession() - session1.beginTransaction() - def session2 = sessionFactory.openStatelessSession() - def session3 = sessionFactory.openSession() - - def value1 = new Value("Value 1") - session1.save(value1) - session2.insert(new Value("Value 2")) - session3.save(new Value("Value 3")) - session1.delete(value1) - - session2.close() - session1.getTransaction().commit() - session1.close() - session3.close() - - rootSpan.end() - scope.close() + def value1 = new Value("Value 1") + session1.save(value1) + session2.insert(new Value("Value 2")) + session3.save(new Value("Value 3")) + session1.delete(value1) + session2.close() + session1.getTransaction().commit() + session1.close() + session3.close() + } expect: assertTraces(1) { diff --git a/instrumentation/java-concurrent/akka-2.5-testing/src/test/groovy/AkkaExecutorInstrumentationTest.groovy b/instrumentation/java-concurrent/akka-2.5-testing/src/test/groovy/AkkaExecutorInstrumentationTest.groovy index b4fc60c7c0..41d81d06b6 100644 --- a/instrumentation/java-concurrent/akka-2.5-testing/src/test/groovy/AkkaExecutorInstrumentationTest.groovy +++ b/instrumentation/java-concurrent/akka-2.5-testing/src/test/groovy/AkkaExecutorInstrumentationTest.groovy @@ -27,6 +27,8 @@ import java.util.concurrent.RejectedExecutionException import java.util.concurrent.ThreadPoolExecutor import java.util.concurrent.TimeUnit +import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace + /** * Test executor instrumentation for Akka specific classes. * This is to large extent a copy of ExecutorInstrumentationTest. @@ -54,9 +56,7 @@ class AkkaExecutorInstrumentationTest extends AgentTestRunner { new Runnable() { @Override void run() { - def parentSpan = TEST_TRACER.spanBuilder("parent").startSpan() - def parentScope = TEST_TRACER.withSpan(parentSpan) - try { + runUnderTrace("parent") { // this child will have a span def child1 = new AkkaAsyncChild() // this child won't @@ -65,9 +65,6 @@ class AkkaExecutorInstrumentationTest extends AgentTestRunner { m(pool, child2) child1.waitForCompletion() child2.waitForCompletion() - } finally { - parentSpan.end() - parentScope.close() } } }.run() @@ -111,9 +108,7 @@ class AkkaExecutorInstrumentationTest extends AgentTestRunner { new Runnable() { @Override void run() { - def parentSpan = TEST_TRACER.spanBuilder("parent").startSpan() - def parentScope = TEST_TRACER.withSpan(parentSpan) - try { + runUnderTrace("parent") { try { for (int i = 0; i < 20; ++i) { // Our current instrumentation instrumentation does not behave very well @@ -140,9 +135,6 @@ class AkkaExecutorInstrumentationTest extends AgentTestRunner { for (AkkaAsyncChild child : children) { child.unblock() } - } finally { - parentSpan.end() - parentScope.close() } } }.run() diff --git a/instrumentation/java-concurrent/scala-testing/src/test/groovy/ScalaExecutorInstrumentationTest.groovy b/instrumentation/java-concurrent/scala-testing/src/test/groovy/ScalaExecutorInstrumentationTest.groovy index e845a2aefd..83b6aae316 100644 --- a/instrumentation/java-concurrent/scala-testing/src/test/groovy/ScalaExecutorInstrumentationTest.groovy +++ b/instrumentation/java-concurrent/scala-testing/src/test/groovy/ScalaExecutorInstrumentationTest.groovy @@ -27,6 +27,8 @@ import java.util.concurrent.RejectedExecutionException import java.util.concurrent.ThreadPoolExecutor import java.util.concurrent.TimeUnit +import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace + /** * Test executor instrumentation for Scala specific classes. * This is to large extent a copy of ExecutorInstrumentationTest. @@ -54,9 +56,7 @@ class ScalaExecutorInstrumentationTest extends AgentTestRunner { new Runnable() { @Override void run() { - def parentSpan = TEST_TRACER.spanBuilder("parent").startSpan() - def parentScope = TEST_TRACER.withSpan(parentSpan) - try { + runUnderTrace("parent") { // this child will have a span def child1 = new ScalaAsyncChild() // this child won't @@ -65,9 +65,6 @@ class ScalaExecutorInstrumentationTest extends AgentTestRunner { m(pool, child2) child1.waitForCompletion() child2.waitForCompletion() - } finally { - parentSpan.end() - parentScope.close() } } }.run() @@ -111,9 +108,7 @@ class ScalaExecutorInstrumentationTest extends AgentTestRunner { new Runnable() { @Override void run() { - def parentSpan = TEST_TRACER.spanBuilder("parent").startSpan() - def parentScope = TEST_TRACER.withSpan(parentSpan) - try { + runUnderTrace("parent") { try { for (int i = 0; i < 20; ++i) { // Our current instrumentation instrumentation does not behave very well @@ -140,9 +135,6 @@ class ScalaExecutorInstrumentationTest extends AgentTestRunner { for (ScalaAsyncChild child : children) { child.unblock() } - } finally { - parentSpan.end() - parentScope.close() } } }.run() diff --git a/instrumentation/java-concurrent/src/slickTest/groovy/CompletableFutureTest.groovy b/instrumentation/java-concurrent/src/slickTest/groovy/CompletableFutureTest.groovy index b7b006d664..02353b95dc 100644 --- a/instrumentation/java-concurrent/src/slickTest/groovy/CompletableFutureTest.groovy +++ b/instrumentation/java-concurrent/src/slickTest/groovy/CompletableFutureTest.groovy @@ -23,6 +23,8 @@ import java.util.concurrent.TimeUnit import java.util.function.Function import java.util.function.Supplier +import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace + /** * Note: ideally this should live with the rest of ExecutorInstrumentationTest, * but this code needs java8 so we put it here for now. @@ -53,16 +55,11 @@ class CompletableFutureTest extends AgentTestRunner { def result = new Supplier() { @Override String get() { - def parentSpan = TEST_TRACER.spanBuilder("parent").startSpan() - def parentScope = TEST_TRACER.withSpan(parentSpan) - try { + runUnderTrace("parent") { return CompletableFuture.supplyAsync(supplier, pool) .thenCompose({ s -> CompletableFuture.supplyAsync(new AppendingSupplier(s), differentPool) }) .thenApply(function) .get() - } finally { - parentSpan.end() - parentScope.close() } } }.get() diff --git a/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy b/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy index ecc5c9fccc..61eba10488 100644 --- a/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy +++ b/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy @@ -37,6 +37,7 @@ import java.util.concurrent.ThreadPoolExecutor import java.util.concurrent.TimeUnit import java.util.concurrent.TimeoutException +import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace import static org.junit.Assume.assumeTrue class ExecutorInstrumentationTest extends AgentTestRunner { @@ -81,9 +82,7 @@ class ExecutorInstrumentationTest extends AgentTestRunner { new Runnable() { @Override void run() { - def parentSpan = TEST_TRACER.spanBuilder("parent").startSpan() - def parentScope = TEST_TRACER.withSpan(parentSpan) - try { + runUnderTrace("parent") { // this child will have a span def child1 = new JavaAsyncChild() // this child won't @@ -92,9 +91,6 @@ class ExecutorInstrumentationTest extends AgentTestRunner { m(pool, child2) child1.waitForCompletion() child2.waitForCompletion() - } finally { - parentSpan.end() - parentScope.close() } } }.run() @@ -171,13 +167,8 @@ class ExecutorInstrumentationTest extends AgentTestRunner { new Runnable() { @Override void run() { - def parentSpan = TEST_TRACER.spanBuilder("parent").startSpan() - def parentScope = TEST_TRACER.withSpan(parentSpan) - try { + runUnderTrace("parent") { m(pool, w(child)) - } finally { - parentSpan.end() - parentScope.close() } } }.run() @@ -219,9 +210,7 @@ class ExecutorInstrumentationTest extends AgentTestRunner { new Runnable() { @Override void run() { - def parentSpan = TEST_TRACER.spanBuilder("parent").startSpan() - def parentScope = TEST_TRACER.withSpan(parentSpan) - try { + runUnderTrace("parent") { try { for (int i = 0; i < 20; ++i) { // Our current instrumentation instrumentation does not behave very well @@ -248,9 +237,6 @@ class ExecutorInstrumentationTest extends AgentTestRunner { for (JavaAsyncChild child : children) { child.unblock() } - } finally { - parentSpan.end() - parentScope.close() } } }.run() diff --git a/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy b/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy index f93b87245f..9ceface34c 100644 --- a/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy +++ b/instrumentation/netty-4.1/src/test/groovy/Netty41ClientTest.groovy @@ -226,13 +226,8 @@ class Netty41ClientTest extends HttpClientTest { class TracedClass { int tracedMethod(String method) { - def span = TEST_TRACER.spanBuilder("tracedMethod").startSpan() - def scope = TEST_TRACER.withSpan(span) - try { - return doRequest(method, server.address.resolve("/success")) - } finally { - span.end() - scope.close() + runUnderTrace("tracedMethod") { + doRequest(method, server.address.resolve("/success")) } } } diff --git a/instrumentation/opentelemetry-api-0.2/src/test/groovy/OpenTelemetryApiTest.groovy b/instrumentation/opentelemetry-api-0.2/src/test/groovy/OpenTelemetryApiTest.groovy index efa0becc1f..bbdebe1617 100644 --- a/instrumentation/opentelemetry-api-0.2/src/test/groovy/OpenTelemetryApiTest.groovy +++ b/instrumentation/opentelemetry-api-0.2/src/test/groovy/OpenTelemetryApiTest.groovy @@ -17,6 +17,7 @@ import io.opentelemetry.auto.test.AgentTestRunner import unshaded.io.opentelemetry.OpenTelemetry import unshaded.io.opentelemetry.trace.Status +import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace import static unshaded.io.opentelemetry.trace.Span.Kind.PRODUCER class OpenTelemetryApiTest extends AgentTestRunner { @@ -54,12 +55,10 @@ class OpenTelemetryApiTest extends AgentTestRunner { def "capture span with implicit parent"() { when: def tracer = OpenTelemetry.getTracerFactory().get("test") - def parentSpan = tracer.spanBuilder("parent").startSpan() - def parentScope = tracer.withSpan(parentSpan) - def testSpan = tracer.spanBuilder("test").startSpan() - testSpan.end() - parentSpan.end() - parentScope.close() + runUnderTrace("parent") { + def testSpan = tracer.spanBuilder("test").startSpan() + testSpan.end() + } then: assertTraces(1) { diff --git a/instrumentation/twilio-6.6/src/test/groovy/test/TwilioClientTest.groovy b/instrumentation/twilio-6.6/src/test/groovy/test/TwilioClientTest.groovy index 65a37420a3..8194c5932c 100644 --- a/instrumentation/twilio-6.6/src/test/groovy/test/TwilioClientTest.groovy +++ b/instrumentation/twilio-6.6/src/test/groovy/test/TwilioClientTest.groovy @@ -39,6 +39,7 @@ import org.apache.http.impl.client.HttpClientBuilder import java.util.concurrent.ExecutionException import java.util.concurrent.TimeUnit +import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace import static io.opentelemetry.trace.Span.Kind.CLIENT class TwilioClientTest extends AgentTestRunner { @@ -126,16 +127,13 @@ class TwilioClientTest extends AgentTestRunner { 1 * twilioRestClient.request(_) >> new Response(new ByteArrayInputStream(MESSAGE_RESPONSE_BODY.getBytes()), 200) - def testSpan = TEST_TRACER.spanBuilder("test").startSpan() - def testScope = TEST_TRACER.withSpan(testSpan) - - Message message = Message.creator( - new PhoneNumber("+1 555 720 5913"), // To number - new PhoneNumber("+1 555 555 5215"), // From number - "Hello world!" // SMS body - ).create(twilioRestClient) - testSpan.end() - testScope.close() + Message message = runUnderTrace("test") { + Message.creator( + new PhoneNumber("+1 555 720 5913"), // To number + new PhoneNumber("+1 555 555 5215"), // From number + "Hello world!" // SMS body + ).create(twilioRestClient) + } expect: @@ -175,19 +173,15 @@ class TwilioClientTest extends AgentTestRunner { 1 * twilioRestClient.request(_) >> new Response(new ByteArrayInputStream(CALL_RESPONSE_BODY.getBytes()), 200) - def testSpan = TEST_TRACER.spanBuilder("test").startSpan() - def testScope = TEST_TRACER.withSpan(testSpan) + Call call = runUnderTrace("test") { + Call.creator( + new PhoneNumber("+15558881234"), // To number + new PhoneNumber("+15559994321"), // From number - Call call = Call.creator( - new PhoneNumber("+15558881234"), // To number - new PhoneNumber("+15559994321"), // From number - - // Read TwiML at this URL when a call connects (hold music) - new URI("http://twimlets.com/holdmusic?Bucket=com.twilio.music.ambient") - ).create(twilioRestClient) - - testSpan.end() - testScope.close() + // Read TwiML at this URL when a call connects (hold music) + new URI("http://twimlets.com/holdmusic?Bucket=com.twilio.music.ambient") + ).create(twilioRestClient) + } expect: @@ -251,18 +245,13 @@ class TwilioClientTest extends AgentTestRunner { .httpClient(networkHttpClient) .build() - - def testSpan = TEST_TRACER.spanBuilder("test").startSpan() - def testScope = TEST_TRACER.withSpan(testSpan) - - Message message = Message.creator( - new PhoneNumber("+1 555 720 5913"), // To number - new PhoneNumber("+1 555 555 5215"), // From number - "Hello world!" // SMS body - ).create(realTwilioRestClient) - - testSpan.end() - testScope.close() + Message message = runUnderTrace("test") { + Message.creator( + new PhoneNumber("+1 555 720 5913"), // To number + new PhoneNumber("+1 555 555 5215"), // From number + "Hello world!" // SMS body + ).create(realTwilioRestClient) + } expect: @@ -355,16 +344,13 @@ class TwilioClientTest extends AgentTestRunner { .httpClient(networkHttpClient) .build() - def testSpan = TEST_TRACER.spanBuilder("test").startSpan() - def testScope = TEST_TRACER.withSpan(testSpan) - - Message message = Message.creator( - new PhoneNumber("+1 555 720 5913"), // To number - new PhoneNumber("+1 555 555 5215"), // From number - "Hello world!" // SMS body - ).create(realTwilioRestClient) - testSpan.end() - testScope.close() + Message message = runUnderTrace("test") { + Message.creator( + new PhoneNumber("+1 555 720 5913"), // To number + new PhoneNumber("+1 555 555 5215"), // From number + "Hello world!" // SMS body + ).create(realTwilioRestClient) + } expect: @@ -470,23 +456,20 @@ class TwilioClientTest extends AgentTestRunner { .httpClient(networkHttpClient) .build() - def testSpan = TEST_TRACER.spanBuilder("test").startSpan() - def testScope = TEST_TRACER.withSpan(testSpan) + Message message = runUnderTrace("test") { - ListenableFuture future = Message.creator( - new PhoneNumber("+1 555 720 5913"), // To number - new PhoneNumber("+1 555 555 5215"), // From number - "Hello world!" // SMS body - ).createAsync(realTwilioRestClient) + ListenableFuture future = Message.creator( + new PhoneNumber("+1 555 720 5913"), // To number + new PhoneNumber("+1 555 555 5215"), // From number + "Hello world!" // SMS body + ).createAsync(realTwilioRestClient) - Message message - try { - message = future.get(10, TimeUnit.SECONDS) - } finally { - // Give the future callback a chance to run - Thread.sleep(1000) - testSpan.end() - testScope.close() + try { + return future.get(10, TimeUnit.SECONDS) + } finally { + // Give the future callback a chance to run + Thread.sleep(1000) + } } expect: @@ -663,23 +646,20 @@ class TwilioClientTest extends AgentTestRunner { when: - def testSpan = TEST_TRACER.spanBuilder("test").startSpan() - def testScope = TEST_TRACER.withSpan(testSpan) + Message message = runUnderTrace("test") { - ListenableFuture future = Message.creator( - new PhoneNumber("+1 555 720 5913"), // To number - new PhoneNumber("+1 555 555 5215"), // From number - "Hello world!" // SMS body - ).createAsync(twilioRestClient) + ListenableFuture future = Message.creator( + new PhoneNumber("+1 555 720 5913"), // To number + new PhoneNumber("+1 555 555 5215"), // From number + "Hello world!" // SMS body + ).createAsync(twilioRestClient) - Message message - try { - message = future.get(10, TimeUnit.SECONDS) - } finally { - // Give the future callback a chance to run - Thread.sleep(1000) - testSpan.end() - testScope.close() + try { + return future.get(10, TimeUnit.SECONDS) + } finally { + // Give the future callback a chance to run + Thread.sleep(1000) + } } then: diff --git a/testing/src/main/groovy/io/opentelemetry/auto/test/log/events/LogEventsTestBase.groovy b/testing/src/main/groovy/io/opentelemetry/auto/test/log/events/LogEventsTestBase.groovy index 6d718047b8..11381bbfa2 100644 --- a/testing/src/main/groovy/io/opentelemetry/auto/test/log/events/LogEventsTestBase.groovy +++ b/testing/src/main/groovy/io/opentelemetry/auto/test/log/events/LogEventsTestBase.groovy @@ -22,6 +22,7 @@ import io.opentelemetry.trace.Tracer import spock.lang.Unroll import static io.opentelemetry.auto.test.utils.ConfigUtils.withConfigOverride +import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace /** * This class represents the standard test cases that new logging library integrations MUST @@ -44,17 +45,13 @@ abstract class LogEventsTestBase extends AgentTestRunner { def "capture #testMethod (#capture)"() { setup: - def parentSpan = tracer.spanBuilder("test").startSpan() - def parentScope = tracer.withSpan(parentSpan) - - def logger = createLogger("abc") - withConfigOverride(Config.LOGS_EVENTS_THRESHOLD, "WARN") { - logger."$testMethod"("xyz") + runUnderTrace("test") { + def logger = createLogger("abc") + withConfigOverride(Config.LOGS_EVENTS_THRESHOLD, "WARN") { + logger."$testMethod"("xyz") + } } - parentSpan.end() - parentScope.close() - expect: assertTraces(1) { trace(0, 1) {