Skip to content

Commit aa17186

Browse files
committed
Restore parent scope stack when completing ScopeCoroutines
1 parent 0a1776a commit aa17186

File tree

5 files changed

+32
-10
lines changed

5 files changed

+32
-10
lines changed

dd-java-agent/instrumentation/kotlin-coroutines/coroutines-1.5/src/test/groovy/KotlinCoroutineInstrumentationTest.groovy

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import datadog.trace.core.DDSpan
22
import datadog.trace.instrumentation.kotlin.coroutines.AbstractKotlinCoroutineInstrumentationTest
33
import kotlinx.coroutines.CoroutineDispatcher
4-
import spock.lang.Ignore
54

65
class KotlinCoroutineInstrumentationTest extends AbstractKotlinCoroutineInstrumentationTest<KotlinCoroutineTests> {
76

@@ -44,7 +43,6 @@ class KotlinCoroutineInstrumentationTest extends AbstractKotlinCoroutineInstrume
4443
[dispatcherName, dispatcher] << dispatchersToTest
4544
}
4645

47-
@Ignore("Not working: disconnected trace")
4846
def "kotlin trace consistent after flow"() {
4947
setup:
5048
KotlinCoroutineTests kotlinTest = new KotlinCoroutineTests(dispatcher)

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.util.Collections;
1616
import java.util.Map;
1717
import kotlinx.coroutines.AbstractCoroutine;
18+
import kotlinx.coroutines.Job;
1819
import net.bytebuddy.asm.Advice;
1920
import net.bytebuddy.description.type.TypeDescription;
2021
import net.bytebuddy.matcher.ElementMatcher;
@@ -112,8 +113,18 @@ public static void onStartInvocation(@Advice.This final AbstractCoroutine<?> cor
112113
public static class JobSupportAfterCompletionInternalAdvice {
113114
@Advice.OnMethodEnter
114115
public static void onCompletionInternal(@Advice.This final AbstractCoroutine<?> coroutine) {
115-
// close the scope if needed
116-
closeScopeStateContext(coroutine);
116+
Job parent = null;
117+
try {
118+
// extract parent when completing scope coroutines so we can restore their scope stack
119+
if (coroutine instanceof kotlinx.coroutines.internal.ScopeCoroutine) {
120+
parent =
121+
((kotlinx.coroutines.internal.ScopeCoroutine) coroutine)
122+
.getParent$kotlinx_coroutines_core();
123+
}
124+
} catch (Throwable ignore) {
125+
// fallback to restoring the current active scope stack after completing the coroutine
126+
}
127+
closeScopeStateContext(coroutine, parent);
117128
}
118129
}
119130
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,12 @@ public static void initializeScopeStateContext(final AbstractCoroutine<?> corout
3636
}
3737
}
3838

39-
public static void closeScopeStateContext(final AbstractCoroutine<?> coroutine) {
39+
public static void closeScopeStateContext(
40+
final AbstractCoroutine<?> coroutine, final Job parent) {
4041
final ScopeStateCoroutineContext scopeStackContext =
4142
getScopeStateContext(coroutine.getContext());
4243
if (scopeStackContext != null) {
43-
scopeStackContext.maybeCloseScopeAndCancelContinuation(coroutine);
44+
scopeStackContext.maybeCloseScopeAndCancelContinuation(coroutine, parent);
4445
}
4546
}
4647
}

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

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,20 @@ public ScopeState updateThreadContext(@NotNull final CoroutineContext coroutineC
5252
}
5353

5454
/** If there's a context item for the coroutine then try to close it */
55-
public void maybeCloseScopeAndCancelContinuation(final Job coroutine) {
55+
public void maybeCloseScopeAndCancelContinuation(final Job coroutine, final Job parent) {
5656
final ScopeStateCoroutineContextItem contextItem = contextItemPerCoroutine.get(coroutine);
5757
if (contextItem != null) {
58-
final ScopeState currentThreadScopeState = AgentTracer.get().newScopeState();
59-
currentThreadScopeState.fetchFromActive();
58+
ScopeState currentThreadScopeState = null;
59+
if (parent != null) {
60+
final ScopeStateCoroutineContextItem parentItem = contextItemPerCoroutine.get(parent);
61+
if (parentItem != null) {
62+
currentThreadScopeState = parentItem.getScopeState();
63+
}
64+
}
65+
if (currentThreadScopeState == null) {
66+
currentThreadScopeState = AgentTracer.get().newScopeState();
67+
currentThreadScopeState.fetchFromActive();
68+
}
6069

6170
contextItem.maybeCloseScopeAndCancelContinuation();
6271
contextItemPerCoroutine.remove(coroutine);
@@ -107,6 +116,10 @@ public ScopeStateCoroutineContextItem() {
107116
coroutineScopeState = AgentTracer.get().newScopeState();
108117
}
109118

119+
public ScopeState getScopeState() {
120+
return coroutineScopeState;
121+
}
122+
110123
public void activate() {
111124
coroutineScopeState.activate();
112125

dd-java-agent/instrumentation/kotlin-coroutines/src/testFixtures/groovy/datadog/trace/instrumentation/kotlin/coroutines/AbstractKotlinCoroutineInstrumentationTest.groovy

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,6 @@ abstract class AbstractKotlinCoroutineInstrumentationTest<T extends CoreKotlinCo
406406
[dispatcherName, dispatcher] << dispatchersToTest
407407
}
408408

409-
@Ignore("Not working: disconnected trace")
410409
def "kotlin trace consistent after delay"() {
411410
setup:
412411
CoreKotlinCoroutineTests kotlinTest = getCoreKotlinCoroutineTestsInstance(dispatcher)

0 commit comments

Comments
 (0)