From 1e853968f9d7e3fbba96f64b3be77675ee7f463d Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Wed, 17 Jun 2026 01:22:50 +0300 Subject: [PATCH] fix: mint a call-unique callKey for RequestContext and ExchangeContext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DispatchContext's default callKey appends a process-unique counter (traceId:spanId:n) so it stays unique even when the trace/span pair is not — the no-op instrumentation context shares constant ids, an inbound W3C trace shares one trace id across spans, and some tracers reuse a span id across sibling calls. RequestContext and ExchangeContext, however, still defaulted to the plain traceId:spanId derivation. On the normal promotion path this was harmless: toRequestContext and toExchangeContext pass the head's minted key forward explicitly. But a RequestContext or ExchangeContext constructed directly off-chain from instrumentation that shares a trace/span id received a non-unique key, so two such instances collided in ContextStore — put rejects the duplicate, and set silently clobbers and evicts the live entry. Default both to the same minted, call-unique key DispatchContext uses, so every link in the chain is collision-safe by default. As with DispatchContext, this makes two default-constructed instances structurally unequal; the equality tests pin an explicit key, and new tests cover the shared-trace/span collision case. --- .../sdk/core/http/context/DispatchContext.kt | 11 +++--- .../sdk/core/http/context/ExchangeContext.kt | 11 +++--- .../sdk/core/http/context/RequestContext.kt | 11 +++--- .../core/http/context/ExchangeContextTest.kt | 33 ++++++++++++++++-- .../core/http/context/RequestContextTest.kt | 34 +++++++++++++++++-- 5 files changed, 84 insertions(+), 16 deletions(-) diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/DispatchContext.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/DispatchContext.kt index 448db098..921a7da6 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/DispatchContext.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/DispatchContext.kt @@ -58,10 +58,9 @@ public data class DispatchContext( * (`traceId:spanId`). This portion is not call-unique on its own — the no-op context * shares constant ids, an inbound trace shares a trace id across spans, and a span id * may be reused across sibling calls — so [mintCallKey] appends a process-unique - * counter to it for the actual key. Retained as the fallback derivation that - * [RequestContext] and [ExchangeContext] use when constructed directly. + * counter to it for the actual key. */ - internal fun deriveCallKey(instrumentationContext: InstrumentationContext): String = + private fun deriveCallKey(instrumentationContext: InstrumentationContext): String = instrumentationContext.traceId.value + ":" + instrumentationContext.spanId.value /** @@ -76,8 +75,12 @@ public data class DispatchContext( * process-unique counter to [deriveCallKey]'s trace/span derivation * (`traceId:spanId:n`). The counter disambiguates calls that would otherwise share a * trace/span pair, so distinct calls never collide in [ContextStore]. + * + * Shared with [RequestContext] and [ExchangeContext], which mint the same call-unique + * default key when constructed directly off-chain (rather than promoted from a + * [DispatchContext]), so every link in the chain is collision-safe by default. */ - private fun mintCallKey(instrumentationContext: InstrumentationContext): String = + internal fun mintCallKey(instrumentationContext: InstrumentationContext): String = deriveCallKey(instrumentationContext) + ":" + mintCounter.incrementAndGet() } } diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/ExchangeContext.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/ExchangeContext.kt index 499af570..ccdcf30d 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/ExchangeContext.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/ExchangeContext.kt @@ -18,13 +18,16 @@ import org.dexpace.sdk.core.instrumentation.InstrumentationContext * chain's [callKey] from the [RequestContext] it was promoted from. * * As the terminal link this is the context whose [close] should be called to evict the - * chain's [ContextStore] entry. The [callKey] defaults to the trace/span derivation when - * constructed directly; in the normal flow it is supplied by - * [RequestContext.toExchangeContext]. + * chain's [ContextStore] entry. In the normal flow the [callKey] is supplied by + * [RequestContext.toExchangeContext]. When this context is constructed directly off-chain, the + * [callKey] defaults to a freshly minted, call-unique key (`traceId:spanId:n`) — the same + * collision-safe derivation [DispatchContext] uses — so two directly-constructed contexts that + * share a trace/span id never collide in [ContextStore]. Two such default-constructed instances + * are therefore not structurally equal; pin an explicit [callKey] if you need equality. */ public data class ExchangeContext( override val instrumentationContext: InstrumentationContext, val request: Request, val response: Response, - override val callKey: String = DispatchContext.deriveCallKey(instrumentationContext), + override val callKey: String = DispatchContext.mintCallKey(instrumentationContext), ) : CallContext diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/RequestContext.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/RequestContext.kt index 564fe23e..969ff74b 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/RequestContext.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/RequestContext.kt @@ -17,14 +17,17 @@ import org.dexpace.sdk.core.instrumentation.InstrumentationContext * promotes this into an [ExchangeContext]. Inherits the chain's [callKey] from the * [DispatchContext] it was promoted from. * - * The [callKey] defaults to the trace/span derivation when constructed directly; in the - * normal flow it is supplied by [DispatchContext.toRequestContext] so the whole chain shares - * one store slot. + * In the normal flow the [callKey] is supplied by [DispatchContext.toRequestContext] so the + * whole chain shares one store slot. When this context is constructed directly off-chain, the + * [callKey] defaults to a freshly minted, call-unique key (`traceId:spanId:n`) — the same + * collision-safe derivation [DispatchContext] uses — so two directly-constructed contexts that + * share a trace/span id never collide in [ContextStore]. Two such default-constructed instances + * are therefore not structurally equal; pin an explicit [callKey] if you need equality. */ public data class RequestContext( override val instrumentationContext: InstrumentationContext, val request: Request, - override val callKey: String = DispatchContext.deriveCallKey(instrumentationContext), + override val callKey: String = DispatchContext.mintCallKey(instrumentationContext), ) : CallContext { /** * Promotes this request context into an [ExchangeContext] bound to [response] and stores diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/ExchangeContextTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/ExchangeContextTest.kt index 0ad32567..e166a1a6 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/ExchangeContextTest.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/ExchangeContextTest.kt @@ -41,12 +41,41 @@ class ExchangeContextTest { val instr = FakeInstrumentationContext(TraceId(owned("eq"))) val req = request() val resp = response() - val a = ExchangeContext(instr, req, resp) - val b = ExchangeContext(instr, req, resp) + // Pin an explicit call key so the two instances are constructed identically: the + // default key is now call-unique, so two default-keyed instances are deliberately + // distinct (see the call-key uniqueness test below). + val key = owned("eq-key") + val a = ExchangeContext(instr, req, resp, key) + val b = ExchangeContext(instr, req, resp, key) assertEquals(a, b) assertEquals(a.hashCode(), b.hashCode()) } + @Test + fun `two directly-constructed contexts sharing a trace and span id receive distinct call keys`() { + // A directly-constructed exchange context — built off-chain from instrumentation that + // shares a trace/span id (an inbound W3C trace, or a tracer reusing a span id across + // sibling calls) — must still get a call-unique key. FakeInstrumentationContext defaults + // to a fixed span id, so both instances below share the SAME trace id AND span id: the + // exact collision case. The default call key must distinguish them, or they would clobber + // each other in ContextStore (which rejects duplicate keys). + val sharedId = owned("collision") + val a = ExchangeContext(FakeInstrumentationContext(TraceId(sharedId)), request(), response()) + val b = ExchangeContext(FakeInstrumentationContext(TraceId(sharedId)), request(), response()) + ownedIds.add(a.callKey) + ownedIds.add(b.callKey) + + assertEquals(a.instrumentationContext.traceId, b.instrumentationContext.traceId) + assertEquals(a.instrumentationContext.spanId, b.instrumentationContext.spanId) + assertNotEquals(a.callKey, b.callKey) + + // Both register in the store without one rejecting or evicting the other. + ContextStore.put(a.callKey, a) + ContextStore.put(b.callKey, b) + assertSame(a, ContextStore.get(a.callKey)) + assertSame(b, ContextStore.get(b.callKey)) + } + @Test fun `copy with same fields is equal to original`() { val instr = FakeInstrumentationContext(TraceId(owned("copy-equal"))) diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/RequestContextTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/RequestContextTest.kt index b7d77276..594214c7 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/RequestContextTest.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/RequestContextTest.kt @@ -11,6 +11,7 @@ import org.dexpace.sdk.core.instrumentation.TraceId import kotlin.test.AfterTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNotEquals import kotlin.test.assertSame class RequestContextTest { @@ -55,13 +56,42 @@ class RequestContextTest { fun `data class equality is by content`() { val instr = FakeInstrumentationContext(TraceId(owned("eq"))) val req = request() - val a = RequestContext(instr, req) - val b = RequestContext(instr, req) + // Pin an explicit call key so the two instances are constructed identically: the + // default key is now call-unique, so two default-keyed instances are deliberately + // distinct (see the call-key uniqueness test below). + val key = owned("eq-key") + val a = RequestContext(instr, req, key) + val b = RequestContext(instr, req, key) assertEquals(a, b) assertEquals(a.hashCode(), b.hashCode()) assertEquals(a, a.copy()) } + @Test + fun `two directly-constructed contexts sharing a trace and span id receive distinct call keys`() { + // A directly-constructed request context — built off-chain from instrumentation that + // shares a trace/span id (an inbound W3C trace, or a tracer reusing a span id across + // sibling calls) — must still get a call-unique key. FakeInstrumentationContext defaults + // to a fixed span id, so both instances below share the SAME trace id AND span id: the + // exact collision case. The default call key must distinguish them, or they would clobber + // each other in ContextStore (which rejects duplicate keys). + val sharedId = owned("collision") + val a = RequestContext(FakeInstrumentationContext(TraceId(sharedId)), request()) + val b = RequestContext(FakeInstrumentationContext(TraceId(sharedId)), request()) + ownedIds.add(a.callKey) + ownedIds.add(b.callKey) + + assertEquals(a.instrumentationContext.traceId, b.instrumentationContext.traceId) + assertEquals(a.instrumentationContext.spanId, b.instrumentationContext.spanId) + assertNotEquals(a.callKey, b.callKey) + + // Both register in the store without one rejecting or evicting the other. + ContextStore.put(a.callKey, a) + ContextStore.put(b.callKey, b) + assertSame(a, ContextStore.get(a.callKey)) + assertSame(b, ContextStore.get(b.callKey)) + } + @Test fun `close evicts entry keyed by call key`() { val id = owned("close")