Skip to content

Commit 53b850c

Browse files
authored
fix: mint a call-unique callKey for RequestContext and ExchangeContext (#125)
PR: #125
1 parent de34bd2 commit 53b850c

5 files changed

Lines changed: 84 additions & 16 deletions

File tree

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/DispatchContext.kt

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,9 @@ public data class DispatchContext(
5858
* (`traceId:spanId`). This portion is not call-unique on its own — the no-op context
5959
* shares constant ids, an inbound trace shares a trace id across spans, and a span id
6060
* may be reused across sibling calls — so [mintCallKey] appends a process-unique
61-
* counter to it for the actual key. Retained as the fallback derivation that
62-
* [RequestContext] and [ExchangeContext] use when constructed directly.
61+
* counter to it for the actual key.
6362
*/
64-
internal fun deriveCallKey(instrumentationContext: InstrumentationContext): String =
63+
private fun deriveCallKey(instrumentationContext: InstrumentationContext): String =
6564
instrumentationContext.traceId.value + ":" + instrumentationContext.spanId.value
6665

6766
/**
@@ -76,8 +75,12 @@ public data class DispatchContext(
7675
* process-unique counter to [deriveCallKey]'s trace/span derivation
7776
* (`traceId:spanId:n`). The counter disambiguates calls that would otherwise share a
7877
* trace/span pair, so distinct calls never collide in [ContextStore].
78+
*
79+
* Shared with [RequestContext] and [ExchangeContext], which mint the same call-unique
80+
* default key when constructed directly off-chain (rather than promoted from a
81+
* [DispatchContext]), so every link in the chain is collision-safe by default.
7982
*/
80-
private fun mintCallKey(instrumentationContext: InstrumentationContext): String =
83+
internal fun mintCallKey(instrumentationContext: InstrumentationContext): String =
8184
deriveCallKey(instrumentationContext) + ":" + mintCounter.incrementAndGet()
8285
}
8386
}

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/ExchangeContext.kt

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,16 @@ import org.dexpace.sdk.core.instrumentation.InstrumentationContext
1818
* chain's [callKey] from the [RequestContext] it was promoted from.
1919
*
2020
* As the terminal link this is the context whose [close] should be called to evict the
21-
* chain's [ContextStore] entry. The [callKey] defaults to the trace/span derivation when
22-
* constructed directly; in the normal flow it is supplied by
23-
* [RequestContext.toExchangeContext].
21+
* chain's [ContextStore] entry. In the normal flow the [callKey] is supplied by
22+
* [RequestContext.toExchangeContext]. When this context is constructed directly off-chain, the
23+
* [callKey] defaults to a freshly minted, call-unique key (`traceId:spanId:n`) — the same
24+
* collision-safe derivation [DispatchContext] uses — so two directly-constructed contexts that
25+
* share a trace/span id never collide in [ContextStore]. Two such default-constructed instances
26+
* are therefore not structurally equal; pin an explicit [callKey] if you need equality.
2427
*/
2528
public data class ExchangeContext(
2629
override val instrumentationContext: InstrumentationContext,
2730
val request: Request,
2831
val response: Response,
29-
override val callKey: String = DispatchContext.deriveCallKey(instrumentationContext),
32+
override val callKey: String = DispatchContext.mintCallKey(instrumentationContext),
3033
) : CallContext

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/RequestContext.kt

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,17 @@ import org.dexpace.sdk.core.instrumentation.InstrumentationContext
1717
* promotes this into an [ExchangeContext]. Inherits the chain's [callKey] from the
1818
* [DispatchContext] it was promoted from.
1919
*
20-
* The [callKey] defaults to the trace/span derivation when constructed directly; in the
21-
* normal flow it is supplied by [DispatchContext.toRequestContext] so the whole chain shares
22-
* one store slot.
20+
* In the normal flow the [callKey] is supplied by [DispatchContext.toRequestContext] so the
21+
* whole chain shares one store slot. When this context is constructed directly off-chain, the
22+
* [callKey] defaults to a freshly minted, call-unique key (`traceId:spanId:n`) — the same
23+
* collision-safe derivation [DispatchContext] uses — so two directly-constructed contexts that
24+
* share a trace/span id never collide in [ContextStore]. Two such default-constructed instances
25+
* are therefore not structurally equal; pin an explicit [callKey] if you need equality.
2326
*/
2427
public data class RequestContext(
2528
override val instrumentationContext: InstrumentationContext,
2629
val request: Request,
27-
override val callKey: String = DispatchContext.deriveCallKey(instrumentationContext),
30+
override val callKey: String = DispatchContext.mintCallKey(instrumentationContext),
2831
) : CallContext {
2932
/**
3033
* Promotes this request context into an [ExchangeContext] bound to [response] and stores

sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/ExchangeContextTest.kt

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,41 @@ class ExchangeContextTest {
4141
val instr = FakeInstrumentationContext(TraceId(owned("eq")))
4242
val req = request()
4343
val resp = response()
44-
val a = ExchangeContext(instr, req, resp)
45-
val b = ExchangeContext(instr, req, resp)
44+
// Pin an explicit call key so the two instances are constructed identically: the
45+
// default key is now call-unique, so two default-keyed instances are deliberately
46+
// distinct (see the call-key uniqueness test below).
47+
val key = owned("eq-key")
48+
val a = ExchangeContext(instr, req, resp, key)
49+
val b = ExchangeContext(instr, req, resp, key)
4650
assertEquals(a, b)
4751
assertEquals(a.hashCode(), b.hashCode())
4852
}
4953

54+
@Test
55+
fun `two directly-constructed contexts sharing a trace and span id receive distinct call keys`() {
56+
// A directly-constructed exchange context — built off-chain from instrumentation that
57+
// shares a trace/span id (an inbound W3C trace, or a tracer reusing a span id across
58+
// sibling calls) — must still get a call-unique key. FakeInstrumentationContext defaults
59+
// to a fixed span id, so both instances below share the SAME trace id AND span id: the
60+
// exact collision case. The default call key must distinguish them, or they would clobber
61+
// each other in ContextStore (which rejects duplicate keys).
62+
val sharedId = owned("collision")
63+
val a = ExchangeContext(FakeInstrumentationContext(TraceId(sharedId)), request(), response())
64+
val b = ExchangeContext(FakeInstrumentationContext(TraceId(sharedId)), request(), response())
65+
ownedIds.add(a.callKey)
66+
ownedIds.add(b.callKey)
67+
68+
assertEquals(a.instrumentationContext.traceId, b.instrumentationContext.traceId)
69+
assertEquals(a.instrumentationContext.spanId, b.instrumentationContext.spanId)
70+
assertNotEquals(a.callKey, b.callKey)
71+
72+
// Both register in the store without one rejecting or evicting the other.
73+
ContextStore.put(a.callKey, a)
74+
ContextStore.put(b.callKey, b)
75+
assertSame(a, ContextStore.get(a.callKey))
76+
assertSame(b, ContextStore.get(b.callKey))
77+
}
78+
5079
@Test
5180
fun `copy with same fields is equal to original`() {
5281
val instr = FakeInstrumentationContext(TraceId(owned("copy-equal")))

sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/context/RequestContextTest.kt

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import org.dexpace.sdk.core.instrumentation.TraceId
1111
import kotlin.test.AfterTest
1212
import kotlin.test.Test
1313
import kotlin.test.assertEquals
14+
import kotlin.test.assertNotEquals
1415
import kotlin.test.assertSame
1516

1617
class RequestContextTest {
@@ -55,13 +56,42 @@ class RequestContextTest {
5556
fun `data class equality is by content`() {
5657
val instr = FakeInstrumentationContext(TraceId(owned("eq")))
5758
val req = request()
58-
val a = RequestContext(instr, req)
59-
val b = RequestContext(instr, req)
59+
// Pin an explicit call key so the two instances are constructed identically: the
60+
// default key is now call-unique, so two default-keyed instances are deliberately
61+
// distinct (see the call-key uniqueness test below).
62+
val key = owned("eq-key")
63+
val a = RequestContext(instr, req, key)
64+
val b = RequestContext(instr, req, key)
6065
assertEquals(a, b)
6166
assertEquals(a.hashCode(), b.hashCode())
6267
assertEquals(a, a.copy())
6368
}
6469

70+
@Test
71+
fun `two directly-constructed contexts sharing a trace and span id receive distinct call keys`() {
72+
// A directly-constructed request context — built off-chain from instrumentation that
73+
// shares a trace/span id (an inbound W3C trace, or a tracer reusing a span id across
74+
// sibling calls) — must still get a call-unique key. FakeInstrumentationContext defaults
75+
// to a fixed span id, so both instances below share the SAME trace id AND span id: the
76+
// exact collision case. The default call key must distinguish them, or they would clobber
77+
// each other in ContextStore (which rejects duplicate keys).
78+
val sharedId = owned("collision")
79+
val a = RequestContext(FakeInstrumentationContext(TraceId(sharedId)), request())
80+
val b = RequestContext(FakeInstrumentationContext(TraceId(sharedId)), request())
81+
ownedIds.add(a.callKey)
82+
ownedIds.add(b.callKey)
83+
84+
assertEquals(a.instrumentationContext.traceId, b.instrumentationContext.traceId)
85+
assertEquals(a.instrumentationContext.spanId, b.instrumentationContext.spanId)
86+
assertNotEquals(a.callKey, b.callKey)
87+
88+
// Both register in the store without one rejecting or evicting the other.
89+
ContextStore.put(a.callKey, a)
90+
ContextStore.put(b.callKey, b)
91+
assertSame(a, ContextStore.get(a.callKey))
92+
assertSame(b, ContextStore.get(b.callKey))
93+
}
94+
6595
@Test
6696
fun `close evicts entry keyed by call key`() {
6797
val id = owned("close")

0 commit comments

Comments
 (0)