Improve variable naming and comments based on code review

src/workerd/api/actor.c++

@@ -24,18 +24,18 @@ public:
     auto& context = IoContext::current();
 
     return context.getMetrics().wrapActorSubrequestClient(context.getSubrequest(
-        [&](TraceContext& spans, IoChannelFactory& ioChannelFactory) {
-      if (spans.span.isObserved()) {
-        spans.span.setTag("actor_id"_kjc, kj::str(actorId));
+        [&](TraceContext& tracing, IoChannelFactory& ioChannelFactory) {
+      if (tracing.span.isObserved()) {
+        tracing.span.setTag("actor_id"_kjc, kj::str(actorId));
       }
 
       // Lazily initialize actorChannel
       if (actorChannel == kj::none) {
-        actorChannel = context.getColoLocalActorChannel(channelId, actorId, spans.span);
+        actorChannel = context.getColoLocalActorChannel(channelId, actorId, tracing.span);
       }
 
       return KJ_REQUIRE_NONNULL(actorChannel)
-          ->startRequest({.cfBlobJson = kj::mv(cfStr), .spans = spans});
+          ->startRequest({.cfBlobJson = kj::mv(cfStr), .tracing = tracing});
     },
         {.inHouse = true,
           .wrapMetrics = true,
@@ -65,19 +65,19 @@ public:
     auto& context = IoContext::current();
 
     return context.getMetrics().wrapActorSubrequestClient(context.getSubrequest(
-        [&](TraceContext& spans, IoChannelFactory& ioChannelFactory) {
-      if (spans.span.isObserved()) {
-        spans.span.setTag("actor_id"_kjc, id->toString());
+        [&](TraceContext& tracing, IoChannelFactory& ioChannelFactory) {
+      if (tracing.span.isObserved()) {
+        tracing.span.setTag("actor_id"_kjc, id->toString());
       }
 
       // Lazily initialize actorChannel
       if (actorChannel == kj::none) {
         actorChannel = context.getGlobalActorChannel(channelId, id->getInner(),
-            kj::mv(locationHint), mode, enableReplicaRouting, spans.span);
+            kj::mv(locationHint), mode, enableReplicaRouting, tracing.span);
       }
 
       return KJ_REQUIRE_NONNULL(actorChannel)
-          ->startRequest({.cfBlobJson = kj::mv(cfStr), .spans = spans});
+          ->startRequest({.cfBlobJson = kj::mv(cfStr), .tracing = tracing});
     },
         {.inHouse = true,
           .wrapMetrics = true,

src/workerd/io/io-channels.h

@@ -88,7 +88,7 @@ class IoChannelFactory {
     kj::Maybe<kj::String> cfBlobJson;
 
     // Specifies the parent span for the subrequest for tracing purposes.
-    TraceParentContext spans = TraceParentContext(nullptr, nullptr);
+    TraceParentContext tracing = TraceParentContext(nullptr, nullptr);
 
     // Serialized JSON value to pass in ew_compat field of control header to FL. If this subrequest
     // does not go directly to FL, this value is ignored. Flags marked with `$neededByFl` in

src/workerd/io/io-context.c++

@@ -790,8 +790,8 @@ kj::Own<WorkerInterface> IoContext::getSubrequestNoChecks(
     limeSpan = makeLimeTraceSpan(kj::ConstString(kj::mv(n)));
   }
 
-  TraceContext spans(kj::mv(span), kj::mv(limeSpan));
-  auto ret = func(spans, getIoChannelFactory());
+  TraceContext tracing(kj::mv(span), kj::mv(limeSpan));
+  auto ret = func(tracing, getIoChannelFactory());
 
   if (options.wrapMetrics) {
     auto& metrics = getMetrics();
@@ -800,11 +800,11 @@ kj::Own<WorkerInterface> IoContext::getSubrequestNoChecks(
         kj::mv(ret), getHeaderIds().contentEncoding, metrics);
   }
 
-  if (spans.span.isObserved()) {
-    ret = ret.attach(kj::mv(spans.span));
+  if (tracing.span.isObserved()) {
+    ret = ret.attach(kj::mv(tracing.span));
   }
-  if (spans.limeSpan.isObserved()) {
-    ret = ret.attach(kj::mv(spans.limeSpan));
+  if (tracing.limeSpan.isObserved()) {
+    ret = ret.attach(kj::mv(tracing.limeSpan));
   }
 
   return kj::mv(ret);
@@ -820,8 +820,9 @@ kj::Own<WorkerInterface> IoContext::getSubrequest(
 kj::Own<WorkerInterface> IoContext::getSubrequestChannel(
     uint channel, bool isInHouse, kj::Maybe<kj::String> cfBlobJson, kj::ConstString operationName) {
   return getSubrequest(
-      [&](TraceContext& spans, IoChannelFactory& channelFactory) {
-    return getSubrequestChannelImpl(channel, isInHouse, kj::mv(cfBlobJson), spans, channelFactory);
+      [&](TraceContext& tracing, IoChannelFactory& channelFactory) {
+    return getSubrequestChannelImpl(
+        channel, isInHouse, kj::mv(cfBlobJson), tracing, channelFactory);
   },
       SubrequestOptions{
         .inHouse = isInHouse,
@@ -835,8 +836,9 @@ kj::Own<WorkerInterface> IoContext::getSubrequestChannelNoChecks(uint channel,
     kj::Maybe<kj::String> cfBlobJson,
     kj::Maybe<kj::ConstString> operationName) {
   return getSubrequestNoChecks(
-      [&](TraceContext& spans, IoChannelFactory& channelFactory) {
-    return getSubrequestChannelImpl(channel, isInHouse, kj::mv(cfBlobJson), spans, channelFactory);
+      [&](TraceContext& tracing, IoChannelFactory& channelFactory) {
+    return getSubrequestChannelImpl(
+        channel, isInHouse, kj::mv(cfBlobJson), tracing, channelFactory);
   },
       SubrequestOptions{
         .inHouse = isInHouse,
@@ -848,11 +850,11 @@ kj::Own<WorkerInterface> IoContext::getSubrequestChannelNoChecks(uint channel,
 kj::Own<WorkerInterface> IoContext::getSubrequestChannelImpl(uint channel,
     bool isInHouse,
     kj::Maybe<kj::String> cfBlobJson,
-    TraceContext& spans,
+    TraceContext& tracing,
     IoChannelFactory& channelFactory) {
   IoChannelFactory::SubrequestMetadata metadata{
     .cfBlobJson = kj::mv(cfBlobJson),
-    .spans = spans,
+    .tracing = tracing,
     .featureFlagsForFl = worker->getIsolate().getFeatureFlagsForFl(),
   };
 

src/workerd/io/io-context.h

@@ -861,7 +861,7 @@ class IoContext final: public kj::Refcounted, private kj::TaskSet::ErrorHandler
   kj::Own<WorkerInterface> getSubrequestChannelImpl(uint channel,
       bool isInHouse,
       kj::Maybe<kj::String> cfBlobJson,
-      TraceContext& spans,
+      TraceContext& tracing,
       IoChannelFactory& channelFactory);
 
   friend class IoContext_IncomingRequest;

src/workerd/io/trace.c++

@@ -583,12 +583,12 @@ WorkerTracer::WorkerTracer(
     : pipelineLogLevel(pipelineLogLevel),
       trace(kj::mv(trace)),
       parentPipeline(kj::mv(parentPipeline)),
-      weakRef(kj::refcounted<WeakRef<WorkerTracer>>(kj::Badge<WorkerTracer>{}, *this)) {}
+      self(kj::refcounted<WeakRef<WorkerTracer>>(kj::Badge<WorkerTracer>{}, *this)) {}
 WorkerTracer::WorkerTracer(PipelineLogLevel pipelineLogLevel)
     : pipelineLogLevel(pipelineLogLevel),
       trace(kj::refcounted<Trace>(
           kj::none, kj::none, kj::none, kj::none, kj::none, nullptr, kj::none)),
-      weakRef(kj::refcounted<WeakRef<WorkerTracer>>(kj::Badge<WorkerTracer>{}, *this)) {}
+      self(kj::refcounted<WeakRef<WorkerTracer>>(kj::Badge<WorkerTracer>{}, *this)) {}
 
 void WorkerTracer::log(kj::Date timestamp, LogLevel logLevel, kj::String message, bool isSpan) {
   if (trace->exceededLogLimit) {

src/workerd/io/trace.h

@@ -379,12 +379,15 @@ class WorkerTracer final: public kj::Refcounted {
       PipelineLogLevel pipelineLogLevel);
   explicit WorkerTracer(PipelineLogLevel pipelineLogLevel);
   ~WorkerTracer() {
-    weakRef->invalidate();
+    self->invalidate();
   }
   KJ_DISALLOW_COPY_AND_MOVE(WorkerTracer);
 
   // Adds log line to trace.  For Spectre, timestamp should only be as accurate as JS Date.now().
+  // The isSpan parameter allows for logging spans, which will be emitted after regular logs. There
+  // can be at most MAX_LIME_SPANS spans in a trace.
   void log(kj::Date timestamp, LogLevel logLevel, kj::String message, bool isSpan = false);
+  // Add a span, which will be represented as a log.
   void addSpan(const Span& span, kj::String spanContext);
 
   // TODO(soon): Eventually:
@@ -418,7 +421,7 @@ class WorkerTracer final: public kj::Refcounted {
   void setTrace(rpc::Trace::Reader reader);
 
   kj::Own<WeakRef<WorkerTracer>> addWeakRef() {
-    return weakRef->addRef();
+    return self->addRef();
   }
 
 private:
@@ -428,7 +431,12 @@ class WorkerTracer final: public kj::Refcounted {
   // own an instance of the pipeline to make sure it doesn't get destroyed
   // before we're finished tracing
   kj::Maybe<kj::Own<PipelineTracer>> parentPipeline;
-  kj::Own<WeakRef<WorkerTracer>> weakRef;
+  // A weak reference for the internal span submitter. We use this so that the span submitter can
+  // add spans while the tracer exists, but does not artifically prolong the lifetime of the tracer
+  // which would interfere with span submission (traces get submitted when the worker returns its
+  // response, but with e.g. waitUntil() the worker can still be performing tasks afterwards so the
+  // span submitter may exist for longer than the tracer).
+  kj::Own<WeakRef<WorkerTracer>> self;
 };
 
 // =======================================================================================
@@ -677,7 +685,9 @@ struct TraceContext {
 // without modifying them. In particular, add functions like newLimeChild() here to make it easier
 // to add a span for the right parent.
 struct TraceParentContext {
-  TraceParentContext(TraceContext& spans): parentSpan(spans.span), limeParentSpan(spans.limeSpan) {}
+  TraceParentContext(TraceContext& tracing)
+      : parentSpan(tracing.span),
+        limeParentSpan(tracing.limeSpan) {}
   TraceParentContext(SpanParent span, SpanParent limeSpan)
       : parentSpan(kj::mv(span)),
         limeParentSpan(kj::mv(limeSpan)) {}