Skip to content

[WO-1581] Enable span status and renaming - #7348

Merged
jmorrell-cloudflare merged 1 commit into
mainfrom
jmorrell/add-span-status
Sep 24, 2026
Merged

jmorrell-cloudflare merged 1 commit into
mainfrom
jmorrell/add-span-status

Conversation

@jmorrell-cloudflare

@jmorrell-cloudflare jmorrell-cloudflare commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor
  • Introduces a new STW event spanUpdate to mirror spanOpen and spanClose. Currently only supports updating name and status but could be expanded to other properties if there is ever a need.
type SpanStatusCode = "unset" | "ok" | "error";

interface SpanStatus {
  readonly code: SpanStatusCode;
  readonly message?: string;
}

type SpanUpdateInfo =
  | { readonly type: "name"; readonly name: string }
  | { readonly type: "status"; readonly status: SpanStatus };

interface SpanUpdate {
  readonly type: "spanUpdate";
  readonly info: SpanUpdateInfo;
}
  • Extends our span model with two new methods to match OpenTelemetry:
interface TracingSpanStatus {
  code: TracingSpanStatusCode;
  message?: string;
}

span.updateName(name: string): this
span.setStatus(status: TracingSpanStatus): this

Comment thread src/workerd/api/tracing.h
@ask-bonk

ask-bonk Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

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

Adds user-tracing span status serialization and tail-stream exposure.

  1. P1: Posted a suggestion for the missing InvocationSpanState::setStatus() implementation, which currently leaves the class abstract and prevents compilation.

github run

@github-actions

github-actions Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

@codecov-commenter

codecov-commenter commented Sep 13, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 28.01418% with 203 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.11%. Comparing base (84338b9) to head (1f6faa7).

Files with missing lines Patch % Lines
src/workerd/api/tracing.c++ 0.00% 73 Missing ⚠️
src/workerd/io/trace.c++ 49.05% 45 Missing and 9 partials ⚠️
src/workerd/io/trace-stream.c++ 0.00% 39 Missing and 2 partials ⚠️
src/workerd/io/tracer.c++ 0.00% 21 Missing ⚠️
src/workerd/io/trace-test.c++ 72.97% 0 Missing and 10 partials ⚠️
src/workerd/server/server.c++ 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7348      +/-   ##
==========================================
- Coverage   38.12%   38.11%   -0.02%     
==========================================
  Files         858      858              
  Lines      262589   262871     +282     
  Branches    24314    24383      +69     
==========================================
+ Hits       100125   100205      +80     
- Misses     148816   148995     +179     
- Partials    13648    13671      +23     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jmorrell-cloudflare
jmorrell-cloudflare force-pushed the jmorrell/add-span-status branch 2 times, most recently from fe46ea3 to 7c97753 Compare September 13, 2026 22:06
@jmorrell-cloudflare jmorrell-cloudflare changed the title [WO-1581] Add span status [WO-1581] Enable span status and renaming Sep 13, 2026
@jmorrell-cloudflare
jmorrell-cloudflare marked this pull request as ready for review September 13, 2026 23:08
@jmorrell-cloudflare
jmorrell-cloudflare requested review from a team as code owners September 13, 2026 23:08

@NuroDev NuroDev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from a types PoV

Comment thread types/defines/trace.d.ts
Comment thread src/workerd/io/trace-stream.c++ Outdated
Comment thread src/workerd/io/trace.h Outdated
Comment thread src/workerd/io/tracer.c++ Outdated
Comment thread src/workerd/io/trace.c++ Outdated
Comment thread src/workerd/io/trace-test.c++ Outdated
Comment thread src/workerd/io/trace.c++
Comment thread src/workerd/io/trace.c++

@fhanau fhanau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure to squash before merge.

…panClose`. Currently only supports updating `name` and `status` but could be expanded to other properties if there is ever a need.

```ts
type SpanStatusCode = "unset" | "ok" | "error";

interface SpanStatus {
  readonly code: SpanStatusCode;
  readonly message?: string;
}

type SpanUpdateInfo =
  | { readonly type: "name"; readonly name: string }
  | { readonly type: "status"; readonly status: SpanStatus };

interface SpanUpdate {
  readonly type: "spanUpdate";
  readonly info: SpanUpdateInfo;
}
```

- Extends our span model with two new methods to match OpenTelemetry:

```ts
interface TracingSpanStatus {
  code: TracingSpanStatusCode;
  message?: string;
}

span.updateName(name: string): this
span.setStatus(status: TracingSpanStatus): this
```
@jmorrell-cloudflare
jmorrell-cloudflare merged commit 911aada into main Sep 24, 2026
24 of 25 checks passed
@jmorrell-cloudflare
jmorrell-cloudflare deleted the jmorrell/add-span-status branch September 24, 2026 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants