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")