feat(traces): migrate trace propagation to dd-trace-rs#1089
feat(traces): migrate trace propagation to dd-trace-rs#1089duncanista wants to merge 3 commits intomainfrom
Conversation
Replace local trace extraction/propagation implementation with datadog-opentelemetry crate from dd-trace-rs. This removes maintenance burden for Datadog/TraceContext propagator logic. - Add datadog-opentelemetry dependency - Remove local propagation modules (context, error, text_map_propagator) - Use DatadogCompositePropagator from dd-trace-rs with thin wrapper for ot-baggage-* header extraction (not yet supported upstream) - Update all consumers to import directly from datadog-opentelemetry - Adapt to u128 trace_id, Sampling struct, SamplingPriority types
|
Merge needs to be held until dd-trace-rs changes are on main or released |
…ace IDs The truncation is intentional — Datadog protocol transmits the lower 64 bits via x-datadog-trace-id, with upper bits in _dd.p.tid tag.
There was a problem hiding this comment.
Pull request overview
This PR migrates trace context propagation from Bottlecap’s local implementation to datadog-opentelemetry (from dd-trace-rs), removing local Datadog/W3C propagation code while keeping a small wrapper to support ot-baggage-* extraction.
Changes:
- Add
datadog-opentelemetrydependency and adapt config to satisfy its propagation configuration trait. - Remove local propagation/context modules and update call sites to use upstream
SpanContext,Sampling,TracePropagationStyle, and header constants. - Keep a thin
DatadogCompositePropagatorwrapper to addot-baggage-*extraction not supported upstream yet.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| bottlecap/src/traces/propagation/text_map_propagator.rs | Deleted local header/W3C TraceContext propagators and tests. |
| bottlecap/src/traces/propagation/error.rs | Deleted local propagation error type. |
| bottlecap/src/traces/context.rs | Deleted local SpanContext/Sampling types in favor of upstream. |
| bottlecap/src/traces/mod.rs | Removes context module export now that it’s deleted. |
| bottlecap/src/traces/propagation/mod.rs | Replaces local multi-propagator logic with wrapper around upstream composite + baggage extraction. |
| bottlecap/src/traces/propagation/carrier.rs | Switches carrier traits to upstream and introduces JsonCarrier newtypes for serde_json::Value. |
| bottlecap/src/otlp/transform.rs | Imports Datadog trace-id-high constant from upstream. |
| bottlecap/src/lifecycle/listener.rs | Updates injected headers to use upstream context/constants and u128 trace IDs. |
| bottlecap/src/lifecycle/invocation/processor.rs | Updates extraction to use upstream propagator and JSON carriers; adjusts for u128 trace IDs. |
| bottlecap/src/lifecycle/invocation/processor_service.rs | Updates SpanContext import to upstream. |
| bottlecap/src/lifecycle/invocation/span_inferrer.rs | Replaces local propagator trait usage with DatadogCompositePropagator. |
| bottlecap/src/lifecycle/invocation/context.rs | Switches to upstream SpanContext. |
| bottlecap/src/lifecycle/invocation/triggers/step_function_event.rs | Updates to upstream SpanContext/Sampling and adds local _dd.p.* parsing helper. |
| bottlecap/src/lifecycle/invocation/triggers/sqs_event.rs | Updates to upstream SpanContext/Sampling and u128 trace IDs. |
| bottlecap/src/config/trace_propagation_style.rs | Uses upstream TracePropagationStyle and keeps only deserialization logic. |
| bottlecap/src/config/mod.rs | Implements upstream PropagationConfig for Config; updates tests to match supported styles. |
| bottlecap/src/config/env.rs | Adjusts env parsing tests/defaults to supported upstream style names. |
| bottlecap/src/config/yaml.rs | Adjusts YAML sample/tests to supported upstream style names. |
| bottlecap/Cargo.toml | Adds git dependency on datadog-opentelemetry. |
| bottlecap/Cargo.lock | Updates lockfile with new dependency tree introduced by datadog-opentelemetry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| Some(context) | ||
| } | ||
|
|
||
| fn attach_baggage(context: &mut SpanContext, carrier: &dyn Extractor) { |
There was a problem hiding this comment.
attach_baggage checks key.strip_prefix("ot-baggage-") on the raw key returned by carrier.keys(), which is case-sensitive. For carriers that don't normalize header keys (e.g., JSON event headers), baggage headers like OT-Baggage-foo won't be extracted. Consider doing an ASCII-lowercase prefix check (or normalizing keys) so extraction is case-insensitive like HTTP headers.
| fn extract_propagation_tags(tags_str: &str) -> HashMap<String, String> { | ||
| tags_str | ||
| .split(',') | ||
| .filter_map(|pair| { | ||
| let (k, v) = pair.split_once('=')?; | ||
| if k.starts_with("_dd.p.") { | ||
| Some((k.to_string(), v.to_string())) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
extract_propagation_tags duplicates the same _dd.p.* parsing logic added in lifecycle/invocation/processor.rs. Consider centralizing this into a shared helper (e.g., in traces::propagation) to avoid the two implementations drifting (and potentially add trimming of whitespace around pairs/keys in one place).
| if let Some(request_id) = span.meta.get("request_id") { | ||
| self.context_buffer.add_tracer_span(request_id, span); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn extract_propagation_tags(carrier: &HashMap<String, String>) -> HashMap<String, String> { | ||
| let carrier_tags = carrier.get(DATADOG_TAGS_KEY).map_or("", String::as_str); | ||
| carrier_tags | ||
| .split(',') | ||
| .filter_map(|pair| { | ||
| let (k, v) = pair.split_once('=')?; | ||
| if k.starts_with("_dd.p.") { | ||
| Some((k.to_string(), v.to_string())) |
There was a problem hiding this comment.
This _dd.p.* parsing helper is now duplicated (also in step_function_event.rs). Consider moving this logic into a single shared helper to keep propagation tag parsing consistent across trigger sources (and to make it easier to apply improvements like trimming/validation in one place).
| fn inject_span_context_to_headers(headers: &mut HeaderMap, span_context: &SpanContext) { | ||
| headers.insert( | ||
| DATADOG_TRACE_ID_KEY, | ||
| span_context | ||
| .trace_id | ||
| .to_string() | ||
| .parse() | ||
| .expect("Failed to parse trace id"), | ||
| { | ||
| #[allow(clippy::cast_possible_truncation)] // Datadog protocol uses lower 64 bits | ||
| let trace_id = span_context.trace_id as u64; | ||
| trace_id | ||
| } | ||
| .to_string() | ||
| .parse() | ||
| .expect("Failed to parse trace id"), | ||
| ); | ||
|
|
There was a problem hiding this comment.
SpanContext.trace_id is now a u128, but this uses as u64 which silently truncates. If the intended behavior is to send the lower 64 bits for Datadog headers, make that explicit (mask/helper) to document intent. Also, this helper currently only injects a subset of the Datadog propagation state into headers; consider serializing all relevant _dd.p.* propagation tags from span_context.tags into x-datadog-tags so important fields (e.g., 128-bit high bits and other propagated tags) aren't lost.
| #[allow(clippy::cast_possible_truncation)] // Datadog protocol uses lower 64 bits | ||
| { | ||
| context.invocation_span.trace_id = sc.trace_id as u64; | ||
| } |
There was a problem hiding this comment.
sc.trace_id as u64 silently truncates the u128 trace id when storing it into invocation_span.trace_id. If the system relies on lower-64-bits semantics, prefer an explicit conversion (mask/helper) and ensure higher-order bits are preserved via _dd.p.tid in tags when needed.
| #[allow(clippy::cast_possible_truncation)] // Datadog protocol uses lower 64 bits | |
| { | |
| context.invocation_span.trace_id = sc.trace_id as u64; | |
| } | |
| // Datadog protocol uses the lower 64 bits for the trace_id field; preserve full 128 bits via _dd.p.tid | |
| let full_trace_id = sc.trace_id; | |
| let lower_64_trace_id = (full_trace_id & 0xffff_ffff_ffff_ffff) as u64; | |
| context.invocation_span.trace_id = lower_64_trace_id; | |
| // Store full 128-bit trace id as 32-char hex in Datadog internal tag | |
| context | |
| .invocation_span | |
| .meta | |
| .entry("_dd.p.tid".to_string()) | |
| .or_insert_with(|| format!("{:032x}", full_trace_id)); |
| #[allow(clippy::cast_possible_truncation)] // Datadog protocol uses lower 64 bits | ||
| { | ||
| trace_id = sc.trace_id as u64; | ||
| } | ||
| parent_id = sc.span_id; | ||
| tags.extend(sc.tags.clone()); |
There was a problem hiding this comment.
Same truncation issue as earlier: sc.trace_id as u64 will drop higher 64 bits. Consider an explicit lower-64-bits conversion and ensure _dd.p.tid is present in the propagated tags when the upper bits are non-zero.
| #[allow(clippy::cast_possible_truncation)] // Datadog protocol uses lower 64 bits | |
| { | |
| trace_id = sc.trace_id as u64; | |
| } | |
| parent_id = sc.span_id; | |
| tags.extend(sc.tags.clone()); | |
| // Datadog protocol uses the lower 64 bits for the trace id field. | |
| let full_trace_id = sc.trace_id; | |
| let lower_64 = (full_trace_id & ((1u128 << 64) - 1)) as u64; | |
| let upper_64 = full_trace_id >> 64; | |
| trace_id = lower_64; | |
| parent_id = sc.span_id; | |
| tags.extend(sc.tags.clone()); | |
| // When the upper 64 bits are non-zero, ensure _dd.p.tid is present | |
| // so that the full 128-bit trace id can be reconstructed downstream. | |
| if upper_64 != 0 && !tags.contains_key("_dd.p.tid") { | |
| let upper_hex = format!("{:016x}", upper_64); | |
| tags.insert("_dd.p.tid".to_string(), upper_hex); | |
| } |
Overview
Migrate trace context propagation from a local implementation to the datadog-opentelemetry crate in dd-trace-rs. This removes ~750 lines of propagation logic (Datadog headers, W3C TraceContext) that we were maintaining locally in favor of the shared upstream
implementation.
Key changes:
Testing