feat(core): Extract and refactor span and scope implementations from …

…tracer (#8321) Add dedicated noop methods for span, span context and scope Remove unused eligibleForDropping method Improve Javadoc
DataDog · Feb 7, 2025 · 3f4b7a8 · 3f4b7a8
1 parent 7ebf343
commit 3f4b7a8
Show file tree

Hide file tree

Showing 47 changed files with 952 additions and 791 deletions.
diff --git a/...rc/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ConcurrentState.java b/...rc/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ConcurrentState.java
@@ -4,7 +4,6 @@
 
 import datadog.trace.bootstrap.ContextStore;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
-import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
 import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -32,7 +31,7 @@ private ConcurrentState() {}
   public static <K> ConcurrentState captureScope(
       ContextStore<K, ConcurrentState> contextStore, K key, AgentScope scope) {
     if (scope != null && scope.isAsyncPropagating()) {
-      if (scope.span() instanceof AgentTracer.NoopAgentSpan) {
+      if (!scope.span().isValid()) {
         return null;
       }
       final ConcurrentState state = contextStore.putIfAbsent(key, FACTORY);

diff --git a/...main/java/datadog/trace/instrumentation/akka/concurrent/AkkaActorCellInstrumentation.java b/...main/java/datadog/trace/instrumentation/akka/concurrent/AkkaActorCellInstrumentation.java
@@ -4,6 +4,7 @@
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan;
 import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 
@@ -13,7 +14,6 @@
 import datadog.trace.agent.tooling.InstrumenterModule;
 import datadog.trace.bootstrap.InstrumentationContext;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
-import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
 import datadog.trace.bootstrap.instrumentation.java.concurrent.AdviceUtils;
 import datadog.trace.bootstrap.instrumentation.java.concurrent.State;
 import java.util.Map;
@@ -72,11 +72,11 @@ public static AgentScope enter(
         return null;
       }
       // If there is a noop span in the active scope, we can clean all the way to this scope
-      if (activeSpan() instanceof AgentTracer.NoopAgentSpan) {
+      if (activeSpan() == noopSpan()) {
         return activeScope;
       }
       // Create an active scope with a noop span, and clean all the way to the previous scope
-      localScope = activateSpan(AgentTracer.NoopAgentSpan.INSTANCE, false);
+      localScope = activateSpan(noopSpan(), false);
       return activeScope;
     }
 

diff --git a/...c/main/java/datadog/trace/instrumentation/akka/concurrent/AkkaMailboxInstrumentation.java b/...c/main/java/datadog/trace/instrumentation/akka/concurrent/AkkaMailboxInstrumentation.java
@@ -4,6 +4,7 @@
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan;
 import static java.util.Collections.singletonList;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 
@@ -12,7 +13,6 @@
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.agent.tooling.InstrumenterModule;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
-import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
 import datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter;
 import java.util.Collection;
 import java.util.EnumMap;
@@ -69,11 +69,11 @@ public static AgentScope enter() {
         return null;
       }
       // If there is a noop span in the active scope, we can clean all the way to this scope
-      if (activeSpan() instanceof AgentTracer.NoopAgentSpan) {
+      if (activeSpan() == noopSpan()) {
         return activeScope;
       }
       // Create an active scope with a noop span, and clean all the way to the previous scope
-      activateSpan(AgentTracer.NoopAgentSpan.INSTANCE, false);
+      activateSpan(noopSpan(), false);
       return activeScope;
     }
 

diff --git a/...11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSHttpClientInstrumentation.java b/...11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSHttpClientInstrumentation.java
@@ -12,7 +12,6 @@
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
-import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
 import net.bytebuddy.asm.Advice;
 
 /**
@@ -50,7 +49,7 @@ public static void methodExit(
       // check name in case TracingRequestHandler failed to activate the span
       if (scope != null
           && (AwsNameCache.spanName(request).equals(scope.span().getSpanName())
-              || scope.span() instanceof AgentTracer.NoopAgentSpan)) {
+              || !scope.span().isValid())) {
         scope.close();
       }
 

diff --git a/....0/src/main/java/datadog/trace/instrumentation/aws/v0/RequestExecutorInstrumentation.java b/....0/src/main/java/datadog/trace/instrumentation/aws/v0/RequestExecutorInstrumentation.java
@@ -10,7 +10,6 @@
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
-import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
 import net.bytebuddy.asm.Advice;
 
 /**
@@ -47,7 +46,7 @@ public static void methodExit(
       // check name in case TracingRequestHandler failed to activate the span
       if (scope != null
           && (AwsNameCache.spanName(request).equals(scope.span().getSpanName())
-              || scope.span() instanceof AgentTracer.NoopAgentSpan)) {
+              || !scope.span().isValid())) {
         scope.close();
       }
 

diff --git a/...-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsHttpClientInstrumentation.java b/...-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsHttpClientInstrumentation.java
@@ -15,7 +15,6 @@
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.agent.tooling.InstrumenterModule;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
-import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
@@ -76,7 +75,7 @@ public static AgentScope methodEnter(
       final AgentScope scope = activeScope();
       // check name in case TracingExecutionInterceptor failed to activate the span
       if (scope != null
-          && (scope.span() instanceof AgentTracer.NoopAgentSpan
+          && ((!scope.span().isValid())
               || AwsSdkClientDecorator.DECORATE
                   .spanName(requestExecutionContext.executionAttributes())
                   .equals(scope.span().getSpanName()))) {

diff --git a/...java/datadog/trace/instrumentation/netty40/NettyChannelHandlerContextInstrumentation.java b/...java/datadog/trace/instrumentation/netty40/NettyChannelHandlerContextInstrumentation.java
@@ -5,6 +5,7 @@
 import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope;
 import static datadog.trace.instrumentation.netty40.AttributeKeys.SPAN_ATTRIBUTE_KEY;
 import static datadog.trace.instrumentation.netty40.NettyChannelPipelineInstrumentation.ADDITIONAL_INSTRUMENTATION_NAMES;
 import static datadog.trace.instrumentation.netty40.NettyChannelPipelineInstrumentation.INSTRUMENTATION_NAME;
@@ -16,7 +17,6 @@
 import datadog.trace.agent.tooling.InstrumenterModule;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
-import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
 import datadog.trace.instrumentation.netty40.client.NettyHttpClientDecorator;
 import datadog.trace.instrumentation.netty40.server.NettyHttpServerDecorator;
 import io.netty.channel.ChannelHandlerContext;
@@ -72,7 +72,7 @@ public static AgentScope scopeSpan(@Advice.This final ChannelHandlerContext ctx)
       final AgentSpan channelSpan = ctx.channel().attr(SPAN_ATTRIBUTE_KEY).get();
       if (channelSpan == null || channelSpan == activeSpan()) {
         // don't modify the scope
-        return AgentTracer.NoopAgentScope.INSTANCE;
+        return noopScope();
       }
       return activateSpan(channelSpan);
     }

diff --git a/...java/datadog/trace/instrumentation/netty41/NettyChannelHandlerContextInstrumentation.java b/...java/datadog/trace/instrumentation/netty41/NettyChannelHandlerContextInstrumentation.java
@@ -5,6 +5,7 @@
 import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope;
 import static datadog.trace.instrumentation.netty41.AttributeKeys.SPAN_ATTRIBUTE_KEY;
 import static datadog.trace.instrumentation.netty41.NettyChannelPipelineInstrumentation.ADDITIONAL_INSTRUMENTATION_NAMES;
 import static datadog.trace.instrumentation.netty41.NettyChannelPipelineInstrumentation.INSTRUMENTATION_NAME;
@@ -16,7 +17,6 @@
 import datadog.trace.agent.tooling.InstrumenterModule;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
-import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
 import datadog.trace.instrumentation.netty41.client.NettyHttpClientDecorator;
 import datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator;
 import io.netty.channel.ChannelHandlerContext;
@@ -72,7 +72,7 @@ public static AgentScope scopeSpan(@Advice.This final ChannelHandlerContext ctx)
       final AgentSpan channelSpan = ctx.channel().attr(SPAN_ATTRIBUTE_KEY).get();
       if (channelSpan == null || channelSpan == activeSpan()) {
         // don't modify the scope
-        return AgentTracer.NoopAgentScope.INSTANCE;
+        return noopScope();
       }
       return activateSpan(channelSpan);
     }

diff --git a/dd-java-agent/instrumentation/okhttp-2/src/test/groovy/OkHttp2AsyncTest.groovy b/dd-java-agent/instrumentation/okhttp-2/src/test/groovy/OkHttp2AsyncTest.groovy
@@ -7,6 +7,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentTracer
 import java.util.concurrent.CountDownLatch
 import java.util.concurrent.atomic.AtomicReference
 
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
 import static java.util.concurrent.TimeUnit.SECONDS
 
 abstract class OkHttp2AsyncTest extends OkHttp2Test {
@@ -52,7 +53,7 @@ abstract class OkHttp2AsyncTest extends OkHttp2Test {
   def "callbacks should carry context with error = #error" () {
 
     when:
-    def captured = AgentTracer.noopSpan()
+    def captured = noopSpan()
     try {
       TraceUtils.runUnderTrace("parent", {
         doRequest(method, url, ["Datadog-Meta-Lang": "java"], "", { captured = AgentTracer.activeSpan() })

diff --git a/dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3AsyncTest.groovy b/dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3AsyncTest.groovy
@@ -13,6 +13,7 @@ import okhttp3.internal.http.HttpMethod
 import java.util.concurrent.CountDownLatch
 import java.util.concurrent.atomic.AtomicReference
 
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
 import static java.util.concurrent.TimeUnit.SECONDS
 
 abstract class OkHttp3AsyncTest extends OkHttp3Test {
@@ -52,7 +53,7 @@ abstract class OkHttp3AsyncTest extends OkHttp3Test {
   def "callbacks should carry context with error = #error" () {
 
     when:
-    def captured = AgentTracer.noopSpan()
+    def captured = noopSpan()
     try {
       TraceUtils.runUnderTrace("parent", {
         doRequest(method, url, ["Datadog-Meta-Lang": "java"], "", { captured = AgentTracer.activeSpan() })

diff --git a/...elemetry-0.3/src/main/java/datadog/trace/instrumentation/opentelemetry/TypeConverter.java b/...elemetry-0.3/src/main/java/datadog/trace/instrumentation/opentelemetry/TypeConverter.java
@@ -1,9 +1,12 @@
 package datadog.trace.instrumentation.opentelemetry;
 
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext;
+
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
-import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
 import datadog.trace.bootstrap.instrumentation.api.AttachableWrapper;
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.trace.Span;
@@ -16,16 +19,16 @@ public class TypeConverter {
   private final OtelScope noopScopeWrapper;
 
   public TypeConverter() {
-    noopSpanWrapper = new OtelSpan(AgentTracer.NoopAgentSpan.INSTANCE, this);
-    noopContextWrapper = new OtelSpanContext(AgentTracer.NoopContext.INSTANCE);
-    noopScopeWrapper = new OtelScope(AgentTracer.NoopAgentScope.INSTANCE);
+    noopSpanWrapper = new OtelSpan(noopSpan(), this);
+    noopContextWrapper = new OtelSpanContext(noopSpanContext());
+    noopScopeWrapper = new OtelScope(noopScope());
   }
 
   public AgentSpan toAgentSpan(final Span span) {
     if (span instanceof OtelSpan) {
       return ((OtelSpan) span).asAgentSpan();
     }
-    return null == span ? null : AgentTracer.NoopAgentSpan.INSTANCE;
+    return null == span ? null : noopSpan();
   }
 
   public Span toSpan(final AgentSpan agentSpan) {
@@ -42,7 +45,7 @@ public Span toSpan(final AgentSpan agentSpan) {
       attachableSpanWrapper.attachWrapper(spanWrapper);
       return spanWrapper;
     }
-    if (agentSpan == AgentTracer.NoopAgentSpan.INSTANCE) {
+    if (agentSpan == noopSpan()) {
       return noopSpanWrapper;
     }
     return new OtelSpan(agentSpan, this);
@@ -62,7 +65,7 @@ public Scope toScope(final AgentScope scope) {
       attachableScopeWrapper.attachWrapper(otScope);
       return otScope;
     }
-    if (scope == AgentTracer.NoopAgentScope.INSTANCE) {
+    if (scope == noopScope()) {
       return noopScopeWrapper;
     }
     return new OtelScope(scope);
@@ -73,7 +76,7 @@ public SpanContext toSpanContext(final AgentSpanContext context) {
       return null;
     }
     // avoid a new SpanContext wrapper allocation for the noop context
-    if (context == AgentTracer.NoopContext.INSTANCE) {
+    if (context == noopSpanContext()) {
       return noopContextWrapper;
     }
     return new OtelSpanContext(context);
@@ -83,6 +86,6 @@ public AgentSpanContext toContext(final SpanContext spanContext) {
     if (spanContext instanceof OtelSpanContext) {
       return ((OtelSpanContext) spanContext).getDelegate();
     }
-    return null == spanContext ? null : AgentTracer.NoopContext.INSTANCE;
+    return null == spanContext ? null : noopSpanContext();
   }
 }
diff --git a/.../instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/TypeConverterTest.groovy b/.../instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/TypeConverterTest.groovy
@@ -2,7 +2,6 @@ import datadog.trace.agent.test.AgentTestRunner
 import datadog.trace.api.DDSpanId
 import datadog.trace.api.DDTraceId
 import datadog.trace.api.sampling.PrioritySampling
-import datadog.trace.bootstrap.instrumentation.api.AgentTracer
 import datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopPathwayContext
 import datadog.trace.bootstrap.instrumentation.api.ScopeSource
 import datadog.trace.core.DDSpan
@@ -12,11 +11,15 @@ import datadog.trace.core.propagation.PropagationTags
 import datadog.trace.core.scopemanager.ContinuableScopeManager
 import datadog.trace.instrumentation.opentelemetry.TypeConverter
 
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
+
 class TypeConverterTest extends AgentTestRunner {
   TypeConverter typeConverter = new TypeConverter()
 
   def "should avoid the noop span wrapper allocation"() {
-    def noopAgentSpan = AgentTracer.NoopAgentSpan.INSTANCE
+    def noopAgentSpan = noopSpan()
     expect:
     typeConverter.toSpan(noopAgentSpan) is typeConverter.toSpan(noopAgentSpan)
   }
@@ -33,13 +36,13 @@ class TypeConverterTest extends AgentTestRunner {
   }
 
   def "should avoid the noop context wrapper allocation"() {
-    def noopContext = AgentTracer.NoopContext.INSTANCE
+    def noopContext = noopSpanContext()
     expect:
     typeConverter.toSpanContext(noopContext) is typeConverter.toSpanContext(noopContext)
   }
 
   def "should avoid the noop scope wrapper allocation"() {
-    def noopScope = AgentTracer.NoopAgentScope.INSTANCE
+    def noopScope = noopScope()
     expect:
     typeConverter.toScope(noopScope) is typeConverter.toScope(noopScope)
   }

diff --git a/...ing/api-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/TypeConverter.java b/...ing/api-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/TypeConverter.java
@@ -1,9 +1,12 @@
 package datadog.trace.instrumentation.opentracing31;
 
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext;
+
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
-import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
 import datadog.trace.bootstrap.instrumentation.api.AttachableWrapper;
 import datadog.trace.instrumentation.opentracing.LogHandler;
 import io.opentracing.Scope;
@@ -19,16 +22,16 @@ public class TypeConverter {
 
   public TypeConverter(final LogHandler logHandler) {
     this.logHandler = logHandler;
-    noopSpanWrapper = new OTSpan(AgentTracer.NoopAgentSpan.INSTANCE, this, logHandler);
-    noopContextWrapper = new OTSpanContext(AgentTracer.NoopContext.INSTANCE);
-    noopScopeWrapper = new OTScopeManager.OTScope(AgentTracer.NoopAgentScope.INSTANCE, false, this);
+    noopSpanWrapper = new OTSpan(noopSpan(), this, logHandler);
+    noopContextWrapper = new OTSpanContext(noopSpanContext());
+    noopScopeWrapper = new OTScopeManager.OTScope(noopScope(), false, this);
   }
 
   public AgentSpan toAgentSpan(final Span span) {
     if (span instanceof OTSpan) {
       return ((OTSpan) span).asAgentSpan();
     }
-    return null == span ? null : AgentTracer.NoopAgentSpan.INSTANCE;
+    return null == span ? null : noopSpan();
   }
 
   public OTSpan toSpan(final AgentSpan agentSpan) {
@@ -45,7 +48,7 @@ public OTSpan toSpan(final AgentSpan agentSpan) {
       attachableSpanWrapper.attachWrapper(spanWrapper);
       return spanWrapper;
     }
-    if (agentSpan == AgentTracer.NoopAgentSpan.INSTANCE) {
+    if (agentSpan == noopSpan()) {
       return noopSpanWrapper;
     }
     return new OTSpan(agentSpan, this, logHandler);
@@ -68,7 +71,7 @@ public Scope toScope(final AgentScope scope, final boolean finishSpanOnClose) {
       attachableScopeWrapper.attachWrapper(otScope);
       return otScope;
     }
-    if (scope == AgentTracer.NoopAgentScope.INSTANCE) {
+    if (scope == noopScope()) {
       return noopScopeWrapper;
     }
     return new OTScopeManager.OTScope(scope, finishSpanOnClose, this);
@@ -79,7 +82,7 @@ public SpanContext toSpanContext(final AgentSpanContext context) {
       return null;
     }
     // avoid a new SpanContext wrapper allocation for the noop context
-    if (context == AgentTracer.NoopContext.INSTANCE) {
+    if (context == noopSpanContext()) {
       return noopContextWrapper;
     }
     return new OTSpanContext(context);
@@ -89,6 +92,6 @@ public AgentSpanContext toContext(final SpanContext spanContext) {
     if (spanContext instanceof OTSpanContext) {
       return ((OTSpanContext) spanContext).getDelegate();
     }
-    return null == spanContext ? null : AgentTracer.NoopContext.INSTANCE;
+    return null == spanContext ? null : noopSpanContext();
   }
 }