Remove activeScope() use in OpenTracing shim

mcculls · mcculls · commit f451c5386a53 · 2025-03-03T17:51:57.000Z
This was only used to implement the deprecated 'active()' OpenTracing method.
We take a best effort approach of using a 'fake' scope which lets the caller
close the active scope if it matches the expected span, and otherwise logs a
warning (or throws an exception in strict mode.)
diff --git a/dd-java-agent/instrumentation/opentracing/api-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/GlobalTracerInstrumentation.java b/dd-java-agent/instrumentation/opentracing/api-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/GlobalTracerInstrumentation.java
@@ -44,6 +44,7 @@ public String[] helperClassNames() {
       packageName + ".OTTextMapSetter",
       packageName + ".OTScopeManager",
       packageName + ".OTScopeManager$OTScope",
+      packageName + ".OTScopeManager$FakeScope",
       packageName + ".TypeConverter",
       packageName + ".OTSpan",
       packageName + ".OTSpanContext",
diff --git a/dd-java-agent/instrumentation/opentracing/api-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/OTScopeManager.java b/dd-java-agent/instrumentation/opentracing/api-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/OTScopeManager.java
@@ -1,5 +1,6 @@
 package datadog.trace.instrumentation.opentracing31;
 
+import datadog.trace.api.Config;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
@@ -8,8 +9,12 @@
 import io.opentracing.Scope;
 import io.opentracing.ScopeManager;
 import io.opentracing.Span;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class OTScopeManager implements ScopeManager {
+  static final Logger log = LoggerFactory.getLogger(OTScopeManager.class);
+
   private final TypeConverter converter;
   private final AgentTracer.TracerAPI tracer;
 
@@ -33,8 +38,12 @@ public Scope activate(final Span span, final boolean finishSpanOnClose) {
   @Deprecated
   @Override
   public Scope active() {
+    AgentSpan agentSpan = tracer.activeSpan();
+    if (null == agentSpan) {
+      return null;
+    }
     // WARNING... Making an assumption about finishSpanOnClose
-    return converter.toScope(tracer.activeScope(), false);
+    return new OTScope(new FakeScope(agentSpan), false, converter);
   }
 
   static class OTScope implements Scope, TraceScope {
@@ -67,4 +76,33 @@ public boolean isFinishSpanOnClose() {
       return finishSpanOnClose;
     }
   }
+
+  private final class FakeScope implements AgentScope {
+    private final AgentSpan agentSpan;
+
+    FakeScope(AgentSpan agentSpan) {
+      this.agentSpan = agentSpan;
+    }
+
+    @Override
+    public AgentSpan span() {
+      return agentSpan;
+    }
+
+    @Override
+    public byte source() {
+      return ScopeSource.MANUAL.id();
+    }
+
+    @Override
+    public void close() {
+      if (agentSpan == tracer.activeSpan()) {
+        tracer.closeActiveSpan();
+      } else if (Config.get().isScopeStrictMode()) {
+        throw new RuntimeException("Tried to close scope when not on top");
+      } else {
+        log.warn("Tried to close scope when not on top");
+      }
+    }
+  }
 }
diff --git a/dd-java-agent/instrumentation/opentracing/api-0.31/src/test/groovy/OpenTracing31Test.groovy b/dd-java-agent/instrumentation/opentracing/api-0.31/src/test/groovy/OpenTracing31Test.groovy
@@ -258,7 +258,7 @@ class OpenTracing31Test extends AgentTestRunner {
     firstScope.close()
 
     then:
-    tracer.scopeManager().active().delegate == secondScope.delegate
+    tracer.scopeManager().active().span() == secondScope.span()
     _ * TEST_PROFILING_CONTEXT_INTEGRATION._
     0 * _
 
diff --git a/dd-java-agent/instrumentation/opentracing/api-0.32/src/main/java/datadog/trace/instrumentation/opentracing32/GlobalTracerInstrumentation.java b/dd-java-agent/instrumentation/opentracing/api-0.32/src/main/java/datadog/trace/instrumentation/opentracing32/GlobalTracerInstrumentation.java
@@ -35,6 +35,7 @@ public String[] helperClassNames() {
       packageName + ".OTTextMapInjectSetter",
       packageName + ".OTScopeManager",
       packageName + ".OTScopeManager$OTScope",
+      packageName + ".OTScopeManager$FakeScope",
       packageName + ".TypeConverter",
       packageName + ".OTSpan",
       packageName + ".OTSpanContext",
diff --git a/dd-java-agent/instrumentation/opentracing/api-0.32/src/main/java/datadog/trace/instrumentation/opentracing32/OTScopeManager.java b/dd-java-agent/instrumentation/opentracing/api-0.32/src/main/java/datadog/trace/instrumentation/opentracing32/OTScopeManager.java
@@ -1,5 +1,6 @@
 package datadog.trace.instrumentation.opentracing32;
 
+import datadog.trace.api.Config;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
@@ -8,8 +9,12 @@
 import io.opentracing.Scope;
 import io.opentracing.ScopeManager;
 import io.opentracing.Span;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class OTScopeManager implements ScopeManager {
+  static final Logger log = LoggerFactory.getLogger(OTScopeManager.class);
+
   private final TypeConverter converter;
   private final AgentTracer.TracerAPI tracer;
 
@@ -38,8 +43,12 @@ public Scope activate(final Span span, final boolean finishSpanOnClose) {
   @Deprecated
   @Override
   public Scope active() {
+    AgentSpan agentSpan = tracer.activeSpan();
+    if (null == agentSpan) {
+      return null;
+    }
     // WARNING... Making an assumption about finishSpanOnClose
-    return converter.toScope(tracer.activeScope(), false);
+    return new OTScope(new FakeScope(agentSpan), false, converter);
   }
 
   @Override
@@ -77,4 +86,33 @@ public boolean isFinishSpanOnClose() {
       return finishSpanOnClose;
     }
   }
+
+  private final class FakeScope implements AgentScope {
+    private final AgentSpan agentSpan;
+
+    FakeScope(AgentSpan agentSpan) {
+      this.agentSpan = agentSpan;
+    }
+
+    @Override
+    public AgentSpan span() {
+      return agentSpan;
+    }
+
+    @Override
+    public byte source() {
+      return ScopeSource.MANUAL.id();
+    }
+
+    @Override
+    public void close() {
+      if (agentSpan == tracer.activeSpan()) {
+        tracer.closeActiveSpan();
+      } else if (Config.get().isScopeStrictMode()) {
+        throw new RuntimeException("Tried to close scope when not on top");
+      } else {
+        log.warn("Tried to close scope when not on top");
+      }
+    }
+  }
 }
diff --git a/dd-java-agent/instrumentation/opentracing/api-0.32/src/test/groovy/OpenTracing32Test.groovy b/dd-java-agent/instrumentation/opentracing/api-0.32/src/test/groovy/OpenTracing32Test.groovy
@@ -273,7 +273,7 @@ class OpenTracing32Test extends AgentTestRunner {
     firstScope.close()
 
     then:
-    tracer.scopeManager().active().delegate == secondScope.delegate
+    tracer.scopeManager().active().span() == secondScope.span()
     _ * TEST_PROFILING_CONTEXT_INTEGRATION._
     0 * _
 
diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java
@@ -972,6 +972,14 @@ public AgentScope activeScope() {
     return scopeManager.active();
   }
 
+  @Override
+  public void closeActiveSpan() {
+    AgentScope activeScope = this.scopeManager.active();
+    if (activeScope != null) {
+      activeScope.close();
+    }
+  }
+
   @Override
   public AgentSpanContext notifyExtensionStart(Object event) {
     return LambdaHandler.notifyStartInvocation(this, event);
diff --git a/dd-trace-ot/build.gradle b/dd-trace-ot/build.gradle
@@ -15,6 +15,7 @@ minimumInstructionCoverage = 0.5
 excludedClassesCoverage += [
   // This is mainly equals() and hashCode()
   "datadog.opentracing.OTScopeManager.OTScope",
+  "datadog.opentracing.OTScopeManager.FakeScope",
   "datadog.opentracing.OTSpan",
   "datadog.opentracing.OTSpanContext",
   "datadog.opentracing.CustomScopeManagerWrapper.CustomScopeState",
diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/OTScopeManager.java b/dd-trace-ot/src/main/java/datadog/opentracing/OTScopeManager.java
@@ -1,5 +1,6 @@
 package datadog.opentracing;
 
+import datadog.trace.api.Config;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
@@ -8,9 +9,13 @@
 import io.opentracing.Scope;
 import io.opentracing.ScopeManager;
 import io.opentracing.Span;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /** One of the two possible scope managers. See CustomScopeManagerWrapper */
 class OTScopeManager implements ScopeManager {
+  static final Logger log = LoggerFactory.getLogger(OTScopeManager.class);
+
   private final TypeConverter converter;
   private final AgentTracer.TracerAPI tracer;
 
@@ -39,8 +44,12 @@ public Scope activate(final Span span, final boolean finishSpanOnClose) {
   @Deprecated
   @Override
   public Scope active() {
+    AgentSpan agentSpan = tracer.activeSpan();
+    if (null == agentSpan) {
+      return null;
+    }
     // WARNING... Making an assumption about finishSpanOnClose
-    return converter.toScope(tracer.activeScope(), false);
+    return new OTScope(new FakeScope(agentSpan), false, converter);
   }
 
   @Override
@@ -83,16 +92,45 @@ public boolean equals(final Object o) {
         return false;
       }
       final OTScope otScope = (OTScope) o;
-      return delegate.equals(otScope.delegate);
+      return delegate.span().equals(otScope.delegate.span());
     }
 
     @Override
     public int hashCode() {
-      return delegate.hashCode();
+      return delegate.span().hashCode();
     }
 
     boolean isFinishSpanOnClose() {
       return finishSpanOnClose;
     }
   }
+
+  private final class FakeScope implements AgentScope {
+    private final AgentSpan agentSpan;
+
+    FakeScope(AgentSpan agentSpan) {
+      this.agentSpan = agentSpan;
+    }
+
+    @Override
+    public AgentSpan span() {
+      return agentSpan;
+    }
+
+    @Override
+    public byte source() {
+      return ScopeSource.MANUAL.id();
+    }
+
+    @Override
+    public void close() {
+      if (agentSpan == tracer.activeSpan()) {
+        tracer.closeActiveSpan();
+      } else if (Config.get().isScopeStrictMode()) {
+        throw new RuntimeException("Tried to close scope when not on top");
+      } else {
+        log.warn("Tried to close scope when not on top");
+      }
+    }
+  }
 }
diff --git a/dd-trace-ot/src/ot33CompatibilityTest/groovy/OT33ApiTest.groovy b/dd-trace-ot/src/ot33CompatibilityTest/groovy/OT33ApiTest.groovy
@@ -63,7 +63,6 @@ class OT33ApiTest extends DDSpecification {
 
     then:
     tracer.activeSpan().delegate == span.delegate
-    coreTracer.activeScope().span() == span.delegate
     coreTracer.activeSpan() == span.delegate
 
     cleanup:
diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/IterationSpansForkedTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/IterationSpansForkedTest.groovy
@@ -40,7 +40,7 @@ class IterationSpansForkedTest extends DDSpecification {
 
     and:
     scope1.span() == span1
-    scopeManager.active().delegate == scope1
+    scopeManager.active().span().delegate == span1
     !spanFinished(span1)
 
     when:
@@ -54,7 +54,7 @@ class IterationSpansForkedTest extends DDSpecification {
 
     and:
     scope2.span() == span2
-    scopeManager.active().delegate == scope2
+    scopeManager.active().span().delegate == span2
     !spanFinished(span2)
 
     when:
@@ -69,7 +69,7 @@ class IterationSpansForkedTest extends DDSpecification {
 
     and:
     scope3.span() == span3
-    scopeManager.active().delegate == scope3
+    scopeManager.active().span().delegate == span3
     !spanFinished(span3)
 
     when:
@@ -96,7 +96,7 @@ class IterationSpansForkedTest extends DDSpecification {
 
     and:
     scope1.span() == span1
-    scopeManager.active().delegate == scope1
+    scopeManager.active().span().delegate == span1
     !spanFinished(span1)
 
     when:
@@ -110,7 +110,7 @@ class IterationSpansForkedTest extends DDSpecification {
 
     and:
     scope2.span() == span2
-    scopeManager.active().delegate == scope2
+    scopeManager.active().span().delegate == span2
     !spanFinished(span2)
 
     when:
@@ -124,7 +124,7 @@ class IterationSpansForkedTest extends DDSpecification {
 
     and:
     scope3.span() == span3
-    scopeManager.active().delegate == scope3
+    scopeManager.active().span().delegate == span3
     !spanFinished(span3)
 
     // close and finish the surrounding (non-iteration) span to complete the trace
@@ -150,7 +150,7 @@ class IterationSpansForkedTest extends DDSpecification {
 
     and:
     scope1.span() == span1
-    scopeManager.active().delegate == scope1
+    scopeManager.active().span().delegate == span1
     !spanFinished(span1)
 
     when:
@@ -168,7 +168,7 @@ class IterationSpansForkedTest extends DDSpecification {
 
     and:
     scope1A1.span() == span1A1
-    scopeManager.active().delegate == scope1A1
+    scopeManager.active().span().delegate == span1A1
     !spanFinished(span1A1)
 
     when:
@@ -182,7 +182,7 @@ class IterationSpansForkedTest extends DDSpecification {
 
     and:
     scope1A2.span() == span1A2
-    scopeManager.active().delegate == scope1A2
+    scopeManager.active().span().delegate == span1A2
     !spanFinished(span1A2)
 
     when:
diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java
@@ -121,6 +121,14 @@ public static AgentScope.Continuation captureSpan(final AgentSpan span) {
     return get().captureSpan(span);
   }
 
+  /**
+   * Closes the scope for the currently active span. Prefer closing the scope returned by {@link
+   * #activateSpan} when available.
+   */
+  public static void closeActiveSpan() {
+    get().closeActiveSpan();
+  }
+
   /**
    * Closes the immediately previous iteration scope. Should be called before creating a new span
    * for {@link #activateNext(AgentSpan)}.
@@ -318,6 +326,8 @@ AgentSpan startSpan(
 
     AgentScope activeScope();
 
+    void closeActiveSpan();
+
     default AgentSpan blackholeSpan() {
       final AgentSpan active = activeSpan();
       return new BlackHoleSpan(active != null ? active.getTraceId() : DDTraceId.ZERO);
@@ -481,6 +491,9 @@ public AgentScope activeScope() {
       return null;
     }
 
+    @Override
+    public void closeActiveSpan() {}
+
     @Override
     public AgentSpan blackholeSpan() {
       return NoopSpan.INSTANCE; // no-op tracer stays no-op
