Skip to content

Commit

Permalink
Use TraceParentContext, simplifying code using SubrequestMetadata
Browse files Browse the repository at this point in the history
  • Loading branch information
fhanau committed Sep 16, 2024
1 parent 5df42cc commit 98cb057
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 12 deletions.
4 changes: 2 additions & 2 deletions src/workerd/api/actor.c++
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public:
}

return KJ_REQUIRE_NONNULL(actorChannel)
->startRequest({.cfBlobJson = kj::mv(cfStr), .parentSpan = spans.span});
->startRequest({.cfBlobJson = kj::mv(cfStr), .spans = spans});
},
{.inHouse = true,
.wrapMetrics = true,
Expand Down Expand Up @@ -77,7 +77,7 @@ public:
}

return KJ_REQUIRE_NONNULL(actorChannel)
->startRequest({.cfBlobJson = kj::mv(cfStr), .parentSpan = spans.span});
->startRequest({.cfBlobJson = kj::mv(cfStr), .spans = spans});
},
{.inHouse = true,
.wrapMetrics = true,
Expand Down
3 changes: 1 addition & 2 deletions src/workerd/io/io-channels.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ class IoChannelFactory {
kj::Maybe<kj::String> cfBlobJson;

// Specifies the parent span for the subrequest for tracing purposes.
SpanParent parentSpan = nullptr;
LimeSpanParent limeParentSpan = nullptr;
TraceParentContext spans = TraceParentContext(nullptr, nullptr);

// Serialized JSON value to pass in ew_compat field of control header to FL. If this subrequest
// does not go directly to FL, this value is ignored. Flags marked with `$neededByFl` in
Expand Down
3 changes: 1 addition & 2 deletions src/workerd/io/io-context.c++
Original file line number Diff line number Diff line change
Expand Up @@ -852,8 +852,7 @@ kj::Own<WorkerInterface> IoContext::getSubrequestChannelImpl(uint channel,
IoChannelFactory& channelFactory) {
IoChannelFactory::SubrequestMetadata metadata{
.cfBlobJson = kj::mv(cfBlobJson),
.parentSpan = spans.span,
.limeParentSpan = spans.limeSpan,
.spans = spans,
.featureFlagsForFl = worker->getIsolate().getFeatureFlagsForFl(),
};

Expand Down
25 changes: 20 additions & 5 deletions src/workerd/io/trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -448,10 +448,9 @@ inline kj::String truncateScriptId(kj::StringPtr id) {
// is currently designed to feed tracing of the Workers Runtime itself for the benefit of the
// developers of the runtime.
//
// However, we might potentially want to give trace workers some access to span tracing as well.
// But, that hasn't been designed yet, and it's not clear if that would be based on the same
// concept of spans or completely separate. In the latter case, these classes should probably
// move to a different header.
// We might potentially want to give trace workers some access to span tracing as well, but with
// that the trace worker and span interfaces should still be largely independent of each other;
// separate span tracing into a separate header.

class SpanBuilder;
class SpanBuilderBase;
Expand Down Expand Up @@ -739,7 +738,7 @@ inline LimeSpanBuilder LimeSpanBuilder::newLimeChild(kj::ConstString operationNa

// Interface to track trace context including both Jaeger and Lime spans.
// TODO(o11y): Consider fleshing this out to make it a proper class, support adding tags/child spans
// to both, ... we expect that tracking lime spans will not needed in all places where we have the
// to both,... We expect that tracking lime spans will not needed in all places where we have the
// existing spans, so synergies will likely be limited.
struct TraceContext {
TraceContext(SpanBuilder span, LimeSpanBuilder limeSpan)
Expand All @@ -753,4 +752,20 @@ struct TraceContext {
LimeSpanBuilder limeSpan;
};

// TraceContext variant tracking span parents instead. This is useful for code interacting with
// IoChannelFactory::SubrequestMetadata, which often needs to pass through both spans together
// without modifying them.
struct TraceParentContext {
TraceParentContext(TraceContext& spans): parentSpan(spans.span), limeParentSpan(spans.limeSpan) {}
TraceParentContext(SpanParent span, LimeSpanParent limeSpan)
: parentSpan(kj::mv(span)),
limeParentSpan(kj::mv(limeSpan)) {}
TraceParentContext(TraceParentContext&& other) = default;
TraceParentContext& operator=(TraceParentContext&& other) = default;
KJ_DISALLOW_COPY(TraceParentContext);

SpanParent parentSpan;
LimeSpanParent limeParentSpan;
};

} // namespace workerd
3 changes: 2 additions & 1 deletion src/workerd/server/server.c++
Original file line number Diff line number Diff line change
Expand Up @@ -2138,7 +2138,8 @@ private:
kj::Maybe<kj::String> cacheName,
kj::Maybe<kj::String> cfBlobJson,
SpanParent parentSpan)
: client(asHttpClient(parent.startRequest({kj::mv(cfBlobJson), kj::mv(parentSpan)}))),
: client(asHttpClient(parent.startRequest(
{kj::mv(cfBlobJson), TraceParentContext(kj::mv(parentSpan), nullptr)}))),
cacheName(kj::mv(cacheName)),
cacheNamespaceHeader(cacheNamespaceHeader) {}

Expand Down

0 comments on commit 98cb057

Please sign in to comment.