Skip to content

Commit 01f18d5

Browse files
brettlangdonbwoebi
andauthored
fix(libdd-trace-utils): apply SpanLink flags masking when v0.5 json encoding (#2314)
# What does this PR do? A tracer flags a `SpanLink` with a 32-bit flags value. Bit 31 is a sentinel: it marks that the tracer explicitly set the sampling decision, as opposed to leaving `flags` at its default of zero. The remaining bits carry the sampling decision itself (bit 0: kept or dropped). # Motivation v0.4 and v0.5 disagree on whether the wire value includes this sentinel. - In v0.4's native msgpack encoding, dd-trace-py adds the sentinel bit before it writes flags. A kept link becomes `0x8000_0001`; a link the tracer explicitly dropped becomes `0x8000_0000`. - v0.5 has no native span-link field, so it encodes links as a JSON array under `meta["_dd.span_links"]`. dd-trace-py's v0.5 JSON encoder never adds the sentinel. A kept link's flags in this JSON is plain 1; the sentinel bit never appears there. libdatadog's v0.5 encoder builds this JSON from the same `SpanLink` struct that v0.4 uses, so flags may already carry the sentinel bit. Before this fix, the v0.5 serializer wrote that raw value straight into the JSON, so a kept link produced `"flags": 2147483649` instead of `1` — a value no v0.5 producer would ever emit, and one that downstream consumers checking against small integers would not recognize as "kept". This PR masks bit 31 off the value the v0.5 serializer writes into flags, while it still decides whether to emit the flags key at all from the raw, unmasked value. Deciding presence from the masked value would break the case of a link the tracer explicitly dropped: raw `0x8000_0000` masks to `0`, indistinguishable from "flags never set" if presence were decided post-mask. With this fix, an explicitly dropped link still emits `"flags": 0`; a link that never set flags emits no flags key at all. # Additional Notes Anything else we should know when reviewing? # How to test the change? - Added `span_link_flags_sentinel_bit_masked_test`: covers the unset, kept, and explicitly-dropped states of the sentinel bit. - `cargo test -p libdd-trace-utils`, `cargo fmt --check`, and `cargo clippy -D warnings` pass. Co-authored-by: bob.weinand <bob.weinand@datadoghq.com>
1 parent d50e49c commit 01f18d5

8 files changed

Lines changed: 238 additions & 13 deletions

File tree

libdd-trace-utils/src/agentless_encoder/mod.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
//! TODO: span normalization (service/name/resource/type truncation + defaults)
2828
2929
use crate::span::v04::{AttributeAnyValue, AttributeArrayValue, Span, SpanEvent, SpanLink};
30-
use crate::span::TraceData;
30+
use crate::span::{TraceData, SPAN_LINK_FLAGS_SET_SENTINEL};
3131
use crate::tracer_metadata::TracerMetadata;
3232
use serde::{
3333
ser::{SerializeMap, SerializeSeq},
@@ -341,9 +341,14 @@ fn encode_span_link<T: TraceData, S: Serializer>(
341341
}),
342342
)?;
343343
}
344-
// `flags == 0` means no sampling decision is available; omit the field.
344+
// When `flags` is 0, no sampling decision exists, so omit the field. Before emission,
345+
// mask off the internal "explicitly set" sentinel (bit 31), because this JSON field uses
346+
// the same `_dd.span_links` key that the v0.5 encoder produces and must match its output.
345347
if link.flags != 0 {
346-
map.serialize_entry("flags", &(link.flags as u64))?;
348+
map.serialize_entry(
349+
"flags",
350+
&((link.flags & !SPAN_LINK_FLAGS_SET_SENTINEL) as u64),
351+
)?;
347352
}
348353
let tracestate: &str = link.tracestate.borrow();
349354
if !tracestate.is_empty() {

libdd-trace-utils/src/agentless_encoder/tests.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,51 @@ fn span_links_serialised_into_meta_as_json_string() {
178178
assert_eq!(link_obj["tracestate"], "dd=s:1");
179179
}
180180

181+
#[cfg_attr(miri, ignore)] // serde_json/rmp_serde overhead is prohibitively slow under Miri
182+
#[test]
183+
fn span_link_flags_sentinel_bit_masked() {
184+
// The internal "explicitly set" sentinel (bit 31) must never appear in the
185+
// `_dd.span_links` JSON, which downstream consumers treat as the real W3C trace-flags value.
186+
// Covers both sentinel states: kept (0x8000_0001) and explicitly dropped (0x8000_0000).
187+
fn encoded_flags(flags: u32) -> serde_json::Value {
188+
let link = SpanLink::<BytesData> {
189+
trace_id: 0x11,
190+
span_id: 0x22,
191+
flags,
192+
..Default::default()
193+
};
194+
let span: Span<BytesData> = Span {
195+
service: bs("svc"),
196+
name: bs("op"),
197+
trace_id: 1,
198+
span_id: 1,
199+
parent_id: 0,
200+
start: 0,
201+
duration: 1,
202+
span_links: vec![link],
203+
..Default::default()
204+
};
205+
let v = json_from_bytes(&encode_payload(&[vec![span]], &base_metadata()).unwrap());
206+
let s = &v["traces"][0]["spans"][0];
207+
let raw = s["meta"]["_dd.span_links"]
208+
.as_str()
209+
.expect("meta[_dd.span_links] must be a string");
210+
let links: serde_json::Value = serde_json::from_str(raw).expect("must be valid JSON");
211+
links[0]["flags"].clone()
212+
}
213+
214+
assert_eq!(
215+
encoded_flags(0x8000_0001),
216+
1,
217+
"the sentinel bit must not leak into the _dd.span_links JSON"
218+
);
219+
assert_eq!(
220+
encoded_flags(0x8000_0000),
221+
0,
222+
"an explicit drop decision must still emit flags: 0, not omit the field"
223+
);
224+
}
225+
181226
#[cfg_attr(miri, ignore)] // serde_json/rmp_serde overhead is prohibitively slow under Miri
182227
#[test]
183228
fn span_events_serialised_into_meta_as_json_string() {

libdd-trace-utils/src/json_log_encoder/mod.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,43 @@ mod tests {
443443
assert_eq!(span_json["span_events"][0]["time_unix_nano"], 123);
444444
}
445445

446+
#[test]
447+
fn span_link_flags_sentinel_bit_masked() {
448+
// The internal "explicitly set" sentinel (bit 31) must never appear in the emitted
449+
// JSON log, which downstream consumers treat as the real W3C trace-flags value.
450+
// Covers both sentinel states: kept (0x8000_0001) and explicitly dropped (0x8000_0000).
451+
use crate::span::v04::SpanLink;
452+
453+
fn encoded_flags(flags: u32) -> Value {
454+
let span = SpanSlice {
455+
span_id: 1,
456+
span_links: vec![SpanLink {
457+
trace_id: 7,
458+
span_id: 8,
459+
flags,
460+
..Default::default()
461+
}],
462+
..Default::default()
463+
};
464+
let mut out = Vec::new();
465+
encode_traces(&[vec![span]], &mut out, MAX).unwrap();
466+
let emitted = lines(&out);
467+
let parsed: Value = serde_json::from_str(&emitted[0]).unwrap();
468+
parsed["traces"][0][0]["span_links"][0]["flags"].clone()
469+
}
470+
471+
assert_eq!(
472+
encoded_flags(0x8000_0001),
473+
1,
474+
"the sentinel bit must not leak into the JSON log"
475+
);
476+
assert_eq!(
477+
encoded_flags(0x8000_0000),
478+
0,
479+
"an explicit drop decision must still emit flags: 0, not omit the field"
480+
);
481+
}
482+
446483
#[test]
447484
fn empty_inner_trace_writes_nothing() {
448485
// One trace containing zero spans: nothing is emitted and no spans counted.

libdd-trace-utils/src/json_log_encoder/span.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
//! `T: Serialize` bound — keeping the public exporter API free of that bound.
1717
1818
use crate::span::v04::{Span, SpanEvent, SpanLink};
19-
use crate::span::TraceData;
19+
use crate::span::{TraceData, SPAN_LINK_FLAGS_SET_SENTINEL};
2020
use serde::ser::{SerializeSeq, SerializeStruct};
2121
use serde::{Serialize, Serializer};
2222
use std::borrow::Borrow;
@@ -102,7 +102,9 @@ impl<T: TraceData> Serialize for LogSpanLink<'_, T> {
102102
state.serialize_field("tracestate", &link.tracestate)?;
103103
}
104104
if has_flags {
105-
state.serialize_field("flags", &link.flags)?;
105+
// Mask off the internal "explicitly set" sentinel (bit 31): this JSON log field
106+
// is consumer-facing and must not expose the internal wire encoding.
107+
state.serialize_field("flags", &(link.flags & !SPAN_LINK_FLAGS_SET_SENTINEL))?;
106108
}
107109
state.end()
108110
}

libdd-trace-utils/src/msgpack_encoder/v04/span_v04.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,3 +281,56 @@ pub fn encode_span<W: RmpWrite, T: TraceData>(
281281

282282
Ok(())
283283
}
284+
285+
#[cfg(test)]
286+
mod tests {
287+
use super::super::to_vec_from_v04;
288+
use crate::span::v04::SpanLink;
289+
use crate::span::BytesData;
290+
291+
#[test]
292+
fn span_link_flags_are_not_masked_on_the_native_v04_wire() {
293+
// The v0.4 native msgpack wire format is the only place where the sentinel bit (bit 31)
294+
// stays in the raw value. The Datadog Agent and other tracers already decode this exact
295+
// bit pattern from v0.4 span links.
296+
let link = SpanLink::<BytesData> {
297+
trace_id: 1,
298+
span_id: 2,
299+
flags: 0x8000_0001,
300+
..Default::default()
301+
};
302+
let span = crate::span::v04::Span::<BytesData> {
303+
span_id: 1,
304+
span_links: vec![link],
305+
..Default::default()
306+
};
307+
let bytes = to_vec_from_v04(&[vec![span]]);
308+
let value = rmpv::decode::read_value(&mut &bytes[..]).expect("decode failed");
309+
let traces = value.as_array().expect("top-level must be an array");
310+
let trace = traces[0].as_array().expect("trace must be an array");
311+
let encoded_span = &trace[0];
312+
let span_links = encoded_span
313+
.as_map()
314+
.expect("span must be a map")
315+
.iter()
316+
.find(|(k, _)| k.as_str() == Some("span_links"))
317+
.map(|(_, v)| v)
318+
.expect("span_links must be present")
319+
.as_array()
320+
.expect("span_links must be an array");
321+
let flags = span_links[0]
322+
.as_map()
323+
.expect("span link must be a map")
324+
.iter()
325+
.find(|(k, _)| k.as_str() == Some("flags"))
326+
.map(|(_, v)| v)
327+
.expect("flags must be present")
328+
.as_u64()
329+
.expect("flags must be a u64");
330+
331+
assert_eq!(
332+
flags, 0x8000_0001,
333+
"v0.4's native wire format must carry flags raw, including the sentinel bit"
334+
);
335+
}
336+
}

libdd-trace-utils/src/otlp_encoder/mapper.rs

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
1212
use super::OtlpResourceInfo;
1313
use crate::span::v04::{Span, SpanEvent, SpanLink};
14-
use crate::span::TraceData;
14+
use crate::span::{TraceData, SPAN_LINK_FLAGS_SET_SENTINEL};
1515
use std::borrow::Borrow;
1616

1717
use libdd_trace_protobuf::opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest as ProtoReq;
@@ -469,8 +469,9 @@ fn map_span_link<T: TraceData>(link: &SpanLink<T>) -> ProtoLink {
469469
.collect(),
470470
dropped_attributes_count: 0,
471471
// W3C trace flags of the linked context (sampled bit, etc.); carry them through so OTLP
472-
// consumers see the same link metadata the tracer recorded.
473-
flags: link.flags,
472+
// consumers see the same link metadata the tracer recorded. Bit 31 is an internal
473+
// "explicitly set" sentinel that must not leak into OTLP's flags field.
474+
flags: link.flags & !SPAN_LINK_FLAGS_SET_SENTINEL,
474475
}
475476
}
476477

@@ -1005,6 +1006,42 @@ mod tests {
10051006
);
10061007
}
10071008

1009+
#[test]
1010+
fn span_link_flags_sentinel_bit_masked() {
1011+
// The internal "explicitly set" sentinel (bit 31) must never appear in OTLP's
1012+
// Link.flags, which downstream consumers treat as the real W3C trace-flags value.
1013+
// Covers both sentinel states: kept (0x8000_0001) and explicitly dropped (0x8000_0000).
1014+
fn mapped_flags(flags: u32) -> u32 {
1015+
let mut span: Span<BytesData> = Span {
1016+
trace_id: 1,
1017+
span_id: 2,
1018+
name: libdd_tinybytes::BytesString::from_static("s"),
1019+
start: 0,
1020+
duration: 1,
1021+
..Default::default()
1022+
};
1023+
span.span_links.push(SpanLink {
1024+
trace_id: 0x11,
1025+
span_id: 0x22,
1026+
flags,
1027+
..Default::default()
1028+
});
1029+
let req = map_traces_to_otlp(vec![vec![span]], &OtlpResourceInfo::default(), false);
1030+
req.resource_spans[0].scope_spans[0].spans[0].links[0].flags
1031+
}
1032+
1033+
assert_eq!(
1034+
mapped_flags(0x8000_0001),
1035+
1,
1036+
"OTLP Link.flags must not carry the internal sentinel bit"
1037+
);
1038+
assert_eq!(
1039+
mapped_flags(0x8000_0000),
1040+
0,
1041+
"an explicit drop decision must still map to flags: 0"
1042+
);
1043+
}
1044+
10081045
#[test]
10091046
fn test_otel_trace_semantics_enabled() {
10101047
// With OTel-semantics on, the DD-promoted attributes (service.name/operation.name/

libdd-trace-utils/src/span/mod.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,20 @@ use std::marker::PhantomData;
1919
use std::ptr::NonNull;
2020
use std::{fmt, ptr};
2121

22+
/// A `SpanLink`'s `flags` field reserves bit 31 to mean "a value was explicitly set", separate
23+
/// from the sampling decision carried in the low bits. The sentinel bit distinguishes
24+
/// `flags == 0` (never set) from an explicit decision of `0`, for example a dropped context.
25+
/// Without the sentinel, both cases look identical on the wire.
26+
///
27+
/// Every non-JSON wire format keeps this bit raw, except OTLP. The native v0.4 msgpack format
28+
/// (`msgpack_encoder::v04::span_v04`), the v1 msgpack format, and the native protobuf format
29+
/// (`libdd_trace_protobuf::pb::SpanLink`) all keep the sentinel raw in `flags`. Tracers already
30+
/// send the bit set in these formats. JSON formats and OTLP protobuf must mask this bit before
31+
/// they emit `flags`, because those consumers treat `flags` as the real W3C trace-flags value.
32+
/// The JSON formats are the v0.5 `_dd.span_links` dictionary, agentless JSON, and structured
33+
/// JSON logging.
34+
pub(crate) const SPAN_LINK_FLAGS_SET_SENTINEL: u32 = 1 << 31;
35+
2236
/// Trait representing the requirements for a type to be used as a Span "string" type.
2337
/// Note: Borrow<str> is not required by the derived traits, but allows to access HashMap elements
2438
/// from a static str and check if the string is empty.

libdd-trace-utils/src/span/v05/mod.rs

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
pub mod dict;
55

66
use crate::span::v04::{AttributeAnyValue, AttributeArrayValue, SpanEvent, SpanLink};
7-
use crate::span::{SharedDictBytes, SpanText, TraceData};
7+
use crate::span::{SharedDictBytes, SpanText, TraceData, SPAN_LINK_FLAGS_SET_SENTINEL};
88
use anyhow::Result;
99
use indexmap::map::RawEntryApiV1;
1010
use libdd_tinybytes::BytesString;
@@ -43,7 +43,7 @@ pub struct Span {
4343
/// low 64 bits).
4444
/// - `span_id` is the 64-bit id hex-encoded as 16 lowercase chars.
4545
/// - `tracestate` and `attributes` are only emitted when non-empty.
46-
/// - `flags` is only emitted when not zero.
46+
/// - `flags` is only emitted when not zero, with [`SPAN_LINK_FLAGS_SET_SENTINEL`] masked off.
4747
struct SpanLinksSerializerV05<'a, T: TraceData>(&'a [SpanLink<T>]);
4848
struct SpanLinkSerializerV05<'a, T: TraceData>(&'a SpanLink<T>);
4949

@@ -81,7 +81,7 @@ impl<'a, T: TraceData> Serialize for SpanLinkSerializerV05<'a, T> {
8181
)?;
8282
}
8383
if has_flags {
84-
map.serialize_entry("flags", &link.flags)?;
84+
map.serialize_entry("flags", &(link.flags & !SPAN_LINK_FLAGS_SET_SENTINEL))?;
8585
}
8686
map.end()
8787
}
@@ -465,8 +465,8 @@ mod tests {
465465
assert!(meta_json(&dict, &v05_span, "meta_struct").is_none());
466466
}
467467

468-
/// A link with no tracestate and no attributes serializes only hex `trace_id`/`span_id`;
469-
/// `flags` is dropped.
468+
/// A link with no tracestate and no attributes serializes only hex `trace_id`/`span_id`,
469+
/// plus `flags` since it is non-zero.
470470
#[test]
471471
fn span_link_minimal_serialization_test() {
472472
let links = vec![SpanLink::<BytesData> {
@@ -484,6 +484,38 @@ mod tests {
484484
);
485485
}
486486

487+
/// Covers all three [`SPAN_LINK_FLAGS_SET_SENTINEL`] states: unset, kept, and explicitly
488+
/// dropped.
489+
#[test]
490+
fn span_link_flags_sentinel_bit_masked_test() {
491+
let kept = vec![SpanLink::<BytesData> {
492+
span_id: 1,
493+
flags: 0x8000_0001,
494+
..Default::default()
495+
}];
496+
let json = serde_json::to_string(&SpanLinksSerializerV05::<BytesData>(&kept)).unwrap();
497+
let parsed: serde_json::Value = serde_json::from_str(&json).unwrap();
498+
assert_eq!(parsed[0]["flags"], serde_json::json!(1));
499+
500+
let dropped = vec![SpanLink::<BytesData> {
501+
span_id: 2,
502+
flags: 0x8000_0000,
503+
..Default::default()
504+
}];
505+
let json = serde_json::to_string(&SpanLinksSerializerV05::<BytesData>(&dropped)).unwrap();
506+
let parsed: serde_json::Value = serde_json::from_str(&json).unwrap();
507+
assert_eq!(parsed[0]["flags"], serde_json::json!(0));
508+
509+
let unset = vec![SpanLink::<BytesData> {
510+
span_id: 3,
511+
flags: 0,
512+
..Default::default()
513+
}];
514+
let json = serde_json::to_string(&SpanLinksSerializerV05::<BytesData>(&unset)).unwrap();
515+
let parsed: serde_json::Value = serde_json::from_str(&json).unwrap();
516+
assert!(parsed[0].get("flags").is_none());
517+
}
518+
487519
/// Multiple links serialize as an ordered JSON array preserving input order.
488520
#[test]
489521
fn span_links_multiple_serialization_test() {

0 commit comments

Comments
 (0)