From 342e209f1780a97e6c4423715ad0f371a8d09037 Mon Sep 17 00:00:00 2001 From: Razvan Rotari Date: Tue, 2 Jul 2024 11:42:29 +0300 Subject: [PATCH 1/3] When deserializing JSON, accept both int and string in 'intValue' field of AnyValue --- opentelemetry-proto/src/proto.rs | 137 ++++++++++-------- opentelemetry-proto/tests/json_deserialize.rs | 30 ++++ 2 files changed, 109 insertions(+), 58 deletions(-) diff --git a/opentelemetry-proto/src/proto.rs b/opentelemetry-proto/src/proto.rs index 8bc372b889..a5f918ecc6 100644 --- a/opentelemetry-proto/src/proto.rs +++ b/opentelemetry-proto/src/proto.rs @@ -1,3 +1,5 @@ +use serde::Deserialize; + /// provide serde support for proto traceIds and spanIds. /// Those are hex encoded strings in the jsons but they are byte arrays in the proto. /// See https://opentelemetry.io/docs/specs/otlp/#json-protobuf-encoding for more details @@ -74,76 +76,95 @@ pub(crate) mod serializers { } pub fn deserialize_from_value<'de, D>(deserializer: D) -> Result, D::Error> -where - D: Deserializer<'de>, -{ - struct ValueVisitor; + where + D: Deserializer<'de>, + { + struct ValueVisitor; - impl<'de> de::Visitor<'de> for ValueVisitor { - type Value = AnyValue; + #[derive(Deserialize)] + #[serde(untagged)] + enum StringOrInt { + Int(i64), + String(String), + } - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("a JSON object for AnyValue") + impl StringOrInt { + fn get_int<'de, V>(&self) -> Result + where + V: de::MapAccess<'de>, + { + match self { + Self::Int(val) => Ok(*val), + Self::String(val) => Ok(val.parse::().map_err(de::Error::custom)?), + } + } } - fn visit_map(self, mut map: V) -> Result - where - V: de::MapAccess<'de>, - { - let mut value: Option = None; + impl<'de> de::Visitor<'de> for ValueVisitor { + type Value = AnyValue; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a JSON object for AnyValue") + } + + fn visit_map(self, mut map: V) -> Result + where + V: de::MapAccess<'de>, + { + let mut value: Option = None; - while let Some(key) = map.next_key::()? { - let key_str = key.as_str(); - match key_str { - "stringValue" => { - let s = map.next_value()?; - value = Some(any_value::Value::StringValue(s)); - }, - "boolValue" => { - let b = map.next_value()?; - value = Some(any_value::Value::BoolValue(b)); - }, - "intValue" => { - let value_str = map.next_value::()?; - let int_value = value_str.parse::() - .map_err(de::Error::custom)?; - value = Some(any_value::Value::IntValue(int_value)); - }, - "doubleValue" => { - let d = map.next_value()?; - value = Some(any_value::Value::DoubleValue(d)); - }, - "arrayValue" => { - let a = map.next_value()?; - value = Some(any_value::Value::ArrayValue(a)); - }, - "kvlistValue" => { - let kv = map.next_value()?; - value = Some(any_value::Value::KvlistValue(kv)); - }, - "bytesValue" => { - let bytes = map.next_value()?; - value = Some(any_value::Value::BytesValue(bytes)); - }, - _ => { - //skip unknown keys, and handle error later. - continue + while let Some(key) = map.next_key::()? { + let key_str = key.as_str(); + match key_str { + "stringValue" => { + let s = map.next_value()?; + value = Some(any_value::Value::StringValue(s)); + } + "boolValue" => { + let b = map.next_value()?; + value = Some(any_value::Value::BoolValue(b)); + } + "intValue" => { + let int_value = map.next_value::()?.get_int::()?; + value = Some(any_value::Value::IntValue(int_value)); + } + "doubleValue" => { + let d = map.next_value()?; + value = Some(any_value::Value::DoubleValue(d)); + } + "arrayValue" => { + let a = map.next_value()?; + value = Some(any_value::Value::ArrayValue(a)); + } + "kvlistValue" => { + let kv = map.next_value()?; + value = Some(any_value::Value::KvlistValue(kv)); + } + "bytesValue" => { + let bytes = map.next_value()?; + value = Some(any_value::Value::BytesValue(bytes)); + } + _ => { + //skip unknown keys, and handle error later. + continue; + } } } - } - if let Some(v) = value { - Ok(AnyValue { value: Some(v) }) - } else { - Err(de::Error::custom("Invalid data for AnyValue, no known keys found")) + if let Some(v) = value { + Ok(AnyValue { value: Some(v) }) + } else { + Err(de::Error::custom( + "Invalid data for AnyValue, no known keys found", + )) + } } } + + let value = deserializer.deserialize_map(ValueVisitor)?; + Ok(Some(value)) } - let value = deserializer.deserialize_map(ValueVisitor)?; - Ok(Some(value)) -} - pub fn serialize_u64_to_string(value: &u64, serializer: S) -> Result where S: Serializer, diff --git a/opentelemetry-proto/tests/json_deserialize.rs b/opentelemetry-proto/tests/json_deserialize.rs index f1fcd3d6b9..6632ffadd5 100644 --- a/opentelemetry-proto/tests/json_deserialize.rs +++ b/opentelemetry-proto/tests/json_deserialize.rs @@ -146,6 +146,36 @@ mod json_deserialize { keyvalue.value.unwrap().value.unwrap(), Value::StringValue("my.service".to_string()) ); + + let keyvalue: KeyValue = serde_json::from_str( + r#" +{ + "key": "service.name", + "value": { + "intValue": "303" + } + } + "#, + ) + .unwrap(); + + assert_eq!(keyvalue.key, "service.name".to_string()); + assert_eq!(keyvalue.value.unwrap().value.unwrap(), Value::IntValue(303)); + + let keyvalue: KeyValue = serde_json::from_str( + r#" +{ + "key": "service.name", + "value": { + "intValue": 303 + } + } + "#, + ) + .unwrap(); + + assert_eq!(keyvalue.key, "service.name".to_string()); + assert_eq!(keyvalue.value.unwrap().value.unwrap(), Value::IntValue(303)); } #[test] From 96254abca49f1c4e2100c653aba04cdb5e844f08 Mon Sep 17 00:00:00 2001 From: Razvan Rotari Date: Tue, 2 Jul 2024 12:54:29 +0300 Subject: [PATCH 2/3] Remove unnecessary use statement --- opentelemetry-proto/src/proto.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/opentelemetry-proto/src/proto.rs b/opentelemetry-proto/src/proto.rs index a5f918ecc6..ba4038072f 100644 --- a/opentelemetry-proto/src/proto.rs +++ b/opentelemetry-proto/src/proto.rs @@ -1,5 +1,3 @@ -use serde::Deserialize; - /// provide serde support for proto traceIds and spanIds. /// Those are hex encoded strings in the jsons but they are byte arrays in the proto. /// See https://opentelemetry.io/docs/specs/otlp/#json-protobuf-encoding for more details @@ -59,15 +57,15 @@ pub(crate) mod serializers { Ok(s) => s, Err(e) => return Err(e), // Handle the error or return it }; - + // Attempt to serialize the intValue field if let Err(e) = state.serialize_field("intValue", &i.to_string()) { return Err(e); // Handle the error or return it } - + // Finalize the struct serialization state.end() - }, + } Some(value) => value.serialize(serializer), None => serializer.serialize_none(), }, @@ -188,7 +186,7 @@ pub(crate) mod serializers { let s = value.to_string(); serializer.serialize_str(&s) } - + pub fn deserialize_string_to_i64<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, From b78cc0162de1ca13d81429d19287fabcbf0aca15 Mon Sep 17 00:00:00 2001 From: Razvan Rotari Date: Thu, 4 Jul 2024 11:18:21 +0300 Subject: [PATCH 3/3] Update integration tests --- .../tests/integration_test/expected/traces.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/opentelemetry-otlp/tests/integration_test/expected/traces.json b/opentelemetry-otlp/tests/integration_test/expected/traces.json index 49fcc1ea55..97e2b04d4d 100644 --- a/opentelemetry-otlp/tests/integration_test/expected/traces.json +++ b/opentelemetry-otlp/tests/integration_test/expected/traces.json @@ -88,6 +88,12 @@ "value": { "intValue": "100" } + }, + { + "key": "number/int", + "value": { + "intValue": 100 + } } ] }