Skip to content

Commit fa369ed

Browse files
authored
Merge pull request #7203 from cloudflare/maizatskyi/2026-09-01-rc-span-observer
switch SpanObserver to kj::Rc
2 parents 5e1bb9b + 5c9926b commit fa369ed

7 files changed

Lines changed: 41 additions & 44 deletions

File tree

‎src/workerd/api/tracing.c++‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ kj::LiteralStringConst spanWarningTypeName(SpanWarningType type) {
5353
// ======================================================================================
5454
// SpanImpl
5555

56-
SpanImpl::SpanImpl(kj::Own<workerd::SpanObserver> observer, kj::ConstString operationName)
56+
SpanImpl::SpanImpl(kj::Rc<workerd::SpanObserver> observer, kj::ConstString operationName)
5757
: builder(kj::mv(observer), kj::mv(operationName)) {}
5858

5959
SpanImpl::SpanImpl(decltype(nullptr)): builder(nullptr) {}

‎src/workerd/api/tracing.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ using TagValue = kj::OneOf<bool, double, kj::String>;
3434
class SpanImpl final: public kj::Refcounted {
3535
public:
3636
// Construct an observed span. The builder drives the observer's onOpen immediately.
37-
SpanImpl(kj::Own<workerd::SpanObserver> observer, kj::ConstString operationName);
37+
SpanImpl(kj::Rc<workerd::SpanObserver> observer, kj::ConstString operationName);
3838

3939
// Construct a no-op span (not recording). Used when there is no current user trace span
4040
// (e.g., running outside a traced request) or when we are in a context where we cannot

‎src/workerd/io/trace.c++‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1810,17 +1810,16 @@ void SpanEndData::copyTo(rpc::SpanEndData::Builder builder) const {
18101810

18111811
// ======================================================================================
18121812

1813-
SpanBuilder::SpanBuilder(kj::Maybe<kj::Own<SpanObserver>> observer,
1814-
kj::ConstString operationName,
1815-
kj::Maybe<kj::Date> startTime) {
1816-
KJ_IF_SOME(obs, observer) {
1813+
SpanBuilder::SpanBuilder(
1814+
kj::Rc<SpanObserver> observer, kj::ConstString operationName, kj::Maybe<kj::Date> startTime) {
1815+
if (observer != nullptr) {
18171816
// TODO(o11y): Once we report the user tracing spanOpen event as soon as a span is created, we
18181817
// should be able to fold this virtual call and just get the timestamp directly.
1819-
kj::Date time = startTime.orDefault([&]() { return obs->getTime(); });
1818+
kj::Date time = startTime.orDefault([&]() { return observer->getTime(); });
18201819
// Report spanOpen event for user tracing spans
1821-
obs->onOpen(operationName.clone(), time);
1820+
observer->onOpen(operationName.clone(), time);
18221821
span.emplace(kj::mv(operationName), time);
1823-
this->observer = kj::mv(obs);
1822+
this->observer = kj::mv(observer);
18241823
}
18251824
}
18261825

@@ -1836,21 +1835,21 @@ SpanBuilder::~SpanBuilder() noexcept(false) {
18361835
}
18371836

18381837
void SpanBuilder::end() {
1839-
KJ_IF_SOME(o, observer) {
1838+
if (observer != nullptr) {
18401839
KJ_IF_SOME(s, span) {
18411840
// TODO(performance): Fold this timer call if we are using I/O time, where we will look up
18421841
// I/O time later.
18431842
s.endTime = kj::systemPreciseCalendarClock().now();
1844-
o->onClose(s.endTime, kj::mv(s.tags), kj::mv(s.logs));
1843+
observer->onClose(s.endTime, kj::mv(s.tags), kj::mv(s.logs));
18451844
span = kj::none;
18461845
}
18471846
}
18481847
}
18491848

18501849
void SpanBuilder::setOperationName(kj::ConstString operationName) {
18511850
KJ_IF_SOME(s, span) {
1852-
KJ_IF_SOME(o, observer) {
1853-
o->onUpdateName(operationName.clone());
1851+
if (observer != nullptr) {
1852+
observer->onUpdateName(operationName.clone());
18541853
}
18551854
s.operationName = kj::mv(operationName);
18561855
}

‎src/workerd/io/trace.h‎

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,7 +1127,7 @@ class SpanParent {
11271127
// Make a SpanParent that causes children not to be reported anywhere.
11281128
SpanParent(decltype(nullptr)) {}
11291129

1130-
SpanParent(kj::Maybe<kj::Own<SpanObserver>> observer): observer(kj::mv(observer)) {}
1130+
SpanParent(kj::Rc<SpanObserver> observer): observer(kj::mv(observer)) {}
11311131

11321132
SpanParent(SpanParent&& other) = default;
11331133
SpanParent& operator=(SpanParent&& other) = default;
@@ -1143,7 +1143,7 @@ class SpanParent {
11431143

11441144
// Useful to skip unnecessary code when not observed.
11451145
bool isObserved() {
1146-
return observer != kj::none;
1146+
return observer != nullptr;
11471147
}
11481148

11491149
// Get the underlying SpanObserver representing the parent span.
@@ -1152,7 +1152,8 @@ class SpanParent {
11521152
// trace IDs in a way that is specific to the trace back-end being used. The caller must downcast
11531153
// the `SpanObserver` to the expected observer type in order to extract the trace ID.
11541154
kj::Maybe<SpanObserver&> getObserver() {
1155-
return observer;
1155+
if (observer != nullptr) return *observer;
1156+
return kj::none;
11561157
}
11571158

11581159
// Return the serializable identity of this span for cross-boundary propagation.
@@ -1166,7 +1167,7 @@ class SpanParent {
11661167
static SpanParent fromSpanContext(tracing::SpanContext context);
11671168

11681169
private:
1169-
kj::Maybe<kj::Own<SpanObserver>> observer;
1170+
kj::Rc<SpanObserver> observer;
11701171
};
11711172

11721173
// Whether the span tag is a custom tag added using the user tracing binding, we do not log for
@@ -1188,7 +1189,7 @@ class SpanBuilder {
11881189
//
11891190
// `operationName` should be a string literal with infinite lifetime, or somehow otherwise be
11901191
// attached to the observer observing this span.
1191-
explicit SpanBuilder(kj::Maybe<kj::Own<SpanObserver>> observer,
1192+
explicit SpanBuilder(kj::Rc<SpanObserver> observer,
11921193
kj::ConstString operationName,
11931194
kj::Maybe<kj::Date> startTime = kj::none);
11941195

@@ -1208,7 +1209,7 @@ class SpanBuilder {
12081209

12091210
// Useful to skip unnecessary code when not observed.
12101211
bool isObserved() {
1211-
return observer != kj::none;
1212+
return observer != nullptr;
12121213
}
12131214

12141215
// Get the underlying SpanObserver representing the span.
@@ -1217,7 +1218,8 @@ class SpanBuilder {
12171218
// trace IDs in a way that is specific to the trace back-end being used. The caller must downcast
12181219
// the `SpanObserver` to the expected observer type in order to extract the trace ID.
12191220
kj::Maybe<SpanObserver&> getObserver() {
1220-
return observer;
1221+
if (observer != nullptr) return *observer;
1222+
return kj::none;
12211223
}
12221224

12231225
// Create a new child span.
@@ -1255,7 +1257,7 @@ class SpanBuilder {
12551257
void addLog(kj::Date timestamp, kj::ConstString key, TagValue value);
12561258

12571259
private:
1258-
kj::Maybe<kj::Own<SpanObserver>> observer;
1260+
kj::Rc<SpanObserver> observer;
12591261
// The under-construction span, or null if the span has ended.
12601262
kj::Maybe<Span> span;
12611263

@@ -1274,12 +1276,12 @@ class SpanObserver: public kj::Refcounted {
12741276
// Allocate a new child span.
12751277
//
12761278
// Note that children can be created long after a span has completed.
1277-
[[nodiscard]] virtual kj::Own<SpanObserver> newChild() = 0;
1279+
[[nodiscard]] virtual kj::Rc<SpanObserver> newChild() = 0;
12781280

12791281
// Allocate a child for a span initiated directly by user JavaScript (via
12801282
// `ctx.tracing.enterSpan`). Allows implementations to apply different policies than for
12811283
// runtime-issued spans (notably, edgeworker bypasses its operation-name allowlist here).
1282-
[[nodiscard]] virtual kj::Own<SpanObserver> newChildFromUserCode() {
1284+
[[nodiscard]] virtual kj::Rc<SpanObserver> newChildFromUserCode() {
12831285
return newChild();
12841286
}
12851287

@@ -1325,7 +1327,7 @@ class NonRecordingSpanObserver final: public SpanObserver {
13251327
public:
13261328
explicit NonRecordingSpanObserver(tracing::SpanContext context): context(kj::mv(context)) {}
13271329

1328-
kj::Own<SpanObserver> newChild() override {
1330+
kj::Rc<SpanObserver> newChild() override {
13291331
return {};
13301332
}
13311333
void onOpen(kj::ConstString, kj::Date) override {}
@@ -1339,39 +1341,35 @@ class NonRecordingSpanObserver final: public SpanObserver {
13391341
};
13401342

13411343
inline kj::Maybe<tracing::SpanContext> SpanParent::toSpanContext() {
1342-
KJ_IF_SOME(obs, observer) {
1343-
return obs->toSpanContext();
1344-
}
1344+
if (observer != nullptr) return observer->toSpanContext();
13451345
return kj::none;
13461346
}
13471347

13481348
inline tracing::SpanId SpanParent::getSpanId() {
1349-
KJ_IF_SOME(obs, observer) {
1350-
return obs->getSpanId();
1351-
}
1349+
if (observer != nullptr) return observer->getSpanId();
13521350
return tracing::SpanId::nullId;
13531351
}
13541352

13551353
inline SpanParent SpanParent::fromSpanContext(tracing::SpanContext context) {
1356-
return SpanParent(kj::refcounted<NonRecordingSpanObserver>(kj::mv(context)));
1354+
return SpanParent(kj::rc<NonRecordingSpanObserver>(kj::mv(context)));
13571355
}
13581356

1359-
inline SpanParent::SpanParent(SpanBuilder& builder): observer(mapAddRef(builder.observer)) {}
1357+
inline SpanParent::SpanParent(SpanBuilder& builder): observer(builder.observer.addRef()) {}
13601358

13611359
inline SpanParent SpanParent::addRef() {
1362-
return SpanParent(mapAddRef(observer));
1360+
return SpanParent(observer.addRef());
13631361
}
13641362

13651363
inline SpanBuilder SpanParent::newChild(
13661364
kj::ConstString operationName, kj::Maybe<kj::Date> startTime) {
1367-
return SpanBuilder(observer.map([](kj::Own<SpanObserver>& obs) { return obs->newChild(); }),
1368-
kj::mv(operationName), startTime);
1365+
if (observer == nullptr) return nullptr;
1366+
return SpanBuilder(observer->newChild(), kj::mv(operationName), startTime);
13691367
}
13701368

13711369
inline SpanBuilder SpanBuilder::newChild(
13721370
kj::ConstString operationName, kj::Maybe<kj::Date> startTime) {
1373-
return SpanBuilder(observer.map([](kj::Own<SpanObserver>& obs) { return obs->newChild(); }),
1374-
kj::mv(operationName), startTime);
1371+
if (observer == nullptr) return nullptr;
1372+
return SpanBuilder(observer->newChild(), kj::mv(operationName), startTime);
13751373
}
13761374

13771375
class TraceContext;

‎src/workerd/io/tracer.c++‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -586,12 +586,12 @@ void WorkerTracer::setJsRpcInfo(const tracing::InvocationSpanContext& context,
586586
}
587587
}
588588

589-
kj::Own<SpanObserver> UserSpanObserver::newChild() {
590-
return kj::refcounted<UserSpanObserver>(kj::addRef(*submitter), spanId, traceId, traceFlags);
589+
kj::Rc<SpanObserver> UserSpanObserver::newChild() {
590+
return kj::rc<UserSpanObserver>(kj::addRef(*submitter), spanId, traceId, traceFlags);
591591
}
592592

593-
kj::Own<SpanObserver> UserSpanObserver::newChildFromUserCode() {
594-
return kj::refcounted<UserSpanObserver>(
593+
kj::Rc<SpanObserver> UserSpanObserver::newChildFromUserCode() {
594+
return kj::rc<UserSpanObserver>(
595595
kj::addRef(*submitter), spanId, traceId, traceFlags, /*fromUserCode=*/true);
596596
}
597597

‎src/workerd/io/tracer.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,8 @@ class UserSpanObserver final: public SpanObserver {
272272
fromUserCode(fromUserCode) {}
273273
KJ_DISALLOW_COPY(UserSpanObserver);
274274

275-
kj::Own<SpanObserver> newChild() override;
276-
kj::Own<SpanObserver> newChildFromUserCode() override;
275+
kj::Rc<SpanObserver> newChild() override;
276+
kj::Rc<SpanObserver> newChildFromUserCode() override;
277277
void onOpen(kj::ConstString operationName, kj::Date startTime) override;
278278
void onClose(kj::Date endTime, Span::TagMap&& tags, kj::Vector<Span::Log>&& logs) override;
279279
kj::Date getTime() override;

‎src/workerd/server/server.c++‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3914,7 +3914,7 @@ class Server::WorkerService final: public Service,
39143914
w->setMakeUserRequestSpanFunc(
39153915
[&w = *w, &entropySource = threadContext.getEntropySource()](
39163916
tracing::TraceId traceId, kj::Maybe<tracing::TraceFlags> traceFlags) {
3917-
return SpanParent(kj::refcounted<UserSpanObserver>(
3917+
return SpanParent(kj::rc<UserSpanObserver>(
39183918
kj::refcounted<SequentialSpanSubmitter>(w.getWeakRef(), entropySource), kj::mv(traceId),
39193919
traceFlags));
39203920
});

0 commit comments

Comments
 (0)