From bfb48f31c1d9965240f5eb7b5cfa399162b6af9f Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Wed, 3 Apr 2019 16:49:06 -0400 Subject: [PATCH 01/15] Make sure span is opened and closed with scope in http_url_connection --- .../HttpUrlConnectionInstrumentation.java | 31 +++++++++++-------- .../UrlInstrumentation.java | 19 ++++++------ 2 files changed, 28 insertions(+), 22 deletions(-) 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 d703ba033f..2e473474b8 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 @@ -13,6 +13,7 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.bootstrap.CallDepthThreadLocalMap; import datadog.trace.bootstrap.ContextStore; import datadog.trace.bootstrap.InstrumentationContext; +import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.Tracer; import io.opentracing.propagation.Format; @@ -178,9 +179,11 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { public Span startSpan(final HttpURLConnection connection) { final Tracer.SpanBuilder builder = GlobalTracer.get().buildSpan(OPERATION_NAME); span = builder.start(); - DECORATE.afterStart(span); - DECORATE.onRequest(span, connection); - return span; + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + DECORATE.afterStart(span); + DECORATE.onRequest(span, connection); + return span; + } } public boolean hasSpan() { @@ -196,11 +199,12 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { } public void finishSpan(final Throwable throwable) { - DECORATE.onError(span, throwable); - DECORATE.beforeFinish(span); - span.finish(); - span = null; - finished = true; + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + span = null; + finished = true; + } } public void finishSpan(final int responseCode) { @@ -210,11 +214,12 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { * (e.g. breaks getOutputStream). */ if (responseCode > 0) { - DECORATE.onResponse(span, responseCode); - DECORATE.beforeFinish(span); - span.finish(); - span = null; - finished = true; + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + DECORATE.onResponse(span, responseCode); + DECORATE.beforeFinish(span); + 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 12b7b34e1f..0179540e27 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 @@ -12,6 +12,7 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.api.Config; import datadog.trace.api.DDSpanTypes; import datadog.trace.api.DDTags; +import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; @@ -65,17 +66,17 @@ 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)) { + 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()); + if (Config.get().isHttpClientSplitByDomain()) { + span.setTag(DDTags.SERVICE_NAME, url.getHost()); + } - 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()); - if (Config.get().isHttpClientSplitByDomain()) { - span.setTag(DDTags.SERVICE_NAME, url.getHost()); + Tags.ERROR.set(span, true); + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); } - - Tags.ERROR.set(span, true); - span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); - span.finish(); } } } From 2bba4c559128c24bd90c0c455b5a7417efa075ca Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Wed, 3 Apr 2019 16:48:15 -0400 Subject: [PATCH 02/15] Make sure span is opened and closed with scope in spymemcached --- .../spymemcached/CompletionListener.java | 56 ++++++++++--------- 1 file changed, 31 insertions(+), 25 deletions(-) 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 d6886ed060..85b42d065e 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 @@ -2,6 +2,7 @@ package datadog.trace.instrumentation.spymemcached; import static datadog.trace.instrumentation.spymemcached.MemcacheClientDecorator.DECORATE; +import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.util.GlobalTracer; import java.util.concurrent.CancellationException; @@ -29,40 +30,45 @@ public abstract class CompletionListener { public CompletionListener(final MemcachedConnection connection, final String methodName) { this.connection = connection; span = GlobalTracer.get().buildSpan(OPERATION_NAME).start(); - DECORATE.afterStart(span); - DECORATE.onConnection(span, connection); - DECORATE.onOperation(span, methodName); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + DECORATE.afterStart(span); + DECORATE.onConnection(span, connection); + DECORATE.onOperation(span, methodName); + } } protected void closeAsyncSpan(final T future) { - try { - processResult(span, future); - } catch (final CancellationException e) { - span.setTag(DB_COMMAND_CANCELLED, true); - } catch (final ExecutionException e) { - if (e.getCause() instanceof CancellationException) { - // Looks like underlying OperationFuture wraps CancellationException into ExecutionException + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + try { + processResult(span, future); + } catch (final CancellationException e) { span.setTag(DB_COMMAND_CANCELLED, true); - } else { - DECORATE.onError(span, e.getCause()); + } catch (final ExecutionException e) { + if (e.getCause() instanceof CancellationException) { + // Looks like underlying OperationFuture wraps CancellationException into + // ExecutionException + span.setTag(DB_COMMAND_CANCELLED, true); + } else { + DECORATE.onError(span, e.getCause()); + } + } catch (final InterruptedException e) { + // Avoid swallowing InterruptedException + DECORATE.onError(span, e); + Thread.currentThread().interrupt(); + } catch (final Exception e) { + // This should never happen, just in case to make sure we cover all unexpected exceptions + DECORATE.onError(span, e); + } finally { + DECORATE.beforeFinish(span); } - } catch (final InterruptedException e) { - // Avoid swallowing InterruptedException - DECORATE.onError(span, e); - Thread.currentThread().interrupt(); - } catch (final Exception e) { - // This should never happen, just in case to make sure we cover all unexpected exceptions - DECORATE.onError(span, e); - } finally { - DECORATE.beforeFinish(span); - span.finish(); } } protected void closeSyncSpan(final Throwable thrown) { - DECORATE.onError(span, thrown); - DECORATE.beforeFinish(span); - span.finish(); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + DECORATE.onError(span, thrown); + DECORATE.beforeFinish(span); + } } protected abstract void processResult(Span span, T future) From f3e1eb4f5d085b1702f84cb549fe476dc8fd3e8e Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Thu, 4 Apr 2019 11:28:51 -0400 Subject: [PATCH 03/15] Make sure span is opened and closed with scope in Couchbase --- .../CouchbaseBucketInstrumentation.java | 31 +++++++++++-------- .../CouchbaseClusterInstrumentation.java | 29 ++++++++++------- 2 files changed, 35 insertions(+), 25 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 b7bb935dfe..742605e0b8 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 @@ -14,6 +14,7 @@ import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.api.DDTags; import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.noop.NoopSpan; import io.opentracing.util.GlobalTracer; @@ -113,14 +114,16 @@ public class CouchbaseBucketInstrumentation extends Instrumenter.Default { declaringClass.getSimpleName().replace("CouchbaseAsync", "").replace("DefaultAsync", ""); final String resourceName = className + "." + method.getName(); - // just replace the no-op span. - spanRef.set( - DECORATE.afterStart( - GlobalTracer.get() - .buildSpan("couchbase.call") - .withTag(DDTags.RESOURCE_NAME, resourceName) - .withTag("bucket", bucket) - .start())); + final Span span = + GlobalTracer.get() + .buildSpan("couchbase.call") + .withTag(DDTags.RESOURCE_NAME, resourceName) + .withTag("bucket", bucket) + .start(); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + // just replace the no-op span. + spanRef.set(DECORATE.afterStart(span)); + } } } @@ -136,8 +139,9 @@ public class CouchbaseBucketInstrumentation extends Instrumenter.Default { final Span span = spanRef.getAndSet(null); if (span != null) { - DECORATE.beforeFinish(span); - span.finish(); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + DECORATE.beforeFinish(span); + } } } } @@ -153,9 +157,10 @@ public class CouchbaseBucketInstrumentation extends Instrumenter.Default { public void call(final Throwable throwable) { final Span span = spanRef.getAndSet(null); if (span != null) { - DECORATE.onError(span, throwable); - DECORATE.beforeFinish(span); - span.finish(); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + } } } } 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 17fdfa7574..1235c84ed8 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 @@ -14,6 +14,7 @@ import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.api.DDTags; import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.noop.NoopSpan; import io.opentracing.util.GlobalTracer; @@ -108,13 +109,15 @@ public class CouchbaseClusterInstrumentation extends Instrumenter.Default { declaringClass.getSimpleName().replace("CouchbaseAsync", "").replace("DefaultAsync", ""); final String resourceName = className + "." + method.getName(); - // just replace the no-op span. - spanRef.set( - DECORATE.afterStart( - GlobalTracer.get() - .buildSpan("couchbase.call") - .withTag(DDTags.RESOURCE_NAME, resourceName) - .start())); + final Span span = + GlobalTracer.get() + .buildSpan("couchbase.call") + .withTag(DDTags.RESOURCE_NAME, resourceName) + .start(); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + // just replace the no-op span. + spanRef.set(DECORATE.afterStart(scope.span())); + } } } @@ -130,8 +133,9 @@ public class CouchbaseClusterInstrumentation extends Instrumenter.Default { final Span span = spanRef.getAndSet(null); if (span != null) { - DECORATE.beforeFinish(span); - span.finish(); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + DECORATE.beforeFinish(span); + } } } } @@ -147,9 +151,10 @@ public class CouchbaseClusterInstrumentation extends Instrumenter.Default { public void call(final Throwable throwable) { final Span span = spanRef.getAndSet(null); if (span != null) { - DECORATE.onError(span, throwable); - DECORATE.beforeFinish(span); - span.finish(); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + } } } } From bddee3d6e224573c468b90c1fc54047c9434902e Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Thu, 4 Apr 2019 11:36:24 -0400 Subject: [PATCH 04/15] Make sure span is opened and closed with scope in Mongo --- .../mongo/MongoClientInstrumentation.java | 4 ++-- ...tener.java => TracingCommandListener.java} | 23 +++++++++++-------- .../MongoAsyncClientInstrumentation.java | 4 ++-- .../MongoAsyncClientInstrumentationTest.java | 2 +- .../MongoClientInstrumentationTest.java | 2 +- 5 files changed, 19 insertions(+), 16 deletions(-) rename dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/{DDTracingCommandListener.java => TracingCommandListener.java} (64%) diff --git a/dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/MongoClientInstrumentation.java b/dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/MongoClientInstrumentation.java index 53799af582..89ad1875ad 100644 --- a/dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/MongoClientInstrumentation.java +++ b/dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/MongoClientInstrumentation.java @@ -48,7 +48,7 @@ public final class MongoClientInstrumentation extends Instrumenter.Default { "datadog.trace.agent.decorator.ClientDecorator", "datadog.trace.agent.decorator.DatabaseClientDecorator", packageName + ".MongoClientDecorator", - packageName + ".DDTracingCommandListener" + packageName + ".TracingCommandListener" }; } @@ -66,7 +66,7 @@ public final class MongoClientInstrumentation extends Instrumenter.Default { // referencing "this" in the method args causes the class to load under a transformer. // This bypasses the Builder instrumentation. Casting as a workaround. final MongoClientOptions.Builder builder = (MongoClientOptions.Builder) dis; - final DDTracingCommandListener listener = new DDTracingCommandListener(); + final TracingCommandListener listener = new TracingCommandListener(); builder.addCommandListener(listener); } } diff --git a/dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/DDTracingCommandListener.java b/dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/TracingCommandListener.java similarity index 64% rename from dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/DDTracingCommandListener.java rename to dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/TracingCommandListener.java index 4d06125b70..ddeb0ec867 100644 --- a/dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/DDTracingCommandListener.java +++ b/dd-java-agent/instrumentation/mongo-3.1/src/main/java/datadog/trace/instrumentation/mongo/TracingCommandListener.java @@ -6,6 +6,7 @@ import com.mongodb.event.CommandFailedEvent; import com.mongodb.event.CommandListener; import com.mongodb.event.CommandStartedEvent; import com.mongodb.event.CommandSucceededEvent; +import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.util.GlobalTracer; import java.util.Map; @@ -13,23 +14,25 @@ import java.util.concurrent.ConcurrentHashMap; import lombok.extern.slf4j.Slf4j; @Slf4j -public class DDTracingCommandListener implements CommandListener { +public class TracingCommandListener implements CommandListener { private final Map spanMap = new ConcurrentHashMap<>(); @Override public void commandStarted(final CommandStartedEvent event) { final Span span = GlobalTracer.get().buildSpan("mongo.query").start(); - DECORATE.afterStart(span); - DECORATE.onConnection(span, event); - if (event.getConnectionDescription() != null - && event.getConnectionDescription() != null - && event.getConnectionDescription().getServerAddress() != null) { - DECORATE.onPeerConnection( - span, event.getConnectionDescription().getServerAddress().getSocketAddress()); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + DECORATE.afterStart(span); + DECORATE.onConnection(span, event); + if (event.getConnectionDescription() != null + && event.getConnectionDescription() != null + && event.getConnectionDescription().getServerAddress() != null) { + DECORATE.onPeerConnection( + span, event.getConnectionDescription().getServerAddress().getSocketAddress()); + } + DECORATE.onStatement(span, event.getCommand()); + spanMap.put(event.getRequestId(), span); } - DECORATE.onStatement(span, event.getCommand()); - spanMap.put(event.getRequestId(), span); } @Override diff --git a/dd-java-agent/instrumentation/mongo-async-3.3/src/main/java/datadog/trace/instrumentation/mongo/MongoAsyncClientInstrumentation.java b/dd-java-agent/instrumentation/mongo-async-3.3/src/main/java/datadog/trace/instrumentation/mongo/MongoAsyncClientInstrumentation.java index d777515193..d6d86f9c58 100644 --- a/dd-java-agent/instrumentation/mongo-async-3.3/src/main/java/datadog/trace/instrumentation/mongo/MongoAsyncClientInstrumentation.java +++ b/dd-java-agent/instrumentation/mongo-async-3.3/src/main/java/datadog/trace/instrumentation/mongo/MongoAsyncClientInstrumentation.java @@ -48,7 +48,7 @@ public final class MongoAsyncClientInstrumentation extends Instrumenter.Default "datadog.trace.agent.decorator.ClientDecorator", "datadog.trace.agent.decorator.DatabaseClientDecorator", packageName + ".MongoClientDecorator", - packageName + ".DDTracingCommandListener" + packageName + ".TracingCommandListener" }; } @@ -66,7 +66,7 @@ public final class MongoAsyncClientInstrumentation extends Instrumenter.Default // referencing "this" in the method args causes the class to load under a transformer. // This bypasses the Builder instrumentation. Casting as a workaround. final MongoClientSettings.Builder builder = (MongoClientSettings.Builder) dis; - final DDTracingCommandListener listener = new DDTracingCommandListener(); + final TracingCommandListener listener = new TracingCommandListener(); builder.addCommandListener(listener); } } diff --git a/dd-java-agent/src/test/java/datadog/trace/agent/integration/MongoAsyncClientInstrumentationTest.java b/dd-java-agent/src/test/java/datadog/trace/agent/integration/MongoAsyncClientInstrumentationTest.java index b86aa7d4c7..86d2f58c9d 100644 --- a/dd-java-agent/src/test/java/datadog/trace/agent/integration/MongoAsyncClientInstrumentationTest.java +++ b/dd-java-agent/src/test/java/datadog/trace/agent/integration/MongoAsyncClientInstrumentationTest.java @@ -49,7 +49,7 @@ public class MongoAsyncClientInstrumentationTest { public void asyncClientHasListener() { Assert.assertEquals(1, client.getSettings().getCommandListeners().size()); Assert.assertEquals( - "DDTracingCommandListener", + "TracingCommandListener", client.getSettings().getCommandListeners().get(0).getClass().getSimpleName()); } diff --git a/dd-java-agent/src/test/java/datadog/trace/agent/integration/MongoClientInstrumentationTest.java b/dd-java-agent/src/test/java/datadog/trace/agent/integration/MongoClientInstrumentationTest.java index 32adafa674..9942c6e535 100644 --- a/dd-java-agent/src/test/java/datadog/trace/agent/integration/MongoClientInstrumentationTest.java +++ b/dd-java-agent/src/test/java/datadog/trace/agent/integration/MongoClientInstrumentationTest.java @@ -81,7 +81,7 @@ public class MongoClientInstrumentationTest { public void syncClientHasListener() { Assert.assertEquals(1, client.getMongoClientOptions().getCommandListeners().size()); Assert.assertEquals( - "DDTracingCommandListener", + "TracingCommandListener", client.getMongoClientOptions().getCommandListeners().get(0).getClass().getSimpleName()); } From a1e23dfb869a35161bac202700e3349850941159 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Thu, 4 Apr 2019 13:16:44 -0400 Subject: [PATCH 05/15] Make sure span is opened and closed with scope in Cassandra --- .../datastax/cassandra/TracingSession.java | 119 ++++++++++-------- 1 file changed, 66 insertions(+), 53 deletions(-) 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 ab0088dc00..b818565336 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 @@ -15,8 +15,10 @@ import com.datastax.driver.core.Statement; import com.google.common.base.Function; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.Tracer; +import io.opentracing.util.GlobalTracer; import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -58,88 +60,100 @@ public class TracingSession implements Session { @Override public ResultSet execute(final String query) { - final Span span = buildSpan(query); - ResultSet resultSet = null; - try { - resultSet = session.execute(query); - return resultSet; - } finally { - finishSpan(span, resultSet); + try (final Scope scope = startSpanWithScope(query, true)) { + try { + final ResultSet resultSet = session.execute(query); + beforeSpanFinish(scope.span(), resultSet); + return resultSet; + } catch (final RuntimeException e) { + beforeSpanFinish(scope.span(), e); + throw e; + } } } @Override public ResultSet execute(final String query, final Object... values) { - final Span span = buildSpan(query); - ResultSet resultSet = null; - try { - resultSet = session.execute(query, values); - return resultSet; - } finally { - finishSpan(span, resultSet); + try (final Scope scope = startSpanWithScope(query, true)) { + try { + final ResultSet resultSet = session.execute(query, values); + beforeSpanFinish(scope.span(), resultSet); + return resultSet; + } catch (final RuntimeException e) { + beforeSpanFinish(scope.span(), e); + throw e; + } } } @Override public ResultSet execute(final String query, final Map values) { - final Span span = buildSpan(query); - ResultSet resultSet = null; - try { - resultSet = session.execute(query, values); - return resultSet; - } finally { - finishSpan(span, resultSet); + try (final Scope scope = startSpanWithScope(query, true)) { + try { + final ResultSet resultSet = session.execute(query, values); + beforeSpanFinish(scope.span(), resultSet); + return resultSet; + } catch (final RuntimeException e) { + beforeSpanFinish(scope.span(), e); + throw e; + } } } @Override public ResultSet execute(final Statement statement) { final String query = getQuery(statement); - final Span span = buildSpan(query); - ResultSet resultSet = null; - try { - resultSet = session.execute(statement); - return resultSet; - } finally { - finishSpan(span, resultSet); + try (final Scope scope = startSpanWithScope(query, true)) { + try { + final ResultSet resultSet = session.execute(statement); + beforeSpanFinish(scope.span(), resultSet); + return resultSet; + } catch (final RuntimeException e) { + beforeSpanFinish(scope.span(), e); + throw e; + } } } @Override public ResultSetFuture executeAsync(final String query) { - final Span span = buildSpan(query); - final ResultSetFuture future = session.executeAsync(query); - future.addListener(createListener(span, future), executorService); + try (final Scope scope = startSpanWithScope(query, false)) { + final ResultSetFuture future = session.executeAsync(query); + future.addListener(createListener(scope.span(), future), executorService); - return future; + return future; + } } @Override public ResultSetFuture executeAsync(final String query, final Object... values) { - final Span span = buildSpan(query); - final ResultSetFuture future = session.executeAsync(query, values); - future.addListener(createListener(span, future), executorService); + try (final Scope scope = startSpanWithScope(query, false)) { + final ResultSetFuture future = session.executeAsync(query, values); + future.addListener(createListener(scope.span(), future), executorService); - return future; + return future; + } } @Override public ResultSetFuture executeAsync(final String query, final Map values) { - final Span span = buildSpan(query); - final ResultSetFuture future = session.executeAsync(query, values); - future.addListener(createListener(span, future), executorService); + try (final Scope scope = startSpanWithScope(query, false)) { + final ResultSetFuture future = session.executeAsync(query, values); + future.addListener(createListener(scope.span(), future), executorService); - return future; + return future; + } } @Override public ResultSetFuture executeAsync(final Statement statement) { final String query = getQuery(statement); - final Span span = buildSpan(query); - final ResultSetFuture future = session.executeAsync(statement); - future.addListener(createListener(span, future), executorService); + try (final Scope scope = startSpanWithScope(query, false)) { + final ResultSetFuture future = session.executeAsync(statement); + future.addListener(createListener(scope.span(), future), executorService); - return future; + return future; + } } @Override @@ -202,32 +216,31 @@ public class TracingSession implements Session { return new Runnable() { @Override public void run() { - try { - finishSpan(span, future.get()); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + beforeSpanFinish(span, future.get()); } catch (final InterruptedException | ExecutionException e) { - finishSpan(span, e); + beforeSpanFinish(span, e); } } }; } - private Span buildSpan(final String query) { + private Scope startSpanWithScope(final String query, final boolean finishOnClose) { final Span span = tracer.buildSpan("cassandra.execute").start(); + final Scope scope = tracer.scopeManager().activate(span, finishOnClose); DECORATE.afterStart(span); DECORATE.onConnection(span, session); DECORATE.onStatement(span, query); - return span; + return scope; } - private static void finishSpan(final Span span, final ResultSet resultSet) { + private static void beforeSpanFinish(final Span span, final ResultSet resultSet) { DECORATE.onResponse(span, resultSet); DECORATE.beforeFinish(span); - span.finish(); } - private static void finishSpan(final Span span, final Exception e) { + private static void beforeSpanFinish(final Span span, final Exception e) { DECORATE.onError(span, e); DECORATE.beforeFinish(span); - span.finish(); } } From 743ab327b0b1cad7e7b1d5551e90b1a3239fe677 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Thu, 4 Apr 2019 15:24:04 -0400 Subject: [PATCH 06/15] Wrap GRPC span start into scope --- .../grpc/client/TracingClientInterceptor.java | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) 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 cb69c45494..3ed16bf489 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 @@ -16,6 +16,7 @@ import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.Tracer; import io.opentracing.propagation.Format; +import io.opentracing.util.GlobalTracer; public class TracingClientInterceptor implements ClientInterceptor { @@ -36,20 +37,22 @@ public class TracingClientInterceptor implements ClientInterceptor { .buildSpan("grpc.client") .withTag(DDTags.RESOURCE_NAME, method.getFullMethodName()) .start(); - DECORATE.afterStart(span); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + DECORATE.afterStart(span); - final ClientCall result; - try (final Scope ignore = tracer.scopeManager().activate(span, false)) { - // call other interceptors - result = next.newCall(method, callOptions); - } catch (final Throwable e) { - DECORATE.onError(span, e); - DECORATE.beforeFinish(span); - span.finish(); - throw e; + final ClientCall result; + try { + // call other interceptors + result = next.newCall(method, callOptions); + } catch (final Throwable e) { + DECORATE.onError(span, e); + DECORATE.beforeFinish(span); + span.finish(); + throw e; + } + + return new TracingClientCall<>(tracer, span, result); } - - return new TracingClientCall<>(tracer, span, result); } static final class TracingClientCall From 1af748720192cc39eecc9c830e00632eded9fce8 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Thu, 4 Apr 2019 15:38:52 -0400 Subject: [PATCH 07/15] Wrap Rabbitmq channel span into scope --- .../amqp/RabbitChannelInstrumentation.java | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) 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 ee9c2c0929..bf803aac14 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 @@ -189,10 +189,11 @@ public class RabbitChannelInstrumentation extends Instrumenter.Default { public static class ChannelGetAdvice { @Advice.OnMethodEnter public static long takeTimestamp( - @Advice.Local("placeholderScope") Scope scope, @Advice.Local("callDepth") int callDepth) { + @Advice.Local("placeholderScope") Scope placeholderScope, + @Advice.Local("callDepth") int callDepth) { callDepth = CallDepthThreadLocalMap.incrementCallDepth(Channel.class); // Don't want RabbitCommandInstrumentation to mess up our actual parent span. - scope = GlobalTracer.get().scopeManager().activate(NoopSpan.INSTANCE, true); + placeholderScope = GlobalTracer.get().scopeManager().activate(NoopSpan.INSTANCE, true); return System.currentTimeMillis(); } @@ -201,13 +202,13 @@ public class RabbitChannelInstrumentation extends Instrumenter.Default { @Advice.This final Channel channel, @Advice.Argument(0) final String queue, @Advice.Enter final long startTime, - @Advice.Local("placeholderScope") final Scope scope, + @Advice.Local("placeholderScope") final Scope placeholderScope, @Advice.Local("callDepth") final int callDepth, @Advice.Return final GetResponse response, @Advice.Thrown final Throwable throwable) { - if (scope.span() instanceof NoopSpan) { - scope.close(); + if (placeholderScope.span() instanceof NoopSpan) { + placeholderScope.close(); } if (callDepth > 0) { @@ -236,6 +237,8 @@ public class RabbitChannelInstrumentation extends Instrumenter.Default { final Integer length = response == null ? null : response.getBody().length; + // TODO: it would be better if we could actually have span wrapped into the scope started in + // OnMethodEnter final Span span = GlobalTracer.get() .buildSpan("amqp.command") @@ -244,13 +247,15 @@ public class RabbitChannelInstrumentation extends Instrumenter.Default { .withTag("message.size", length) .withTag(Tags.PEER_PORT.getKey(), connection.getPort()) .start(); - CONSUMER_DECORATE.afterStart(span); - CONSUMER_DECORATE.onGet(span, queue); - CONSUMER_DECORATE.onPeerConnection(span, connection.getAddress()); - CONSUMER_DECORATE.onError(span, throwable); - CONSUMER_DECORATE.beforeFinish(span); - span.finish(); - CallDepthThreadLocalMap.reset(Channel.class); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + 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 { + CallDepthThreadLocalMap.reset(Channel.class); + } } } From 480a14b170904622f4ee02ae788c709ea8ffe985 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Thu, 4 Apr 2019 15:45:48 -0400 Subject: [PATCH 08/15] OkHttp: rearrange scope code a bit --- .../trace/instrumentation/okhttp3/TracingCallFactory.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 c27c174c1c..b69dc87634 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 @@ -28,7 +28,7 @@ public class TracingCallFactory implements Call.Factory { final Span span = GlobalTracer.get().buildSpan("okhttp.http").start(); try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { DECORATE.afterStart(scope); - DECORATE.onRequest(scope.span(), request); + DECORATE.onRequest(span, request); /** In case of exception network interceptor is not called */ final OkHttpClient.Builder okBuilder = okHttpClient.newBuilder(); @@ -42,14 +42,13 @@ public class TracingCallFactory implements Call.Factory { @Override public Response intercept(final Chain chain) throws IOException { try (final Scope interceptorScope = - GlobalTracer.get().scopeManager().activate(span, false)) { + GlobalTracer.get().scopeManager().activate(span, true)) { return chain.proceed(chain.request()); } catch (final Exception ex) { DECORATE.onError(scope, ex); throw ex; } finally { DECORATE.beforeFinish(span); - span.finish(); } } }); From 0e7418fde6035038b11f46cad15db3ce87f2ab98 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Thu, 4 Apr 2019 15:49:35 -0400 Subject: [PATCH 09/15] AWS v2: use scope when span is created --- .../instrumentation/aws/v2/TracingExecutionInterceptor.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java8/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java b/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java8/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java index 4ad249fb07..224e93bfa8 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java8/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java +++ b/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java8/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java @@ -34,8 +34,10 @@ public class TracingExecutionInterceptor implements ExecutionInterceptor { public void beforeExecution( final Context.BeforeExecution context, final ExecutionAttributes executionAttributes) { final Span span = GlobalTracer.get().buildSpan("aws.command").start(); - DECORATE.afterStart(span); - executionAttributes.putAttribute(SPAN_ATTRIBUTE, span); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + DECORATE.afterStart(span); + executionAttributes.putAttribute(SPAN_ATTRIBUTE, span); + } } @Override From 4119059e7082213d3624412b140c11d319e40fcd Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Thu, 4 Apr 2019 15:54:21 -0400 Subject: [PATCH 10/15] Jaxrs: use scope when span is opened --- .../jaxrs/ClientTracingFilter.java | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/ClientTracingFilter.java b/dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/ClientTracingFilter.java index be9a3d34f1..a32c1942d4 100644 --- a/dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/ClientTracingFilter.java +++ b/dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/ClientTracingFilter.java @@ -3,6 +3,7 @@ package datadog.trace.instrumentation.jaxrs; import static datadog.trace.instrumentation.jaxrs.JaxRsClientDecorator.DECORATE; import datadog.trace.api.DDTags; +import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.propagation.Format; import io.opentracing.util.GlobalTracer; @@ -21,24 +22,25 @@ public class ClientTracingFilter implements ClientRequestFilter, ClientResponseF @Override public void filter(final ClientRequestContext requestContext) { - final Span span = GlobalTracer.get() .buildSpan("jax-rs.client.call") .withTag(DDTags.RESOURCE_NAME, requestContext.getMethod() + " jax-rs.client.call") .start(); - DECORATE.afterStart(span); - DECORATE.onRequest(span, requestContext); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + DECORATE.afterStart(span); + DECORATE.onRequest(span, requestContext); - log.debug("{} - client span started", span); + log.debug("{} - client span started", span); - GlobalTracer.get() - .inject( - span.context(), - Format.Builtin.HTTP_HEADERS, - new InjectAdapter(requestContext.getHeaders())); + GlobalTracer.get() + .inject( + span.context(), + Format.Builtin.HTTP_HEADERS, + new InjectAdapter(requestContext.getHeaders())); - requestContext.setProperty(SPAN_PROPERTY_NAME, span); + requestContext.setProperty(SPAN_PROPERTY_NAME, span); + } } @Override From e678a62e5bb84b97e07b9eafea2dd25c5a1784eb Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Thu, 4 Apr 2019 15:58:21 -0400 Subject: [PATCH 11/15] Use scope when opening JMS span --- .../jms/JMSMessageConsumerInstrumentation.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) 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 2586260bbb..ed29c3e4c3 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 @@ -10,6 +10,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; +import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.Tracer; @@ -92,15 +93,16 @@ public final class JMSMessageConsumerInstrumentation extends Instrumenter.Defaul } final Span span = spanBuilder.start(); - CONSUMER_DECORATE.afterStart(span); - if (message == null) { - CONSUMER_DECORATE.onReceive(span, method); - } else { - CONSUMER_DECORATE.onConsume(span, message); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { + CONSUMER_DECORATE.afterStart(span); + if (message == null) { + CONSUMER_DECORATE.onReceive(span, method); + } else { + CONSUMER_DECORATE.onConsume(span, message); + } + CONSUMER_DECORATE.onError(span, throwable); + CONSUMER_DECORATE.beforeFinish(span); } - CONSUMER_DECORATE.onError(span, throwable); - CONSUMER_DECORATE.beforeFinish(span); - span.finish(); } } } From a5b5d236e1727ee2776a7e5f84d6e6a8d7629f49 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Thu, 4 Apr 2019 16:04:52 -0400 Subject: [PATCH 12/15] Use Scope when opening span in netty instrumentation --- .../ChannelFutureListenerInstrumentation.java | 11 ++-- .../HttpClientRequestTracingHandler.java | 51 ++++++++++--------- .../HttpServerRequestTracingHandler.java | 40 +++++++-------- .../ChannelFutureListenerInstrumentation.java | 11 ++-- .../HttpClientRequestTracingHandler.java | 51 ++++++++++--------- .../HttpServerRequestTracingHandler.java | 40 +++++++-------- 6 files changed, 104 insertions(+), 100 deletions(-) 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 f23b57834c..89f5e00ec9 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 @@ -83,18 +83,19 @@ public class ChannelFutureListenerInstrumentation extends Instrumenter.Default { if (continuation == null) { return null; } - final TraceScope scope = continuation.activate(); + final TraceScope parentScope = continuation.activate(); final Span errorSpan = GlobalTracer.get() .buildSpan("netty.connect") .withTag(Tags.COMPONENT.getKey(), "netty") .start(); - Tags.ERROR.set(errorSpan, true); - errorSpan.log(singletonMap(ERROR_OBJECT, cause)); - errorSpan.finish(); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(errorSpan, true)) { + Tags.ERROR.set(errorSpan, true); + errorSpan.log(singletonMap(ERROR_OBJECT, cause)); + } - return scope; + return parentScope; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientRequestTracingHandler.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientRequestTracingHandler.java index 656847ebb6..6dbb99246a 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientRequestTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/client/HttpClientRequestTracingHandler.java @@ -8,6 +8,7 @@ import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelOutboundHandlerAdapter; import io.netty.channel.ChannelPromise; import io.netty.handler.codec.http.HttpRequest; +import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.propagation.Format; import io.opentracing.util.GlobalTracer; @@ -24,40 +25,44 @@ public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapt return; } - TraceScope scope = null; + TraceScope parentScope = null; final TraceScope.Continuation continuation = ctx.channel().attr(AttributeKeys.PARENT_CONNECT_CONTINUATION_ATTRIBUTE_KEY).getAndRemove(); if (continuation != null) { - scope = continuation.activate(); + parentScope = continuation.activate(); } final HttpRequest request = (HttpRequest) msg; final Span span = GlobalTracer.get().buildSpan("netty.client.request").start(); - DECORATE.afterStart(span); - DECORATE.onRequest(span, request); - DECORATE.onPeerConnection(span, (InetSocketAddress) ctx.channel().remoteAddress()); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + DECORATE.afterStart(span); + DECORATE.onRequest(span, request); + DECORATE.onPeerConnection(span, (InetSocketAddress) ctx.channel().remoteAddress()); - // AWS calls are often signed, so we can't add headers without breaking the signature. - if (!request.headers().contains("amz-sdk-invocation-id")) { - GlobalTracer.get() - .inject( - span.context(), Format.Builtin.HTTP_HEADERS, new NettyResponseInjectAdapter(request)); + // AWS calls are often signed, so we can't add headers without breaking the signature. + if (!request.headers().contains("amz-sdk-invocation-id")) { + GlobalTracer.get() + .inject( + span.context(), + Format.Builtin.HTTP_HEADERS, + new NettyResponseInjectAdapter(request)); + } + + ctx.channel().attr(AttributeKeys.CLIENT_ATTRIBUTE_KEY).set(span); + + try { + ctx.write(msg, prm); + } catch (final Throwable throwable) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + span.finish(); + throw throwable; + } } - ctx.channel().attr(AttributeKeys.CLIENT_ATTRIBUTE_KEY).set(span); - - try { - ctx.write(msg, prm); - } catch (final Throwable throwable) { - DECORATE.onError(span, throwable); - DECORATE.beforeFinish(span); - span.finish(); - throw throwable; - } - - if (null != scope) { - scope.close(); + if (null != parentScope) { + parentScope.close(); } } } diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerRequestTracingHandler.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerRequestTracingHandler.java index 933c40e6c1..48027ed4d9 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerRequestTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/server/HttpServerRequestTracingHandler.java @@ -29,31 +29,27 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte GlobalTracer.get() .extract(Format.Builtin.HTTP_HEADERS, new NettyRequestExtractAdapter(request)); - final Scope scope = - GlobalTracer.get() - .buildSpan("netty.request") - .asChildOf(extractedContext) - .startActive(false); - final Span span = scope.span(); - DECORATE.afterStart(span); - DECORATE.onRequest(span, request); - DECORATE.onPeerConnection(span, remoteAddress); + final Span span = + GlobalTracer.get().buildSpan("netty.request").asChildOf(extractedContext).start(); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + DECORATE.afterStart(span); + DECORATE.onRequest(span, request); + DECORATE.onPeerConnection(span, remoteAddress); - if (scope instanceof TraceScope) { - ((TraceScope) scope).setAsyncPropagation(true); - } + if (scope instanceof TraceScope) { + ((TraceScope) scope).setAsyncPropagation(true); + } - ctx.channel().attr(AttributeKeys.SERVER_ATTRIBUTE_KEY).set(span); + ctx.channel().attr(AttributeKeys.SERVER_ATTRIBUTE_KEY).set(span); - try { - ctx.fireChannelRead(msg); - } catch (final Throwable throwable) { - DECORATE.onError(span, throwable); - DECORATE.beforeFinish(span); - span.finish(); // Finish the span manually since finishSpanOnClose was false - throw throwable; - } finally { - scope.close(); + try { + ctx.fireChannelRead(msg); + } catch (final Throwable throwable) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + span.finish(); // Finish the span manually since finishSpanOnClose was false + throw throwable; + } } } } 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 f91f8cdcb5..2b7f0ee543 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 @@ -83,18 +83,19 @@ public class ChannelFutureListenerInstrumentation extends Instrumenter.Default { if (continuation == null) { return null; } - final TraceScope scope = continuation.activate(); + final TraceScope parentScope = continuation.activate(); final Span errorSpan = GlobalTracer.get() .buildSpan("netty.connect") .withTag(Tags.COMPONENT.getKey(), "netty") .start(); - Tags.ERROR.set(errorSpan, true); - errorSpan.log(singletonMap(ERROR_OBJECT, cause)); - errorSpan.finish(); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(errorSpan, true)) { + Tags.ERROR.set(errorSpan, true); + errorSpan.log(singletonMap(ERROR_OBJECT, cause)); + } - return scope; + return parentScope; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientRequestTracingHandler.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientRequestTracingHandler.java index 6342472f12..5cc082ec8c 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientRequestTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/client/HttpClientRequestTracingHandler.java @@ -8,6 +8,7 @@ import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelOutboundHandlerAdapter; import io.netty.channel.ChannelPromise; import io.netty.handler.codec.http.HttpRequest; +import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.propagation.Format; import io.opentracing.util.GlobalTracer; @@ -24,40 +25,44 @@ public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapt return; } - TraceScope scope = null; + TraceScope parentScope = null; final TraceScope.Continuation continuation = ctx.channel().attr(AttributeKeys.PARENT_CONNECT_CONTINUATION_ATTRIBUTE_KEY).getAndRemove(); if (continuation != null) { - scope = continuation.activate(); + parentScope = continuation.activate(); } final HttpRequest request = (HttpRequest) msg; final Span span = GlobalTracer.get().buildSpan("netty.client.request").start(); - DECORATE.afterStart(span); - DECORATE.onRequest(span, request); - DECORATE.onPeerConnection(span, (InetSocketAddress) ctx.channel().remoteAddress()); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + DECORATE.afterStart(span); + DECORATE.onRequest(span, request); + DECORATE.onPeerConnection(span, (InetSocketAddress) ctx.channel().remoteAddress()); - // AWS calls are often signed, so we can't add headers without breaking the signature. - if (!request.headers().contains("amz-sdk-invocation-id")) { - GlobalTracer.get() - .inject( - span.context(), Format.Builtin.HTTP_HEADERS, new NettyResponseInjectAdapter(request)); + // AWS calls are often signed, so we can't add headers without breaking the signature. + if (!request.headers().contains("amz-sdk-invocation-id")) { + GlobalTracer.get() + .inject( + span.context(), + Format.Builtin.HTTP_HEADERS, + new NettyResponseInjectAdapter(request)); + } + + ctx.channel().attr(AttributeKeys.CLIENT_ATTRIBUTE_KEY).set(span); + + try { + ctx.write(msg, prm); + } catch (final Throwable throwable) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + span.finish(); + throw throwable; + } } - ctx.channel().attr(AttributeKeys.CLIENT_ATTRIBUTE_KEY).set(span); - - try { - ctx.write(msg, prm); - } catch (final Throwable throwable) { - DECORATE.onError(span, throwable); - DECORATE.beforeFinish(span); - span.finish(); - throw throwable; - } - - if (null != scope) { - scope.close(); + if (null != parentScope) { + parentScope.close(); } } } diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerRequestTracingHandler.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerRequestTracingHandler.java index 791549c1c1..cadfeefd28 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerRequestTracingHandler.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/server/HttpServerRequestTracingHandler.java @@ -29,31 +29,27 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte GlobalTracer.get() .extract(Format.Builtin.HTTP_HEADERS, new NettyRequestExtractAdapter(request)); - final Scope scope = - GlobalTracer.get() - .buildSpan("netty.request") - .asChildOf(extractedContext) - .startActive(false); - final Span span = scope.span(); - DECORATE.afterStart(span); - DECORATE.onRequest(span, request); - DECORATE.onPeerConnection(span, remoteAddress); + final Span span = + GlobalTracer.get().buildSpan("netty.request").asChildOf(extractedContext).start(); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + DECORATE.afterStart(span); + DECORATE.onRequest(span, request); + DECORATE.onPeerConnection(span, remoteAddress); - if (scope instanceof TraceScope) { - ((TraceScope) scope).setAsyncPropagation(true); - } + if (scope instanceof TraceScope) { + ((TraceScope) scope).setAsyncPropagation(true); + } - ctx.channel().attr(AttributeKeys.SERVER_ATTRIBUTE_KEY).set(span); + ctx.channel().attr(AttributeKeys.SERVER_ATTRIBUTE_KEY).set(span); - try { - ctx.fireChannelRead(msg); - } catch (final Throwable throwable) { - DECORATE.onError(span, throwable); - DECORATE.beforeFinish(span); - span.finish(); // Finish the span manually since finishSpanOnClose was false - throw throwable; - } finally { - scope.close(); + try { + ctx.fireChannelRead(msg); + } catch (final Throwable throwable) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + span.finish(); // Finish the span manually since finishSpanOnClose was false + throw throwable; + } } } } From 7b53cebd3d801b0477d46f5f4fffacf94fcd5522 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Fri, 5 Apr 2019 09:59:09 -0400 Subject: [PATCH 13/15] Fix some compiler warnings --- .../jersey/JerseyClientConnectionErrorInstrumentation.java | 4 ++-- .../ResteasyClientConnectionErrorInstrumentation.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/instrumentation/jax-rs-client/connection-error-handling-jersey/src/main/java/datadog/trace/instrumentation/connection_error/jersey/JerseyClientConnectionErrorInstrumentation.java b/dd-java-agent/instrumentation/jax-rs-client/connection-error-handling-jersey/src/main/java/datadog/trace/instrumentation/connection_error/jersey/JerseyClientConnectionErrorInstrumentation.java index 266eaea96f..f826b82340 100644 --- a/dd-java-agent/instrumentation/jax-rs-client/connection-error-handling-jersey/src/main/java/datadog/trace/instrumentation/connection_error/jersey/JerseyClientConnectionErrorInstrumentation.java +++ b/dd-java-agent/instrumentation/jax-rs-client/connection-error-handling-jersey/src/main/java/datadog/trace/instrumentation/connection_error/jersey/JerseyClientConnectionErrorInstrumentation.java @@ -81,9 +81,9 @@ public final class JerseyClientConnectionErrorInstrumentation extends Instrument @Advice.OnMethodExit(suppress = Throwable.class) public static void handleError( @Advice.FieldValue("requestContext") final ClientRequest context, - @Advice.Return(readOnly = false) Future future) { + @Advice.Return(readOnly = false) Future future) { if (!(future instanceof WrappedFuture)) { - future = new WrappedFuture(future, context); + future = new WrappedFuture<>(future, context); } } } diff --git a/dd-java-agent/instrumentation/jax-rs-client/connection-error-handling-resteasy/src/main/java/datadog/trace/instrumentation/connection_error/resteasy/ResteasyClientConnectionErrorInstrumentation.java b/dd-java-agent/instrumentation/jax-rs-client/connection-error-handling-resteasy/src/main/java/datadog/trace/instrumentation/connection_error/resteasy/ResteasyClientConnectionErrorInstrumentation.java index 5f286250a8..360f5c4ba4 100644 --- a/dd-java-agent/instrumentation/jax-rs-client/connection-error-handling-resteasy/src/main/java/datadog/trace/instrumentation/connection_error/resteasy/ResteasyClientConnectionErrorInstrumentation.java +++ b/dd-java-agent/instrumentation/jax-rs-client/connection-error-handling-resteasy/src/main/java/datadog/trace/instrumentation/connection_error/resteasy/ResteasyClientConnectionErrorInstrumentation.java @@ -81,9 +81,9 @@ public final class ResteasyClientConnectionErrorInstrumentation extends Instrume @Advice.OnMethodExit(suppress = Throwable.class) public static void handleError( @Advice.FieldValue("configuration") final ClientConfiguration context, - @Advice.Return(readOnly = false) Future future) { + @Advice.Return(readOnly = false) Future future) { if (!(future instanceof WrappedFuture)) { - future = new WrappedFuture(future, context); + future = new WrappedFuture<>(future, context); } } } From f4791a17df33a282b75938936459c70f39943d3b Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 8 Apr 2019 11:35:13 -0400 Subject: [PATCH 14/15] Use decorators in Netty's ChannelFutureListenerInstrumentation --- .../netty40/ChannelFutureListenerInstrumentation.java | 6 +++--- .../netty41/ChannelFutureListenerInstrumentation.java | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) 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 89f5e00ec9..84a025b185 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 @@ -1,7 +1,6 @@ package datadog.trace.instrumentation.netty40; import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static io.opentracing.log.Fields.ERROR_OBJECT; import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -12,6 +11,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.context.TraceScope; +import datadog.trace.instrumentation.netty40.server.NettyHttpServerDecorator; import io.netty.channel.ChannelFuture; import io.opentracing.Scope; import io.opentracing.Span; @@ -91,8 +91,8 @@ public class ChannelFutureListenerInstrumentation extends Instrumenter.Default { .withTag(Tags.COMPONENT.getKey(), "netty") .start(); try (final Scope scope = GlobalTracer.get().scopeManager().activate(errorSpan, true)) { - Tags.ERROR.set(errorSpan, true); - errorSpan.log(singletonMap(ERROR_OBJECT, cause)); + NettyHttpServerDecorator.DECORATE.onError(errorSpan, cause); + NettyHttpServerDecorator.DECORATE.beforeFinish(errorSpan); } 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 2b7f0ee543..116eae6de2 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 @@ -1,7 +1,6 @@ package datadog.trace.instrumentation.netty41; import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static io.opentracing.log.Fields.ERROR_OBJECT; import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -12,6 +11,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.context.TraceScope; +import datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator; import io.netty.channel.ChannelFuture; import io.opentracing.Scope; import io.opentracing.Span; @@ -91,8 +91,8 @@ public class ChannelFutureListenerInstrumentation extends Instrumenter.Default { .withTag(Tags.COMPONENT.getKey(), "netty") .start(); try (final Scope scope = GlobalTracer.get().scopeManager().activate(errorSpan, true)) { - Tags.ERROR.set(errorSpan, true); - errorSpan.log(singletonMap(ERROR_OBJECT, cause)); + NettyHttpServerDecorator.DECORATE.onError(errorSpan, cause); + NettyHttpServerDecorator.DECORATE.beforeFinish(errorSpan); } return parentScope; From 6fd630831ff1dc407bd7dbb6c9eb4371b5b16e05 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 8 Apr 2019 11:54:47 -0400 Subject: [PATCH 15/15] 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(); } }