Move jsRpcSession span ownership into JsRpcSessionCustomEvent#6703
Closed
Ankcorn wants to merge 2 commits intocloudflare:mainfrom
Closed
Move jsRpcSession span ownership into JsRpcSessionCustomEvent#6703Ankcorn wants to merge 2 commits intocloudflare:mainfrom
jsRpcSession span ownership into JsRpcSessionCustomEvent#6703Ankcorn wants to merge 2 commits intocloudflare:mainfrom
Conversation
Ankcorn
changed the title
jsRpcSession span ownership into JsRpcSessionCustomEvent
Adds Case 7 to tracing-hierarchy-instrumentation-test: an RPC call made inside enterSpan() must produce a jsRpcSession user span whose parent is the enterSpan, not the top-level onset span. This is the RPC analog of the existing fetchInsideEnterSpan case.
The jsRpcSession user span is now created in Fetcher::getJsRpcClient() — a new helper that returns the WorkerInterface plus the span — and transferred to the JsRpcSessionCustomEvent where it lives as a member SpanBuilder until the event is destroyed (i.e. session end). Previously the span was created inside getClientWithTracing() and attached to the WorkerInterface via .attach(). The event itself had no visibility into the span, which made it impossible for callImpl() to reach it for runtime enrichment (e.g. AI Gateway binding span tags). Behavioural notes: - Span created on the caller side before startRequest() so its ID is available for USER_SPAN_CONTEXT_PROPAGATION (the callee's onset event reports this span as its parent). - Direct channel variants (uint, IoOwn<SubrequestChannel>) get a jsRpcSession span. OutgoingFactory variants (DurableObject stubs, cross-process actors) already create their own outer span (e.g. durable_object_subrequest), so jsRpcSession is omitted to avoid redundancy. This matches pre-existing behaviour for those variants. - ioContext.now() (I/O time) is used for the explicit start time so we remain Spectre-safe and deterministic in test mode.
Ankcorn
force-pushed
the
tankcorn/jsrpc-span-parenting
branch
from
1ca14e7 to
04e17f6
Compare
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.
Today the span is buried in a
TraceContext.attach()-ed to theWorkerInterface— the event can't reach it. This blocks runtime span enrichment fromcallImpl's response continuation (e.g. AI Gateway binding tags, #6695).Fetcher::getJsRpcClient()returning{worker, sessionSpan}, and stores theSpanBuilderonJsRpcSessionCustomEventfor the session lifetime.startRequest()so it works asuserSpanParentforUSER_SPAN_CONTEXT_PROPAGATION.OutgoingFactoryvariants keep their existing outer span — nojsRpcSessionstacked on top.Unblocks #6695
With the span reachable from the event,
callImplcan apply enrichment directly viaevent->applyEnrichment(...).This lets #6695 delete:
currentEnrichmentTc_raw pointer field onFetcher(with its "safe because JS is single-threaded" comment)getEnrichmentTraceContext()virtual side-channel + every overrideKJ_IF_SOME(tc, clientWithTracing.traceContext)branch + manualkj::heap<TraceContext>lifetime jugglingWhat stays in #6695: callee-side
setJsrpcSessionAttributesAPI — i.e. the actual feature.