From 940b0eee2063f2942d7997f378de149c95b5ef0c Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Tue, 31 Dec 2024 09:04:48 -0800 Subject: [PATCH 1/5] initial commit --- opentelemetry-proto/src/proto.rs | 18 +++++ .../tonic/opentelemetry.proto.metrics.v1.rs | 70 +++++++++++++++++++ opentelemetry-proto/tests/grpc_build.rs | 40 +++++++++++ 3 files changed, 128 insertions(+) diff --git a/opentelemetry-proto/src/proto.rs b/opentelemetry-proto/src/proto.rs index 792e1fc945..196f4676a5 100644 --- a/opentelemetry-proto/src/proto.rs +++ b/opentelemetry-proto/src/proto.rs @@ -182,6 +182,24 @@ pub(crate) mod serializers { let s: String = Deserialize::deserialize(deserializer)?; s.parse::().map_err(de::Error::custom) } + pub fn serialize_vec_u64_to_strings(vec: &Vec, serializer: S) -> Result + where + S: Serializer, + { + let str_vec: Vec = vec.iter().map(|&num| num.to_string()).collect(); + serializer.collect_seq(str_vec) + } + + pub fn deserialize_strings_to_vec_u64<'de, D>(deserializer: D) -> Result, D::Error> + where + D: Deserializer<'de>, + { + let str_vec: Vec = Deserialize::deserialize(deserializer)?; + str_vec + .into_iter() + .map(|s| s.parse::().map_err(de::Error::custom)) + .collect() + } } #[cfg(feature = "gen-tonic-messages")] diff --git a/opentelemetry-proto/src/proto/tonic/opentelemetry.proto.metrics.v1.rs b/opentelemetry-proto/src/proto/tonic/opentelemetry.proto.metrics.v1.rs index fb04bec6cc..a0f1440189 100644 --- a/opentelemetry-proto/src/proto/tonic/opentelemetry.proto.metrics.v1.rs +++ b/opentelemetry-proto/src/proto/tonic/opentelemetry.proto.metrics.v1.rs @@ -431,6 +431,13 @@ pub struct HistogramDataPoint { /// value must be equal to the sum of the "count" fields in buckets if a /// histogram is provided. #[prost(fixed64, tag = "4")] + #[cfg_attr( + feature = "with-serde", + serde( + serialize_with = "crate::proto::serializers::serialize_u64_to_string", + deserialize_with = "crate::proto::serializers::deserialize_string_to_u64" + ) + )] pub count: u64, /// sum of the values in the population. If count is zero then this field /// must be zero. @@ -450,6 +457,13 @@ pub struct HistogramDataPoint { /// The number of elements in bucket_counts array must be by one greater than /// the number of elements in explicit_bounds array. #[prost(fixed64, repeated, tag = "6")] + #[cfg_attr( + feature = "with-serde", + serde( + serialize_with = "crate::proto::serializers::serialize_vec_u64_to_strings", + deserialize_with = "crate::proto::serializers::deserialize_strings_to_vec_u64" + ) + )] pub bucket_counts: ::prost::alloc::vec::Vec, /// explicit_bounds specifies buckets with explicitly defined bounds for values. /// @@ -503,17 +517,38 @@ pub struct ExponentialHistogramDataPoint { /// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January /// 1970. #[prost(fixed64, tag = "2")] + #[cfg_attr( + feature = "with-serde", + serde( + serialize_with = "crate::proto::serializers::serialize_u64_to_string", + deserialize_with = "crate::proto::serializers::deserialize_string_to_u64" + ) + )] pub start_time_unix_nano: u64, /// TimeUnixNano is required, see the detailed comments above Metric. /// /// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January /// 1970. #[prost(fixed64, tag = "3")] + #[cfg_attr( + feature = "with-serde", + serde( + serialize_with = "crate::proto::serializers::serialize_u64_to_string", + deserialize_with = "crate::proto::serializers::deserialize_string_to_u64" + ) + )] pub time_unix_nano: u64, /// count is the number of values in the population. Must be /// non-negative. This value must be equal to the sum of the "bucket_counts" /// values in the positive and negative Buckets plus the "zero_count" field. #[prost(fixed64, tag = "4")] + #[cfg_attr( + feature = "with-serde", + serde( + serialize_with = "crate::proto::serializers::serialize_u64_to_string", + deserialize_with = "crate::proto::serializers::deserialize_string_to_u64" + ) + )] pub count: u64, /// sum of the values in the population. If count is zero then this field /// must be zero. @@ -551,6 +586,13 @@ pub struct ExponentialHistogramDataPoint { /// Implementations MAY consider the zero bucket to have probability /// mass equal to (zero_count / count). #[prost(fixed64, tag = "7")] + #[cfg_attr( + feature = "with-serde", + serde( + serialize_with = "crate::proto::serializers::serialize_u64_to_string", + deserialize_with = "crate::proto::serializers::deserialize_string_to_u64" + ) + )] pub zero_count: u64, /// positive carries the positive range of exponential bucket counts. #[prost(message, optional, tag = "8")] @@ -628,15 +670,36 @@ pub struct SummaryDataPoint { /// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January /// 1970. #[prost(fixed64, tag = "2")] + #[cfg_attr( + feature = "with-serde", + serde( + serialize_with = "crate::proto::serializers::serialize_u64_to_string", + deserialize_with = "crate::proto::serializers::deserialize_string_to_u64" + ) + )] pub start_time_unix_nano: u64, /// TimeUnixNano is required, see the detailed comments above Metric. /// /// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January /// 1970. #[prost(fixed64, tag = "3")] + #[cfg_attr( + feature = "with-serde", + serde( + serialize_with = "crate::proto::serializers::serialize_u64_to_string", + deserialize_with = "crate::proto::serializers::deserialize_string_to_u64" + ) + )] pub time_unix_nano: u64, /// count is the number of values in the population. Must be non-negative. #[prost(fixed64, tag = "4")] + #[cfg_attr( + feature = "with-serde", + serde( + serialize_with = "crate::proto::serializers::serialize_u64_to_string", + deserialize_with = "crate::proto::serializers::deserialize_string_to_u64" + ) + )] pub count: u64, /// sum of the values in the population. If count is zero then this field /// must be zero. @@ -704,6 +767,13 @@ pub struct Exemplar { /// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January /// 1970. #[prost(fixed64, tag = "2")] + #[cfg_attr( + feature = "with-serde", + serde( + serialize_with = "crate::proto::serializers::serialize_u64_to_string", + deserialize_with = "crate::proto::serializers::deserialize_string_to_u64" + ) + )] pub time_unix_nano: u64, /// (Optional) Span ID of the exemplar trace. /// span_id may be missing if the measurement is not recorded inside a trace diff --git a/opentelemetry-proto/tests/grpc_build.rs b/opentelemetry-proto/tests/grpc_build.rs index d09a13cd64..fdf136f780 100644 --- a/opentelemetry-proto/tests/grpc_build.rs +++ b/opentelemetry-proto/tests/grpc_build.rs @@ -97,20 +97,60 @@ fn build_tonic() { // the proto file uses u64 for timestamp // Thus, special serializer and deserializer are needed for path in [ + //trace "trace.v1.Span.start_time_unix_nano", "trace.v1.Span.end_time_unix_nano", "trace.v1.Span.Event.time_unix_nano", + //logs "logs.v1.LogRecord.time_unix_nano", "logs.v1.LogRecord.observed_time_unix_nano", + //metrics "metrics.v1.HistogramDataPoint.start_time_unix_nano", "metrics.v1.HistogramDataPoint.time_unix_nano", "metrics.v1.NumberDataPoint.start_time_unix_nano", "metrics.v1.NumberDataPoint.time_unix_nano", + "metrics.v1.ExponentialHistogramDataPoint.start_time_unix_nano", + "metrics.v1.ExponentialHistogramDataPoint.time_unix_nano", + "metrics.v1.SummaryDataPoint.start_time_unix_nano", + "metrics.v1.SummaryDataPoint.time_unix_nano", + "metrics.v1.Exemplar.time_unix_nano", ] { builder = builder .field_attribute(path, "#[cfg_attr(feature = \"with-serde\", serde(serialize_with = \"crate::proto::serializers::serialize_u64_to_string\", deserialize_with = \"crate::proto::serializers::deserialize_string_to_u64\"))]") } + // special serializer and deserializer for metrics count + // OTLP/JSON format may uses string for count + // the proto file uses u64 for count + // Thus, special serializer and deserializer are needed + for path in [ + // metrics count and bucket fields + "metrics.v1.HistogramDataPoint.count", + "metrics.v1.ExponentialHistogramDataPoint.count", + "metrics.v1.ExponentialHistogramDataPoint.zero_count", + "metrics.v1.SummaryDataPoint.count", + ] { + builder = builder.field_attribute( + path, + "#[cfg_attr(feature = \"with-serde\", serde(serialize_with = \"crate::proto::serializers::serialize_u64_to_string\", deserialize_with = \"crate::proto::serializers::deserialize_string_to_u64\"))]", + ); + } + + // special serializer and deserializer for metrics bucket counts + // OTLP/JSON format may uses string for bucket counts + // the proto file uses u64 for bucket counts + // Thus, special serializer and deserializer are needed + for path in [ + "metrics.v1.HistogramDataPoint.bucket_counts", + "metrics.v1.ExponentialHistogramDataPoint.positive.bucket_counts", + "metrics.v1.ExponentialHistogramDataPoint.negative.bucket_counts", + ] { + builder = builder.field_attribute( + path, + "#[cfg_attr(feature = \"with-serde\", serde(serialize_with = \"crate::proto::serializers::serialize_vec_u64_to_strings\", deserialize_with = \"crate::proto::serializers::deserialize_strings_to_vec_u64\"))]", + ); + } + // special serializer and deserializer for value // The Value::value field must be hidden builder = builder From 3fa2e649f4b54c967fa9ce0fc5c14fdff25af773 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 6 Jan 2025 22:35:38 -0800 Subject: [PATCH 2/5] fix --- opentelemetry-proto/tests/json_serde.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/opentelemetry-proto/tests/json_serde.rs b/opentelemetry-proto/tests/json_serde.rs index 389541cce7..cb11646aad 100644 --- a/opentelemetry-proto/tests/json_serde.rs +++ b/opentelemetry-proto/tests/json_serde.rs @@ -978,11 +978,11 @@ mod json_serde { ], "startTimeUnixNano": "1544712660300000000", "timeUnixNano": "1544712660300000000", - "count": 2, + "count": "2", "sum": 2.0, "bucketCounts": [ - 1, - 1 + "1", + "1" ], "explicitBounds": [ 1.0 @@ -1090,9 +1090,9 @@ mod json_serde { { "startTimeUnixNano": "1544712660300000000", "timeUnixNano": "1544712660300000000", - "count": 2, + "count": "2", "sum": 2, - "bucketCounts": [1,1], + "bucketCounts": ["1","1"], "explicitBounds": [1], "min": 0, "max": 2, From 808505959d46f199453dc94507f6301d754b970d Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Tue, 7 Jan 2025 01:53:01 -0800 Subject: [PATCH 3/5] add NaN serialization/deserialization --- opentelemetry-proto/src/proto.rs | 58 +++++++++++++++++++ .../tonic/opentelemetry.proto.metrics.v1.rs | 14 +++++ opentelemetry-proto/tests/grpc_build.rs | 13 +++++ 3 files changed, 85 insertions(+) diff --git a/opentelemetry-proto/src/proto.rs b/opentelemetry-proto/src/proto.rs index 196f4676a5..bd02d1ecff 100644 --- a/opentelemetry-proto/src/proto.rs +++ b/opentelemetry-proto/src/proto.rs @@ -200,6 +200,64 @@ pub(crate) mod serializers { .map(|s| s.parse::().map_err(de::Error::custom)) .collect() } + + +// Special serializer and deserializer for NaN, Infinity, and -Infinity +pub fn serialize_f64_special(value: &f64, serializer: S) -> Result +where + S: Serializer, +{ + if value.is_nan() { + serializer.serialize_str("NaN") + } else if value.is_infinite() { + if value.is_sign_positive() { + serializer.serialize_str("Infinity") + } else { + serializer.serialize_str("-Infinity") + } + } else { + serializer.serialize_f64(*value) + } +} + +pub fn deserialize_f64_special<'de, D>(deserializer: D) -> Result +where + D: Deserializer<'de>, +{ + struct F64Visitor; + + impl<'de> de::Visitor<'de> for F64Visitor { + type Value = f64; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a float or a string representing NaN, Infinity, or -Infinity") + } + + fn visit_f64(self, value: f64) -> Result + where + E: de::Error, + { + Ok(value) + } + + fn visit_str(self, value: &str) -> Result + where + E: de::Error, + { + match value { + "NaN" => Ok(f64::NAN), + "Infinity" => Ok(f64::INFINITY), + "-Infinity" => Ok(f64::NEG_INFINITY), + _ => Err(E::custom(format!( + "Invalid string for f64: expected NaN, Infinity, or -Infinity but got '{}'", + value + ))), + } + } + } + + deserializer.deserialize_any(F64Visitor) + } } #[cfg(feature = "gen-tonic-messages")] diff --git a/opentelemetry-proto/src/proto/tonic/opentelemetry.proto.metrics.v1.rs b/opentelemetry-proto/src/proto/tonic/opentelemetry.proto.metrics.v1.rs index a0f1440189..6f3feebd50 100644 --- a/opentelemetry-proto/src/proto/tonic/opentelemetry.proto.metrics.v1.rs +++ b/opentelemetry-proto/src/proto/tonic/opentelemetry.proto.metrics.v1.rs @@ -738,11 +738,25 @@ pub mod summary_data_point { /// The quantile of a distribution. Must be in the interval /// \[0.0, 1.0\]. #[prost(double, tag = "1")] + #[cfg_attr( + feature = "with-serde", + serde( + serialize_with = "crate::proto::serializers::serialize_f64_special", + deserialize_with = "crate::proto::serializers::deserialize_f64_special" + ) + )] pub quantile: f64, /// The value at the given quantile of a distribution. /// /// Quantile values must NOT be negative. #[prost(double, tag = "2")] + #[cfg_attr( + feature = "with-serde", + serde( + serialize_with = "crate::proto::serializers::serialize_f64_special", + deserialize_with = "crate::proto::serializers::deserialize_f64_special" + ) + )] pub value: f64, } } diff --git a/opentelemetry-proto/tests/grpc_build.rs b/opentelemetry-proto/tests/grpc_build.rs index fdf136f780..dd0ea70f97 100644 --- a/opentelemetry-proto/tests/grpc_build.rs +++ b/opentelemetry-proto/tests/grpc_build.rs @@ -151,6 +151,19 @@ fn build_tonic() { ); } + // Special handling for floating-point fields that might contain NaN, Infinity, or -Infinity + // TODO: More needs to be added here as we find more fields that need this special handling + for path in [ + // metrics + "metrics.v1.SummaryDataPoint.ValueAtQuantile.value", + "metrics.v1.SummaryDataPoint.ValueAtQuantile.quantile", + ] { + builder = builder.field_attribute( + path, + "#[cfg_attr(feature = \"with-serde\", serde(serialize_with = \"crate::proto::serializers::serialize_f64_special\", deserialize_with = \"crate::proto::serializers::deserialize_f64_special\"))]", + ); + } + // special serializer and deserializer for value // The Value::value field must be hidden builder = builder From 03d5e9a496154c86459a7df13c19c680d1053d6b Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Tue, 28 Jan 2025 15:55:44 -0800 Subject: [PATCH 4/5] add tests for NaN --- opentelemetry-proto/tests/json_serde.rs | 181 ++++++++++++++++++++++++ 1 file changed, 181 insertions(+) diff --git a/opentelemetry-proto/tests/json_serde.rs b/opentelemetry-proto/tests/json_serde.rs index cb11646aad..17ba4eb710 100644 --- a/opentelemetry-proto/tests/json_serde.rs +++ b/opentelemetry-proto/tests/json_serde.rs @@ -1503,4 +1503,185 @@ mod json_serde { } } } + #[cfg(feature = "metrics")] + mod metrics_with_nan { + use super::*; + use opentelemetry_proto::tonic::metrics::v1::summary_data_point::ValueAtQuantile; + use opentelemetry_proto::tonic::metrics::v1::Summary; + use opentelemetry_proto::tonic::metrics::v1::SummaryDataPoint; + + fn value_with_nan() -> ExportMetricsServiceRequest { + ExportMetricsServiceRequest { + resource_metrics: vec![ResourceMetrics { + resource: Some(Resource { + attributes: vec![KeyValue { + key: String::from("service.name"), + value: None, + }], + dropped_attributes_count: 0, + }), + scope_metrics: vec![ScopeMetrics { + scope: None, + metrics: vec![Metric { + name: String::from("example_metric"), + description: String::from("A sample metric with NaN values"), + unit: String::from("1"), + metadata: vec![], + data: Some( + opentelemetry_proto::tonic::metrics::v1::metric::Data::Summary( + Summary { + data_points: vec![SummaryDataPoint { + attributes: vec![], + start_time_unix_nano: 0, + time_unix_nano: 0, + count: 100, + sum: 500.0, + quantile_values: vec![ + ValueAtQuantile { + quantile: 0.5, + value: f64::NAN, + }, + ValueAtQuantile { + quantile: 0.9, + value: f64::NAN, + }, + ], + flags: 0, + }], + }, + ), + ), + }], + schema_url: String::new(), + }], + schema_url: String::new(), + }], + } + } + + // language=json + const CANONICAL_WITH_NAN: &str = r#"{ + "resourceMetrics": [ + { + "resource": { + "attributes": [ + { + "key": "service.name", + "value": null + } + ], + "droppedAttributesCount": 0 + }, + "scopeMetrics": [ + { + "scope": null, + "metrics": [ + { + "name": "example_metric", + "description": "A sample metric with NaN values", + "unit": "1", + "metadata": [], + "summary": { + "dataPoints": [ + { + "attributes": [], + "startTimeUnixNano": "0", + "timeUnixNano": "0", + "count": "100", + "sum": 500.0, + "quantileValues": [ + { + "quantile": 0.5, + "value": "NaN" + }, + { + "quantile": 0.9, + "value": "NaN" + } + ], + "flags": 0 + } + ] + } + } + ], + "schemaUrl": "" + } + ], + "schemaUrl": "" + } + ] + }"#; + + #[test] + fn serialize_with_nan() { + let input: ExportMetricsServiceRequest = value_with_nan(); + + // Serialize the structure to JSON + let actual = serde_json::to_string_pretty(&input).expect("serialization must succeed"); + + // Normalize both the actual and expected JSON for comparison + let actual_value: serde_json::Value = + serde_json::from_str(&actual).expect("valid JSON"); + let expected_value: serde_json::Value = + serde_json::from_str(CANONICAL_WITH_NAN).expect("valid JSON"); + + // Compare the normalized JSON values + assert_eq!(actual_value, expected_value); + } + + #[test] + fn deserialize_with_nan() { + let actual: ExportMetricsServiceRequest = + serde_json::from_str(CANONICAL_WITH_NAN).expect("deserialization must succeed"); + + // Ensure the deserialized structure matches the expected values + assert_eq!(actual.resource_metrics.len(), 1); + + let resource_metric = &actual.resource_metrics[0]; + assert_eq!( + resource_metric.resource.as_ref().unwrap().attributes.len(), + 1 + ); + assert_eq!( + resource_metric.resource.as_ref().unwrap().attributes[0].key, + "service.name" + ); + assert!(resource_metric.resource.as_ref().unwrap().attributes[0] + .value + .is_none()); + + assert_eq!(resource_metric.scope_metrics.len(), 1); + + let scope_metric = &resource_metric.scope_metrics[0]; + assert!(scope_metric.scope.is_none()); + assert_eq!(scope_metric.metrics.len(), 1); + + let metric = &scope_metric.metrics[0]; + assert_eq!(metric.name, "example_metric"); + assert_eq!(metric.description, "A sample metric with NaN values"); + assert_eq!(metric.unit, "1"); + + if let Some(opentelemetry_proto::tonic::metrics::v1::metric::Data::Summary(summary)) = + &metric.data + { + assert_eq!(summary.data_points.len(), 1); + + let data_point = &summary.data_points[0]; + assert_eq!(data_point.attributes.len(), 0); + assert_eq!(data_point.start_time_unix_nano, 0); + assert_eq!(data_point.time_unix_nano, 0); + assert_eq!(data_point.count, 100); + assert_eq!(data_point.sum, 500.0); + + assert_eq!(data_point.quantile_values.len(), 2); + + // Verify that quantile values are NaN + assert!(data_point.quantile_values[0].value.is_nan()); + assert!(data_point.quantile_values[1].value.is_nan()); + } else { + panic!("Expected metric data to be of type Summary"); + } + } + } } From 67c6a13915f05bc97765e327860bcb28b5cab93b Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Tue, 28 Jan 2025 16:07:30 -0800 Subject: [PATCH 5/5] add test for quantile --- opentelemetry-proto/tests/json_serde.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/opentelemetry-proto/tests/json_serde.rs b/opentelemetry-proto/tests/json_serde.rs index 17ba4eb710..800ffaa12a 100644 --- a/opentelemetry-proto/tests/json_serde.rs +++ b/opentelemetry-proto/tests/json_serde.rs @@ -1678,7 +1678,9 @@ mod json_serde { // Verify that quantile values are NaN assert!(data_point.quantile_values[0].value.is_nan()); + assert!(data_point.quantile_values[0].quantile == 0.5); assert!(data_point.quantile_values[1].value.is_nan()); + assert!(data_point.quantile_values[1].quantile == 0.9); } else { panic!("Expected metric data to be of type Summary"); }