diff --git a/libdd-trace-protobuf/build.rs b/libdd-trace-protobuf/build.rs index 983b1b5ffb..dcecdad2f8 100644 --- a/libdd-trace-protobuf/build.rs +++ b/libdd-trace-protobuf/build.rs @@ -93,7 +93,7 @@ fn generate_protobuf() { ); config.field_attribute( ".pb.Span.meta", - "#[serde(default)] #[serde(deserialize_with = \"crate::deserializers::deserialize_null_into_default\")]", + "#[serde(default)] #[serde(deserialize_with = \"crate::deserializers::deserialize_map_with_nullable_values\")]", ); config.field_attribute( ".pb.Span.metrics", diff --git a/libdd-trace-protobuf/src/deserializers.rs b/libdd-trace-protobuf/src/deserializers.rs index a90652635a..2c239bac03 100644 --- a/libdd-trace-protobuf/src/deserializers.rs +++ b/libdd-trace-protobuf/src/deserializers.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use serde::{Deserialize, Deserializer}; +use std::collections::HashMap; pub fn deserialize_null_into_default<'de, D, T>(deserializer: D) -> Result where @@ -12,6 +13,23 @@ where Ok(opt.unwrap_or_default()) } +/// Deserialize a HashMap where null values are skipped. +pub fn deserialize_map_with_nullable_values<'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let opt: Option>> = Option::deserialize(deserializer)?; + match opt { + None => Ok(HashMap::new()), + Some(map) => Ok(map + .into_iter() + .filter_map(|(k, v)| v.map(|val| (k, val))) + .collect()), + } +} + pub fn is_default(t: &T) -> bool { t == &T::default() } diff --git a/libdd-trace-protobuf/src/pb.rs b/libdd-trace-protobuf/src/pb.rs index 5234baf86d..06f46ba561 100644 --- a/libdd-trace-protobuf/src/pb.rs +++ b/libdd-trace-protobuf/src/pb.rs @@ -301,7 +301,9 @@ pub struct Span { /// @gotags: json:"meta,omitempty" msg:"meta,omitempty" #[prost(map = "string, string", tag = "10")] #[serde(default)] - #[serde(deserialize_with = "crate::deserializers::deserialize_null_into_default")] + #[serde( + deserialize_with = "crate::deserializers::deserialize_map_with_nullable_values" + )] pub meta: ::std::collections::HashMap< ::prost::alloc::string::String, ::prost::alloc::string::String, diff --git a/libdd-trace-utils/src/msgpack_decoder/decode/string.rs b/libdd-trace-utils/src/msgpack_decoder/decode/string.rs index 215fdd941a..9fff188c82 100644 --- a/libdd-trace-utils/src/msgpack_decoder/decode/string.rs +++ b/libdd-trace-utils/src/msgpack_decoder/decode/string.rs @@ -56,6 +56,7 @@ pub fn read_nullable_string<'a>(buf: &mut &'a [u8]) -> Result<&'a str, DecodeErr /// # Errors /// Fails if the buffer does not contain a valid map length prefix, /// or if any key or value is not a valid utf8 msgpack string. +/// Null values are skipped (key not inserted into map). #[inline] pub fn read_str_map_to_strings<'a>( buf: &mut &'a [u8], @@ -67,8 +68,11 @@ pub fn read_str_map_to_strings<'a>( let mut map = HashMap::with_capacity(len.try_into().expect("Unable to cast map len to usize")); for _ in 0..len { let key = read_string_ref(buf)?; - let value = read_string_ref(buf)?; - map.insert(key, value); + // Only insert if value is not null + if !handle_null_marker(buf) { + let value = read_string_ref(buf)?; + map.insert(key, value); + } } Ok(map) } @@ -78,6 +82,7 @@ pub fn read_str_map_to_strings<'a>( /// # Errors /// Fails if the buffer does not contain a valid map length prefix, /// or if any key or value is not a valid utf8 msgpack string. +/// Null values are skipped (key not inserted into map). #[inline] pub fn read_nullable_str_map_to_strings<'a>( buf: &mut &'a [u8], diff --git a/libdd-trace-utils/src/msgpack_decoder/v04/mod.rs b/libdd-trace-utils/src/msgpack_decoder/v04/mod.rs index 0a214b5997..c9dd29892b 100644 --- a/libdd-trace-utils/src/msgpack_decoder/v04/mod.rs +++ b/libdd-trace-utils/src/msgpack_decoder/v04/mod.rs @@ -492,6 +492,39 @@ mod tests { assert!(decoded_span.span_links.is_empty()); } + #[test] + fn test_decoder_meta_with_null_values() { + let mut span = create_test_json_span(1, 2, 0, 0, false); + span["meta"] = json!({ + "key1": "value1", + "key2": null, // This null value should be skipped + "key3": "value3" + }); + + let encoded_data = rmp_serde::to_vec_named(&vec![vec![span]]).unwrap(); + let (decoded_traces, _) = + from_bytes(libdd_tinybytes::Bytes::from(encoded_data)).expect("Decoding failed"); + + assert_eq!(1, decoded_traces.len()); + assert_eq!(1, decoded_traces[0].len()); + let decoded_span = &decoded_traces[0][0]; + + assert_eq!( + "value1", + decoded_span.meta[&BytesString::from_slice("key1".as_ref()).unwrap()].as_str() + ); + assert!( + !decoded_span + .meta + .contains_key(&BytesString::from_slice("key2".as_ref()).unwrap()), + "Null value should be skipped, but key was present" + ); + assert_eq!( + "value3", + decoded_span.meta[&BytesString::from_slice("key3".as_ref()).unwrap()].as_str() + ); + } + #[test] fn test_decoder_read_string_wrong_format() { let span = SpanBytes { diff --git a/libdd-trace-utils/src/trace_utils.rs b/libdd-trace-utils/src/trace_utils.rs index 4b74c149fb..00f556f145 100644 --- a/libdd-trace-utils/src/trace_utils.rs +++ b/libdd-trace-utils/src/trace_utils.rs @@ -1197,4 +1197,55 @@ mod tests { 1.0 ); } + + #[test] + fn test_rmp_serde_deserialize_meta_with_null_values() { + // Create a JSON representation with null value in meta + let span_json = json!({ + "service": "test-service", + "name": "test_name", + "resource": "test-resource", + "trace_id": 1_u64, + "span_id": 2_u64, + "parent_id": 0_u64, + "start": 0_i64, + "duration": 5_i64, + "error": 0_i32, + "meta": { + "service": "test-service", + "env": "test-env", + "runtime-id": "test-runtime-id-value", + "problematic_key": null // Ensure this null value does not cause an error + }, + "metrics": {}, + "type": "", + "meta_struct": {}, + "span_links": [], + "span_events": [] + }); + + let traces_json = vec![vec![span_json]]; + let encoded_data = rmp_serde::to_vec(&traces_json).unwrap(); + let traces: Vec> = rmp_serde::from_read(&encoded_data[..]) + .expect("Failed to deserialize traces with null values in meta"); + + assert_eq!(1, traces.len()); + assert_eq!(1, traces[0].len()); + let decoded_span = &traces[0][0]; + + assert_eq!("test-service", decoded_span.service); + assert_eq!("test_name", decoded_span.name); + assert_eq!("test-resource", decoded_span.resource); + assert_eq!("test-service", decoded_span.meta.get("service").unwrap()); + assert_eq!("test-env", decoded_span.meta.get("env").unwrap()); + assert_eq!( + "test-runtime-id-value", + decoded_span.meta.get("runtime-id").unwrap() + ); + // Assert that the null value was filtered out (key not present in map) + assert!( + !decoded_span.meta.contains_key("problematic_key"), + "Null value should be skipped, but key was present" + ); + } }