From 8b102c4d67a79347795f81d4396e50add4a917b7 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 13 May 2024 21:42:20 +0000 Subject: [PATCH 1/3] initial changes --- .../expected/failed_traces.json | 2 +- .../integration_test/expected/traces.json | 2 +- .../tests/integration_test/tests/traces.rs | 10 +- opentelemetry-proto/src/proto.rs | 99 +++++++++++++++++-- opentelemetry-proto/tests/grpc_build.rs | 4 + opentelemetry-proto/tests/json_deserialize.rs | 2 +- 6 files changed, 101 insertions(+), 18 deletions(-) diff --git a/opentelemetry-otlp/tests/integration_test/expected/failed_traces.json b/opentelemetry-otlp/tests/integration_test/expected/failed_traces.json index 160c86e1c2..1d065f4b2d 100644 --- a/opentelemetry-otlp/tests/integration_test/expected/failed_traces.json +++ b/opentelemetry-otlp/tests/integration_test/expected/failed_traces.json @@ -86,7 +86,7 @@ { "key": "bogons", "value": { - "intValue": 100 + "intValue": "100" } } ] diff --git a/opentelemetry-otlp/tests/integration_test/expected/traces.json b/opentelemetry-otlp/tests/integration_test/expected/traces.json index cc85443e76..49fcc1ea55 100644 --- a/opentelemetry-otlp/tests/integration_test/expected/traces.json +++ b/opentelemetry-otlp/tests/integration_test/expected/traces.json @@ -86,7 +86,7 @@ { "key": "bogons", "value": { - "intValue": 100 + "intValue": "100" } } ] diff --git a/opentelemetry-otlp/tests/integration_test/tests/traces.rs b/opentelemetry-otlp/tests/integration_test/tests/traces.rs index 45507e5035..64d59e911e 100644 --- a/opentelemetry-otlp/tests/integration_test/tests/traces.rs +++ b/opentelemetry-otlp/tests/integration_test/tests/traces.rs @@ -58,12 +58,12 @@ pub async fn traces() -> Result<(), Box> { Ok(()) } -pub fn assert_traces_results(result: &str, _: &str) { - // let left = read_spans_from_json(File::open(expected).unwrap()); - // let right = read_spans_from_json(File::open(result).unwrap()); +pub fn assert_traces_results(result: &str, expected: &str) { + let left = read_spans_from_json(File::open(expected).unwrap()); + let right = read_spans_from_json(File::open(result).unwrap()); + + TraceAsserter::new(left, right).assert(); - // TraceAsserter::new(left, right).assert(); - // // we cannot read result json file because the timestamp was represents as string instead of u64. // need to fix it on json file exporter diff --git a/opentelemetry-proto/src/proto.rs b/opentelemetry-proto/src/proto.rs index 2d10d7feab..fd7ddd7652 100644 --- a/opentelemetry-proto/src/proto.rs +++ b/opentelemetry-proto/src/proto.rs @@ -3,7 +3,7 @@ /// See https://opentelemetry.io/docs/specs/otlp/#json-protobuf-encoding for more details #[cfg(all(feature = "with-serde", feature = "gen-tonic-messages"))] pub(crate) mod serializers { - use crate::tonic::common::v1::any_value::Value; + use crate::tonic::common::v1::any_value::{self, Value}; use crate::tonic::common::v1::AnyValue; use serde::de::{self, MapAccess, Visitor}; use serde::ser::SerializeStruct; @@ -52,23 +52,86 @@ pub(crate) mod serializers { // Serialize any_value::Value using its own implementation // If value is None, it will be serialized as such match value { - Some(value) => value.value.serialize(serializer), + Some(any_value) => match &any_value.value { + Some(Value::IntValue(i)) => serialize_i64_to_string(i, serializer), + Some(value) => value.serialize(serializer), + None => serializer.serialize_none(), + }, None => serializer.serialize_none(), } - } pub fn deserialize_from_value<'de, D>(deserializer: D) -> Result, D::Error> - where - D: Deserializer<'de>, - { - // Deserialize any_value::Value using its own implementation - let value = Option::::deserialize(deserializer)?; +where + D: Deserializer<'de>, +{ + struct ValueVisitor; + + 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") + } - // Wrap the deserialized value in AnyValue - Ok(Some(AnyValue { value })) + 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 + } + } + } + + 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)) +} + pub fn serialize_u64_to_string(value: &u64, serializer: S) -> Result where S: Serializer, @@ -84,6 +147,22 @@ pub(crate) mod serializers { let s: String = Deserialize::deserialize(deserializer)?; s.parse::().map_err(de::Error::custom) } + + pub fn serialize_i64_to_string(value: &i64, serializer: S) -> Result + where + S: Serializer, + { + let s = value.to_string(); + serializer.serialize_str(&s) + } + + pub fn deserialize_string_to_i64<'de, D>(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let s: String = Deserialize::deserialize(deserializer)?; + s.parse::().map_err(de::Error::custom) + } } #[cfg(feature = "gen-tonic-messages")] diff --git a/opentelemetry-proto/tests/grpc_build.rs b/opentelemetry-proto/tests/grpc_build.rs index e63fd412ba..baaca42b05 100644 --- a/opentelemetry-proto/tests/grpc_build.rs +++ b/opentelemetry-proto/tests/grpc_build.rs @@ -90,6 +90,10 @@ fn build_tonic() { builder = builder .field_attribute("common.v1.KeyValue.value", "#[cfg_attr(feature =\"with-serde\", serde(serialize_with = \"crate::proto::serializers::serialize_to_value\", deserialize_with = \"crate::proto::serializers::deserialize_from_value\"))]"); + // add custom serializer and deserializer for Value.intValue + builder = builder + .field_attribute("common.v1.any_value.Value.IntValue11", "#[cfg_attr(feature =\"with-serde\", serde(serialize_with = \"crate::proto::serializers::serialize_i64_to_string\", deserialize_with = \"crate::proto::serializers::deserialize_string_to_i64\"))]"); + builder .out_dir(out_dir.path()) .compile(TONIC_PROTO_FILES, TONIC_INCLUDES) diff --git a/opentelemetry-proto/tests/json_deserialize.rs b/opentelemetry-proto/tests/json_deserialize.rs index f1fcd3d6b9..08fd2424c0 100644 --- a/opentelemetry-proto/tests/json_deserialize.rs +++ b/opentelemetry-proto/tests/json_deserialize.rs @@ -117,7 +117,7 @@ mod json_deserialize { let value: Value = serde_json::from_str( r#" { - "intValue": 123 + "intValue": "123" }"#, ) .unwrap(); From a34d22962dd6fe404a7d7f3a5c94f04791de6739 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 13 May 2024 21:43:47 +0000 Subject: [PATCH 2/3] fix --- opentelemetry-proto/tests/grpc_build.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/opentelemetry-proto/tests/grpc_build.rs b/opentelemetry-proto/tests/grpc_build.rs index baaca42b05..e63fd412ba 100644 --- a/opentelemetry-proto/tests/grpc_build.rs +++ b/opentelemetry-proto/tests/grpc_build.rs @@ -90,10 +90,6 @@ fn build_tonic() { builder = builder .field_attribute("common.v1.KeyValue.value", "#[cfg_attr(feature =\"with-serde\", serde(serialize_with = \"crate::proto::serializers::serialize_to_value\", deserialize_with = \"crate::proto::serializers::deserialize_from_value\"))]"); - // add custom serializer and deserializer for Value.intValue - builder = builder - .field_attribute("common.v1.any_value.Value.IntValue11", "#[cfg_attr(feature =\"with-serde\", serde(serialize_with = \"crate::proto::serializers::serialize_i64_to_string\", deserialize_with = \"crate::proto::serializers::deserialize_string_to_i64\"))]"); - builder .out_dir(out_dir.path()) .compile(TONIC_PROTO_FILES, TONIC_INCLUDES) From 8813ae149c122b68e8ede179b5c7fed6d594837f Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 13 May 2024 22:56:14 +0000 Subject: [PATCH 3/3] fix unit test --- opentelemetry-proto/tests/json_deserialize.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-proto/tests/json_deserialize.rs b/opentelemetry-proto/tests/json_deserialize.rs index 08fd2424c0..f1fcd3d6b9 100644 --- a/opentelemetry-proto/tests/json_deserialize.rs +++ b/opentelemetry-proto/tests/json_deserialize.rs @@ -117,7 +117,7 @@ mod json_deserialize { let value: Value = serde_json::from_str( r#" { - "intValue": "123" + "intValue": 123 }"#, ) .unwrap();