From 30f23b2fe4b453bcd03e95ed3cce743f9368aac8 Mon Sep 17 00:00:00 2001 From: Felix Hanau Date: Wed, 19 Nov 2025 22:59:00 +0000 Subject: [PATCH] [nfc] Clean up r2GetClient(), removing getHttpClientWithSpans() It turns out that the getHttpClient variant with trace context parameter is easier to use. --- src/workerd/api/r2-bucket.c++ | 15 +++++++++------ src/workerd/io/io-context.c++ | 29 ----------------------------- src/workerd/io/io-context.h | 13 ------------- 3 files changed, 9 insertions(+), 48 deletions(-) diff --git a/src/workerd/api/r2-bucket.c++ b/src/workerd/api/r2-bucket.c++ index 7253fd0eea1..a797d712a78 100644 --- a/src/workerd/api/r2-bucket.c++ +++ b/src/workerd/api/r2-bucket.c++ @@ -25,17 +25,20 @@ namespace workerd::api::public_beta { kj::Own r2GetClient( IoContext& context, uint subrequestChannel, R2UserTracing user) { - kj::Vector tags; - tags.add("rpc.service"_kjc, kj::ConstString("r2"_kjc)); - tags.add(user.method.key, kj::ConstString(kj::str(user.method.value))); + auto traceSpan = context.makeTraceSpan(user.op); + auto userSpan = context.makeUserTraceSpan(user.op); + TraceContext traceContext(kj::mv(traceSpan), kj::mv(userSpan)); + traceContext.userSpan.setTag("rpc.service"_kjc, "r2"_kjc); + traceContext.userSpan.setTag(user.method.key, user.method.value); KJ_IF_SOME(b, user.bucket) { - tags.add("cloudflare.r2.bucket"_kjc, kj::ConstString(kj::str(b))); + traceContext.userSpan.setTag("cloudflare.r2.bucket"_kjc, b); } KJ_IF_SOME(tag, user.extraTag) { - tags.add(tag.key, kj::ConstString(kj::str(tag.value))); + traceContext.userSpan.setTag(tag.key, tag.value); } - return context.getHttpClientWithSpans(subrequestChannel, true, kj::none, user.op, kj::mv(tags)); + return context.getHttpClient(subrequestChannel, true, kj::none, traceContext) + .attach(kj::mv(traceContext)); } static bool isWholeNumber(double x) { diff --git a/src/workerd/io/io-context.c++ b/src/workerd/io/io-context.c++ index 0cea5dd89a0..d265e0598dd 100644 --- a/src/workerd/io/io-context.c++ +++ b/src/workerd/io/io-context.c++ @@ -960,26 +960,6 @@ kj::Own IoContext::getSubrequestChannel( }); } -kj::Own IoContext::getSubrequestChannelWithSpans(uint channel, - bool isInHouse, - kj::Maybe cfBlobJson, - kj::ConstString operationName, - kj::Vector tags) { - return getSubrequest( - [&](TraceContext& tracing, IoChannelFactory& channelFactory) { - for (Span::Tag& tag: tags) { - tracing.userSpan.setTag(kj::mv(tag.key), kj::mv(tag.value)); - } - return getSubrequestChannelImpl( - channel, isInHouse, kj::mv(cfBlobJson), tracing, channelFactory); - }, - SubrequestOptions{ - .inHouse = isInHouse, - .wrapMetrics = !isInHouse, - .operationName = kj::mv(operationName), - }); -} - kj::Own IoContext::getSubrequestChannelNoChecks(uint channel, bool isInHouse, kj::Maybe cfBlobJson, @@ -1018,15 +998,6 @@ kj::Own IoContext::getHttpClient( getSubrequestChannel(channel, isInHouse, kj::mv(cfBlobJson), kj::mv(operationName))); } -kj::Own IoContext::getHttpClientWithSpans(uint channel, - bool isInHouse, - kj::Maybe cfBlobJson, - kj::ConstString operationName, - kj::Vector tags) { - return asHttpClient(getSubrequestChannelWithSpans( - channel, isInHouse, kj::mv(cfBlobJson), kj::mv(operationName), kj::mv(tags))); -} - kj::Own IoContext::getHttpClient( uint channel, bool isInHouse, kj::Maybe cfBlobJson, TraceContext& traceContext) { return asHttpClient(getSubrequestChannel(channel, isInHouse, kj::mv(cfBlobJson), traceContext)); diff --git a/src/workerd/io/io-context.h b/src/workerd/io/io-context.h index 29aab69d772..900763a6b2d 100644 --- a/src/workerd/io/io-context.h +++ b/src/workerd/io/io-context.h @@ -854,12 +854,6 @@ class IoContext final: public kj::Refcounted, private kj::TaskSet::ErrorHandler kj::Own getSubrequestChannel( uint channel, bool isInHouse, kj::Maybe cfBlobJson, TraceContext& traceContext); - kj::Own getSubrequestChannelWithSpans(uint channel, - bool isInHouse, - kj::Maybe cfBlobJson, - kj::ConstString operationName, - kj::Vector tags); - // Like getSubrequestChannel() but doesn't enforce limits. Use for trusted paths only. kj::Own getSubrequestChannelNoChecks(uint channel, bool isInHouse, @@ -876,13 +870,6 @@ class IoContext final: public kj::Refcounted, private kj::TaskSet::ErrorHandler kj::Own getHttpClient( uint channel, bool isInHouse, kj::Maybe cfBlobJson, TraceContext& traceContext); - // As above, but with list of span tags to add, analogous to getSubrequestChannelWithSpans(). - kj::Own getHttpClientWithSpans(uint channel, - bool isInHouse, - kj::Maybe cfBlobJson, - kj::ConstString operationName, - kj::Vector tags); - // Convenience methods that call getSubrequest*() and adapt the returned WorkerInterface objects // to HttpClient. kj::Own getHttpClientNoChecks(uint channel,