Skip to content

Commit 2de8a73

Browse files
amarzialiamarziali
authored
Merge branch 'master' into andrea.marziali/spring-completablefuture
2 parents 3402955 + 32046a3 commit 2de8a73

File tree

17 files changed

+88
-176
lines changed

17 files changed

+88
-176
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/coroutines-1.5/src/test/kotlin/KotlinCoroutineTests.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ class KotlinCoroutineTests(dispatcher: CoroutineDispatcher) : CoreKotlinCoroutin
4646
emit(1)
4747
}.flowOn(Dispatchers.IO)
4848
val ff = f.single()
49-
// FIXME: This span is detached
49+
5050
childSpan("outside-flow").activateAndUse {
5151
println("hello $ff")
5252
}

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

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,24 @@
11
package datadog.trace.instrumentation.kotlin.coroutines;
22

3+
import java.lang.invoke.MethodHandle;
4+
import java.lang.invoke.MethodHandles;
5+
import java.lang.invoke.MethodType;
36
import kotlin.coroutines.CoroutineContext;
47
import kotlinx.coroutines.AbstractCoroutine;
8+
import kotlinx.coroutines.ChildHandle;
59
import kotlinx.coroutines.Job;
10+
import kotlinx.coroutines.JobNode;
11+
import kotlinx.coroutines.JobSupport;
612
import org.jetbrains.annotations.Nullable;
13+
import org.slf4j.Logger;
14+
import org.slf4j.LoggerFactory;
715

816
public class CoroutineContextHelper {
17+
private static final Logger log = LoggerFactory.getLogger(CoroutineContextHelper.class);
18+
919
/*
1020
IntelliJ shows a warning here for Job being out of bounds, but that's not true, the class compiles.
1121
*/
12-
1322
@Nullable
1423
@SuppressWarnings("unchecked")
1524
public static Job getJob(final CoroutineContext context) {
@@ -40,7 +49,57 @@ public static void closeScopeStateContext(final AbstractCoroutine<?> coroutine)
4049
final ScopeStateCoroutineContext scopeStackContext =
4150
getScopeStateContext(coroutine.getContext());
4251
if (scopeStackContext != null) {
43-
scopeStackContext.maybeCloseScopeAndCancelContinuation(coroutine);
52+
scopeStackContext.maybeCloseScopeAndCancelContinuation(coroutine, getParentJob(coroutine));
53+
}
54+
}
55+
56+
private static final MethodHandle PARENT_HANDLE_METHOD;
57+
private static final MethodHandle PARENT_HANDLE_FIELD;
58+
private static final MethodHandle JOB_FIELD;
59+
60+
static {
61+
MethodHandle parentHandleMethod = null;
62+
MethodHandle parentHandleField = null;
63+
MethodHandle jobField = null;
64+
65+
MethodHandles.Lookup lookup = MethodHandles.publicLookup();
66+
try {
67+
// Kotlin coroutines 1.5+
68+
parentHandleMethod =
69+
lookup.findVirtual(
70+
JobSupport.class,
71+
"getParentHandle$kotlinx_coroutines_core",
72+
MethodType.methodType(ChildHandle.class));
73+
jobField = lookup.findGetter(JobNode.class, "job", JobSupport.class);
74+
} catch (Throwable ignore) {
75+
try {
76+
// Kotlin coroutines 1.3
77+
parentHandleField = lookup.findGetter(JobSupport.class, "parentHandle", ChildHandle.class);
78+
jobField = lookup.findGetter(JobNode.class, "job", Job.class);
79+
} catch (Throwable e) {
80+
log.debug("Unable to access parent handle", e);
81+
}
82+
}
83+
84+
PARENT_HANDLE_METHOD = parentHandleMethod;
85+
PARENT_HANDLE_FIELD = parentHandleField;
86+
JOB_FIELD = jobField;
87+
}
88+
89+
private static Job getParentJob(JobSupport coroutine) {
90+
try {
91+
Object parentHandle = null;
92+
if (null != PARENT_HANDLE_METHOD) {
93+
parentHandle = PARENT_HANDLE_METHOD.invoke(coroutine);
94+
} else if (null != PARENT_HANDLE_FIELD) {
95+
parentHandle = PARENT_HANDLE_FIELD.invoke(coroutine);
96+
}
97+
if (parentHandle instanceof JobNode) {
98+
return (Job) JOB_FIELD.invoke((JobNode) parentHandle);
99+
}
100+
} catch (Throwable e) {
101+
log.debug("Unable to extract parent job", e);
44102
}
103+
return null;
45104
}
46105
}

‎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: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import datadog.trace.core.DDSpan
55
import kotlinx.coroutines.CoroutineDispatcher
66
import kotlinx.coroutines.Dispatchers
77
import kotlinx.coroutines.ThreadPoolDispatcherKt
8-
import spock.lang.Ignore
98
import spock.lang.Shared
109

1110
abstract class AbstractKotlinCoroutineInstrumentationTest<T extends CoreKotlinCoroutineTests> extends AgentTestRunner {
@@ -366,7 +365,6 @@ abstract class AbstractKotlinCoroutineInstrumentationTest<T extends CoreKotlinCo
366365
[dispatcherName, dispatcher] << dispatchersToTest
367366
}
368367

369-
@Ignore("Not working: disconnected trace")
370368
def "kotlin trace consistent with timeout"() {
371369
setup:
372370
CoreKotlinCoroutineTests kotlinTest = getCoreKotlinCoroutineTestsInstance(dispatcher)
@@ -389,7 +387,7 @@ abstract class AbstractKotlinCoroutineInstrumentationTest<T extends CoreKotlinCo
389387
}
390388
span(2) {
391389
operationName "3-after-timeout"
392-
childOf span(5)
390+
childOf span(1)
393391
}
394392
span(3) {
395393
operationName "4-after-timeout-2"
@@ -406,7 +404,6 @@ abstract class AbstractKotlinCoroutineInstrumentationTest<T extends CoreKotlinCo
406404
[dispatcherName, dispatcher] << dispatchersToTest
407405
}
408406

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

‎dd-java-agent/instrumentation/kotlin-coroutines/src/testFixtures/kotlin/datadog/trace/instrumentation/kotlin/coroutines/CoreKotlinCoroutineTests.kt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,6 @@ abstract class CoreKotlinCoroutineTests(private val dispatcher: CoroutineDispatc
320320
delay(10)
321321
}
322322
}
323-
// FIXME: This span is detached
324323
childSpan("3-after-timeout").activateAndUse {
325324
delay(10)
326325
}
@@ -351,7 +350,6 @@ abstract class CoreKotlinCoroutineTests(private val dispatcher: CoroutineDispatc
351350
}
352351
}
353352

354-
// FIXME: This span is detached
355353
tracedChild("after-process")
356354

357355
6

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,16 +55,6 @@ public Scope toScope(final AgentScope scope) {
5555
if (scope == null) {
5656
return null;
5757
}
58-
if (scope instanceof AttachableWrapper) {
59-
AttachableWrapper attachableScopeWrapper = (AttachableWrapper) scope;
60-
Object wrapper = attachableScopeWrapper.getWrapper();
61-
if (wrapper instanceof Scope) {
62-
return (Scope) wrapper;
63-
}
64-
Scope otScope = new OtelScope(scope);
65-
attachableScopeWrapper.attachWrapper(otScope);
66-
return otScope;
67-
}
6858
if (scope == noopScope()) {
6959
return noopScopeWrapper;
7060
}

‎dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/TypeConverterTest.groovy‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,17 @@
11
import datadog.trace.agent.test.AgentTestRunner
22
import datadog.trace.api.DDSpanId
33
import datadog.trace.api.DDTraceId
4-
import datadog.trace.api.sampling.PrioritySampling
54
import datadog.trace.api.datastreams.NoopPathwayContext
5+
import datadog.trace.api.sampling.PrioritySampling
66
import datadog.trace.core.DDSpan
77
import datadog.trace.core.DDSpanContext
88
import datadog.trace.core.PendingTrace
99
import datadog.trace.core.propagation.PropagationTags
10-
import datadog.trace.core.scopemanager.ContinuableScopeManager
1110
import datadog.trace.instrumentation.opentelemetry.TypeConverter
1211

1312
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
14-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1513
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
14+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1615

1716
class TypeConverterTest extends AgentTestRunner {
1817
TypeConverter typeConverter = new TypeConverter()
@@ -46,20 +45,6 @@ class TypeConverterTest extends AgentTestRunner {
4645
typeConverter.toScope(noopScope) is typeConverter.toScope(noopScope)
4746
}
4847

49-
def "should avoid extra allocation for a scope wrapper"() {
50-
def scopeManager = new ContinuableScopeManager(0, false)
51-
def context = createTestSpanContext()
52-
def span1 = new DDSpan("test", 0, context, null)
53-
def span2 = new DDSpan("test", 0, context, null)
54-
def scope1 = scopeManager.activateManualSpan(span1)
55-
def scope2 = scopeManager.activateManualSpan(span2)
56-
expect:
57-
// return the same wrapper for the same scope
58-
typeConverter.toScope(scope1) is typeConverter.toScope(scope1)
59-
// return distinct wrapper for another context
60-
!typeConverter.toScope(scope1).is(typeConverter.toScope(scope2))
61-
}
62-
6348
def createTestSpanContext() {
6449
def trace = Stub(PendingTrace)
6550
return new DDSpanContext(

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

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,19 +58,6 @@ public Scope toScope(final AgentScope scope, final boolean finishSpanOnClose) {
5858
if (scope == null) {
5959
return null;
6060
}
61-
if (scope instanceof AttachableWrapper) {
62-
AttachableWrapper attachableScopeWrapper = (AttachableWrapper) scope;
63-
Object wrapper = attachableScopeWrapper.getWrapper();
64-
if (wrapper instanceof OTScopeManager.OTScope) {
65-
OTScopeManager.OTScope attachedScopeWrapper = (OTScopeManager.OTScope) wrapper;
66-
if (attachedScopeWrapper.isFinishSpanOnClose() == finishSpanOnClose) {
67-
return (Scope) wrapper;
68-
}
69-
}
70-
Scope otScope = new OTScopeManager.OTScope(scope, finishSpanOnClose, this);
71-
attachableScopeWrapper.attachWrapper(otScope);
72-
return otScope;
73-
}
7461
if (scope == noopScope()) {
7562
return noopScopeWrapper;
7663
}

‎dd-java-agent/instrumentation/opentracing/api-0.31/src/test/groovy/TypeConverterTest.groovy‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,18 @@
11
import datadog.trace.agent.test.AgentTestRunner
22
import datadog.trace.api.DDSpanId
33
import datadog.trace.api.DDTraceId
4-
import datadog.trace.api.sampling.PrioritySampling
54
import datadog.trace.api.datastreams.NoopPathwayContext
5+
import datadog.trace.api.sampling.PrioritySampling
66
import datadog.trace.core.DDSpan
77
import datadog.trace.core.DDSpanContext
88
import datadog.trace.core.PendingTrace
99
import datadog.trace.core.propagation.PropagationTags
10-
import datadog.trace.core.scopemanager.ContinuableScopeManager
1110
import datadog.trace.instrumentation.opentracing.DefaultLogHandler
1211
import datadog.trace.instrumentation.opentracing31.TypeConverter
1312

1413
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope
15-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1614
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
15+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpanContext
1716

1817
class TypeConverterTest extends AgentTestRunner {
1918
TypeConverter typeConverter = new TypeConverter(new DefaultLogHandler())
@@ -51,23 +50,6 @@ class TypeConverterTest extends AgentTestRunner {
5150
typeConverter.toScope(noopScope, false) is typeConverter.toScope(noopScope, true)
5251
}
5352

54-
def "should avoid extra allocation for a scope wrapper"() {
55-
def scopeManager = new ContinuableScopeManager(0, false)
56-
def context = createTestSpanContext()
57-
def span1 = new DDSpan("test", 0, context, null)
58-
def span2 = new DDSpan("test", 0, context, null)
59-
def scope1 = scopeManager.activateManualSpan(span1)
60-
def scope2 = scopeManager.activateManualSpan(span2)
61-
expect:
62-
// return the same wrapper for the same scope
63-
typeConverter.toScope(scope1, true) is typeConverter.toScope(scope1, true)
64-
typeConverter.toScope(scope1, false) is typeConverter.toScope(scope1, false)
65-
!typeConverter.toScope(scope1, true).is(typeConverter.toScope(scope1, false))
66-
!typeConverter.toScope(scope1, false).is(typeConverter.toScope(scope1, true))
67-
// return distinct wrapper for another context
68-
!typeConverter.toScope(scope1, true).is(typeConverter.toScope(scope2, true))
69-
}
70-
7153
def createTestSpanContext() {
7254
def trace = Stub(PendingTrace)
7355
return new DDSpanContext(

0 commit comments

Comments
 (0)