Skip to content

Commit 68cd955

Browse files
committed
[o11y] Do not throw error if WorkerTracer is unused for alarm event
1 parent e10895c commit 68cd955

File tree

3 files changed

+28
-0
lines changed

3 files changed

+28
-0
lines changed

src/workerd/io/tracer.c++

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,10 @@ WorkerTracer::~WorkerTracer() noexcept(false) {
130130
return;
131131
}
132132

133+
if (destroyed) {
134+
return;
135+
}
136+
133137
// Report the outcome event if STWs are present. All worker events need to call setEventInfo at
134138
// the start of the invocation to submit the onset event before any other tail events.
135139
KJ_IF_SOME(writer, maybeTailStreamWriter) {
@@ -157,6 +161,11 @@ WorkerTracer::~WorkerTracer() noexcept(false) {
157161
}
158162
};
159163

164+
void WorkerTracer::setDestroyed() {
165+
maybeTailStreamWriter = kj::none;
166+
destroyed = true;
167+
}
168+
160169
constexpr kj::LiteralStringConst logSizeExceeded =
161170
"[\"Log size limit exceeded: More than 256KB of data (across console.log statements, exception, request metadata and headers) was logged during a single request. Subsequent data for this request will not be recorded in logs, appear when tailing this Worker's logs, or in Tail Workers.\"]"_kjc;
162171

src/workerd/io/tracer.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ class BaseTracer: public kj::Refcounted {
137137

138138
SpanParent makeUserRequestSpan();
139139

140+
virtual void setDestroyed() = 0;
141+
140142
using MakeUserRequestSpanFunc = kj::Function<SpanParent()>;
141143

142144
// Allow setting the user request span after the tracer has been created so its observer can
@@ -179,6 +181,13 @@ class WorkerTracer final: public BaseTracer {
179181
virtual ~WorkerTracer() noexcept(false);
180182
KJ_DISALLOW_COPY_AND_MOVE(WorkerTracer);
181183

184+
// Used to indicate that no events are expected for this tracer, we can deallocate its tail stream
185+
// writer early (if present) and won't log a warning about deallocating a WorkerTracer that didn't
186+
// receive any events. Intended for (hopefully rare) cases where we end up not using a
187+
// WorkerInterface but can only determine this after the WorkerInterface and associated
188+
// WorkerTracer have already been allocated.
189+
void setDestroyed() override;
190+
182191
void addLog(const tracing::InvocationSpanContext& context,
183192
kj::Date timestamp,
184193
LogLevel logLevel,
@@ -233,5 +242,8 @@ class WorkerTracer final: public BaseTracer {
233242
kj::Maybe<kj::Rc<PipelineTracer>> parentPipeline;
234243

235244
kj::Maybe<kj::Own<tracing::TailStreamWriter>> maybeTailStreamWriter;
245+
246+
// Indicates that we are not expecting any events for this tail stream writer, that
247+
bool destroyed;
236248
};
237249
} // namespace workerd

src/workerd/io/worker-entrypoint.c++

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,13 @@ kj::Promise<WorkerInterface::AlarmResult> WorkerEntrypoint::runAlarmImpl(
631631
auto& actor = KJ_REQUIRE_NONNULL(context.getActor(), "alarm() should only work with actors");
632632

633633
KJ_IF_SOME(promise, actor.getAlarm(scheduledTime)) {
634+
// Mark the WorkerTracer as destroyed so that we don't log a warning about it being allocated
635+
// without being used. Ideally we'd avoid this by detecting that an alarm is already scheduled
636+
// before allocating the WorkerInterface/WorkerTracer.
637+
KJ_IF_SOME(t, incomingRequest->getWorkerTracer()) {
638+
t.setDestroyed();
639+
}
640+
634641
// There is a pre-existing alarm for `scheduledTime`, we can just wait for its result.
635642
// TODO(someday) If the request responsible for fulfilling this alarm were to be cancelled, then
636643
// we could probably take over and try to fulfill it ourselves. Maybe we'd want to loop on

0 commit comments

Comments
 (0)