feat(WO-1143): add SpanKind to SpanOpen and Outcome STW events

[repository]

We currently determine SpanKind in our streaming tail worker based on the names/attributes of the spans, but we'd like this kind of determination to be made directly in the runtime when spans are crated.

This adds the field SpanKind to the Onset and SpanOpen for the STW, which can be "one of Client, Server, Internal, Producer, or Consumer" as per OTel spec.

If not specified, spans created via UserSpanObserver::reportStart will default to Internal.

This is the first of three changes. Please also see:

Exposing SpanKind on Onset to JS, and determining it by the onset info type
https://github.com/cloudflare/workerd/pull/TBD
repository/workerd@lngo/spankind-1...repository:workerd:lngo/spankind-2

Exposing SpanKind on SpanOpen to JS, adding methods to set it, and setting it at various call sites
https://github.com/cloudflare/workerd/pull/TBD
repository/workerd@lngo/spankind-1...repository:workerd:lngo/spankind-3

[ask-bonk]

I'm Bonk, and I've done a quick review of your PR.

This PR adds SpanKind (matching the OTel spec) to SpanOpen, Onset, and related data structures, threading it through serialization and the span submission/reporting interfaces.

Findings (ranked by severity):

1. [MEDIUM] Tests don't verify spanKind round-trips through serialization/clone. The existing Read/Write SpanOpen works and Read/Write Onset works tests (src/workerd/io/trace-test.c++) only exercise the default SpanKind::INTERNAL and never assert the field value. A serialization bug (e.g. copyTo or the reader constructor silently dropping the field) would not be caught. I'd recommend adding test cases that construct SpanOpen and Onset with a non-default SpanKind (e.g., CLIENT or SERVER) and assert it survives:

   • copyTo → reader → constructor round-trip
   • clone()

   For example, for SpanOpen:

   KJ_TEST("Read/Write SpanOpen preserves SpanKind") {
     capnp::MallocMessageBuilder builder;
     auto infoBuilder = builder.initRoot<rpc::Trace::SpanOpen>();
   
     SpanOpen info(0x2a2a2a2a2a2a2a2a, "foo"_kjc, SpanKind::CLIENT);
     KJ_ASSERT(info.spanKind == SpanKind::CLIENT);
     info.copyTo(infoBuilder);
   
     auto reader = infoBuilder.asReader();
     SpanOpen info2(reader);
     KJ_ASSERT(info2.spanKind == SpanKind::CLIENT);
   
     SpanOpen info3 = info.clone();
     KJ_ASSERT(info3.spanKind == SpanKind::CLIENT);
   }

   And similarly for Onset with SpanKind::SERVER.

2. [LOW] CompleteSpan has spanKind field but UserSpanData capnp does not. This is fine because CompleteSpan/UserSpanData is legacy (marked obsolete26) and the CompleteSpan is decomposed into SpanOpenData + SpanEndData before transmission. Just noting for awareness — no action needed.

Everything else looks correct: cap'n proto changes are purely additive (wire-compatible), serialization/deserialization is consistent across all paths, defaults are sensible, and the SpanKind enum values match the OTel spec.

---

This review was generated by an AI assistant and may contain inaccuracies.

[ask-bonk]

Review posted successfully on PR #6471. The main finding is a medium-severity gap in test coverage -- the existing serialization round-trip tests for SpanOpen and Onset don't verify that spanKind survives serialization/deserialization/clone, since they only use the default value. I provided concrete test code suggestions. The rest of the PR looks correct: cap'n proto changes are wire-compatible, all serialization paths are consistent, and the enum values match the OTel spec.

github run