fix: mint a call-unique callKey for RequestContext and ExchangeContext#125
Merged
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #124
Problem
A
CallContext'scallKeyis the slot under which a call is tracked inContextStore, and it must be unique per call:ContextStore.putrejects a duplicate key, andset(used on the promotion path) silently overwrites — and so evicts — a live entry if two calls collide.traceId:spanIdis not a safe key on its own: the no-op instrumentation context shares constant ids across untraced calls, an inbound W3C trace shares one trace id across its spans, and some tracers reuse a span id across sibling client calls.DispatchContextalready defends against this — its defaultcallKeyappends a process-unique counter (traceId:spanId:n).RequestContextandExchangeContext, however, still defaulted to the plaintraceId:spanIdderivation.On the normal promotion path that was harmless:
toRequestContext/toExchangeContextpass the head's minted key forward explicitly, so these defaults were never exercised. But aRequestContextorExchangeContextconstructed directly off-chain from instrumentation that shares a trace/span id received a non-unique key, so two such instances collided inContextStore—putrejects the second, orsetclobbers the first and evicts a live entry. The result was an inconsistency: the head of the chain was collision-safe by default while the other two links were not.Change
Default
RequestContextandExchangeContextto the same minted, call-unique keyDispatchContextuses, so every link in the chain is collision-safe by default regardless of how it is constructed.mintCallKeyis shared as the common derivation;deriveCallKeyis now a private implementation detail of it.As with
DispatchContext, minting a unique default makes two default-constructed instances structurally unequal. The data-class equality tests pin an explicitcallKeyto construct identical instances, and new tests assert that two directly-constructed contexts sharing a trace and span id get distinct keys and both register in the store without evicting each other.No public API signature changes (
apiCheckclean); the constructors' default-argument values change but their types do not.