Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .brazil.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"com.squareup.okhttp3:okhttp:5.*": "OkHttp3-5.x",
"com.squareup.okio:okio-jvm:3.*": "OkioJvm-3.x",
"io.opentelemetry:opentelemetry-api:1.*": "Maven-io-opentelemetry_opentelemetry-api-1.x",
"io.opentelemetry:opentelemetry-extension-kotlin:1.*": "Maven-io-opentelemetry_opentelemetry-extension-kotlin-1.x",
"org.slf4j:slf4j-api:2.*": "Maven-org-slf4j_slf4j-api-2.x",
"aws.sdk.kotlin.crt:aws-crt-kotlin:0.9.*": "AwsCrtKotlin-0.9.x",
"aws.sdk.kotlin.crt:aws-crt-kotlin:0.8.*": "AwsCrtKotlin-0.8.x",
Expand Down
8 changes: 8 additions & 0 deletions .changes/ec884c41-7bda-4033-a80b-5da20836a64b.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "ec884c41-7bda-4033-a80b-5da20836a64b",
"type": "bugfix",
"description": "Fix OpenTelemetry span concurrency by using Span.asContextElement() instead of Span.makeCurrent()",
"issues": [
"https://github.com/smithy-lang/smithy-kotlin/issues/1211"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We could mention the issue in the pull request description as well

]
}
1 change: 1 addition & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ okhttp4 = { module = "com.squareup.okhttp3:okhttp", version.ref = "okhttp4-versi
okhttp-coroutines = { module = "com.squareup.okhttp3:okhttp-coroutines", version.ref = "okhttp-version" }
opentelemetry-api = { module = "io.opentelemetry:opentelemetry-api", version.ref = "otel-version" }
opentelemetry-sdk-testing = {module = "io.opentelemetry:opentelemetry-sdk-testing", version.ref = "otel-version" }
opentelemetry-kotlin-extension = { module = "io.opentelemetry:opentelemetry-extension-kotlin", version.ref = "otel-version" }
slf4j-api = { module = "org.slf4j:slf4j-api", version.ref = "slf4j-version" }
slf4j-api-v1x = { module = "org.slf4j:slf4j-api", version.ref = "slf4j-v1x-version" }
slf4j-simple = { module = "org.slf4j:slf4j-simple", version.ref = "slf4j-version" }
Expand Down
3 changes: 3 additions & 0 deletions runtime/observability/telemetry-api/api/telemetry-api.api
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ public final class aws/smithy/kotlin/runtime/telemetry/metrics/UpDownCounter$Def

public abstract class aws/smithy/kotlin/runtime/telemetry/trace/AbstractTraceSpan : aws/smithy/kotlin/runtime/telemetry/trace/TraceSpan {
public fun <init> ()V
public fun asContextElement ()Lkotlin/coroutines/CoroutineContext;
public fun close ()V
public fun emitEvent (Ljava/lang/String;Laws/smithy/kotlin/runtime/collections/Attributes;)V
public fun getSpanContext ()Laws/smithy/kotlin/runtime/telemetry/trace/SpanContext;
Expand Down Expand Up @@ -424,6 +425,7 @@ public final class aws/smithy/kotlin/runtime/telemetry/trace/SpanStatus : java/l

public abstract interface class aws/smithy/kotlin/runtime/telemetry/trace/TraceSpan : aws/smithy/kotlin/runtime/telemetry/context/Scope {
public static final field Companion Laws/smithy/kotlin/runtime/telemetry/trace/TraceSpan$Companion;
public abstract fun asContextElement ()Lkotlin/coroutines/CoroutineContext;
public abstract fun close ()V
public abstract fun emitEvent (Ljava/lang/String;Laws/smithy/kotlin/runtime/collections/Attributes;)V
public abstract fun getSpanContext ()Laws/smithy/kotlin/runtime/telemetry/trace/SpanContext;
Expand All @@ -437,6 +439,7 @@ public final class aws/smithy/kotlin/runtime/telemetry/trace/TraceSpan$Companion
}

public final class aws/smithy/kotlin/runtime/telemetry/trace/TraceSpan$DefaultImpls {
public static fun asContextElement (Laws/smithy/kotlin/runtime/telemetry/trace/TraceSpan;)Lkotlin/coroutines/CoroutineContext;
public static synthetic fun emitEvent$default (Laws/smithy/kotlin/runtime/telemetry/trace/TraceSpan;Ljava/lang/String;Laws/smithy/kotlin/runtime/collections/Attributes;ILjava/lang/Object;)V
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ package aws.smithy.kotlin.runtime.telemetry.trace

import aws.smithy.kotlin.runtime.collections.AttributeKey
import aws.smithy.kotlin.runtime.collections.Attributes
import kotlin.coroutines.CoroutineContext
import kotlin.coroutines.EmptyCoroutineContext

/**
* An abstract implementation of a trace span. By default, this class uses no-op implementations for all members unless
Expand All @@ -18,4 +20,5 @@ public abstract class AbstractTraceSpan : TraceSpan {
override operator fun <T : Any> set(key: AttributeKey<T>, value: T) { }
override fun mergeAttributes(attributes: Attributes) { }
override fun close() { }
override fun asContextElement(): CoroutineContext = EmptyCoroutineContext
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public suspend inline fun <R> withSpan(
// or else traces may be disconnected from their parent
val updatedCtx = coroutineContext[TelemetryProviderContext]?.provider?.contextManager?.current()
val telemetryCtxElement = (updatedCtx?.let { TelemetryContextElement(it) } ?: coroutineContext[TelemetryContextElement]) ?: EmptyCoroutineContext
withContext(context + TraceSpanContext(span) + telemetryCtxElement) {
withContext(context + TraceSpanContext(span) + telemetryCtxElement + span.asContextElement()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we set up tests to confirm this is working as expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very complex setup, involving two Docker containers and a lot of visual inspection. I'm not sure setting up unit tests would be worth the effort

block(span)
}
} catch (ex: Exception) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import aws.smithy.kotlin.runtime.collections.AttributeKey
import aws.smithy.kotlin.runtime.collections.Attributes
import aws.smithy.kotlin.runtime.collections.emptyAttributes
import aws.smithy.kotlin.runtime.telemetry.context.Scope
import kotlin.coroutines.CoroutineContext
import kotlin.coroutines.EmptyCoroutineContext

/**
* Represents a single operation/task within a trace. Each trace contains a root span and
Expand All @@ -27,6 +29,11 @@ public interface TraceSpan : Scope {
*/
public val spanContext: SpanContext

/**
* A representation of this span as a [CoroutineContext] element
*/
public fun asContextElement(): CoroutineContext = EmptyCoroutineContext

/**
* Set an attribute on the span
* @param key the attribute key to use
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ kotlin {
jvmMain {
dependencies {
api(libs.opentelemetry.api)
api(libs.opentelemetry.kotlin.extension)
}
}
all {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import aws.smithy.kotlin.runtime.collections.Attributes
import aws.smithy.kotlin.runtime.telemetry.context.Context
import aws.smithy.kotlin.runtime.telemetry.trace.*
import io.opentelemetry.api.OpenTelemetry
import io.opentelemetry.extension.kotlin.asContextElement
import kotlin.coroutines.CoroutineContext
import io.opentelemetry.api.trace.Span as OtelSpan
import io.opentelemetry.api.trace.SpanContext as OtelSpanContext
import io.opentelemetry.api.trace.SpanKind as OtelSpanKind
Expand Down Expand Up @@ -62,11 +64,11 @@ private class OtelSpanContextImpl(private val otelSpanContext: OtelSpanContext)
internal class OtelTraceSpanImpl(
private val otelSpan: OtelSpan,
) : TraceSpan {

private val spanScope = otelSpan.makeCurrent()

override val spanContext: SpanContext
get() = OtelSpanContextImpl(otelSpan.spanContext)

override fun asContextElement(): CoroutineContext = otelSpan.asContextElement()

override fun <T : Any> set(key: AttributeKey<T>, value: T) {
key.otelAttrKeyOrNull(value)?.let { otelKey ->
otelSpan.setAttribute(otelKey, value)
Expand All @@ -90,7 +92,6 @@ internal class OtelTraceSpanImpl(

override fun close() {
otelSpan.end()
spanScope.close()
}
}

Expand Down
Loading