Skip to content

feat(WO-1143): determine SpanOpen's SpanKind, serialize to JS#6477

Open
repository wants to merge 4 commits intolngo/spankind-1from
lngo/spankind-3
Open

feat(WO-1143): determine SpanOpen's SpanKind, serialize to JS#6477
repository wants to merge 4 commits intolngo/spankind-1from
lngo/spankind-3

Conversation

@repository
Copy link
Copy Markdown
Member

This adds the ability to set SpanKind on user tracing spans to TraceContext::makeUserTraceSpan. Then at various trace instrumentation site, we set it to the appropriate value depending on the operation.

If not otherwise specified, user spans created via TraceContext::makeUserTraceSpan will default to SpanKind::CLIENT, as most bindings are implemented as calls to external services.

Please also see:
#6471

Operation SpanKind Why
KV
kv_get CLIENT Outbound call to KV service
kv_getWithMetadata CLIENT Outbound call to KV service
kv_list CLIENT Outbound call to KV service
kv_put CLIENT Outbound call to KV service
kv_delete CLIENT Outbound call to KV service
Cache API
cache_match CLIENT Outbound call to Cache API
cache_put CLIENT Outbound call to Cache API
cache_delete CLIENT Outbound call to Cache API
R2
r2_head CLIENT Outbound call to R2
r2_get CLIENT Outbound call to R2
r2_put CLIENT Outbound call to R2
r2_delete CLIENT Outbound call to R2
r2_list CLIENT Outbound call to R2
r2_createMultipartUpload CLIENT Outbound call to R2
r2_uploadPart CLIENT Outbound call to R2
r2_completeMultipartUpload CLIENT Outbound call to R2
r2_abortMultipartUpload CLIENT Outbound call to R2
R2 Admin
r2_create_bucket CLIENT Outbound call to R2 admin
r2_list_buckets CLIENT Outbound call to R2 admin
r2_delete_bucket CLIENT Outbound call to R2 admin
Durable Object Storage
durable_object_storage_get CLIENT Outbound call to DO storage
durable_object_storage_getAlarm CLIENT Outbound call to DO storage
durable_object_storage_list CLIENT Outbound call to DO storage
durable_object_storage_put CLIENT Outbound call to DO storage
durable_object_storage_delete CLIENT Outbound call to DO storage
durable_object_storage_deleteAlarm CLIENT Outbound call to DO storage
durable_object_storage_deleteAll CLIENT Outbound call to DO storage
durable_object_storage_setAlarm PRODUCER Schedules future alarm invocation
durable_object_storage_transaction INTERNAL Local transaction coordination
durable_object_storage_sync INTERNAL Local flush of pending writes
durable_object_storage_getCurrentBookmark INTERNAL Local state query
durable_object_storage_waitForBookmark INTERNAL Local state query
Durable Object SQL
durable_object_storage_exec CLIENT Outbound call to DO SQL
durable_object_storage_ingest CLIENT Outbound call to DO SQL
durable_object_storage_getDatabaseSize INTERNAL Local metadata read
Durable Object KV Storage
durable_object_storage_kv_get CLIENT Outbound call to DO KV storage
durable_object_storage_kv_list CLIENT Outbound call to DO KV storage
durable_object_storage_kv_put CLIENT Outbound call to DO KV storage
durable_object_storage_kv_delete CLIENT Outbound call to DO KV storage
Queues
queue_send PRODUCER Enqueues message for async consumer
queue_send (sendWithResponse) PRODUCER Enqueues message for async consumer
queue_send (sendBatch) PRODUCER Enqueues batch for async consumer
queue_send (sendBatchWithResponse) PRODUCER Enqueues batch for async consumer
Memory Cache
memory_cache_read CLIENT Outbound call to memory cache
Fetch
fetch subrequests CLIENT Outbound HTTP call to external service

Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk bot left a comment

Choose a reason for hiding this comment

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

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

This PR adds SpanKind to user tracing spans across KV, Cache, R2, DO storage, Queues, Memory Cache, and Fetch instrumentation sites, then serializes it to the spanOpen JS event. The changes look well-structured and consistent.

One issue found:

  1. [MEDIUM] User-created spans via TracingModule::startSpan default to CLIENT instead of INTERNALsrc/workerd/api/tracing-module.c++:32 calls makeUserTraceSpan without specifying SpanKind, so it inherits the new default CLIENT. Per the OpenTelemetry spec, manually-created spans should default to INTERNAL. A user calling tracing.startSpan("my-operation") would get spanKind: "client" which incorrectly implies an outbound network call.

    Suggested fix in src/workerd/api/tracing-module.c++ line 32:

    TraceContext traceContext = ioContext.makeUserTraceSpan(kj::ConstString(kj::mv(name)), SpanKind::INTERNAL);

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

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Apr 1, 2026

Review posted successfully. Here's a summary:

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

Summary: This PR adds SpanKind to user tracing spans across KV, Cache, R2, DO storage, Queues, Memory Cache, and Fetch instrumentation sites, and serializes it to the spanOpen JS event.

One issue found:

  1. [MEDIUM] User-created spans via TracingModule::startSpan default to CLIENT instead of INTERNALsrc/workerd/api/tracing-module.c++:32 calls makeUserTraceSpan without specifying SpanKind, so it inherits the new default CLIENT. Per the OpenTelemetry spec, manually-created spans should default to INTERNAL. A user calling tracing.startSpan("my-operation") would get spanKind: "client" which incorrectly implies an outbound network call. Suggested fix provided in the review.

github run

@repository repository marked this pull request as ready for review April 1, 2026 15:25
@repository repository requested review from a team as code owners April 1, 2026 15:25
@repository repository requested review from vicb and removed request for a team April 1, 2026 15:25
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.

1 participant