Skip to content

[o11y][nfc] Require getEventInfo() to be defined to ensure EventInfo is present #5798

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
fhanau wants to merge 1 commit into
base: main
Choose a base branch
from
+29 −34
Open

[o11y][nfc] Require getEventInfo() to be defined to ensure EventInfo is present #5798

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
31 changes: 15 additions & 16 deletions src/workerd/api/hibernatable-web-socket.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,26 +185,26 @@ HibernatableWebSocketCustomEvent::HibernatableWebSocketCustomEvent(
manager(manager) {}

// TODO(cleanup): Try to reduce duplication with consumeParams()
kj::Maybe<tracing::EventInfo> HibernatableWebSocketCustomEvent::getEventInfo() const {
tracing::EventInfo HibernatableWebSocketCustomEvent::getEventInfo() const {
// Try to extract event type from params if available
KJ_SWITCH_ONEOF(params) {
KJ_CASE_ONEOF(socketParams, HibernatableSocketParams) {
KJ_SWITCH_ONEOF(socketParams.eventType) {
KJ_CASE_ONEOF(text, HibernatableSocketParams::Text) {
return tracing::EventInfo(tracing::HibernatableWebSocketEventInfo(
tracing::HibernatableWebSocketEventInfo::Message()));
return tracing::HibernatableWebSocketEventInfo(
tracing::HibernatableWebSocketEventInfo::Message());
}
KJ_CASE_ONEOF(data, HibernatableSocketParams::Data) {
return tracing::EventInfo(tracing::HibernatableWebSocketEventInfo(
tracing::HibernatableWebSocketEventInfo::Message()));
return tracing::HibernatableWebSocketEventInfo(
tracing::HibernatableWebSocketEventInfo::Message());
}
KJ_CASE_ONEOF(close, HibernatableSocketParams::Close) {
return tracing::EventInfo(tracing::HibernatableWebSocketEventInfo(
tracing::HibernatableWebSocketEventInfo::Close{close.code, close.wasClean}));
return tracing::HibernatableWebSocketEventInfo(
tracing::HibernatableWebSocketEventInfo::Close{close.code, close.wasClean});
}
KJ_CASE_ONEOF(error, HibernatableSocketParams::Error) {
return tracing::EventInfo(tracing::HibernatableWebSocketEventInfo(
tracing::HibernatableWebSocketEventInfo::Error()));
return tracing::HibernatableWebSocketEventInfo(
tracing::HibernatableWebSocketEventInfo::Error());
}
}
}
Expand All @@ -214,17 +214,16 @@ kj::Maybe<tracing::EventInfo> HibernatableWebSocketCustomEvent::getEventInfo() c
switch (payload.which()) {
case rpc::HibernatableWebSocketEventMessage::Payload::TEXT:
case rpc::HibernatableWebSocketEventMessage::Payload::DATA:
return tracing::EventInfo(tracing::HibernatableWebSocketEventInfo(
tracing::HibernatableWebSocketEventInfo::Message()));
return tracing::HibernatableWebSocketEventInfo(
tracing::HibernatableWebSocketEventInfo::Message());
case rpc::HibernatableWebSocketEventMessage::Payload::CLOSE: {
auto close = payload.getClose();
return tracing::EventInfo(tracing::HibernatableWebSocketEventInfo(
tracing::HibernatableWebSocketEventInfo::Close{
close.getCode(), close.getWasClean()}));
return tracing::HibernatableWebSocketEventInfo(
tracing::HibernatableWebSocketEventInfo::Close{close.getCode(), close.getWasClean()});
}
case rpc::HibernatableWebSocketEventMessage::Payload::ERROR:
return tracing::EventInfo(tracing::HibernatableWebSocketEventInfo(
tracing::HibernatableWebSocketEventInfo::Error()));
return tracing::HibernatableWebSocketEventInfo(
tracing::HibernatableWebSocketEventInfo::Error());
}
KJ_UNREACHABLE;
}
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/hibernatable-web-socket.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class HibernatableWebSocketCustomEvent final: public WorkerInterface::CustomEven
return typeId;
}

kj::Maybe<tracing::EventInfo> getEventInfo() const override;
tracing::EventInfo getEventInfo() const override;

kj::Promise<Result> notSupported() override {
KJ_UNIMPLEMENTED("hibernatable web socket event not supported");
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/api/queue.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ StartQueueEventResponse startQueueEvent(EventTarget& globalEventTarget,

} // namespace

kj::Maybe<tracing::EventInfo> QueueCustomEvent::getEventInfo() const {
tracing::EventInfo QueueCustomEvent::getEventInfo() const {
kj::String queueName;
uint32_t batchSize;
KJ_SWITCH_ONEOF(params) {
Expand All @@ -544,7 +544,7 @@ kj::Maybe<tracing::EventInfo> QueueCustomEvent::getEventInfo() const {
}
}

return tracing::EventInfo(tracing::QueueEventInfo(kj::mv(queueName), batchSize));
return tracing::QueueEventInfo(kj::mv(queueName), batchSize);
}

kj::Promise<WorkerInterface::CustomEvent::Result> QueueCustomEvent::run(
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/queue.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ class QueueCustomEvent final: public WorkerInterface::CustomEvent, public kj::Re
return EVENT_TYPE;
}

kj::Maybe<tracing::EventInfo> getEventInfo() const override;
tracing::EventInfo getEventInfo() const override;

QueueRetryBatch getRetryBatch() const {
return {.retry = result.retryBatch.retry, .delaySeconds = result.retryBatch.delaySeconds};
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/api/trace.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,8 +639,8 @@ kj::Promise<void> sendTracesToExportedHandler(kj::Own<IoContext::IncomingRequest
}
} // namespace

kj::Maybe<tracing::EventInfo> TraceCustomEvent::getEventInfo() const {
return tracing::EventInfo(tracing::TraceEventInfo(traces));
tracing::EventInfo TraceCustomEvent::getEventInfo() const {
return tracing::TraceEventInfo(traces);
}

auto TraceCustomEvent::run(kj::Own<IoContext::IncomingRequest> incomingRequest,
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/trace.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ class TraceCustomEvent final: public WorkerInterface::CustomEvent {
return typeId;
}

kj::Maybe<tracing::EventInfo> getEventInfo() const override;
tracing::EventInfo getEventInfo() const override;

kj::Promise<Result> notSupported() override {
KJ_UNIMPLEMENTED("trace event not supported");
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/api/worker-rpc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,8 +468,8 @@ class JsRpcSessionCustomEvent final: public WorkerInterface::CustomEvent {
return typeId;
}

kj::Maybe<tracing::EventInfo> getEventInfo() const override {
return tracing::EventInfo(tracing::JsRpcEventInfo(nullptr));
tracing::EventInfo getEventInfo() const override {
return tracing::JsRpcEventInfo(nullptr);
}

rpc::JsRpcTarget::Client getCap() {
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/io/trace-stream.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -907,8 +907,8 @@ class TailStreamTarget final: public rpc::TailStreamTarget::Server {
};
} // namespace

kj::Maybe<EventInfo> TailStreamCustomEvent::getEventInfo() const {
return EventInfo(TraceEventInfo(kj::Array<TraceEventInfo::TraceItem>(nullptr)));
EventInfo TailStreamCustomEvent::getEventInfo() const {
return TraceEventInfo(kj::Array<TraceEventInfo::TraceItem>(nullptr));
}

kj::Promise<WorkerInterface::CustomEvent::Result> TailStreamCustomEvent::run(
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/io/trace-stream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class TailStreamCustomEvent final: public WorkerInterface::CustomEvent {
return typeId;
}

kj::Maybe<tracing::EventInfo> getEventInfo() const override;
tracing::EventInfo getEventInfo() const override;

void failed(const kj::Exception& e) override {
capFulfiller->reject(kj::cp(e));
Expand Down
4 changes: 1 addition & 3 deletions src/workerd/io/worker-entrypoint.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,9 +798,7 @@ kj::Promise<WorkerInterface::CustomEvent::Result> WorkerEntrypoint::customEvent(
// any user code executes (particularly important for actors whose constructors may run
// during delivered()).
KJ_IF_SOME(t, incomingRequest->getWorkerTracer()) {
KJ_IF_SOME(eventInfo, event->getEventInfo()) {
t.setEventInfo(*incomingRequest, kj::mv(eventInfo));
}
t.setEventInfo(*incomingRequest, event->getEventInfo());
}

auto promise = event->run(kj::mv(incomingRequest), entrypointName, kj::mv(props), waitUntilTasks)
Expand Down
4 changes: 1 addition & 3 deletions src/workerd/io/worker-interface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,7 @@ class WorkerInterface: public kj::HttpService {

// Get event info for tracing.
// Return none if this event type doesn't need tracing.
virtual kj::Maybe<tracing::EventInfo> getEventInfo() const {
return kj::none;
}
virtual tracing::EventInfo getEventInfo() const = 0;

// If the CustomEvent fails before any of the other methods are called, this may be invoked
// to report the failure reason.
Expand Down
Loading