Skip to content

Commit

Permalink
Merge pull request #1445 from cloudflare/jphillips/trace-script-version
Browse files Browse the repository at this point in the history
Add script version to worker trace events.
  • Loading branch information
jp4a50 authored Nov 30, 2023
2 parents c34d69e + b7e14e6 commit d794a1a
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 12 deletions.
5 changes: 5 additions & 0 deletions src/workerd/api/trace.c++
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ TraceItem::TraceItem(jsg::Lock& js, const Trace& trace)
exceptions(getTraceExceptions(trace)),
diagnosticChannelEvents(getTraceDiagnosticChannelEvents(js, trace)),
scriptName(trace.scriptName.map([](auto& name) { return kj::str(name); })),
scriptVersionId(trace.scriptVersionId.map([](auto& id) { return kj::str(id); })),
dispatchNamespace(trace.dispatchNamespace.map([](auto& ns) { return kj::str(ns); })),
scriptTags(getTraceScriptTags(trace)),
outcome(getTraceOutcome(trace)),
Expand Down Expand Up @@ -213,6 +214,10 @@ kj::Maybe<kj::StringPtr> TraceItem::getScriptName() {
return scriptName.map([](auto& name) -> kj::StringPtr { return name; });
}

jsg::Optional<kj::StringPtr> TraceItem::getScriptVersionId() {
return scriptVersionId.map([](auto& scriptVersionId) -> kj::StringPtr { return scriptVersionId; });
}

jsg::Optional<kj::StringPtr> TraceItem::getDispatchNamespace() {
return dispatchNamespace.map([](auto& ns) -> kj::StringPtr { return ns; });
}
Expand Down
7 changes: 7 additions & 0 deletions src/workerd/api/trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ class TraceItem final: public jsg::Object {
kj::ArrayPtr<jsg::Ref<TraceException>> getExceptions();
kj::ArrayPtr<jsg::Ref<TraceDiagnosticChannelEvent>> getDiagnosticChannelEvents();
kj::Maybe<kj::StringPtr> getScriptName();
// TODO(someday): we expose this as jsg::Optional for now, since that ensures that the property will
// not show up when calling JSON.stringify() on the object if it has not been explicitly set.
// At some point, we may want to align the behaviour with getScriptName() which shows up as
// `null` when not explicitly set.
jsg::Optional<kj::StringPtr> getScriptVersionId();
jsg::Optional<kj::StringPtr> getDispatchNamespace();
jsg::Optional<kj::Array<kj::StringPtr>> getScriptTags();
kj::StringPtr getOutcome();
Expand All @@ -85,6 +90,7 @@ class TraceItem final: public jsg::Object {
JSG_LAZY_READONLY_INSTANCE_PROPERTY(exceptions, getExceptions);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(diagnosticsChannelEvents, getDiagnosticChannelEvents);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(scriptName, getScriptName);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(scriptVersionId, getScriptVersionId);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(dispatchNamespace, getDispatchNamespace);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(scriptTags, getScriptTags);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(outcome, getOutcome);
Expand All @@ -97,6 +103,7 @@ class TraceItem final: public jsg::Object {
kj::Array<jsg::Ref<TraceException>> exceptions;
kj::Array<jsg::Ref<TraceDiagnosticChannelEvent>> diagnosticChannelEvents;
kj::Maybe<kj::String> scriptName;
jsg::Optional<kj::String> scriptVersionId;
kj::Maybe<kj::String> dispatchNamespace;
jsg::Optional<kj::Array<kj::String>> scriptTags;
kj::String outcome;
Expand Down
34 changes: 23 additions & 11 deletions src/workerd/io/trace.c++
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,11 @@ Trace::Exception::Exception(kj::Date timestamp, kj::String name, kj::String mess
: timestamp(timestamp), name(kj::mv(name)), message(kj::mv(message)) {}

Trace::Trace(kj::Maybe<kj::String> stableId, kj::Maybe<kj::String> scriptName,
kj::Maybe<kj::String> dispatchNamespace, kj::Array<kj::String> scriptTags)
kj::Maybe<kj::String> scriptVersionId, kj::Maybe<kj::String> dispatchNamespace,
kj::Array<kj::String> scriptTags)
: stableId(kj::mv(stableId)),
scriptName(kj::mv(scriptName)),
scriptVersionId(kj::mv(scriptVersionId)),
dispatchNamespace(kj::mv(dispatchNamespace)),
scriptTags(kj::mv(scriptTags)) {}
Trace::Trace(rpc::Trace::Reader reader) {
Expand All @@ -214,11 +216,14 @@ void Trace::copyTo(rpc::Trace::Builder builder) {
builder.setOutcome(outcome);
builder.setCpuTime(cpuTime / kj::MILLISECONDS);
builder.setWallTime(wallTime / kj::MILLISECONDS);
KJ_IF_SOME(s, scriptName) {
builder.setScriptName(s);
KJ_IF_SOME(name, scriptName) {
builder.setScriptName(name);
}
KJ_IF_SOME(id, scriptVersionId) {
builder.setScriptVersionId(id);
}
KJ_IF_SOME(s, dispatchNamespace) {
builder.setDispatchNamespace(s);
KJ_IF_SOME(ns, dispatchNamespace) {
builder.setDispatchNamespace(ns);
}

{
Expand Down Expand Up @@ -296,13 +301,18 @@ void Trace::mergeFrom(rpc::Trace::Reader reader, PipelineLogLevel pipelineLogLev
wallTime = reader.getWallTime() * kj::MILLISECONDS;

// mergeFrom() is called both when deserializing traces from a sandboxed
// worker and when deserializing traces sent to a sandboxed trace worker. In
// the former case, the trace's scriptName is already set and the deserialized
// value is missing, so we need to be careful not to overwrite the set value.
// worker and when deserializing traces sent to a sandboxed trace worker. In
// the former case, the trace's scriptName (and other fields like
// scriptVersionId) are already set and the deserialized value is missing, so
// we need to be careful not to overwrite the set value.
if (reader.hasScriptName()) {
scriptName = kj::str(reader.getScriptName());
}

if (reader.hasScriptVersionId()) {
scriptVersionId = kj::str(reader.getScriptVersionId());
}

if (reader.hasDispatchNamespace()) {
dispatchNamespace = kj::str(reader.getDispatchNamespace());
}
Expand Down Expand Up @@ -438,8 +448,10 @@ kj::Promise<kj::Array<kj::Own<Trace>>> PipelineTracer::onComplete() {

kj::Own<WorkerTracer> PipelineTracer::makeWorkerTracer(
PipelineLogLevel pipelineLogLevel, kj::Maybe<kj::String> stableId,
kj::Maybe<kj::String> scriptName, kj::Maybe<kj::String> dispatchNamespace, kj::Array<kj::String> scriptTags) {
auto trace = kj::refcounted<Trace>(kj::mv(stableId), kj::mv(scriptName), kj::mv(dispatchNamespace), kj::mv(scriptTags));
kj::Maybe<kj::String> scriptName, kj::Maybe<kj::String> scriptVersionId,
kj::Maybe<kj::String> dispatchNamespace, kj::Array<kj::String> scriptTags) {
auto trace = kj::refcounted<Trace>(kj::mv(stableId), kj::mv(scriptName), kj::mv(scriptVersionId),
kj::mv(dispatchNamespace), kj::mv(scriptTags));
traces.add(kj::addRef(*trace));
return kj::refcounted<WorkerTracer>(kj::addRef(*this), kj::mv(trace), pipelineLogLevel);
}
Expand All @@ -450,7 +462,7 @@ WorkerTracer::WorkerTracer(kj::Own<PipelineTracer> parentPipeline,
parentPipeline(kj::mv(parentPipeline)) {}
WorkerTracer::WorkerTracer(PipelineLogLevel pipelineLogLevel)
: pipelineLogLevel(pipelineLogLevel),
trace(kj::refcounted<Trace>(nullptr, nullptr, nullptr, nullptr)) {}
trace(kj::refcounted<Trace>(kj::none, kj::none, kj::none, kj::none, nullptr)) {}

void WorkerTracer::log(kj::Date timestamp, LogLevel logLevel, kj::String message) {
if (trace->exceededLogLimit) {
Expand Down
6 changes: 5 additions & 1 deletion src/workerd/io/trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ enum class PipelineLogLevel {
// Collects trace information about the handling of a worker/pipeline fetch event.
class Trace final : public kj::Refcounted {
public:
explicit Trace(kj::Maybe<kj::String> stableId, kj::Maybe<kj::String> scriptName, kj::Maybe<kj::String> dispatchNamespace, kj::Array<kj::String> scriptTags);
explicit Trace(kj::Maybe<kj::String> stableId, kj::Maybe<kj::String> scriptName,
kj::Maybe<kj::String> scriptVersionId, kj::Maybe<kj::String> dispatchNamespace,
kj::Array<kj::String> scriptTags);
Trace(rpc::Trace::Reader reader);
~Trace() noexcept(false);
KJ_DISALLOW_COPY_AND_MOVE(Trace);
Expand Down Expand Up @@ -231,6 +233,7 @@ class Trace final : public kj::Refcounted {
// trace, if any.

kj::Maybe<kj::String> scriptName;
kj::Maybe<kj::String> scriptVersionId;
kj::Maybe<kj::String> dispatchNamespace;
kj::Array<kj::String> scriptTags;

Expand Down Expand Up @@ -292,6 +295,7 @@ class PipelineTracer final : public kj::Refcounted {
kj::Own<WorkerTracer> makeWorkerTracer(PipelineLogLevel pipelineLogLevel,
kj::Maybe<kj::String> stableId,
kj::Maybe<kj::String> scriptName,
kj::Maybe<kj::String> scriptVersionId,
kj::Maybe<kj::String> dispatchNamespace,
kj::Array<kj::String> scriptTags);
// Makes a tracer for a worker stage.
Expand Down
1 change: 1 addition & 0 deletions src/workerd/io/worker-interface.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ struct Trace @0x8e8d911203762d34 {

outcome @2 :EventOutcome;
scriptName @4 :Text;
scriptVersionId @19 :Text;

eventTimestampNs @5 :Int64;

Expand Down

0 comments on commit d794a1a

Please sign in to comment.