feat: add W3CTraceContext with tracestate support#173
Conversation
|
IIRC we have some downstream extension libs that supports W3C trace context - https://github.com/fast/fastrace-reqwest/blob/main/src/lib.rs But there can be some difference from the changeset here. |
|
Thanks for the pointer! Yes, This PR is complementary —
|
|
Thanks for putting this together. I think this PR works as a boundary helper, but we should be clear about the limitation of that design. Since The cleaner alternative would be a breaking change: make I think we should decide which direction we want before merging. If this PR stays on the boundary-helper path, the docs should state that limitation explicitly, and |
Per andylokandy's review comment on fast#173, this commit strengthens the documentation and implementation of W3CTraceContext to clarify its boundary-helper role and handle repeated tracestate headers correctly. Changes: 1. Expanded W3CTraceContext top-level docs to explicitly state the round-trip limitation: extracting span_context and later using SpanContext::from_span or current_local_parent() loses tracestate. Added concrete example of the loss scenario. 2. Updated decode_headers to join multiple tracestate headers with commas per W3C Trace Context §3.3.1.1. Empty or whitespace-only tracestate values are filtered before joining to ensure proper normalization (e.g., ["", ""] → None, not ","). 3. Added three new tests covering repeated tracestate headers: - repeated non-empty headers joined with commas - mixed empty/non-empty headers with filtering - all-empty headers normalized to None All 17 W3C trace context tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Thank you for the detailed feedback, @andylokandy. After discussing with the team, we're staying on the boundary-helper path for this PR, and I've pushed a commit addressing your specific asks. Design decisionThe boundary-helper approach aligns well with how RisingWave (and likely other users) already propagate trace context: the inbound The breaking-change path (carrying tracestate through the span chain) would be the "cleaner" abstraction, but the cost is significant: Changes in this commit (
|
Introduce `W3CTraceContext`, a boundary/header wrapper that pairs a `SpanContext` with an optional W3C `tracestate` string. This enables vendor-specific key-value pairs to survive propagation across process boundaries via the standard `traceparent`/`tracestate` header pair. Key design decisions: - Separate type from `SpanContext` to preserve `SpanContext: Copy` - Empty tracestate strings normalized to `None` for API consistency - Case-insensitive header matching per W3C spec (no allocation) - Explicit documentation that tracestate is NOT retained through fastrace's internal span machinery Closes fast#171
Join two-line doc comment to single line to satisfy nightly cargo fmt --all --check. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Per andylokandy's review comment on fast#173, this commit strengthens the documentation and implementation of W3CTraceContext to clarify its boundary-helper role and handle repeated tracestate headers correctly. Changes: 1. Expanded W3CTraceContext top-level docs to explicitly state the round-trip limitation: extracting span_context and later using SpanContext::from_span or current_local_parent() loses tracestate. Added concrete example of the loss scenario. 2. Updated decode_headers to join multiple tracestate headers with commas per W3C Trace Context §3.3.1.1. Empty or whitespace-only tracestate values are filtered before joining to ensure proper normalization (e.g., ["", ""] → None, not ","). 3. Added three new tests covering repeated tracestate headers: - repeated non-empty headers joined with commas - mixed empty/non-empty headers with filtering - all-empty headers normalized to None All 17 W3C trace context tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
205a8a0 to
bcece82
Compare
|
@TennyZhuang Thank you! |
Summary
Adds
W3CTraceContext, a boundary/header wrapper that extendsSpanContextwith optional W3Ctracestatesupport. This enables vendor-specific key-value pairs to survive propagation across process boundaries via the standardtraceparent/tracestateheader pair.Closes #171
Design
SpanContextto preserveSpanContext: Copy.W3CTraceContextisClonebut notCopysince it holds anOption<String>.W3CTraceContextis explicitly scoped for encoding/decoding W3C headers at RPC injection/extraction points. Thetracestatefield is not carried through fastrace's internal span machinery.decode(..., Some(""))andwith_tracestate("")normalize toNonefor API consistency.eq_ignore_ascii_case) per W3C spec, avoiding per-header allocation.API
Test plan