Skip to content

Add preview metadata to buffered tail traces #6375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Ankcorn wants to merge 1 commit into
base: main
Choose a base branch
from
+94 −1
Open

Add preview metadata to buffered tail traces #6375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/workerd/api/trace.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,12 @@ TraceItem::TraceItem(jsg::Lock& js, const Trace& trace)
scriptTags(getTraceScriptTags(trace)),
tailAttributes(trace.tailAttributes.map(
[](auto& tags) { return KJ_MAP(tag, tags) { return tag.clone(); }; })),
preview(trace.preview.map([](auto& p) {
return jsg::Dict<kj::String>{.fields = kj::arr(
jsg::Dict<kj::String>::Field{.name = kj::str("id"), .value = kj::str(p.id)},
jsg::Dict<kj::String>::Field{.name = kj::str("slug"), .value = kj::str(p.slug)},
jsg::Dict<kj::String>::Field{.name = kj::str("name"), .value = kj::str(p.name)})};
})),
durableObjectId(mapCopyString(trace.durableObjectId)),
executionModel(kj::str(trace.executionModel)),
outcome(kj::str(trace.outcome)),
Expand Down Expand Up @@ -306,6 +312,14 @@ jsg::Optional<jsg::Dict<TraceItem::TailAttributeValue>> TraceItem::getTailAttrib
});
}

jsg::Optional<jsg::Dict<kj::String>> TraceItem::getPreview() {
return preview.map([](const jsg::Dict<kj::String>& p) {
return jsg::Dict<kj::String>{.fields = KJ_MAP(field, p.fields) {
return jsg::Dict<kj::String>::Field{.name = kj::str(field.name), .value = kj::str(field.value)};
}};
});
}

jsg::Optional<kj::StringPtr> TraceItem::getDurableObjectId() {
return durableObjectId.map([](auto& id) -> kj::StringPtr { return id; });
}
Expand Down
3 changes: 3 additions & 0 deletions src/workerd/api/trace.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ class TraceItem final: public jsg::Object {
jsg::Optional<ScriptVersion> getScriptVersion();
jsg::Optional<kj::StringPtr> getDispatchNamespace();
jsg::Optional<kj::Array<kj::StringPtr>> getScriptTags();
jsg::Optional<jsg::Dict<kj::String>> getPreview();
Copy link
Contributor

@jp4a50 jp4a50 Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should make this a JSG struct instead of a JSG dict. I think dicts are meant to be for when you don't know the keys at compile time. See getScriptVersion for inspiration.

jsg::Optional<jsg::Dict<TailAttributeValue>> getTailAttributes();
jsg::Optional<kj::StringPtr> getDurableObjectId();
kj::StringPtr getExecutionModel();
Expand All @@ -124,6 +125,7 @@ class TraceItem final: public jsg::Object {
JSG_LAZY_READONLY_INSTANCE_PROPERTY(dispatchNamespace, getDispatchNamespace);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(scriptTags, getScriptTags);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(tailAttributes, getTailAttributes);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(preview, getPreview);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(durableObjectId, getDurableObjectId);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(outcome, getOutcome);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(executionModel, getExecutionModel);
Expand All @@ -146,6 +148,7 @@ class TraceItem final: public jsg::Object {
kj::Maybe<kj::String> dispatchNamespace;
jsg::Optional<kj::Array<kj::String>> scriptTags;
kj::Maybe<kj::Array<tracing::Attribute>> tailAttributes;
jsg::Optional<jsg::Dict<kj::String>> preview;
kj::Maybe<kj::String> durableObjectId;
kj::String executionModel;
kj::String outcome;
Expand Down
21 changes: 21 additions & 0 deletions src/workerd/io/trace-test.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,12 +577,33 @@ KJ_TEST("Read/Write TailEvent with Multiple Attributes") {
KJ_ASSERT(attrs2[1].name == "bar"_kj);
}

KJ_TEST("Trace with Preview") {
auto trace = kj::refcounted<Trace>(kj::str("test-stable-id"), kj::str("test-script"),
kj::none, // scriptVersion
kj::str("test-namespace"), kj::str("test-script-id"),
kj::Array<kj::String>(), // scriptTags
kj::str("test-entrypoint"), ExecutionModel::STATELESS,
TracePreview(kj::str("63bafce9179948688866bb22268eb1c6"), kj::str("feature-my-branch"),
kj::str("feature/my-branch")));

capnp::MallocMessageBuilder builder;
auto traceBuilder = builder.initRoot<rpc::Trace>();
trace->copyTo(traceBuilder);

auto trace2 = kj::refcounted<Trace>(traceBuilder.asReader());
auto& preview = KJ_ASSERT_NONNULL(trace2->preview);
KJ_ASSERT(preview.id == "63bafce9179948688866bb22268eb1c6"_kj);
KJ_ASSERT(preview.slug == "feature-my-branch"_kj);
KJ_ASSERT(preview.name == "feature/my-branch"_kj);
}

KJ_TEST("Trace with Durable Object ID") {
auto trace = kj::refcounted<Trace>(kj::str("test-stable-id"), kj::str("test-script"),
kj::none, // scriptVersion
kj::str("test-namespace"), kj::str("test-script-id"),
kj::Array<kj::String>(), // scriptTags
kj::str("test-entrypoint"), ExecutionModel::DURABLE_OBJECT,
kj::none, // preview
kj::str("abc123def456") // durableObjectId
);

Expand Down
33 changes: 32 additions & 1 deletion src/workerd/io/trace.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,11 +498,32 @@ TraceEventInfo TraceEventInfo::clone() const {
return TraceEventInfo(KJ_MAP(item, traces) { return item.clone(); });
}

TracePreview::TracePreview(kj::String id, kj::String slug, kj::String name)
: id(kj::mv(id)),
slug(kj::mv(slug)),
name(kj::mv(name)) {}

TracePreview::TracePreview(rpc::Trace::Preview::Reader reader)
: id(kj::str(reader.getId())),
slug(kj::str(reader.getSlug())),
name(kj::str(reader.getName())) {}

void TracePreview::copyTo(rpc::Trace::Preview::Builder builder) const {
builder.setId(id);
builder.setSlug(slug);
builder.setName(name);
}

TracePreview TracePreview::clone() const {
return TracePreview(kj::str(id), kj::str(slug), kj::str(name));
}

TraceEventInfo::TraceItem::TraceItem(kj::Maybe<kj::String> scriptName)
: scriptName(kj::mv(scriptName)) {}

TraceEventInfo::TraceItem::TraceItem(rpc::Trace::TraceEventInfo::TraceItem::Reader reader)
: scriptName(kj::str(reader.getScriptName())) {}
: scriptName(reader.hasScriptName() ? kj::Maybe<kj::String>(kj::str(reader.getScriptName()))
Copy link
Author

@Ankcorn Ankcorn Mar 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opencode drive by changed this because it thinks the old implementation has a bug...

IMO seems suss and I'm going to revert this line but hello @danlapid I just wanted to flag it to you incase opencode smarter than me

Copy link
Contributor

@jp4a50 jp4a50 Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's right. The previous code didn't preserve the emptiness of the scriptName capnp field during a serialization round trip (which seems to be used by TraceCustomEvent::sendRpc method for sending traces over an RPC interface).

I think this matters in practice too since the worker may be deployed without a script name in some cases. I suspect the failures you're seeing on the internal build are due to this changing behaviour in cases where script name is empty. It likely results in a trace item having a null scriptName instead of empty string. We might need to put the change behind a compatibility date if so, because even though it's a bug fix it's a change in API behaviour.

: kj::none) {}

void TraceEventInfo::TraceItem::copyTo(
rpc::Trace::TraceEventInfo::TraceItem::Builder builder) const {
Expand Down Expand Up @@ -697,6 +718,7 @@ Trace::Trace(kj::Maybe<kj::String> stableId,
kj::Array<kj::String> scriptTags,
kj::Maybe<kj::String> entrypoint,
ExecutionModel executionModel,
kj::Maybe<tracing::TracePreview> preview,
kj::Maybe<kj::String> durableObjectId)
: stableId(kj::mv(stableId)),
scriptName(kj::mv(scriptName)),
Expand All @@ -705,6 +727,7 @@ Trace::Trace(kj::Maybe<kj::String> stableId,
scriptId(kj::mv(scriptId)),
scriptTags(kj::mv(scriptTags)),
entrypoint(kj::mv(entrypoint)),
preview(kj::mv(preview)),
durableObjectId(kj::mv(durableObjectId)),
executionModel(executionModel) {}
Trace::Trace(rpc::Trace::Reader reader) {
Expand Down Expand Up @@ -764,6 +787,10 @@ void Trace::copyTo(rpc::Trace::Builder builder) const {
builder.setEntrypoint(e);
}

KJ_IF_SOME(p, preview) {
p.copyTo(builder.initPreview());
}

KJ_IF_SOME(id, durableObjectId) {
builder.setDurableObjectId(id);
}
Expand Down Expand Up @@ -874,6 +901,10 @@ void Trace::mergeFrom(rpc::Trace::Reader reader, PipelineLogLevel pipelineLogLev
entrypoint = kj::str(reader.getEntrypoint());
}

if (reader.hasPreview()) {
preview = tracing::TracePreview(reader.getPreview());
}

if (reader.hasDurableObjectId()) {
durableObjectId = kj::str(reader.getDurableObjectId());
}
Expand Down
17 changes: 17 additions & 0 deletions src/workerd/io/trace.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,21 @@ struct EmailEventInfo final {
};

// Describes a buffered tail worker request
struct TracePreview final {
explicit TracePreview(kj::String id, kj::String slug, kj::String name);
TracePreview(rpc::Trace::Preview::Reader reader);
TracePreview(TracePreview&&) = default;
TracePreview& operator=(TracePreview&&) = default;
KJ_DISALLOW_COPY(TracePreview);

kj::String id;
kj::String slug;
kj::String name;

void copyTo(rpc::Trace::Preview::Builder builder) const;
TracePreview clone() const;
};

struct TraceEventInfo final {
struct TraceItem;

Expand Down Expand Up @@ -885,6 +900,7 @@ class Trace final: public kj::Refcounted {
kj::Array<kj::String> scriptTags,
kj::Maybe<kj::String> entrypoint,
ExecutionModel executionModel,
kj::Maybe<tracing::TracePreview> preview = kj::none,
kj::Maybe<kj::String> durableObjectId = kj::none);
Trace(rpc::Trace::Reader reader);
~Trace() noexcept(false);
Expand All @@ -908,6 +924,7 @@ class Trace final: public kj::Refcounted {
kj::Array<kj::String> scriptTags;
kj::Maybe<kj::Array<tracing::Attribute>> tailAttributes;
kj::Maybe<kj::String> entrypoint;
kj::Maybe<tracing::TracePreview> preview;
kj::Maybe<kj::String> durableObjectId;

kj::Vector<tracing::Log> logs;
Expand Down
7 changes: 7 additions & 0 deletions src/workerd/io/worker-interface.capnp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,12 @@ struct Trace @0x8e8d911203762d34 {
rawSize @2 :UInt32;
}

struct Preview {
id @0 :Text;
slug @1 :Text;
name @2 :Text;
}

struct TraceEventInfo {
struct TraceItem {
scriptName @0 :Text;
Expand Down Expand Up @@ -167,6 +173,7 @@ struct Trace @0x8e8d911203762d34 {
scriptTags @14 :List(Text);

entrypoint @22 :Text;
preview @29 :Preview;
durableObjectId @27 :Text;
tailAttributes @28 :List(Attribute);

Expand Down
Loading