Skip to content

Commit 1c727ec

Browse files
yannhamclaude
andcommitted
fix: gate VecMap PartialEq behind test-utils, fix FFI key dedup and deletion
VecMap PartialEq is now HashMap-based (order-independent) and only available in test/test-utils builds. FFI delete functions use remove_slow for correct has/del semantics. get_keys deduplicates output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a99876a commit 1c727ec

5 files changed

Lines changed: 19 additions & 11 deletions

File tree

datadog-sidecar-ffi/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ libdd-crashtracker-ffi = { path = "../libdd-crashtracker-ffi", features = ["coll
3535

3636
[dev-dependencies]
3737
http = "1.1"
38+
libdd-trace-utils = { path = "../libdd-trace-utils", features = ["test-utils"] }
3839
tempfile = { version = "3.3" }
3940

4041
[lints.rust]

datadog-sidecar-ffi/src/span.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use libdd_trace_utils::span::v04::{
77
AttributeAnyValueBytes, AttributeArrayValueBytes, SpanBytes, SpanEventBytes, SpanLinkBytes,
88
VecMap,
99
};
10-
use libdd_trace_utils::span::vec_map::Tombstone;
1110
use std::borrow::Cow;
1211
use std::collections::HashMap;
1312
use std::ffi::{c_char, CString};
@@ -67,12 +66,6 @@ fn remove_vec_map_slow<V>(map: &mut VecMap<BytesString, V>, key: CharSlice) {
6766
map.remove_slow(&bytes_str_key);
6867
}
6968

70-
#[inline]
71-
fn remove_vec_map<V: Tombstone>(map: &mut VecMap<BytesString, V>, key: CharSlice) {
72-
let bytes_str_key = convert_char_slice_to_bytes_string(key);
73-
map.remove_unset(&bytes_str_key);
74-
}
75-
7669
#[inline]
7770
fn exists_vec_map<V>(map: &VecMap<BytesString, V>, key: CharSlice) -> bool {
7871
let bytes_str_key = convert_char_slice_to_bytes_string(key);
@@ -87,6 +80,7 @@ fn get_vec_map_keys<'a, V>(
8780
) -> *mut CharSlice<'a> {
8881
let mut keys: Vec<&str> = map.iter().map(|(k, _)| k.as_str()).collect();
8982
keys.sort_unstable();
83+
keys.dedup();
9084

9185
let slices: Box<[CharSlice]> = keys
9286
.iter()
@@ -350,7 +344,7 @@ pub extern "C" fn ddog_add_span_meta(span: &mut SpanBytes, key: CharSlice, value
350344

351345
#[no_mangle]
352346
pub extern "C" fn ddog_del_span_meta(span: &mut SpanBytes, key: CharSlice) {
353-
remove_vec_map(&mut span.meta, key);
347+
remove_vec_map_slow(&mut span.meta, key);
354348
}
355349

356350
#[no_mangle]

datadog-sidecar-ffi/tests/span.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ fn test_span_debug_log_output() {
186186
ddog_set_span_name(span, CharSlice::from("debug-span"));
187187
let debug_output = ddog_span_debug_log(span);
188188

189-
let expected_output = CharSlice::from("Span { service: , name: debug-span, resource: , type: , trace_id: 0, span_id: 0, parent_id: 0, start: 0, duration: 0, error: 0, meta: [], metrics: [], meta_struct: [], span_links: [], span_events: [] }");
189+
let expected_output = CharSlice::from("Span { service: , name: debug-span, resource: , type: , trace_id: 0, span_id: 0, parent_id: 0, start: 0, duration: 0, error: 0, meta: VecMap([]), metrics: VecMap([]), meta_struct: VecMap([]), span_links: [], span_events: [] }");
190190

191191
assert_eq!(debug_output, expected_output);
192192

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ fn is_empty_str<T: Borrow<str>>(value: &T) -> bool {
7676
/// let _ = span.meta.get("foo");
7777
/// }
7878
/// ```
79-
#[derive(Debug, Default, PartialEq, Serialize)]
79+
#[derive(Debug, Default, Serialize)]
80+
#[cfg_attr(any(test, feature = "test-utils"), derive(PartialEq))]
8081
pub struct Span<T: TraceData> {
8182
pub service: T::Text,
8283
pub name: T::Text,

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,21 @@ use std::hash::Hash;
3131
/// Explicit deduplication is currently being done automatically and on-the-fly during
3232
/// serialization. If needed, in the future, we might trigger deduplication on other events, for
3333
/// example at insertion if the size is bigger than a threshold.
34-
#[derive(Clone, Debug, PartialEq, Default)]
34+
#[derive(Clone, Debug, Default)]
3535
pub struct VecMap<K, V>(Vec<(K, V)>);
3636

37+
#[cfg(any(test, feature = "test-utils"))]
38+
impl<K: Eq + Hash, V: PartialEq> PartialEq for VecMap<K, V> {
39+
fn eq(&self, other: &Self) -> bool {
40+
let lhs: HashMap<&K, &V> = self.0.iter().map(|(k, v)| (k, v)).collect();
41+
let rhs: HashMap<&K, &V> = other.0.iter().map(|(k, v)| (k, v)).collect();
42+
lhs == rhs
43+
}
44+
}
45+
46+
#[cfg(any(test, feature = "test-utils"))]
47+
impl<K: Eq + Hash, V: PartialEq> Eq for VecMap<K, V> {}
48+
3749
/// Values that can be replaced by a special tombstone value that makes them to be ignored later in
3850
/// the pipeline. This makes their removal from a [VecMap] a bit more efficient (still a linear
3951
/// scan, but no shifting involved). Technically equivalent to [Default], but of course having a

0 commit comments

Comments
 (0)