Skip to content

Commit

Permalink
Replace scriptVersionId with scriptVersion object in worker trace eve…
Browse files Browse the repository at this point in the history
…nts.
  • Loading branch information
jp4a50 committed Dec 6, 2023
1 parent d8abb2b commit b9f5ecc
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 24 deletions.
30 changes: 27 additions & 3 deletions src/workerd/api/trace.c++
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ kj::Array<jsg::Ref<TraceDiagnosticChannelEvent>> getTraceDiagnosticChannelEvents
};
}

kj::Maybe<kj::Own<workerd::ScriptVersion::Reader>> getTraceScriptVersion(const Trace& trace) {
return trace.scriptVersion.map([](const auto& version) { return capnp::clone(*version); });
}

double getTraceExceptionTimestamp(const Trace::Exception& ex) {
if (isPredictableModeForTest()) {
return 0;
Expand Down Expand Up @@ -172,7 +176,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); })),
scriptVersion(getTraceScriptVersion(trace)),
dispatchNamespace(trace.dispatchNamespace.map([](auto& ns) { return kj::str(ns); })),
scriptTags(getTraceScriptTags(trace)),
outcome(getTraceOutcome(trace)),
Expand Down Expand Up @@ -214,8 +218,8 @@ 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<ScriptVersion> TraceItem::getScriptVersion() {
return scriptVersion.map([](auto& version) { return ScriptVersion(*version); });
}

jsg::Optional<kj::StringPtr> TraceItem::getDispatchNamespace() {
Expand Down Expand Up @@ -416,6 +420,26 @@ jsg::JsValue TraceDiagnosticChannelEvent::getMessage(jsg::Lock& js) {

double TraceDiagnosticChannelEvent::getTimestamp() { return timestamp; }

ScriptVersion::ScriptVersion(workerd::ScriptVersion::Reader version)
: id{[&]() -> kj::Maybe<kj::String> {
if (version.hasId()) {
return kj::str(version.getId());
}
return kj::none;
}()},
tag{[&]() -> kj::Maybe<kj::String> {
if (version.hasTag()) {
return kj::str(version.getTag());
}
return kj::none;
}()},
message{[&]() -> kj::Maybe<kj::String> {
if (version.hasMessage()) {
return kj::str(version.getMessage());
}
return kj::none;
}()} {}

TraceItem::CustomEventInfo::CustomEventInfo(const Trace& trace,
const Trace::CustomEventInfo& eventInfo)
: eventInfo(eventInfo) {}
Expand Down
22 changes: 15 additions & 7 deletions src/workerd/api/trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,17 @@ class TailEvent final: public ExtendableEvent {
}
};

struct ScriptVersion {
public:
explicit ScriptVersion(workerd::ScriptVersion::Reader version);

jsg::Optional<kj::String> id;
jsg::Optional<kj::String> tag;
jsg::Optional<kj::String> message;

JSG_STRUCT(id, tag, message);
};

class TraceItem final: public jsg::Object {
public:
class FetchEventInfo;
Expand All @@ -71,11 +82,7 @@ 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<ScriptVersion> getScriptVersion();
jsg::Optional<kj::StringPtr> getDispatchNamespace();
jsg::Optional<kj::Array<kj::StringPtr>> getScriptTags();
kj::StringPtr getOutcome();
Expand All @@ -90,7 +97,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(scriptVersion, getScriptVersion);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(dispatchNamespace, getDispatchNamespace);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(scriptTags, getScriptTags);
JSG_LAZY_READONLY_INSTANCE_PROPERTY(outcome, getOutcome);
Expand All @@ -103,7 +110,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::Own<workerd::ScriptVersion::Reader>> scriptVersion;
kj::Maybe<kj::String> dispatchNamespace;
jsg::Optional<kj::Array<kj::String>> scriptTags;
kj::String outcome;
Expand Down Expand Up @@ -433,6 +440,7 @@ class TraceCustomEventImpl final: public WorkerInterface::CustomEvent {
};

#define EW_TRACE_ISOLATE_TYPES \
api::ScriptVersion, \
api::TailEvent, \
api::TraceItem, \
api::TraceItem::AlarmEventInfo, \
Expand Down
7 changes: 7 additions & 0 deletions src/workerd/io/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ wd_cc_capnp_library(
visibility = ["//visibility:public"],
deps = [
":outcome_capnp",
":script_version_capnp",
":trimmed-supported-compatibility-date",
"@capnp-cpp//src/capnp/compat:http-over-capnp_capnp",
],
Expand All @@ -150,6 +151,12 @@ wd_cc_capnp_library(
visibility = ["//visibility:public"],
)

wd_cc_capnp_library(
name = "script_version_capnp",
srcs = ["script-version.capnp"],
visibility = ["//visibility:public"],
)

[kj_test(
src = f,
deps = [
Expand Down
17 changes: 17 additions & 0 deletions src/workerd/io/script-version.capnp
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Copyright (c) 2023 Cloudflare, Inc.
# Licensed under the Apache 2.0 license found in the LICENSE file or at:
# https://opensource.org/licenses/Apache-2.0

@0xd3b6bb739ff1b77a;

using Cxx = import "/capnp/c++.capnp";
$Cxx.namespace("workerd");

struct ScriptVersion {
id @0 :Text;
# An ID that should be used to uniquely identify this version.
tag @1 :Text;
# An optional tag to associate with this version.
message @2 :Text;
# An optional message that can be used to describe this version.
}
19 changes: 10 additions & 9 deletions src/workerd/io/trace.c++
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// https://opensource.org/licenses/Apache-2.0

#include <workerd/io/trace.h>
#include <capnp/message.h>
#include <capnp/schema.h>
#include <kj/compat/http.h>
#include <kj/debug.h>
Expand Down Expand Up @@ -185,11 +186,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> scriptVersionId, kj::Maybe<kj::String> dispatchNamespace,
kj::Maybe<kj::Own<ScriptVersion::Reader>> scriptVersion, kj::Maybe<kj::String> dispatchNamespace,
kj::Array<kj::String> scriptTags)
: stableId(kj::mv(stableId)),
scriptName(kj::mv(scriptName)),
scriptVersionId(kj::mv(scriptVersionId)),
scriptVersion(kj::mv(scriptVersion)),
dispatchNamespace(kj::mv(dispatchNamespace)),
scriptTags(kj::mv(scriptTags)) {}
Trace::Trace(rpc::Trace::Reader reader) {
Expand Down Expand Up @@ -219,8 +220,8 @@ void Trace::copyTo(rpc::Trace::Builder builder) {
KJ_IF_SOME(name, scriptName) {
builder.setScriptName(name);
}
KJ_IF_SOME(id, scriptVersionId) {
builder.setScriptVersionId(id);
KJ_IF_SOME(version, scriptVersion) {
builder.setScriptVersion(*version);
}
KJ_IF_SOME(ns, dispatchNamespace) {
builder.setDispatchNamespace(ns);
Expand Down Expand Up @@ -303,14 +304,14 @@ void Trace::mergeFrom(rpc::Trace::Reader reader, PipelineLogLevel pipelineLogLev
// 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 (and other fields like
// scriptVersionId) are already set and the deserialized value is missing, so
// scriptVersion) 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.hasScriptVersion()) {
scriptVersion = capnp::clone(reader.getScriptVersion());
}

if (reader.hasDispatchNamespace()) {
Expand Down Expand Up @@ -448,9 +449,9 @@ 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> scriptVersionId,
kj::Maybe<kj::String> scriptName, kj::Maybe<kj::Own<ScriptVersion::Reader>> scriptVersion,
kj::Maybe<kj::String> dispatchNamespace, kj::Array<kj::String> scriptTags) {
auto trace = kj::refcounted<Trace>(kj::mv(stableId), kj::mv(scriptName), kj::mv(scriptVersionId),
auto trace = kj::refcounted<Trace>(kj::mv(stableId), kj::mv(scriptName), kj::mv(scriptVersion),
kj::mv(dispatchNamespace), kj::mv(scriptTags));
traces.add(kj::addRef(*trace));
return kj::refcounted<WorkerTracer>(kj::addRef(*this), kj::mv(trace), pipelineLogLevel);
Expand Down
8 changes: 4 additions & 4 deletions src/workerd/io/trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ enum class PipelineLogLevel {
class Trace final : public kj::Refcounted {
public:
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);
kj::Maybe<kj::Own<ScriptVersion::Reader>> scriptVersion,
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 @@ -233,7 +233,7 @@ class Trace final : public kj::Refcounted {
// trace, if any.

kj::Maybe<kj::String> scriptName;
kj::Maybe<kj::String> scriptVersionId;
kj::Maybe<kj::Own<ScriptVersion::Reader>> scriptVersion;
kj::Maybe<kj::String> dispatchNamespace;
kj::Array<kj::String> scriptTags;

Expand Down Expand Up @@ -295,7 +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::Own<ScriptVersion::Reader>> scriptVersion,
kj::Maybe<kj::String> dispatchNamespace,
kj::Array<kj::String> scriptTags);
// Makes a tracer for a worker stage.
Expand Down
3 changes: 2 additions & 1 deletion src/workerd/io/worker-interface.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ $Cxx.namespace("workerd::rpc");
using import "/capnp/compat/http-over-capnp.capnp".HttpMethod;
using import "/capnp/compat/http-over-capnp.capnp".HttpService;
using import "/workerd/io/outcome.capnp".EventOutcome;
using import "/workerd/io/script-version.capnp".ScriptVersion;

struct Trace @0x8e8d911203762d34 {
logs @0 :List(Log);
Expand Down Expand Up @@ -39,7 +40,7 @@ struct Trace @0x8e8d911203762d34 {

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

eventTimestampNs @5 :Int64;

Expand Down

0 comments on commit b9f5ecc

Please sign in to comment.