Implement @WithSpan support for kotlin coroutines #8870
Conversation
.../java/io/opentelemetry/javaagent/tooling/ignore/AdditionalLibraryIgnoredTypesConfigurer.java
Outdated
Show resolved
Hide resolved
...nt/instrumentation/kotlinxcoroutines/instrumentationannotations/WithSpanInstrumentation.java
Show resolved
Hide resolved
...nt/instrumentation/kotlinxcoroutines/instrumentationannotations/WithSpanInstrumentation.java
Outdated
Show resolved
Hide resolved
...nt/instrumentation/kotlinxcoroutines/instrumentationannotations/WithSpanInstrumentation.java
Outdated
Show resolved
Hide resolved
...nt/instrumentation/kotlinxcoroutines/instrumentationannotations/WithSpanInstrumentation.java
Outdated
Show resolved
Hide resolved
...nt/instrumentation/kotlinxcoroutines/instrumentationannotations/WithSpanInstrumentation.java
Outdated
Show resolved
Hide resolved
...nt/instrumentation/kotlinxcoroutines/instrumentationannotations/WithSpanInstrumentation.java
Outdated
Show resolved
Hide resolved
...nt/instrumentation/kotlinxcoroutines/instrumentationannotations/WithSpanInstrumentation.java
Outdated
Show resolved
Hide resolved
|
|
||
| public static Context enterCoroutine( | ||
| int label, Continuation<?> continuation, MethodRequest request) { | ||
| // label 0 means that coroutine is started, any other label means that coroutine is resumed |
There was a problem hiding this comment.
Do you know of any documentation/kotlin design doc that'd explain the label value? I couldn't find any, unfortunately.
There was a problem hiding this comment.
https://www.youtube.com/watch?v=YrrUCSi72E8 was useful if I remember correctly (around 7:30 there is something about labels). Basically it should work so that when coroutine is suspended it returns the label where it should be resumed and on resume it is called with the same label.
There was a problem hiding this comment.
Thanks! I'll watch that
...nt/instrumentation/kotlinxcoroutines/instrumentationannotations/WithSpanInstrumentation.java
Show resolved
Hide resolved
...telemetry/javaagent/instrumentation/kotlinxcoroutines/KotlinCoroutinesInstrumentationTest.kt
Show resolved
Hide resolved
|
This PR will solve the wrong span duration reported at this issue #6502? |
| @@ -10,25 +10,35 @@ muzzle { | |||
| group.set("org.jetbrains.kotlinx") | |||
| module.set("kotlinx-coroutines-core") | |||
| versions.set("[1.0.0,1.3.8)") | |||
| extraDependency(project(":instrumentation-annotations")) | |||
| extraDependency("io.opentelemetry:opentelemetry-api:1.27.0") | |||
There was a problem hiding this comment.
can we also keep the original muzzle definition and use excludeInstrumentationName, to make sure the kotlin instrumentation is still applied in the absence of these dependencies on the user classpath?
There was a problem hiding this comment.
or possibly extract out into separate module?
.../opentelemetry/javaagent/instrumentation/instrumentationannotations/KotlinCoroutineUtil.java
Show resolved
Hide resolved
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
Resolves #8812
With kotlin coroutines current
@WithSpanhandling that starts span on method entry and ends it when method exits doesn't work well because method will be exited when it calls a blocking operation and later called again to resume the coroutine. This creates multiple spans for the same method and messes up context propagation. This pr explores an alternative@WithSpanimplementation for coroutines where span is started when method is first called and ended when it returns a result, calls to resume the continuation will only restore the scope and not start a new span.There is also support for the
@SpanAttributeannotation, but it supports less types than what is supported for the regular@WithSpanhandling. Support for@AddingSpanAttributesannotation has not been implemented.