From f6a343113b9a721409e3a406ef903eed62e3a2e7 Mon Sep 17 00:00:00 2001 From: Thomas Ankcorn Date: Thu, 30 Apr 2026 13:42:17 +0000 Subject: [PATCH 1/2] test: verify jsRpcSession span is parented to enclosing enterSpan 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. --- .../tracing-hierarchy-instrumentation-test.js | 17 +++++++++++++++++ .../test/tracing/tracing-hierarchy-mock.js | 11 +++++++++-- .../test/tracing/tracing-hierarchy-test.js | 14 ++++++++++++++ .../test/tracing/tracing-hierarchy-test.wd-test | 8 ++++++-- 4 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/cloudflare/internal/test/tracing/tracing-hierarchy-instrumentation-test.js b/src/cloudflare/internal/test/tracing/tracing-hierarchy-instrumentation-test.js index 9176167c932..c244412d205 100644 --- a/src/cloudflare/internal/test/tracing/tracing-hierarchy-instrumentation-test.js +++ b/src/cloudflare/internal/test/tracing/tracing-hierarchy-instrumentation-test.js @@ -143,6 +143,23 @@ export const validateHierarchy = { // waitForCompletion() awaits them all. BaseTracer::WeakRef prevents the abandoned // SpanImpl from pinning the tracer past end-of-request. + // ---------- Case 7: jsRpcInsideEnterSpan ---------- + // The jsRpcSession span created by an RPC binding call must be a child of the + // enterSpan it was called from, not the top-level onset span. This is the RPC + // equivalent of case 3 (fetchInsideEnterSpan). + { + const outer = findSpanByName(state, 'hierarchy-rpc-outer'); + assert.strictEqual(outer.case, 'jsRpcInsideEnterSpan'); + assert.ok(outer.closed); + const rpcSpan = findSpanByName( + state, + 'jsRpcSession', + (s) => s.invocationId === outer.invocationId + ); + assertParent(rpcSpan, outer, 'jsRpcInsideEnterSpan'); + assertTopLevelParent(outer, 'jsRpcInsideEnterSpan'); + } + console.log('All tracing-hierarchy tests passed!'); }, }; diff --git a/src/cloudflare/internal/test/tracing/tracing-hierarchy-mock.js b/src/cloudflare/internal/test/tracing/tracing-hierarchy-mock.js index a4f1a52e067..7f05990167b 100644 --- a/src/cloudflare/internal/test/tracing/tracing-hierarchy-mock.js +++ b/src/cloudflare/internal/test/tracing/tracing-hierarchy-mock.js @@ -2,10 +2,17 @@ // Licensed under the Apache 2.0 license found in the LICENSE file or at: // https://opensource.org/licenses/Apache-2.0 -// Minimal fetch target used by tracing-hierarchy-test. Just echoes the request path so -// the runtime-generated "fetch" span has something to observe. +// Minimal fetch + RPC target used by tracing-hierarchy-test. +import { WorkerEntrypoint } from 'cloudflare:workers'; + export default { async fetch(request) { return new Response('ok', { status: 200 }); }, }; + +export class RpcTarget extends WorkerEntrypoint { + async ping() { + return 'pong'; + } +} diff --git a/src/cloudflare/internal/test/tracing/tracing-hierarchy-test.js b/src/cloudflare/internal/test/tracing/tracing-hierarchy-test.js index 87a6e59bd5e..44ebaebc8bf 100644 --- a/src/cloudflare/internal/test/tracing/tracing-hierarchy-test.js +++ b/src/cloudflare/internal/test/tracing/tracing-hierarchy-test.js @@ -113,3 +113,17 @@ export const abandonedPromiseSpan = { }); }, }; + +export const jsRpcInsideEnterSpan = { + async test(ctrl, env, ctx) { + const { withSpan } = env.tracingTest; + // An RPC call inside enterSpan should produce a jsRpcSession user span whose + // parent is the enterSpan, not the top-level onset span. This is the RPC + // equivalent of fetchInsideEnterSpan. + await withSpan('hierarchy-rpc-outer', async (outer) => { + outer.setAttribute('case', 'jsRpcInsideEnterSpan'); + const result = await env.rpcTarget.ping(); + assert.strictEqual(result, 'pong'); + }); + }, +}; diff --git a/src/cloudflare/internal/test/tracing/tracing-hierarchy-test.wd-test b/src/cloudflare/internal/test/tracing/tracing-hierarchy-test.wd-test index a5782bcca9a..d0c7813ffd7 100644 --- a/src/cloudflare/internal/test/tracing/tracing-hierarchy-test.wd-test +++ b/src/cloudflare/internal/test/tracing/tracing-hierarchy-test.wd-test @@ -11,13 +11,17 @@ const unitTests :Workerd.Config = ( modules = [ (name = "worker", esModule = embed "tracing-hierarchy-test.js"), ], - compatibilityFlags = ["experimental", "nodejs_compat"], + compatibilityFlags = ["experimental", "nodejs_compat", "rpc"], streamingTails = ["tail"], bindings = [ ( name = "fetchTarget", service = "tracing-hierarchy-mock" ), + ( + name = "rpcTarget", + service = (name = "tracing-hierarchy-mock", entrypoint = "RpcTarget") + ), ( name = "tracingTest", wrapped = ( @@ -29,7 +33,7 @@ const unitTests :Workerd.Config = ( ), ( name = "tracing-hierarchy-mock", worker = ( - compatibilityFlags = ["experimental", "nodejs_compat"], + compatibilityFlags = ["experimental", "nodejs_compat", "rpc"], modules = [ (name = "worker", esModule = embed "tracing-hierarchy-mock.js"), ], From 04e17f61fb4fc38202b7ae9dbead7b8a9cb2264b Mon Sep 17 00:00:00 2001 From: Thomas Ankcorn Date: Thu, 30 Apr 2026 13:42:29 +0000 Subject: [PATCH 2/2] refactor: move jsRpcSession span ownership into JsRpcSessionCustomEvent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) 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. --- src/workerd/api/http.c++ | 49 ++++++++++++++++++++++++++++++++++-- src/workerd/api/http.h | 12 +++++++++ src/workerd/api/worker-rpc.h | 6 +++++ 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/workerd/api/http.c++ b/src/workerd/api/http.c++ index 1b86fe02681..d8a1977b387 100644 --- a/src/workerd/api/http.c++ +++ b/src/workerd/api/http.c++ @@ -2121,9 +2121,9 @@ kj::Maybe> Fetcher::getRpcMethodInternal(jsg::Lock& js, rpc::JsRpcTarget::Client Fetcher::getClientForOneCall( jsg::Lock& js, kj::Vector& path) { auto& ioContext = IoContext::current(); - auto worker = getClient(ioContext, kj::none, "jsRpcSession"_kjc); + auto [worker, sessionSpan] = getJsRpcClient(ioContext); auto event = kj::heap( - JsRpcSessionCustomEvent::WORKER_RPC_EVENT_TYPE); + JsRpcSessionCustomEvent::WORKER_RPC_EVENT_TYPE, kj::mv(sessionSpan)); auto result = event->getCap(); @@ -2407,6 +2407,51 @@ kj::Own Fetcher::getClient( return clientWithTracing.client.attach(kj::mv(clientWithTracing.traceContext)); } +Fetcher::JsRpcClient Fetcher::getJsRpcClient(IoContext& ioContext) { + // The jsRpcSession span is owned by JsRpcSessionCustomEvent and lives for the session. + // OutgoingFactory variants create their own outer span, so we skip jsRpcSession for them. + auto withSessionSpan = [&](auto startRequest) -> JsRpcClient { + auto sessionSpan = ioContext.getCurrentUserTraceSpan().newChild( + "jsRpcSession"_kjc, ioContext.now()); + auto sessionSpanParent = SpanParent(sessionSpan); + auto worker = ioContext.getSubrequest( + [&](TraceContext& tracing, IoChannelFactory& channelFactory) { + return startRequest(channelFactory, + IoChannelFactory::SubrequestMetadata{ + .parentSpan = tracing.getInternalSpanParent(), + .userSpanParent = sessionSpanParent.addRef(), + }); + }, {.inHouse = isInHouse, .wrapMetrics = !isInHouse}); + return {kj::mv(worker), kj::mv(sessionSpan)}; + }; + + KJ_SWITCH_ONEOF(channelOrClientFactory) { + // Service binding (e.g. env.MyService) — create jsRpcSession span. + KJ_CASE_ONEOF(channel, uint) { + return withSessionSpan([&](IoChannelFactory& channelFactory, + IoChannelFactory::SubrequestMetadata metadata) { + return channelFactory.startSubrequest(channel, kj::mv(metadata)); + }); + } + // Direct in-process channel handle — create jsRpcSession span. + KJ_CASE_ONEOF(channel, IoOwn) { + return withSessionSpan([&](IoChannelFactory&, + IoChannelFactory::SubrequestMetadata metadata) { + return channel->startRequest(kj::mv(metadata)); + }); + } + // DurableObject stub (env.MyActor.get(id)) — factory creates durable_object_subrequest, skip. + KJ_CASE_ONEOF(outgoingFactory, IoOwn) { + return {outgoingFactory->newSingleUseClient(kj::none), SpanBuilder(nullptr)}; + } + // Cross-process actor — factory creates its own outer span, skip. + KJ_CASE_ONEOF(outgoingFactory, kj::Own) { + return {outgoingFactory->newSingleUseClient(ioContext, kj::none), SpanBuilder(nullptr)}; + } + } + KJ_UNREACHABLE; +} + Fetcher::ClientWithTracing Fetcher::getClientWithTracing( IoContext& ioContext, kj::Maybe cfStr, kj::ConstString operationName) { KJ_SWITCH_ONEOF(channelOrClientFactory) { diff --git a/src/workerd/api/http.h b/src/workerd/api/http.h index df9f1a4c4f9..4a63ead13ff 100644 --- a/src/workerd/api/http.h +++ b/src/workerd/api/http.h @@ -338,6 +338,18 @@ class Fetcher: public JsRpcClientProvider { kj::Own getClient( IoContext& ioContext, kj::Maybe cfStr, kj::ConstString operationName); + // Worker interface plus the user span representing this jsRpc session, if any. The span is + // created for direct channel variants but not for OutgoingFactory variants (which create their + // own outer span). The span is intended to be transferred to JsRpcSessionCustomEvent. + struct JsRpcClient { + kj::Own worker; + SpanBuilder sessionSpan; + }; + + // Get a worker interface for a jsRpc session call, along with the jsRpcSession span (if one + // should be created for this Fetcher variant). + JsRpcClient getJsRpcClient(IoContext& ioContext); + // Result of getClient call that includes optional trace context struct ClientWithTracing { kj::Own client; diff --git a/src/workerd/api/worker-rpc.h b/src/workerd/api/worker-rpc.h index 1d9e4ed9246..d8e38e9ff83 100644 --- a/src/workerd/api/worker-rpc.h +++ b/src/workerd/api/worker-rpc.h @@ -467,12 +467,14 @@ class RpcStubDisposalGroup { class JsRpcSessionCustomEvent final: public WorkerInterface::CustomEvent { public: JsRpcSessionCustomEvent(uint16_t typeId, + SpanBuilder jsRpcSessionSpan = SpanBuilder(nullptr), kj::Maybe wrapperModule = kj::none, kj::PromiseFulfillerPair paf = kj::newPromiseAndFulfiller()) : capFulfiller(kj::mv(paf.fulfiller)), clientCap(kj::mv(paf.promise)), typeId(typeId), + jsRpcSessionSpan(kj::mv(jsRpcSessionSpan)), wrapperModule(kj::mv(wrapperModule)) {} ~JsRpcSessionCustomEvent() noexcept(false) { @@ -529,6 +531,10 @@ class JsRpcSessionCustomEvent final: public WorkerInterface::CustomEvent { // limited return type. kj::Maybe clientCap; uint16_t typeId; + // Span representing this jsRpc session. Created before startRequest() so the callee can + // reference its ID for trace context propagation. Lives until this event is destroyed + // (i.e., until the session ends), which gives the correct span lifetime. + SpanBuilder jsRpcSessionSpan; kj::Maybe wrapperModule;