From a92d3073c375f6016a379611dc3c4ca66ccd0921 Mon Sep 17 00:00:00 2001 From: Felix Hanau Date: Sat, 30 Aug 2025 20:38:49 -0400 Subject: [PATCH] STOR-4395 Improve span parenting for Hibernate events, add TODOs for other events For some customEvents, we did not create trace scopes so far. Note that this only affects the internal tracing system. --- src/workerd/api/hibernatable-web-socket.c++ | 2 ++ src/workerd/api/worker-rpc.c++ | 4 ++++ src/workerd/io/io-context.c++ | 2 -- src/workerd/io/trace-stream.c++ | 3 +++ 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/workerd/api/hibernatable-web-socket.c++ b/src/workerd/api/hibernatable-web-socket.c++ index 7764688cba9..439753d9f02 100644 --- a/src/workerd/api/hibernatable-web-socket.c++ +++ b/src/workerd/api/hibernatable-web-socket.c++ @@ -107,6 +107,8 @@ kj::Promise HibernatableWebSocketCustomEve co_await context.run( [entrypointName = entrypointName, &context, eventParameters = kj::mv(eventParameters), props = kj::mv(props)](Worker::Lock& lock) mutable { + jsg::AsyncContextFrame::StorageScope traceScope = context.makeAsyncTraceScope(lock); + KJ_SWITCH_ONEOF(eventParameters.eventType) { KJ_CASE_ONEOF(text, HibernatableSocketParams::Text) { return lock.getGlobalScope().sendHibernatableWebSocketMessage(kj::mv(text.message), diff --git a/src/workerd/api/worker-rpc.c++ b/src/workerd/api/worker-rpc.c++ index b1e152a0f31..d3705455d8c 100644 --- a/src/workerd/api/worker-rpc.c++ +++ b/src/workerd/api/worker-rpc.c++ @@ -936,6 +936,10 @@ class JsRpcTargetBase: public rpc::JsRpcTarget::Server { // Note: No need to topUpActor() since this is the start of a top-level request, so the // actor will already have been topped up by IncomingRequest::delivered(). return ctx.run([this, &ctx, callContext](Worker::Lock& lock) mutable { + // TODO(later): Create trace scope for STOR-4395. Is this the right place to do so, or + // should we try to do it earlier to capture any spans created in a constructor using + // Actor::ensureConstructed())? + jsg::AsyncContextFrame::StorageScope traceScope = ctx.makeAsyncTraceScope(lock); return callImpl(lock, ctx, callContext); }); }) {} diff --git a/src/workerd/io/io-context.c++ b/src/workerd/io/io-context.c++ index 2e7dbc056de..6ac4c8142fa 100644 --- a/src/workerd/io/io-context.c++ +++ b/src/workerd/io/io-context.c++ @@ -1049,8 +1049,6 @@ SpanParent IoContext::getCurrentTraceSpan() { } SpanParent IoContext::getCurrentUserTraceSpan() { - // TODO(o11y): Add support for retrieving span from storage scope lock for more accurate span - // context, as with Jaeger spans. KJ_IF_SOME(workerTracer, getWorkerTracer()) { return workerTracer.getUserRequestSpan(); } diff --git a/src/workerd/io/trace-stream.c++ b/src/workerd/io/trace-stream.c++ index 3e95c2f07e5..a8154302ec6 100644 --- a/src/workerd/io/trace-stream.c++ +++ b/src/workerd/io/trace-stream.c++ @@ -612,6 +612,9 @@ class TailStreamTarget final: public rpc::TailStreamTarget::Server { ioContext .run([this, &ioContext, reportContext, ownReportContext = kj::mv(ownReportContext)]( Worker::Lock& lock) mutable -> kj::Promise { + // TODO(later): STOR-4395 This method is generally called several times in a single + // customEvent. Should an async trace scope be created each time? + auto params = reportContext.getParams(); KJ_ASSERT(params.hasEvents(), "Events are required."); auto eventReaders = params.getEvents();