From 6fd630831ff1dc407bd7dbb6c9eb4371b5b16e05 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 8 Apr 2019 11:54:47 -0400 Subject: [PATCH] Stop using `scopeManager().activate(span, true)` --- .../CouchbaseBucketInstrumentation.java | 6 ++-- .../CouchbaseClusterInstrumentation.java | 6 ++-- .../datastax/cassandra/TracingSession.java | 32 ++++++++++++------- .../grpc/client/TracingClientInterceptor.java | 6 ++-- .../grpc/server/TracingServerInterceptor.java | 12 +++---- .../HttpUrlConnectionInstrumentation.java | 6 ++-- .../UrlInstrumentation.java | 3 +- .../JMSMessageConsumerInstrumentation.java | 3 +- .../ChannelFutureListenerInstrumentation.java | 3 +- .../ChannelFutureListenerInstrumentation.java | 3 +- .../okhttp3/TracingCallFactory.java | 3 +- .../amqp/RabbitChannelInstrumentation.java | 5 +-- .../spymemcached/CompletionListener.java | 6 ++-- 13 files changed, 59 insertions(+), 35 deletions(-) diff --git a/dd-java-agent/instrumentation/couchbase-2.0/src/main/java/datadog/trace/instrumentation/couchbase/client/CouchbaseBucketInstrumentation.java b/dd-java-agent/instrumentation/couchbase-2.0/src/main/java/datadog/trace/instrumentation/couchbase/client/CouchbaseBucketInstrumentation.java index 742605e0b8..50ed987a31 100644 --- a/dd-java-agent/instrumentation/couchbase-2.0/src/main/java/datadog/trace/instrumentation/couchbase/client/CouchbaseBucketInstrumentation.java +++ b/dd-java-agent/instrumentation/couchbase-2.0/src/main/java/datadog/trace/instrumentation/couchbase/client/CouchbaseBucketInstrumentation.java @@ -139,8 +139,9 @@ public class CouchbaseBucketInstrumentation extends Instrumenter.Default { final Span span = spanRef.getAndSet(null); if (span != null) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { DECORATE.beforeFinish(span); + span.finish(); } } } @@ -157,9 +158,10 @@ public class CouchbaseBucketInstrumentation extends Instrumenter.Default { public void call(final Throwable throwable) { final Span span = spanRef.getAndSet(null); if (span != null) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { DECORATE.onError(span, throwable); DECORATE.beforeFinish(span); + span.finish(); } } } diff --git a/dd-java-agent/instrumentation/couchbase-2.0/src/main/java/datadog/trace/instrumentation/couchbase/client/CouchbaseClusterInstrumentation.java b/dd-java-agent/instrumentation/couchbase-2.0/src/main/java/datadog/trace/instrumentation/couchbase/client/CouchbaseClusterInstrumentation.java index 1235c84ed8..b5549d014a 100644 --- a/dd-java-agent/instrumentation/couchbase-2.0/src/main/java/datadog/trace/instrumentation/couchbase/client/CouchbaseClusterInstrumentation.java +++ b/dd-java-agent/instrumentation/couchbase-2.0/src/main/java/datadog/trace/instrumentation/couchbase/client/CouchbaseClusterInstrumentation.java @@ -133,8 +133,9 @@ public class CouchbaseClusterInstrumentation extends Instrumenter.Default { final Span span = spanRef.getAndSet(null); if (span != null) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { DECORATE.beforeFinish(span); + span.finish(); } } } @@ -151,9 +152,10 @@ public class CouchbaseClusterInstrumentation extends Instrumenter.Default { public void call(final Throwable throwable) { final Span span = spanRef.getAndSet(null); if (span != null) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { DECORATE.onError(span, throwable); DECORATE.beforeFinish(span); + span.finish(); } } } diff --git a/dd-java-agent/instrumentation/datastax-cassandra-3/src/main/java/datadog/trace/instrumentation/datastax/cassandra/TracingSession.java b/dd-java-agent/instrumentation/datastax-cassandra-3/src/main/java/datadog/trace/instrumentation/datastax/cassandra/TracingSession.java index b818565336..39d4fc7c5c 100644 --- a/dd-java-agent/instrumentation/datastax-cassandra-3/src/main/java/datadog/trace/instrumentation/datastax/cassandra/TracingSession.java +++ b/dd-java-agent/instrumentation/datastax-cassandra-3/src/main/java/datadog/trace/instrumentation/datastax/cassandra/TracingSession.java @@ -60,7 +60,7 @@ public class TracingSession implements Session { @Override public ResultSet execute(final String query) { - try (final Scope scope = startSpanWithScope(query, true)) { + try (final Scope scope = startSpanWithScope(query)) { try { final ResultSet resultSet = session.execute(query); beforeSpanFinish(scope.span(), resultSet); @@ -68,13 +68,15 @@ public class TracingSession implements Session { } catch (final RuntimeException e) { beforeSpanFinish(scope.span(), e); throw e; + } finally { + scope.span().finish(); } } } @Override public ResultSet execute(final String query, final Object... values) { - try (final Scope scope = startSpanWithScope(query, true)) { + try (final Scope scope = startSpanWithScope(query)) { try { final ResultSet resultSet = session.execute(query, values); beforeSpanFinish(scope.span(), resultSet); @@ -82,13 +84,15 @@ public class TracingSession implements Session { } catch (final RuntimeException e) { beforeSpanFinish(scope.span(), e); throw e; + } finally { + scope.span().finish(); } } } @Override public ResultSet execute(final String query, final Map values) { - try (final Scope scope = startSpanWithScope(query, true)) { + try (final Scope scope = startSpanWithScope(query)) { try { final ResultSet resultSet = session.execute(query, values); beforeSpanFinish(scope.span(), resultSet); @@ -96,6 +100,8 @@ public class TracingSession implements Session { } catch (final RuntimeException e) { beforeSpanFinish(scope.span(), e); throw e; + } finally { + scope.span().finish(); } } } @@ -103,7 +109,7 @@ public class TracingSession implements Session { @Override public ResultSet execute(final Statement statement) { final String query = getQuery(statement); - try (final Scope scope = startSpanWithScope(query, true)) { + try (final Scope scope = startSpanWithScope(query)) { try { final ResultSet resultSet = session.execute(statement); beforeSpanFinish(scope.span(), resultSet); @@ -111,13 +117,15 @@ public class TracingSession implements Session { } catch (final RuntimeException e) { beforeSpanFinish(scope.span(), e); throw e; + } finally { + scope.span().finish(); } } } @Override public ResultSetFuture executeAsync(final String query) { - try (final Scope scope = startSpanWithScope(query, false)) { + try (final Scope scope = startSpanWithScope(query)) { final ResultSetFuture future = session.executeAsync(query); future.addListener(createListener(scope.span(), future), executorService); @@ -127,7 +135,7 @@ public class TracingSession implements Session { @Override public ResultSetFuture executeAsync(final String query, final Object... values) { - try (final Scope scope = startSpanWithScope(query, false)) { + try (final Scope scope = startSpanWithScope(query)) { final ResultSetFuture future = session.executeAsync(query, values); future.addListener(createListener(scope.span(), future), executorService); @@ -137,7 +145,7 @@ public class TracingSession implements Session { @Override public ResultSetFuture executeAsync(final String query, final Map values) { - try (final Scope scope = startSpanWithScope(query, false)) { + try (final Scope scope = startSpanWithScope(query)) { final ResultSetFuture future = session.executeAsync(query, values); future.addListener(createListener(scope.span(), future), executorService); @@ -148,7 +156,7 @@ public class TracingSession implements Session { @Override public ResultSetFuture executeAsync(final Statement statement) { final String query = getQuery(statement); - try (final Scope scope = startSpanWithScope(query, false)) { + try (final Scope scope = startSpanWithScope(query)) { final ResultSetFuture future = session.executeAsync(statement); future.addListener(createListener(scope.span(), future), executorService); @@ -216,18 +224,20 @@ public class TracingSession implements Session { return new Runnable() { @Override public void run() { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { beforeSpanFinish(span, future.get()); } catch (final InterruptedException | ExecutionException e) { beforeSpanFinish(span, e); + } finally { + span.finish(); } } }; } - private Scope startSpanWithScope(final String query, final boolean finishOnClose) { + private Scope startSpanWithScope(final String query) { final Span span = tracer.buildSpan("cassandra.execute").start(); - final Scope scope = tracer.scopeManager().activate(span, finishOnClose); + final Scope scope = tracer.scopeManager().activate(span, false); DECORATE.afterStart(span); DECORATE.onConnection(span, session); DECORATE.onStatement(span, query); diff --git a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/client/TracingClientInterceptor.java b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/client/TracingClientInterceptor.java index 3ed16bf489..bdb1a97faf 100644 --- a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/client/TracingClientInterceptor.java +++ b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/client/TracingClientInterceptor.java @@ -131,14 +131,14 @@ public class TracingClientInterceptor implements ClientInterceptor { public void onClose(final Status status, final Metadata trailers) { DECORATE.onClose(span, status); // Finishes span. - try (final Scope ignored = tracer.scopeManager().activate(span, true)) { + try (final Scope ignored = tracer.scopeManager().activate(span, false)) { delegate().onClose(status, trailers); - DECORATE.beforeFinish(span); } catch (final Throwable e) { DECORATE.onError(span, e); + throw e; + } finally { DECORATE.beforeFinish(span); span.finish(); - throw e; } } diff --git a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/TracingServerInterceptor.java b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/TracingServerInterceptor.java index 1aefc79242..d54fc23630 100644 --- a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/TracingServerInterceptor.java +++ b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/TracingServerInterceptor.java @@ -140,7 +140,7 @@ public class TracingServerInterceptor implements ServerInterceptor { @Override public void onCancel() { // Finishes span. - try (final Scope scope = tracer.scopeManager().activate(span, true)) { + try (final Scope scope = tracer.scopeManager().activate(span, false)) { if (scope instanceof TraceScope) { ((TraceScope) scope).setAsyncPropagation(true); } @@ -149,19 +149,19 @@ public class TracingServerInterceptor implements ServerInterceptor { if (scope instanceof TraceScope) { ((TraceScope) scope).setAsyncPropagation(false); } - DECORATE.beforeFinish(span); } catch (final Throwable e) { DECORATE.onError(span, e); + throw e; + } finally { DECORATE.beforeFinish(span); span.finish(); - throw e; } } @Override public void onComplete() { // Finishes span. - try (final Scope scope = tracer.scopeManager().activate(span, true)) { + try (final Scope scope = tracer.scopeManager().activate(span, false)) { if (scope instanceof TraceScope) { ((TraceScope) scope).setAsyncPropagation(true); } @@ -169,12 +169,12 @@ public class TracingServerInterceptor implements ServerInterceptor { if (scope instanceof TraceScope) { ((TraceScope) scope).setAsyncPropagation(false); } - DECORATE.beforeFinish(span); } catch (final Throwable e) { DECORATE.onError(span, e); + throw e; + } finally { DECORATE.beforeFinish(span); span.finish(); - throw e; } } diff --git a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java index 2e473474b8..11605ef4b6 100644 --- a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java +++ b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java @@ -199,9 +199,10 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { } public void finishSpan(final Throwable throwable) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { DECORATE.onError(span, throwable); DECORATE.beforeFinish(span); + span.finish(); span = null; finished = true; } @@ -214,9 +215,10 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { * (e.g. breaks getOutputStream). */ if (responseCode > 0) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { DECORATE.onResponse(span, responseCode); DECORATE.beforeFinish(span); + span.finish(); span = null; finished = true; } diff --git a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java index 0179540e27..1f6ff9e035 100644 --- a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java +++ b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java @@ -66,7 +66,7 @@ public class UrlInstrumentation extends Instrumenter.Default { .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT) .withTag(Tags.COMPONENT.getKey(), COMPONENT) .start(); - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { Tags.HTTP_URL.set(span, url.toString()); Tags.PEER_PORT.set(span, url.getPort() == -1 ? 80 : url.getPort()); Tags.PEER_HOSTNAME.set(span, url.getHost()); @@ -76,6 +76,7 @@ public class UrlInstrumentation extends Instrumenter.Default { Tags.ERROR.set(span, true); span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + span.finish(); } } } diff --git a/dd-java-agent/instrumentation/jms/src/main/java/datadog/trace/instrumentation/jms/JMSMessageConsumerInstrumentation.java b/dd-java-agent/instrumentation/jms/src/main/java/datadog/trace/instrumentation/jms/JMSMessageConsumerInstrumentation.java index ed29c3e4c3..f11a95b3ff 100644 --- a/dd-java-agent/instrumentation/jms/src/main/java/datadog/trace/instrumentation/jms/JMSMessageConsumerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms/src/main/java/datadog/trace/instrumentation/jms/JMSMessageConsumerInstrumentation.java @@ -93,7 +93,7 @@ public final class JMSMessageConsumerInstrumentation extends Instrumenter.Defaul } final Span span = spanBuilder.start(); - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { CONSUMER_DECORATE.afterStart(span); if (message == null) { CONSUMER_DECORATE.onReceive(span, method); @@ -102,6 +102,7 @@ public final class JMSMessageConsumerInstrumentation extends Instrumenter.Defaul } CONSUMER_DECORATE.onError(span, throwable); CONSUMER_DECORATE.beforeFinish(span); + span.finish(); } } } diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/ChannelFutureListenerInstrumentation.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/ChannelFutureListenerInstrumentation.java index 84a025b185..51ebfc8965 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/ChannelFutureListenerInstrumentation.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/ChannelFutureListenerInstrumentation.java @@ -90,9 +90,10 @@ public class ChannelFutureListenerInstrumentation extends Instrumenter.Default { .buildSpan("netty.connect") .withTag(Tags.COMPONENT.getKey(), "netty") .start(); - try (final Scope scope = GlobalTracer.get().scopeManager().activate(errorSpan, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(errorSpan, false)) { NettyHttpServerDecorator.DECORATE.onError(errorSpan, cause); NettyHttpServerDecorator.DECORATE.beforeFinish(errorSpan); + errorSpan.finish(); } return parentScope; diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/ChannelFutureListenerInstrumentation.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/ChannelFutureListenerInstrumentation.java index 116eae6de2..034d139e89 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/ChannelFutureListenerInstrumentation.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/ChannelFutureListenerInstrumentation.java @@ -90,9 +90,10 @@ public class ChannelFutureListenerInstrumentation extends Instrumenter.Default { .buildSpan("netty.connect") .withTag(Tags.COMPONENT.getKey(), "netty") .start(); - try (final Scope scope = GlobalTracer.get().scopeManager().activate(errorSpan, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(errorSpan, false)) { NettyHttpServerDecorator.DECORATE.onError(errorSpan, cause); NettyHttpServerDecorator.DECORATE.beforeFinish(errorSpan); + errorSpan.finish(); } return parentScope; diff --git a/dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TracingCallFactory.java b/dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TracingCallFactory.java index b69dc87634..31253c8c05 100644 --- a/dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TracingCallFactory.java +++ b/dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TracingCallFactory.java @@ -42,13 +42,14 @@ public class TracingCallFactory implements Call.Factory { @Override public Response intercept(final Chain chain) throws IOException { try (final Scope interceptorScope = - GlobalTracer.get().scopeManager().activate(span, true)) { + GlobalTracer.get().scopeManager().activate(span, false)) { return chain.proceed(chain.request()); } catch (final Exception ex) { DECORATE.onError(scope, ex); throw ex; } finally { DECORATE.beforeFinish(span); + span.finish(); } } }); diff --git a/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitChannelInstrumentation.java b/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitChannelInstrumentation.java index bf803aac14..31e379af18 100644 --- a/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitChannelInstrumentation.java +++ b/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitChannelInstrumentation.java @@ -193,7 +193,7 @@ public class RabbitChannelInstrumentation extends Instrumenter.Default { @Advice.Local("callDepth") int callDepth) { callDepth = CallDepthThreadLocalMap.incrementCallDepth(Channel.class); // Don't want RabbitCommandInstrumentation to mess up our actual parent span. - placeholderScope = GlobalTracer.get().scopeManager().activate(NoopSpan.INSTANCE, true); + placeholderScope = GlobalTracer.get().scopeManager().activate(NoopSpan.INSTANCE, false); return System.currentTimeMillis(); } @@ -247,13 +247,14 @@ public class RabbitChannelInstrumentation extends Instrumenter.Default { .withTag("message.size", length) .withTag(Tags.PEER_PORT.getKey(), connection.getPort()) .start(); - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { CONSUMER_DECORATE.afterStart(span); CONSUMER_DECORATE.onGet(span, queue); CONSUMER_DECORATE.onPeerConnection(span, connection.getAddress()); CONSUMER_DECORATE.onError(span, throwable); CONSUMER_DECORATE.beforeFinish(span); } finally { + span.finish(); CallDepthThreadLocalMap.reset(Channel.class); } } diff --git a/dd-java-agent/instrumentation/spymemcached-2.12/src/main/java/datadog/trace/instrumentation/spymemcached/CompletionListener.java b/dd-java-agent/instrumentation/spymemcached-2.12/src/main/java/datadog/trace/instrumentation/spymemcached/CompletionListener.java index 85b42d065e..07d9d2c4be 100644 --- a/dd-java-agent/instrumentation/spymemcached-2.12/src/main/java/datadog/trace/instrumentation/spymemcached/CompletionListener.java +++ b/dd-java-agent/instrumentation/spymemcached-2.12/src/main/java/datadog/trace/instrumentation/spymemcached/CompletionListener.java @@ -38,7 +38,7 @@ public abstract class CompletionListener { } protected void closeAsyncSpan(final T future) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { try { processResult(span, future); } catch (final CancellationException e) { @@ -60,14 +60,16 @@ public abstract class CompletionListener { DECORATE.onError(span, e); } finally { DECORATE.beforeFinish(span); + span.finish(); } } } protected void closeSyncSpan(final Throwable thrown) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { DECORATE.onError(span, thrown); DECORATE.beforeFinish(span); + span.finish(); } }