refactor!: clean up trace context ids#177
Open
andylokandy wants to merge 12 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors fastrace’s trace/span identifier model for the upcoming 0.8 contract by replacing public numeric ID wrappers with checked APIs, moving W3C propagation details onto SpanContext, and updating reporters/tests/docs to the new parent/trace-state model.
Changes:
- Reworked core tracing types so
TraceId/SpanIdare opaque checked values and root parents are represented withOption<SpanId>. - Moved W3C propagation behavior onto
SpanContext, including trace flags, tracestate storage, andtraceparentencode/decode helpers. - Updated collectors, reporters, tests, benches, and docs to consume the new ID/context APIs.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/statically-disable/src/main.rs |
Updates no-feature smoke test to use checked ID constructors and root contexts. |
README.md |
Adds W3C Trace Context documentation and new ID construction guidance. |
fastrace/tests/lib.rs |
Adjusts integration tests for root contexts, checked IDs, and cloned link contexts. |
fastrace/src/util/tree.rs |
Switches tree-building sentinel logic from zero span IDs to None. |
fastrace/src/span.rs |
Propagates trace flags/state through collect tokens and root span creation. |
fastrace/src/local/local_span.rs |
Refreshes local-span docs/tests for checked IDs and richer collect tokens. |
fastrace/src/local/local_span_stack.rs |
Updates stack tests/helpers for optional parent IDs and trace metadata. |
fastrace/src/local/local_span_line.rs |
Preserves optional parent IDs plus trace flags/state in current token logic. |
fastrace/src/local/local_collector.rs |
Updates local collector tests/helpers for the new collect token shape. |
fastrace/src/lib.rs |
Refreshes crate docs and prelude exports for the new propagation model. |
fastrace/src/collector/mod.rs |
Re-exports new trace-state/flag types and expands SpanRecord/CollectToken. |
fastrace/src/collector/id.rs |
Introduces opaque checked IDs, trace flags/state, and the new SpanContext APIs. |
fastrace/src/collector/global_collector.rs |
Carries optional parents, trace flags, and tracestate through span post-processing. |
fastrace/benches/trace.rs |
Updates benchmarks to construct roots via checked/root context APIs. |
fastrace/benches/compare.rs |
Updates comparison benchmark to the new root context API. |
fastrace-opentelemetry/tests/context.rs |
Adjusts OTel bridge test for optional span IDs. |
fastrace-opentelemetry/src/lib.rs |
Maps byte-based IDs, trace flags, and tracestate into OpenTelemetry types. |
fastrace-opentelemetry/README.md |
Refreshes OpenTelemetry bridge example formatting. |
fastrace-jaeger/src/lib.rs |
Converts Jaeger export to use byte-based ID helpers and optional parent IDs. |
fastrace-datadog/src/lib.rs |
Converts Datadog export to use byte-based ID helpers and optional parent IDs. |
CHANGELOG.md |
Documents the unreleased 0.8 trace propagation changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
535
to
549
| impl serde::Serialize for SpanContext { | ||
| fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> { | ||
| self.encode_w3c_traceparent().serialize(serializer) | ||
| let traceparent = self.encode_traceparent().ok_or_else(|| { | ||
| serde::ser::Error::custom("span context has no span id for traceparent") | ||
| })?; | ||
| traceparent.serialize(serializer) | ||
| } | ||
| } | ||
|
|
||
| impl<'de> serde::Deserialize<'de> for SpanContext { | ||
| fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> { | ||
| let s = String::deserialize(deserializer)?; | ||
| SpanContext::decode_w3c_traceparent(&s) | ||
| SpanContext::decode_traceparent(&s) | ||
| .ok_or_else(|| serde::de::Error::custom("invalid w3c traceparent")) | ||
| } |
Comment on lines
+535
to
+540
| impl serde::Serialize for SpanContext { | ||
| fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> { | ||
| self.encode_w3c_traceparent().serialize(serializer) | ||
| let traceparent = self.encode_traceparent().ok_or_else(|| { | ||
| serde::ser::Error::custom("span context has no span id for traceparent") | ||
| })?; | ||
| traceparent.serialize(serializer) |
Comment on lines
178
to
191
| fn map_links(links: Vec<SpanContext>) -> SpanLinks { | ||
| let links = links | ||
| .into_iter() | ||
| .map(|link| { | ||
| let trace_flags = if link.sampled { | ||
| TraceFlags::SAMPLED | ||
| } else { | ||
| TraceFlags::default() | ||
| }; | ||
| .filter_map(|link| { | ||
| let span_id = link.span_id()?; | ||
| let span_context = OtelSpanContext::new( | ||
| link.trace_id.0.into(), | ||
| link.span_id.0.into(), | ||
| trace_flags, | ||
| OtelTraceId::from_bytes(link.trace_id().to_bytes()), | ||
| OtelSpanId::from_bytes(span_id.to_bytes()), | ||
| map_trace_flags(link.trace_flags()), | ||
| false, | ||
| TraceState::default(), | ||
| map_trace_state(link.trace_state()), | ||
| ); | ||
| Link::with_context(span_context) | ||
| Some(Link::with_context(span_context)) | ||
| }) |
andylokandy
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
INVALIDvaluesSpanContextwithTraceFlags,TraceState,traceparentencode/decode, and header-name constantsOption<SpanId>internally instead of a public zero-span sentinelDesign Notes
TraceIdandSpanIdare no longer public numeric tuple wrappers. Public construction goes throughfrom_bytes,from_hex, orFromStr. All-zero IDs are represented asTraceId::INVALIDandSpanId::INVALID; W3Ctraceparentparse and encode reject those invalid IDs at the propagation boundary.SpanContextis now the single cheap propagation and parent/link context. It carries trace flags and tracestate directly, andSpanremains the live timed object responsible for collection and reporting.