Skip to content

Commit

Permalink
Revert "Move kj::Own<WorkerTracer> to kj::Rc<WorkerTracer>"
Browse files Browse the repository at this point in the history
This reverts commit f2e1730.
  • Loading branch information
jasnell committed Oct 22, 2024
1 parent 416ff97 commit 52b29ed
Show file tree
Hide file tree
Showing 12 changed files with 21 additions and 26 deletions.
4 changes: 2 additions & 2 deletions src/workerd/api/hibernatable-web-socket.c++
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ kj::Promise<WorkerInterface::CustomEvent::Result> HibernatableWebSocketCustomEve

auto eventParameters = consumeParams();

KJ_IF_SOME(tracer, incomingRequest->getMetrics().getWorkerTracer()) {
KJ_IF_SOME(t, incomingRequest->getMetrics().getWorkerTracer()) {
Trace::HibernatableWebSocketEventInfo::Type type =
[&]() -> Trace::HibernatableWebSocketEventInfo::Type {
KJ_SWITCH_ONEOF(eventParameters.eventType) {
Expand All @@ -95,7 +95,7 @@ kj::Promise<WorkerInterface::CustomEvent::Result> HibernatableWebSocketCustomEve
KJ_UNREACHABLE;
}();

tracer->setEventInfo(context.now(), Trace::HibernatableWebSocketEventInfo(kj::mv(type)));
t.setEventInfo(context.now(), Trace::HibernatableWebSocketEventInfo(kj::mv(type)));
}

try {
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/node/diagnostics-channel.c++
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ void Channel::publish(jsg::Lock& js, jsg::Value message) {
Error,
"Diagnostic events cannot be published with SharedArrayBuffer or "
"transferred ArrayBuffer instances");
tracer->addDiagnosticChannelEvent(context.now(), name.toString(js), kj::mv(tmp.data));
tracer.addDiagnosticChannelEvent(context.now(), name.toString(js), kj::mv(tmp.data));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/node/util.c++
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ namespace {
// JSG_KJ_EXCEPTION would not give us that, and we only want to incur the cost
// of creating and capturing the stack when we actually need it.
auto ex = KJ_ASSERT_NONNULL(js.error(message).tryCast<jsg::JsObject>());
tracer->addException(ioContext.now(), ex.get(js, "name"_kj).toString(js),
tracer.addException(ioContext.now(), ex.get(js, "name"_kj).toString(js),
ex.get(js, "message"_kj).toString(js), ex.get(js, "stack"_kj).toString(js));
ioContext.abort(js.exceptionToKj(ex));
} else {
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/api/queue.c++
Original file line number Diff line number Diff line change
Expand Up @@ -530,8 +530,8 @@ kj::Promise<WorkerInterface::CustomEvent::Result> QueueCustomEventImpl::run(
}
}

KJ_IF_SOME(tracer, incomingRequest->getMetrics().getWorkerTracer()) {
tracer->setEventInfo(context.now(), Trace::QueueEventInfo(kj::mv(queueName), batchSize));
KJ_IF_SOME(t, incomingRequest->getMetrics().getWorkerTracer()) {
t.setEventInfo(context.now(), Trace::QueueEventInfo(kj::mv(queueName), batchSize));
}

// Create a custom refcounted type for holding the queueEvent so that we can pass it to the
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/api/trace.c++
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,8 @@ kj::Promise<void> sendTracesToExportedHandler(kj::Own<IoContext::IncomingRequest
auto& context = incomingRequest->getContext();
auto& metrics = incomingRequest->getMetrics();

KJ_IF_SOME(tracer, incomingRequest->getMetrics().getWorkerTracer()) {
tracer->setEventInfo(context.now(), Trace::TraceEventInfo(traces));
KJ_IF_SOME(t, incomingRequest->getMetrics().getWorkerTracer()) {
t.setEventInfo(context.now(), Trace::TraceEventInfo(traces));
}

auto nonEmptyTraces = kj::Vector<kj::Own<Trace>>(kj::size(traces));
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/api/worker-rpc.c++
Original file line number Diff line number Diff line change
Expand Up @@ -1666,8 +1666,8 @@ private:
}

void addTrace(jsg::Lock& js, IoContext& ioctx, kj::StringPtr methodName) override {
KJ_IF_SOME(tracer, ioctx.getMetrics().getWorkerTracer()) {
tracer->setEventInfo(ioctx.now(), Trace::JsRpcEventInfo(kj::str(methodName)));
KJ_IF_SOME(t, ioctx.getMetrics().getWorkerTracer()) {
t.setEventInfo(ioctx.now(), Trace::JsRpcEventInfo(kj::str(methodName)));
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/io/observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class RequestObserver: public kj::Refcounted {
return nullptr;
}

virtual kj::Maybe<kj::Rc<WorkerTracer>> getWorkerTracer() {
virtual kj::Maybe<WorkerTracer&> getWorkerTracer() {
return kj::none;
}

Expand Down
4 changes: 2 additions & 2 deletions src/workerd/io/trace.c++
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ kj::Promise<kj::Array<kj::Own<Trace>>> PipelineTracer::onComplete() {
return kj::mv(paf.promise);
}

kj::Rc<WorkerTracer> PipelineTracer::makeWorkerTracer(PipelineLogLevel pipelineLogLevel,
kj::Own<WorkerTracer> PipelineTracer::makeWorkerTracer(PipelineLogLevel pipelineLogLevel,
ExecutionModel executionModel,
kj::Maybe<kj::String> scriptId,
kj::Maybe<kj::String> stableId,
Expand All @@ -591,7 +591,7 @@ kj::Rc<WorkerTracer> PipelineTracer::makeWorkerTracer(PipelineLogLevel pipelineL
kj::mv(dispatchNamespace), kj::mv(scriptId), kj::mv(scriptTags), kj::mv(entrypoint),
executionModel);
traces.add(kj::addRef(*trace));
return kj::rc<WorkerTracer>(addRefToThis(), kj::mv(trace), pipelineLogLevel);
return kj::refcounted<WorkerTracer>(kj::addRef(*this), kj::mv(trace), pipelineLogLevel);
}

void PipelineTracer::addTrace(rpc::Trace::Reader reader) {
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/io/trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ class PipelineTracer final: public kj::Refcounted, public kj::EnableAddRefToThis
kj::Promise<kj::Array<kj::Own<Trace>>> onComplete();

// Makes a tracer for a worker stage.
kj::Rc<WorkerTracer> makeWorkerTracer(PipelineLogLevel pipelineLogLevel,
kj::Own<WorkerTracer> makeWorkerTracer(PipelineLogLevel pipelineLogLevel,
ExecutionModel executionModel,
kj::Maybe<kj::String> scriptId,
kj::Maybe<kj::String> stableId,
Expand Down
8 changes: 4 additions & 4 deletions src/workerd/io/worker-entrypoint.c++
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ kj::Promise<void> WorkerEntrypoint::request(kj::HttpMethod method,

bool isActor = context.getActor() != kj::none;

KJ_IF_SOME(tracer, incomingRequest->getMetrics().getWorkerTracer()) {
KJ_IF_SOME(t, incomingRequest->getMetrics().getWorkerTracer()) {
auto timestamp = context.now();
kj::String cfJson;
KJ_IF_SOME(c, cfBlobJson) {
Expand All @@ -253,7 +253,7 @@ kj::Promise<void> WorkerEntrypoint::request(kj::HttpMethod method,
return Trace::FetchEventInfo::Header(kj::mv(entry.key), kj::strArray(entry.value, ", "));
};

tracer->setEventInfo(timestamp,
t.setEventInfo(timestamp,
Trace::FetchEventInfo(method, kj::str(url), kj::mv(cfJson), kj::mv(traceHeadersArray)));
}

Expand Down Expand Up @@ -474,7 +474,7 @@ kj::Promise<WorkerInterface::ScheduledResult> WorkerEntrypoint::runScheduled(

KJ_IF_SOME(t, context.getMetrics().getWorkerTracer()) {
double eventTime = (scheduledTime - kj::UNIX_EPOCH) / kj::MILLISECONDS;
t->setEventInfo(context.now(), Trace::ScheduledEventInfo(eventTime, kj::str(cron)));
t.setEventInfo(context.now(), Trace::ScheduledEventInfo(eventTime, kj::str(cron)));
}

// Scheduled handlers run entirely in waitUntil() tasks.
Expand Down Expand Up @@ -532,7 +532,7 @@ kj::Promise<WorkerInterface::AlarmResult> WorkerEntrypoint::runAlarmImpl(
incomingRequest->delivered();

KJ_IF_SOME(t, incomingRequest->getMetrics().getWorkerTracer()) {
t->setEventInfo(context.now(), Trace::AlarmEventInfo(scheduledTime));
t.setEventInfo(context.now(), Trace::AlarmEventInfo(scheduledTime));
}

auto scheduleAlarmResult = co_await actor.scheduleAlarm(scheduledTime);
Expand Down
6 changes: 3 additions & 3 deletions src/workerd/io/worker.c++
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ void sendExceptionToInspector(jsg::Lock& js,

void addExceptionToTrace(jsg::Lock& js,
IoContext& ioContext,
kj::Rc<WorkerTracer>& tracer,
WorkerTracer& tracer,
UncaughtExceptionSource source,
const jsg::JsValue& exception,
const jsg::TypeHandler<Worker::Api::ErrorInterface>& errorTypeHandler) {
Expand Down Expand Up @@ -214,7 +214,7 @@ void addExceptionToTrace(jsg::Lock& js,
}

// TODO(someday): Limit size of exception content?
tracer->addException(timestamp, kj::mv(name), kj::mv(message), kj::mv(stack));
tracer.addException(timestamp, kj::mv(name), kj::mv(message), kj::mv(stack));
}

void reportStartupError(kj::StringPtr id,
Expand Down Expand Up @@ -1842,7 +1842,7 @@ void Worker::handleLog(jsg::Lock& js,
auto& ioContext = IoContext::current();
KJ_IF_SOME(tracer, ioContext.getMetrics().getWorkerTracer()) {
auto timestamp = ioContext.now();
tracer->log(timestamp, level, message());
tracer.log(timestamp, level, message());
}
}

Expand Down
5 changes: 0 additions & 5 deletions src/workerd/util/own-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ inline auto mapAddRef(kj::Maybe<kj::Own<T>>& maybe) -> kj::Maybe<kj::Own<T>> {
return maybe.map([](kj::Own<T>& t) { return kj::addRef(*t); });
}

template <typename T>
inline auto mapAddRef(kj::Maybe<kj::Rc<T>>& maybe) -> kj::Maybe<kj::Rc<T>> {
return maybe.map([](kj::Rc<T>& t) { return t.addRef(); });
}

template <typename T>
inline auto mapAddRef(kj::Maybe<T&> maybe) -> kj::Maybe<kj::Own<T>> {
return maybe.map([](T& t) { return kj::addRef(t); });
Expand Down

0 comments on commit 52b29ed

Please sign in to comment.