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 index 027cb5bbb69..3129d49d036 100644 --- 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 index 6004bb28b9e..e0c18fb9c2c 100644 --- 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.closeActive(); + } else if (Config.get().isScopeStrictMode()) { + throw new RuntimeException("Tried to close " + agentSpan + " scope when not on top"); + } else { + log.warn("Tried to close {} scope when not on top", agentSpan); + } + } + } } 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 index 50675668882..bfa1f5fe9c8 100644 --- 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 index a12f689860d..e2b594e3ad2 100644 --- 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 index b32581b711e..1978ea1b83b 100644 --- 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.closeActive(); + } else if (Config.get().isScopeStrictMode()) { + throw new RuntimeException("Tried to close " + agentSpan + " scope when not on top"); + } else { + log.warn("Tried to close {} scope when not on top", agentSpan); + } + } + } } 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 index 3015ddd1d87..432a44ade2c 100644 --- 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 index 4e043347faf..49eb2fe5728 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -977,6 +977,14 @@ public AgentScope activeScope() { return scopeManager.active(); } + @Override + public void closeActive() { + 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-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java index e34e8d80608..a2707e9df68 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java @@ -55,7 +55,7 @@ public final void close() { byte source = source(); scopeManager.healthMetrics.onScopeCloseError(source); if (source == ScopeSource.MANUAL.id() && scopeManager.strictMode) { - throw new RuntimeException("Tried to close scope when not on top"); + throw new RuntimeException("Tried to close " + span + " scope when not on top"); } } diff --git a/dd-trace-ot/build.gradle b/dd-trace-ot/build.gradle index e6865047c8d..77212c99889 100644 --- 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 index fa0890a7696..b2297abfd19 100644 --- 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.closeActive(); + } else if (Config.get().isScopeStrictMode()) { + throw new RuntimeException("Tried to close " + agentSpan + " scope when not on top"); + } else { + log.warn("Tried to close {} scope when not on top", agentSpan); + } + } + } } diff --git a/dd-trace-ot/src/ot33CompatibilityTest/groovy/OT33ApiTest.groovy b/dd-trace-ot/src/ot33CompatibilityTest/groovy/OT33ApiTest.groovy index 7fd77af215e..163a8ca2aae 100644 --- 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 index 2787add8210..77cf0e41c55 100644 --- 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 index a3212cfab55..789a3e6a388 100644 --- 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,16 @@ public static AgentScope.Continuation captureSpan(final AgentSpan span) { return get().captureSpan(span); } + /** + * Closes the scope for the currently active span. + * + * @deprecated Prefer closing the scope returned by {@link #activateSpan} when available. + */ + @Deprecated + public static void closeActive() { + get().closeActive(); + } + /** * Closes the immediately previous iteration scope. Should be called before creating a new span * for {@link #activateNext(AgentSpan)}. @@ -310,6 +320,8 @@ AgentSpan startSpan( AgentScope.Continuation captureSpan(AgentSpan span); + void closeActive(); + void closePrevious(boolean finishSpan); AgentScope activateNext(AgentSpan span); @@ -463,6 +475,9 @@ public boolean isAsyncPropagationEnabled() { @Override public void setAsyncPropagationEnabled(boolean asyncPropagationEnabled) {} + @Override + public void closeActive() {} + @Override public void closePrevious(final boolean finishSpan) {}