From b7e14e605fea454073952d7e30b52ca47c5a833c Mon Sep 17 00:00:00 2001 From: Jon Phillips Date: Tue, 28 Nov 2023 13:52:58 +0000 Subject: [PATCH] Add script version to worker trace events. --- src/workerd/api/trace.c++ | 5 ++++ src/workerd/api/trace.h | 7 ++++++ src/workerd/io/trace.c++ | 34 ++++++++++++++++++--------- src/workerd/io/trace.h | 6 ++++- src/workerd/io/worker-interface.capnp | 1 + 5 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/workerd/api/trace.c++ b/src/workerd/api/trace.c++ index aac5d794fa9..afc9e1ba02b 100644 --- a/src/workerd/api/trace.c++ +++ b/src/workerd/api/trace.c++ @@ -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)), @@ -213,6 +214,10 @@ kj::Maybe TraceItem::getScriptName() { return scriptName.map([](auto& name) -> kj::StringPtr { return name; }); } +jsg::Optional TraceItem::getScriptVersionId() { + return scriptVersionId.map([](auto& scriptVersionId) -> kj::StringPtr { return scriptVersionId; }); +} + jsg::Optional TraceItem::getDispatchNamespace() { return dispatchNamespace.map([](auto& ns) -> kj::StringPtr { return ns; }); } diff --git a/src/workerd/api/trace.h b/src/workerd/api/trace.h index 9b93320b296..144ee5c2725 100644 --- a/src/workerd/api/trace.h +++ b/src/workerd/api/trace.h @@ -71,6 +71,11 @@ class TraceItem final: public jsg::Object { kj::ArrayPtr> getExceptions(); kj::ArrayPtr> getDiagnosticChannelEvents(); kj::Maybe 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 getScriptVersionId(); jsg::Optional getDispatchNamespace(); jsg::Optional> getScriptTags(); kj::StringPtr getOutcome(); @@ -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); @@ -97,6 +103,7 @@ class TraceItem final: public jsg::Object { kj::Array> exceptions; kj::Array> diagnosticChannelEvents; kj::Maybe scriptName; + jsg::Optional scriptVersionId; kj::Maybe dispatchNamespace; jsg::Optional> scriptTags; kj::String outcome; diff --git a/src/workerd/io/trace.c++ b/src/workerd/io/trace.c++ index d770c41b667..85dd6dd4439 100644 --- a/src/workerd/io/trace.c++ +++ b/src/workerd/io/trace.c++ @@ -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 stableId, kj::Maybe scriptName, - kj::Maybe dispatchNamespace, kj::Array scriptTags) + kj::Maybe scriptVersionId, kj::Maybe dispatchNamespace, + kj::Array 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) { @@ -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); } { @@ -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()); } @@ -438,8 +448,10 @@ kj::Promise>> PipelineTracer::onComplete() { kj::Own PipelineTracer::makeWorkerTracer( PipelineLogLevel pipelineLogLevel, kj::Maybe stableId, - kj::Maybe scriptName, kj::Maybe dispatchNamespace, kj::Array scriptTags) { - auto trace = kj::refcounted(kj::mv(stableId), kj::mv(scriptName), kj::mv(dispatchNamespace), kj::mv(scriptTags)); + kj::Maybe scriptName, kj::Maybe scriptVersionId, + kj::Maybe dispatchNamespace, kj::Array scriptTags) { + auto trace = kj::refcounted(kj::mv(stableId), kj::mv(scriptName), kj::mv(scriptVersionId), + kj::mv(dispatchNamespace), kj::mv(scriptTags)); traces.add(kj::addRef(*trace)); return kj::refcounted(kj::addRef(*this), kj::mv(trace), pipelineLogLevel); } @@ -450,7 +462,7 @@ WorkerTracer::WorkerTracer(kj::Own parentPipeline, parentPipeline(kj::mv(parentPipeline)) {} WorkerTracer::WorkerTracer(PipelineLogLevel pipelineLogLevel) : pipelineLogLevel(pipelineLogLevel), - trace(kj::refcounted(nullptr, nullptr, nullptr, nullptr)) {} + trace(kj::refcounted(kj::none, kj::none, kj::none, kj::none, nullptr)) {} void WorkerTracer::log(kj::Date timestamp, LogLevel logLevel, kj::String message) { if (trace->exceededLogLimit) { diff --git a/src/workerd/io/trace.h b/src/workerd/io/trace.h index 68f4e12a050..51660f8e0c9 100644 --- a/src/workerd/io/trace.h +++ b/src/workerd/io/trace.h @@ -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 stableId, kj::Maybe scriptName, kj::Maybe dispatchNamespace, kj::Array scriptTags); + explicit Trace(kj::Maybe stableId, kj::Maybe scriptName, + kj::Maybe scriptVersionId, kj::Maybe dispatchNamespace, + kj::Array scriptTags); Trace(rpc::Trace::Reader reader); ~Trace() noexcept(false); KJ_DISALLOW_COPY_AND_MOVE(Trace); @@ -231,6 +233,7 @@ class Trace final : public kj::Refcounted { // trace, if any. kj::Maybe scriptName; + kj::Maybe scriptVersionId; kj::Maybe dispatchNamespace; kj::Array scriptTags; @@ -292,6 +295,7 @@ class PipelineTracer final : public kj::Refcounted { kj::Own makeWorkerTracer(PipelineLogLevel pipelineLogLevel, kj::Maybe stableId, kj::Maybe scriptName, + kj::Maybe scriptVersionId, kj::Maybe dispatchNamespace, kj::Array scriptTags); // Makes a tracer for a worker stage. diff --git a/src/workerd/io/worker-interface.capnp b/src/workerd/io/worker-interface.capnp index 4f78591a96c..9df7f2c784a 100644 --- a/src/workerd/io/worker-interface.capnp +++ b/src/workerd/io/worker-interface.capnp @@ -39,6 +39,7 @@ struct Trace @0x8e8d911203762d34 { outcome @2 :EventOutcome; scriptName @4 :Text; + scriptVersionId @19 :Text; eventTimestampNs @5 :Int64;