Skip to content

Add SpanContext::toTraceparent() - #6942

Merged
mar-cf merged 1 commit into
mainfrom
cache-user-tracing
Aug 14, 2026
Merged

Add SpanContext::toTraceparent()#6942
mar-cf merged 1 commit into
mainfrom
cache-user-tracing

Conversation

@mar-cf

@mar-cf mar-cf commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@mar-cf
mar-cf requested review from a team as code owners August 10, 2026 13:45
@ask-bonk

ask-bonk Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

I've completed my analysis. Let me summarize the review:

The code is clean, correct, and well-tested. Let me verify my analysis:

  1. toTraceparent() correctly returns kj::none for all invalid states (no spanId, no traceFlags, zero traceId, zero spanId).
  2. formatW3CTraceparent() correctly produces W3C-compliant format with proper zero-padding via addHex() (16 chars per 64-bit field) and kj::encodeHex() (2 chars for flags byte).
  3. Tests cover round-tripping, zero-padding, and all kj::none return paths.
  4. No behavioral changes to existing code — this is purely additive.
  5. No backward compat concerns — new methods only.
  6. Memory safety — no lifetime issues, no async patterns, no V8 interaction.

The one minor observation: formatW3CTraceparent is declared as a public free function in the header but is currently only called from SpanContext::toTraceparent(). It's reasonable to expose it for reuse by InvocationSpanContext or other callers in the future, so this is a deliberate design choice, not a problem. The PR title mentions the branch is cache-user-tracing, suggesting more callers are coming.

LGTM

github run

@mar-cf
mar-cf force-pushed the cache-user-tracing branch from 83ee576 to 555941d Compare August 13, 2026 15:52
@codspeed-hq

codspeed-hq Bot commented Aug 13, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 9.19%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 1 regressed benchmark
✅ 71 untouched benchmarks
⏩ 129 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
bm_Promise_Fib10 18.5 µs 20.3 µs -9.19%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing cache-user-tracing (20e92f4) with main (3d43d41)

Open in CodSpeed

Footnotes

  1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@mar-cf
mar-cf force-pushed the cache-user-tracing branch from 555941d to a96f2c6 Compare August 13, 2026 19:37
@mar-cf
mar-cf force-pushed the cache-user-tracing branch from a96f2c6 to 20e92f4 Compare August 14, 2026 09:19
@mar-cf
mar-cf merged commit c15dd17 into main Aug 14, 2026
22 of 23 checks passed
@mar-cf
mar-cf deleted the cache-user-tracing branch August 14, 2026 11:33
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.

3 participants