Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libdd-trace-protobuf/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
18 changes: 18 additions & 0 deletions libdd-trace-protobuf/src/deserializers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T, D::Error>
where
Expand All @@ -12,6 +13,23 @@ where
Ok(opt.unwrap_or_default())
}

/// Deserialize a HashMap<String, String> where null values are skipped.
pub fn deserialize_map_with_nullable_values<'de, D>(
deserializer: D,
) -> Result<HashMap<String, String>, D::Error>
where
D: Deserializer<'de>,
{
let opt: Option<HashMap<String, Option<String>>> = 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: Default + PartialEq>(t: &T) -> bool {
t == &T::default()
}
Expand Down
4 changes: 3 additions & 1 deletion libdd-trace-protobuf/src/pb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 7 additions & 2 deletions libdd-trace-utils/src/msgpack_decoder/decode/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Comment thread
shreyamalpani marked this conversation as resolved.
#[inline]
pub fn read_str_map_to_strings<'a>(
buf: &mut &'a [u8],
Expand All @@ -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)
}
Expand All @@ -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],
Expand Down
33 changes: 33 additions & 0 deletions libdd-trace-utils/src/msgpack_decoder/v04/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
51 changes: 51 additions & 0 deletions libdd-trace-utils/src/trace_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Vec<pb::Span>> = 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"
);
}
}
Loading