Skip to content

Commit 9ffbc3c

Browse files
committed
Combine continuation implementations into one which supports multiple activations.
Call the `hold()` method on the continuation to let multiple threads activate it concurrently and close their scopes without fear of prematurely closing the span. You must then call `cancel()` at some point to avoid discarding traces.
1 parent b74cc24 commit 9ffbc3c

File tree

21 files changed

+174
-327
lines changed

21 files changed

+174
-327
lines changed

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ConcurrentState.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,23 +69,23 @@ public static <K> void closeScope(
6969
if (throwable != null) {
7070
// This might lead to the continuation being consumed early, but it's better to be safe if we
7171
// threw an Exception on entry
72-
state.closeContinuation();
72+
state.cancelContinuation();
7373
}
7474
}
7575

76-
public static <K> void closeAndClearContinuation(
76+
public static <K> void cancelAndClearContinuation(
7777
ContextStore<K, ConcurrentState> contextStore, K key) {
7878
final ConcurrentState state = contextStore.get(key);
7979
if (state == null) {
8080
return;
8181
}
82-
state.closeAndClearContinuation();
82+
state.cancelAndClearContinuation();
8383
}
8484

8585
private boolean captureAndSetContinuation(final AgentScope scope) {
8686
if (CONTINUATION.compareAndSet(this, null, CLAIMED)) {
8787
// lazy write is guaranteed to be seen by getAndSet
88-
CONTINUATION.lazySet(this, scope.captureConcurrent());
88+
CONTINUATION.lazySet(this, scope.capture().hold());
8989
return true;
9090
}
9191
return false;
@@ -99,14 +99,14 @@ private AgentScope activateAndContinueContinuation() {
9999
return null;
100100
}
101101

102-
private void closeContinuation() {
102+
private void cancelContinuation() {
103103
final AgentScope.Continuation continuation = CONTINUATION.get(this);
104104
if (continuation != null && continuation != CLAIMED) {
105105
continuation.cancel();
106106
}
107107
}
108108

109-
private void closeAndClearContinuation() {
109+
private void cancelAndClearContinuation() {
110110
final AgentScope.Continuation continuation = CONTINUATION.get(this);
111111
if (continuation != null && continuation != CLAIMED) {
112112
// We should never be able to reuse this state

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ContinuationClaim.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ final class ContinuationClaim implements AgentScope.Continuation {
77

88
public static final ContinuationClaim CLAIMED = new ContinuationClaim();
99

10+
@Override
11+
public AgentScope.Continuation hold() {
12+
throw new IllegalStateException();
13+
}
14+
1015
@Override
1116
public AgentScope activate() {
1217
throw new IllegalStateException();

dd-java-agent/instrumentation/java-concurrent/java-completablefuture/src/main/java/java/util/concurrent/CompletableFutureAdvice.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public static void exit(
7272
boolean claimed = !wasClaimed && !hadExecutor && zis.getForkJoinTaskTag() == 1;
7373
if (mode == ASYNC || (mode < ASYNC && claimed) || !zis.isLive()) {
7474
contextStore = InstrumentationContext.get(UniCompletion.class, ConcurrentState.class);
75-
ConcurrentState.closeAndClearContinuation(contextStore, zis);
75+
ConcurrentState.cancelAndClearContinuation(contextStore, zis);
7676
}
7777
if (scope != null || throwable != null) {
7878
if (contextStore == null) {

dd-java-agent/instrumentation/kotlin-coroutines/src/main/java/datadog/trace/instrumentation/kotlin/coroutines/ScopeStateCoroutineContext.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ public void maybeInitialize() {
121121
if (!isInitialized) {
122122
final AgentScope activeScope = AgentTracer.get().activeScope();
123123
if (activeScope != null && activeScope.isAsyncPropagating()) {
124-
continuation = activeScope.captureConcurrent();
124+
continuation = activeScope.capture().hold();
125125
}
126126
isInitialized = true;
127127
}

dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/main/java/datadog/trace/instrumentation/opentelemetry/OtelScope.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,6 @@ public Continuation capture() {
1616
return delegate.capture();
1717
}
1818

19-
@Override
20-
public Continuation captureConcurrent() {
21-
return delegate.captureConcurrent();
22-
}
23-
2419
@Override
2520
public void close() {
2621
delegate.close();

dd-java-agent/instrumentation/opentracing/api-0.31/src/main/java/datadog/trace/instrumentation/opentracing31/OTScopeManager.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,6 @@ public Continuation capture() {
6868
return delegate.capture();
6969
}
7070

71-
@Override
72-
public Continuation captureConcurrent() {
73-
return delegate.captureConcurrent();
74-
}
75-
7671
public boolean isFinishSpanOnClose() {
7772
return finishSpanOnClose;
7873
}

dd-java-agent/instrumentation/opentracing/api-0.32/src/main/java/datadog/trace/instrumentation/opentracing32/OTScopeManager.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,6 @@ public Continuation capture() {
7878
return delegate.capture();
7979
}
8080

81-
@Override
82-
public Continuation captureConcurrent() {
83-
return delegate.captureConcurrent();
84-
}
85-
8681
public boolean isFinishSpanOnClose() {
8782
return finishSpanOnClose;
8883
}

dd-trace-api/src/main/java/datadog/trace/context/NoopTraceScope.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ public static class NoopContinuation implements Continuation {
88

99
private NoopContinuation() {}
1010

11+
@Override
12+
public Continuation hold() {
13+
return this;
14+
}
15+
1116
@Override
1217
public TraceScope activate() {
1318
return NoopTraceScope.INSTANCE;
@@ -24,11 +29,6 @@ public Continuation capture() {
2429
return NoopContinuation.INSTANCE;
2530
}
2631

27-
@Override
28-
public Continuation captureConcurrent() {
29-
return null;
30-
}
31-
3232
@Override
3333
public void close() {}
3434
}

dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,11 @@ public interface TraceScope extends Closeable {
1414
*/
1515
Continuation capture();
1616

17-
/**
18-
* Prevent the trace attached to this TraceScope from reporting until the returned Continuation is
19-
* either activated (and the returned scope is closed), or canceled.
20-
*
21-
* <p>Should be called on the parent thread.
22-
*
23-
* <p>If the returned {@link Continuation} is activated, it needs to be canceled in addition to
24-
* the returned {@link TraceScope} being closed. This is to allow multiple concurrent threads that
25-
* activate the continuation to race in a safe way, and close the scopes without fear of closing
26-
* the related {@code Span} prematurely.
27-
*/
28-
Continuation captureConcurrent();
17+
/** @deprecated Replaced by {@code capture().hold()}. */
18+
@Deprecated
19+
default Continuation captureConcurrent() {
20+
return capture().hold();
21+
}
2922

3023
/** Close the activated context and allow any underlying spans to finish. */
3124
@Override
@@ -61,6 +54,15 @@ default void setAsyncPropagation(boolean value) {
6154
*/
6255
interface Continuation {
6356

57+
/**
58+
* Prevent the trace attached to this scope from reporting until the continuation is explicitly
59+
* cancelled. You must call {@link #cancel()} at some point to avoid discarding traces.
60+
*
61+
* <p>Use this when you want to let multiple threads activate the continuation concurrently and
62+
* close their scopes without fear of prematurely closing the related span.
63+
*/
64+
Continuation hold();
65+
6466
/**
6567
* Activate the continuation.
6668
*

dd-trace-core/src/main/java/datadog/trace/core/scopemanager/AbstractContinuation.java

Lines changed: 0 additions & 34 deletions
This file was deleted.

dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ConcurrentContinuation.java

Lines changed: 0 additions & 95 deletions
This file was deleted.

dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -135,21 +135,9 @@ public final void setAsyncPropagation(final boolean value) {
135135
* @return The new continuation, or null if this scope is not async propagating.
136136
*/
137137
@Override
138-
public final AbstractContinuation capture() {
138+
public final ScopeContinuation capture() {
139139
return isAsyncPropagating
140-
? new SingleContinuation(scopeManager, span, source()).register()
141-
: null;
142-
}
143-
144-
/**
145-
* The continuation returned must be closed or activated or the trace will not finish.
146-
*
147-
* @return The new continuation, or null if this scope is not async propagating.
148-
*/
149-
@Override
150-
public final AbstractContinuation captureConcurrent() {
151-
return isAsyncPropagating
152-
? new ConcurrentContinuation(scopeManager, span, source()).register()
140+
? new ScopeContinuation(scopeManager, span, source()).register()
153141
: null;
154142
}
155143

dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScopeManager.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ public AgentScope activate(
9191
}
9292

9393
public AgentScope.Continuation captureSpan(final AgentSpan span) {
94-
AbstractContinuation continuation =
95-
new SingleContinuation(this, span, ScopeSource.INSTRUMENTATION.id());
94+
ScopeContinuation continuation =
95+
new ScopeContinuation(this, span, ScopeSource.INSTRUMENTATION.id());
9696
continuation.register();
9797
healthMetrics.onCaptureContinuation();
9898
return continuation;
@@ -136,12 +136,12 @@ private AgentScope activate(
136136
}
137137

138138
/**
139-
* Activates a scope for the given {@link AbstractContinuation}.
139+
* Activates a scope for the given {@link ScopeContinuation}.
140140
*
141141
* @param continuation {@code null} if a continuation is re-used
142142
*/
143143
ContinuableScope continueSpan(
144-
final AbstractContinuation continuation, final AgentSpan span, final byte source) {
144+
final ScopeContinuation continuation, final AgentSpan span, final byte source) {
145145
ScopeStack scopeStack = scopeStack();
146146

147147
// optimization: if the top scope is already keeping the same span alive

dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuingScope.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@
55

66
final class ContinuingScope extends ContinuableScope {
77
/** Continuation that created this scope. */
8-
private final AbstractContinuation continuation;
8+
private final ScopeContinuation continuation;
99

1010
ContinuingScope(
1111
final ContinuableScopeManager scopeManager,
1212
final AgentSpan span,
1313
final byte source,
1414
final boolean isAsyncPropagating,
15-
final AbstractContinuation continuation,
15+
final ScopeContinuation continuation,
1616
final Stateful scopeState) {
1717
super(scopeManager, span, source, isAsyncPropagating, scopeState);
1818
this.continuation = continuation;
@@ -21,7 +21,6 @@ final class ContinuingScope extends ContinuableScope {
2121
@Override
2222
void cleanup(final ScopeStack scopeStack) {
2323
super.cleanup(scopeStack);
24-
2524
continuation.cancelFromContinuedScopeClose();
2625
}
2726
}

0 commit comments

Comments
 (0)