diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ConcurrentState.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ConcurrentState.java index 73028d59e8b..e12a896e69e 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ConcurrentState.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ConcurrentState.java @@ -69,23 +69,23 @@ public static void closeScope( if (throwable != null) { // This might lead to the continuation being consumed early, but it's better to be safe if we // threw an Exception on entry - state.closeContinuation(); + state.cancelContinuation(); } } - public static void closeAndClearContinuation( + public static void cancelAndClearContinuation( ContextStore contextStore, K key) { final ConcurrentState state = contextStore.get(key); if (state == null) { return; } - state.closeAndClearContinuation(); + state.cancelAndClearContinuation(); } private boolean captureAndSetContinuation(final AgentScope scope) { if (CONTINUATION.compareAndSet(this, null, CLAIMED)) { // lazy write is guaranteed to be seen by getAndSet - CONTINUATION.lazySet(this, scope.captureConcurrent()); + CONTINUATION.lazySet(this, scope.capture().hold()); return true; } return false; @@ -99,14 +99,14 @@ private AgentScope activateAndContinueContinuation() { return null; } - private void closeContinuation() { + private void cancelContinuation() { final AgentScope.Continuation continuation = CONTINUATION.get(this); if (continuation != null && continuation != CLAIMED) { continuation.cancel(); } } - private void closeAndClearContinuation() { + private void cancelAndClearContinuation() { final AgentScope.Continuation continuation = CONTINUATION.get(this); if (continuation != null && continuation != CLAIMED) { // We should never be able to reuse this state diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ContinuationClaim.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ContinuationClaim.java index d956e3519f3..01089d3144c 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ContinuationClaim.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ContinuationClaim.java @@ -7,6 +7,11 @@ final class ContinuationClaim implements AgentScope.Continuation { public static final ContinuationClaim CLAIMED = new ContinuationClaim(); + @Override + public AgentScope.Continuation hold() { + throw new IllegalStateException(); + } + @Override public AgentScope activate() { throw new IllegalStateException(); diff --git a/dd-java-agent/instrumentation/java-concurrent/java-completablefuture/src/main/java/java/util/concurrent/CompletableFutureAdvice.java b/dd-java-agent/instrumentation/java-concurrent/java-completablefuture/src/main/java/java/util/concurrent/CompletableFutureAdvice.java index bf8bf98c7a8..82c836b9a0e 100644 --- a/dd-java-agent/instrumentation/java-concurrent/java-completablefuture/src/main/java/java/util/concurrent/CompletableFutureAdvice.java +++ b/dd-java-agent/instrumentation/java-concurrent/java-completablefuture/src/main/java/java/util/concurrent/CompletableFutureAdvice.java @@ -72,7 +72,7 @@ public static void exit( boolean claimed = !wasClaimed && !hadExecutor && zis.getForkJoinTaskTag() == 1; if (mode == ASYNC || (mode < ASYNC && claimed) || !zis.isLive()) { contextStore = InstrumentationContext.get(UniCompletion.class, ConcurrentState.class); - ConcurrentState.closeAndClearContinuation(contextStore, zis); + ConcurrentState.cancelAndClearContinuation(contextStore, zis); } if (scope != null || throwable != null) { if (contextStore == null) { diff --git a/dd-java-agent/instrumentation/kotlin-coroutines/src/main/java/datadog/trace/instrumentation/kotlin/coroutines/ScopeStateCoroutineContext.java b/dd-java-agent/instrumentation/kotlin-coroutines/src/main/java/datadog/trace/instrumentation/kotlin/coroutines/ScopeStateCoroutineContext.java index d5982e35131..9030e154e68 100644 --- a/dd-java-agent/instrumentation/kotlin-coroutines/src/main/java/datadog/trace/instrumentation/kotlin/coroutines/ScopeStateCoroutineContext.java +++ b/dd-java-agent/instrumentation/kotlin-coroutines/src/main/java/datadog/trace/instrumentation/kotlin/coroutines/ScopeStateCoroutineContext.java @@ -121,7 +121,7 @@ public void maybeInitialize() { if (!isInitialized) { final AgentScope activeScope = AgentTracer.get().activeScope(); if (activeScope != null && activeScope.isAsyncPropagating()) { - continuation = activeScope.captureConcurrent(); + continuation = activeScope.capture().hold(); } isInitialized = true; } diff --git a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/main/java/datadog/trace/instrumentation/opentelemetry/OtelScope.java b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/main/java/datadog/trace/instrumentation/opentelemetry/OtelScope.java index 144013b86a7..6a387df6f67 100644 --- a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/main/java/datadog/trace/instrumentation/opentelemetry/OtelScope.java +++ b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/main/java/datadog/trace/instrumentation/opentelemetry/OtelScope.java @@ -16,11 +16,6 @@ public Continuation capture() { return delegate.capture(); } - @Override - public Continuation captureConcurrent() { - return delegate.captureConcurrent(); - } - @Override public void close() { delegate.close(); 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 7328d208ffc..d9f6179b5d3 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 @@ -68,11 +68,6 @@ public Continuation capture() { return delegate.capture(); } - @Override - public Continuation captureConcurrent() { - return delegate.captureConcurrent(); - } - public boolean isFinishSpanOnClose() { return finishSpanOnClose; } 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 173d3b4c993..aa4c0f1b131 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 @@ -78,11 +78,6 @@ public Continuation capture() { return delegate.capture(); } - @Override - public Continuation captureConcurrent() { - return delegate.captureConcurrent(); - } - public boolean isFinishSpanOnClose() { return finishSpanOnClose; } diff --git a/dd-trace-api/src/main/java/datadog/trace/context/NoopTraceScope.java b/dd-trace-api/src/main/java/datadog/trace/context/NoopTraceScope.java index 4ec59fb71d2..34d5cffbe19 100644 --- a/dd-trace-api/src/main/java/datadog/trace/context/NoopTraceScope.java +++ b/dd-trace-api/src/main/java/datadog/trace/context/NoopTraceScope.java @@ -8,6 +8,11 @@ public static class NoopContinuation implements Continuation { private NoopContinuation() {} + @Override + public Continuation hold() { + return this; + } + @Override public TraceScope activate() { return NoopTraceScope.INSTANCE; @@ -24,11 +29,6 @@ public Continuation capture() { return NoopContinuation.INSTANCE; } - @Override - public Continuation captureConcurrent() { - return null; - } - @Override public void close() {} } diff --git a/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java b/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java index 692f2857075..f7e5df5f803 100644 --- a/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java +++ b/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java @@ -14,18 +14,11 @@ public interface TraceScope extends Closeable { */ Continuation capture(); - /** - * Prevent the trace attached to this TraceScope from reporting until the returned Continuation is - * either activated (and the returned scope is closed), or canceled. - * - *

Should be called on the parent thread. - * - *

If the returned {@link Continuation} is activated, it needs to be canceled in addition to - * the returned {@link TraceScope} being closed. This is to allow multiple concurrent threads that - * activate the continuation to race in a safe way, and close the scopes without fear of closing - * the related {@code Span} prematurely. - */ - Continuation captureConcurrent(); + /** @deprecated Replaced by {@code capture().hold()}. */ + @Deprecated + default Continuation captureConcurrent() { + return capture().hold(); + } /** Close the activated context and allow any underlying spans to finish. */ @Override @@ -61,6 +54,15 @@ default void setAsyncPropagation(boolean value) { */ interface Continuation { + /** + * Prevent the trace attached to this scope from reporting until the continuation is explicitly + * cancelled. You must call {@link #cancel()} at some point to avoid discarding traces. + * + *

Use this when you want to let multiple threads activate the continuation concurrently and + * close their scopes without fear of prematurely closing the related span. + */ + Continuation hold(); + /** * Activate the continuation. * diff --git a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/AbstractContinuation.java b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/AbstractContinuation.java deleted file mode 100644 index f51253add71..00000000000 --- a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/AbstractContinuation.java +++ /dev/null @@ -1,34 +0,0 @@ -package datadog.trace.core.scopemanager; - -import datadog.trace.bootstrap.instrumentation.api.AgentScope; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -import datadog.trace.bootstrap.instrumentation.api.AgentTraceCollector; - -/** - * This class must not be a nested class of ContinuableScope to avoid an unconstrained chain of - * references (using too much memory). - */ -abstract class AbstractContinuation implements AgentScope.Continuation { - - final ContinuableScopeManager scopeManager; - final AgentSpan spanUnderScope; - final byte source; - final AgentTraceCollector traceCollector; - - public AbstractContinuation( - ContinuableScopeManager scopeManager, AgentSpan spanUnderScope, byte source) { - this.scopeManager = scopeManager; - this.spanUnderScope = spanUnderScope; - this.source = source; - this.traceCollector = spanUnderScope.context().getTraceCollector(); - } - - AbstractContinuation register() { - traceCollector.registerContinuation(this); - return this; - } - - // Called by ContinuableScopeManager when a continued scope is closed - // Can't use cancel() for SingleContinuation because of the "used" check - abstract void cancelFromContinuedScopeClose(); -} diff --git a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ConcurrentContinuation.java b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ConcurrentContinuation.java deleted file mode 100644 index 4136bccf330..00000000000 --- a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ConcurrentContinuation.java +++ /dev/null @@ -1,95 +0,0 @@ -package datadog.trace.core.scopemanager; - -import datadog.trace.bootstrap.instrumentation.api.AgentScope; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; - -/** - * This class must not be a nested class of ContinuableScope to avoid an unconstrained chain of - * references (using too much memory). - * - *

This {@link AbstractContinuation} differs from the {@link SingleContinuation} in that if it is - * activated, it needs to be canceled in addition to the returned {@link AgentScope} being closed. - * This is to allow multiple concurrent threads that activate the continuation to race in a safe - * way, and close the scopes without fear of closing the related {@link AgentSpan} prematurely. - */ -final class ConcurrentContinuation extends AbstractContinuation { - private static final int START = 1; - private static final int CLOSED = Integer.MIN_VALUE >> 1; - private static final int BARRIER = Integer.MIN_VALUE >> 2; - private volatile int count = START; - - private static final AtomicIntegerFieldUpdater COUNT = - AtomicIntegerFieldUpdater.newUpdater(ConcurrentContinuation.class, "count"); - - public ConcurrentContinuation( - ContinuableScopeManager scopeManager, AgentSpan spanUnderScope, byte source) { - super(scopeManager, spanUnderScope, source); - } - - private boolean tryActivate() { - int current = COUNT.incrementAndGet(this); - if (current < START) { - COUNT.decrementAndGet(this); - } - return current > START; - } - - private boolean tryClose() { - int current = COUNT.get(this); - if (current < BARRIER) { - return false; - } - // Now decrement the counter - current = COUNT.decrementAndGet(this); - // Try to close this if we are between START and BARRIER - while (current < START && current > BARRIER) { - if (COUNT.compareAndSet(this, current, CLOSED)) { - return true; - } - current = COUNT.get(this); - } - return false; - } - - @Override - public AgentScope activate() { - if (tryActivate()) { - return scopeManager.continueSpan(this, spanUnderScope, source); - } else { - return null; - } - } - - @Override - public void cancel() { - if (tryClose()) { - traceCollector.cancelContinuation(this); - } - ContinuableScopeManager.log.debug( - "t_id={} -> canceling continuation {}", spanUnderScope.getTraceId(), this); - } - - @Override - public AgentSpan getSpan() { - return spanUnderScope; - } - - @Override - void cancelFromContinuedScopeClose() { - cancel(); - } - - @Override - public String toString() { - int c = COUNT.get(this); - String s = c < BARRIER ? "CANCELED" : String.valueOf(c); - return getClass().getSimpleName() - + "@" - + Integer.toHexString(hashCode()) - + "(" - + s - + ")->" - + spanUnderScope; - } -} 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 85b3514ac88..72b3ef9ccd6 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 @@ -135,21 +135,9 @@ public final void setAsyncPropagation(final boolean value) { * @return The new continuation, or null if this scope is not async propagating. */ @Override - public final AbstractContinuation capture() { + public final ScopeContinuation capture() { return isAsyncPropagating - ? new SingleContinuation(scopeManager, span, source()).register() - : null; - } - - /** - * The continuation returned must be closed or activated or the trace will not finish. - * - * @return The new continuation, or null if this scope is not async propagating. - */ - @Override - public final AbstractContinuation captureConcurrent() { - return isAsyncPropagating - ? new ConcurrentContinuation(scopeManager, span, source()).register() + ? new ScopeContinuation(scopeManager, span, source()).register() : null; } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScopeManager.java b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScopeManager.java index bc31ee1722b..9da444a87c5 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScopeManager.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScopeManager.java @@ -91,8 +91,8 @@ public AgentScope activate( } public AgentScope.Continuation captureSpan(final AgentSpan span) { - AbstractContinuation continuation = - new SingleContinuation(this, span, ScopeSource.INSTRUMENTATION.id()); + ScopeContinuation continuation = + new ScopeContinuation(this, span, ScopeSource.INSTRUMENTATION.id()); continuation.register(); healthMetrics.onCaptureContinuation(); return continuation; @@ -136,12 +136,12 @@ private AgentScope activate( } /** - * Activates a scope for the given {@link AbstractContinuation}. + * Activates a scope for the given {@link ScopeContinuation}. * * @param continuation {@code null} if a continuation is re-used */ ContinuableScope continueSpan( - final AbstractContinuation continuation, final AgentSpan span, final byte source) { + final ScopeContinuation continuation, final AgentSpan span, final byte source) { ScopeStack scopeStack = scopeStack(); // optimization: if the top scope is already keeping the same span alive diff --git a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuingScope.java b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuingScope.java index f929fdb5df1..0ead3d4f725 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuingScope.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuingScope.java @@ -5,14 +5,14 @@ final class ContinuingScope extends ContinuableScope { /** Continuation that created this scope. */ - private final AbstractContinuation continuation; + private final ScopeContinuation continuation; ContinuingScope( final ContinuableScopeManager scopeManager, final AgentSpan span, final byte source, final boolean isAsyncPropagating, - final AbstractContinuation continuation, + final ScopeContinuation continuation, final Stateful scopeState) { super(scopeManager, span, source, isAsyncPropagating, scopeState); this.continuation = continuation; @@ -21,7 +21,6 @@ final class ContinuingScope extends ContinuableScope { @Override void cleanup(final ScopeStack scopeStack) { super.cleanup(scopeStack); - continuation.cancelFromContinuedScopeClose(); } } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ScopeContinuation.java b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ScopeContinuation.java new file mode 100644 index 00000000000..1c90276d093 --- /dev/null +++ b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ScopeContinuation.java @@ -0,0 +1,127 @@ +package datadog.trace.core.scopemanager; + +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTraceCollector; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopAgentScope; +import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; + +/** + * Used to pass async context between workers. A trace will not be reported until all spans and + * continuations are resolved. You must call activate (and close on the returned scope) or cancel on + * each continuation to avoid discarding traces. + * + *

This class must not be a nested class of ContinuableScope to avoid an unconstrained chain of + * references (using too much memory). + */ +final class ScopeContinuation implements AgentScope.Continuation { + private static final AtomicIntegerFieldUpdater COUNT = + AtomicIntegerFieldUpdater.newUpdater(ScopeContinuation.class, "count"); + + // these boundaries were selected to allow for speculative counting and fuzzy checks + private static final int CANCELLED = Integer.MIN_VALUE >> 1; + private static final int HELD = (Integer.MAX_VALUE >> 1) + 1; + + final ContinuableScopeManager scopeManager; + final AgentSpan spanUnderScope; + final byte source; + final AgentTraceCollector traceCollector; + + /** + * When positive this reflects the number of outstanding activations as well as whether there is + * an active hold on the continuation: + * + * + * + * + * + * + * + *
Value Meaning
0Not held or activated
1..HELD-1Activated, not held
HELDHeld, not yet activated
HELD..MAX_INTActivated and held
+ * + * where HELD is at the mid-point between 1 and MAX_INT. + * + *

A negative value of CANCELLED reflects that the continuation has either been activated and + * all associated scopes are now closed, or it has been explicitly cancelled. This value was + * chosen to be half the size of MIN_INT to avoid speculative additions in {@link #activate()} + * from overflowing to a positive count. + */ + private volatile int count = 0; + + ScopeContinuation( + final ContinuableScopeManager scopeManager, + final AgentSpan spanUnderScope, + final byte source) { + this.scopeManager = scopeManager; + this.spanUnderScope = spanUnderScope; + this.source = source; + + this.traceCollector = spanUnderScope.context().getTraceCollector(); + } + + ScopeContinuation register() { + traceCollector.registerContinuation(this); + return this; + } + + @Override + public AgentScope.Continuation hold() { + // update initial count to record that this continuation has a hold + COUNT.compareAndSet(this, 0, HELD); + return this; + } + + @Override + public AgentScope activate() { + if (COUNT.incrementAndGet(this) > 0) { + // speculative update succeeded, continuation can be activated + return scopeManager.continueSpan(this, spanUnderScope, source); + } else { + // continuation cancelled or too many activations; rollback count + COUNT.decrementAndGet(this); + return NoopAgentScope.INSTANCE; + } + } + + @Override + public void cancel() { + int current = count; + while (current >= HELD) { + // remove the hold on this continuation by removing the offset + COUNT.compareAndSet(this, current, current - HELD); + current = count; + } + while (current == 0) { + // no outstanding activations and hold has been removed + if (COUNT.compareAndSet(this, current, CANCELLED)) { + traceCollector.cancelContinuation(this); + return; + } + current = count; + } + } + + void cancelFromContinuedScopeClose() { + if (COUNT.compareAndSet(this, 1, CANCELLED)) { + // fast path: only one activation of the continuation (no hold) + traceCollector.cancelContinuation(this); + } else if (COUNT.decrementAndGet(this) == 0) { + // slow path: multiple activations, all have now closed (no hold) + cancel(); + } /* else there are outstanding activations or hold is in place */ + } + + @Override + public AgentSpan getSpan() { + return spanUnderScope; + } + + @Override + public String toString() { + return getClass().getSimpleName() + + "@" + + Integer.toHexString(hashCode()) + + "->" + + spanUnderScope; + } +} diff --git a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/SingleContinuation.java b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/SingleContinuation.java deleted file mode 100644 index 9fe44679a81..00000000000 --- a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/SingleContinuation.java +++ /dev/null @@ -1,61 +0,0 @@ -package datadog.trace.core.scopemanager; - -import datadog.trace.bootstrap.instrumentation.api.AgentScope; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; - -/** - * This class must not be a nested class of ContinuableScope to avoid an unconstrained chain of - * references (using too much memory). - */ -final class SingleContinuation extends AbstractContinuation { - private static final AtomicIntegerFieldUpdater USED = - AtomicIntegerFieldUpdater.newUpdater(SingleContinuation.class, "used"); - private volatile int used = 0; - - SingleContinuation( - final ContinuableScopeManager scopeManager, - final AgentSpan spanUnderScope, - final byte source) { - super(scopeManager, spanUnderScope, source); - } - - @Override - public AgentScope activate() { - if (USED.compareAndSet(this, 0, 1)) { - return scopeManager.continueSpan(this, spanUnderScope, source); - } else { - ContinuableScopeManager.log.debug( - "Failed to activate continuation. Reusing a continuation not allowed. Spans may be reported separately."); - return scopeManager.continueSpan(null, spanUnderScope, source); - } - } - - @Override - public void cancel() { - if (USED.compareAndSet(this, 0, 1)) { - traceCollector.cancelContinuation(this); - } else { - ContinuableScopeManager.log.debug("Failed to close continuation {}. Already used.", this); - } - } - - @Override - public AgentSpan getSpan() { - return spanUnderScope; - } - - @Override - void cancelFromContinuedScopeClose() { - traceCollector.cancelContinuation(this); - } - - @Override - public String toString() { - return getClass().getSimpleName() - + "@" - + Integer.toHexString(hashCode()) - + "->" - + spanUnderScope; - } -} diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/scopemanager/ScopeAndContinuationLayoutTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/scopemanager/ScopeAndContinuationLayoutTest.groovy index e7b6488d7f2..bba8f7d0028 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/scopemanager/ScopeAndContinuationLayoutTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/scopemanager/ScopeAndContinuationLayoutTest.groovy @@ -14,11 +14,7 @@ class ScopeAndContinuationLayoutTest extends DDSpecification { } def "single continuation layout"() { - expect: layoutAcceptable(SingleContinuation, 32) - } - - def "concurrent continuation layout"() { - expect: layoutAcceptable(ConcurrentContinuation, 32) + expect: layoutAcceptable(ScopeContinuation, 32) } def layoutAcceptable(Class klass, int acceptableSize) { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/scopemanager/ScopeManagerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/scopemanager/ScopeManagerTest.groovy index f63a4134158..71e25290dae 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/scopemanager/ScopeManagerTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/scopemanager/ScopeManagerTest.groovy @@ -254,23 +254,20 @@ class ScopeManagerTest extends DDCoreSpecification { def span = tracer.buildSpan("test", "test").start() def scope = tracer.activateSpan(span) tracer.setAsyncPropagationEnabled(false) - def continuation = concurrent ? scope.captureConcurrent() : scope.capture() + def continuation = scope.capture() then: continuation == null when: tracer.setAsyncPropagationEnabled(true) - continuation = concurrent ? scope.captureConcurrent() : scope.capture() + continuation = scope.capture() then: continuation != null cleanup: continuation.cancel() - - where: - concurrent << [false, true] } def "Continuation.cancel doesn't close parent scope"() { @@ -278,7 +275,7 @@ class ScopeManagerTest extends DDCoreSpecification { def span = tracer.buildSpan("test", "test").start() def scope = tracer.activateSpan(span) tracer.setAsyncPropagationEnabled(true) - def continuation = concurrent ? scope.captureConcurrent() : scope.capture() + def continuation = scope.capture() then: continuation != null @@ -288,9 +285,6 @@ class ScopeManagerTest extends DDCoreSpecification { then: scopeManager.active() == scope - - where: - concurrent << [false, true] } // @Flaky("awaitGC is flaky") @@ -299,7 +293,7 @@ class ScopeManagerTest extends DDCoreSpecification { def span = tracer.buildSpan("test", "test").start() def scopeRef = new AtomicReference(tracer.activateSpan(span)) tracer.setAsyncPropagationEnabled(true) - def continuation = concurrent ? scopeRef.get().captureConcurrent() : scopeRef.get().capture() + def continuation = scopeRef.get().capture() then: continuation != null @@ -320,9 +314,6 @@ class ScopeManagerTest extends DDCoreSpecification { ref.get() == null !spanFinished(span) writer == [] - - where: - concurrent << [false, true] } def "hard reference on continuation does not prevent trace from reporting"() { @@ -330,7 +321,7 @@ class ScopeManagerTest extends DDCoreSpecification { def span = tracer.buildSpan("test", "test").start() def scope = tracer.activateSpan(span) tracer.setAsyncPropagationEnabled(true) - def continuation = concurrent ? scope.captureConcurrent() : scope.capture() + def continuation = scope.capture() then: continuation != null @@ -353,11 +344,7 @@ class ScopeManagerTest extends DDCoreSpecification { writer == [[span]] where: - autoClose | concurrent - true | true - true | false - false | true - false | false + autoClose << [true, false] } def "continuation restores trace"() { @@ -368,7 +355,7 @@ class ScopeManagerTest extends DDCoreSpecification { def childScope = tracer.activateSpan(childSpan) tracer.setAsyncPropagationEnabled(true) - def continuation = concurrentChild ? childScope.captureConcurrent() : childScope.capture() + def continuation = childScope.capture() childScope.close() then: @@ -389,9 +376,6 @@ class ScopeManagerTest extends DDCoreSpecification { when: "activating the continuation" def newScope = continuation.activate() - if (concurrentChild) { - continuation.cancel() - } then: "the continued scope becomes active and span state doesnt change" newScope instanceof ContinuableScope @@ -405,13 +389,10 @@ class ScopeManagerTest extends DDCoreSpecification { writer == [] when: "creating and activating a second continuation" - def newContinuation = concurrentNew ? newScope.captureConcurrent() : newScope.capture() + def newContinuation = newScope.capture() newScope.close() def secondContinuedScope = newContinuation.activate() secondContinuedScope.close() - if (concurrentNew) { - newContinuation.cancel() - } childSpan.finish() writer.waitForTraces(1) @@ -420,13 +401,6 @@ class ScopeManagerTest extends DDCoreSpecification { spanFinished(childSpan) spanFinished(parentSpan) writer == [[childSpan, parentSpan]] - - where: - concurrentChild | concurrentNew - false | false - true | false - false | true - true | true } def "continuation allows adding spans even after other spans were completed"() { @@ -434,14 +408,11 @@ class ScopeManagerTest extends DDCoreSpecification { def span = tracer.buildSpan("test", "test").start() def scope = tracer.activateSpan(span) tracer.setAsyncPropagationEnabled(true) - def continuation = concurrent ? scope.captureConcurrent() : scope.capture() + def continuation = scope.capture() scope.close() span.finish() def newScope = continuation.activate() - if (concurrent) { - continuation.cancel() - } then: "the continuation sets the active scope" newScope instanceof ContinuableScope @@ -468,9 +439,6 @@ class ScopeManagerTest extends DDCoreSpecification { spanFinished(childSpan) childSpan.context().parentId == span.context().spanId writer == [[childSpan, span]] - - where: - concurrent << [false, true] } def "test activating same span multiple times"() { @@ -726,7 +694,7 @@ class ScopeManagerTest extends DDCoreSpecification { def span = tracer.buildSpan("test", "test").start() def scope = tracer.activateSpan(span) tracer.setAsyncPropagationEnabled(true) - def continuation = concurrent ? scope.captureConcurrent() : scope.capture() + def continuation = scope.capture() scope.close() span.finish() @@ -737,9 +705,7 @@ class ScopeManagerTest extends DDCoreSpecification { when: def continuedScope = continuation.activate() - if (concurrent) { - continuation.cancel() - } + AgentSpan secondSpan = tracer.buildSpan("test", "test2").start() AgentScope secondScope = (ContinuableScope) tracer.activateSpan(secondSpan) @@ -760,9 +726,6 @@ class ScopeManagerTest extends DDCoreSpecification { then: writer == [[secondSpan, span]] - - where: - concurrent << [false, true] } def "exception thrown in TraceInterceptor does not leave scope manager in bad state "() { @@ -814,7 +777,7 @@ class ScopeManagerTest extends DDCoreSpecification { def span = tracer.buildSpan("test", "test").start() def scope = tracer.activateSpan(span) tracer.setAsyncPropagationEnabled(true) - def continuation = concurrent ? scope.captureConcurrent() : scope.capture() + def continuation = scope.capture() scope.close() span.finish() @@ -841,7 +804,7 @@ class ScopeManagerTest extends DDCoreSpecification { def span2 = tracer.buildSpan("test", "test").start() def scope2 = tracer.activateSpan(span2) tracer.setAsyncPropagationEnabled(true) - def continuation2 = concurrent ? scope2.captureConcurrent() : scope2.capture() + def continuation2 = scope2.capture() then: continuation2 != null @@ -859,9 +822,6 @@ class ScopeManagerTest extends DDCoreSpecification { spanFinished(span2) scopeManager.scopeStack().depth() == 0 writer == [[span], [span2]] - - where: - concurrent << [false, true] } @Shared @@ -870,7 +830,7 @@ class ScopeManagerTest extends DDCoreSpecification { @Shared AtomicInteger iteration = new AtomicInteger(0) - def "concurrent continuation can be activated and closed in multiple threads"() { + def "continuation can be activated and closed in multiple threads"() { setup: long sendDelayNanos = TimeUnit.MILLISECONDS.toNanos(500 - 100) @@ -879,10 +839,12 @@ class ScopeManagerTest extends DDCoreSpecification { def start = System.nanoTime() def scope = (ContinuableScope) tracer.activateSpan(span) tracer.setAsyncPropagationEnabled(true) - continuation = scope.captureConcurrent() + continuation = scope.capture() scope.close() span.finish() + continuation.hold() + then: ThreadUtils.runConcurrently(8, 512) { int iter = iteration.incrementAndGet() 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 3ac3eb017c7..8338ec947e4 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/OTScopeManager.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/OTScopeManager.java @@ -96,11 +96,6 @@ public Continuation capture() { return delegate.capture(); } - @Override - public Continuation captureConcurrent() { - return delegate.captureConcurrent(); - } - boolean isFinishSpanOnClose() { return finishSpanOnClose; } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentScope.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentScope.java index 396acd80140..7d2ac8aa848 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentScope.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentScope.java @@ -11,14 +11,14 @@ public interface AgentScope extends TraceScope, Closeable { @Override Continuation capture(); - @Override - Continuation captureConcurrent(); - @Override void close(); interface Continuation extends TraceScope.Continuation { + @Override + Continuation hold(); + @Override AgentScope activate(); 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 77876e922ab..0335aa05ab1 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 @@ -997,11 +997,6 @@ public AgentScope.Continuation capture() { return NoopContinuation.INSTANCE; } - @Override - public AgentScope.Continuation captureConcurrent() { - return NoopContinuation.INSTANCE; - } - @Override public void close() {} @@ -1052,17 +1047,22 @@ static class NoopContinuation implements AgentScope.Continuation { static final NoopContinuation INSTANCE = new NoopContinuation(); @Override - public AgentScope activate() { - return NoopAgentScope.INSTANCE; + public AgentScope.Continuation hold() { + return this; } @Override - public void cancel() {} + public AgentScope activate() { + return NoopAgentScope.INSTANCE; + } @Override public AgentSpan getSpan() { return NoopAgentSpan.INSTANCE; } + + @Override + public void cancel() {} } public static final class BlackholeContext extends NoopContext {