Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] fix serialization/deserialization for u64 and "NaN" float fields in Metrics #2491

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
76 changes: 76 additions & 0 deletions opentelemetry-proto/src/proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,82 @@ pub(crate) mod serializers {
let s: String = Deserialize::deserialize(deserializer)?;
s.parse::<i64>().map_err(de::Error::custom)
}
pub fn serialize_vec_u64_to_strings<S>(vec: &Vec<u64>, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
let str_vec: Vec<String> = 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<Vec<u64>, D::Error>
where
D: Deserializer<'de>,
{
let str_vec: Vec<String> = Deserialize::deserialize(deserializer)?;
str_vec
.into_iter()
.map(|s| s.parse::<u64>().map_err(de::Error::custom))
.collect()
}


// Special serializer and deserializer for NaN, Infinity, and -Infinity
pub fn serialize_f64_special<S>(value: &f64, serializer: S) -> Result<S::Ok, S::Error>
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<f64, D::Error>
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<E>(self, value: f64) -> Result<f64, E>
where
E: de::Error,
{
Ok(value)
}

fn visit_str<E>(self, value: &str) -> Result<f64, E>
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")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<u64>,
/// explicit_bounds specifies buckets with explicitly defined bounds for values.
///
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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")]
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -675,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,
}
}
Expand All @@ -704,6 +781,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
Expand Down
53 changes: 53 additions & 0 deletions opentelemetry-proto/tests/grpc_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,20 +97,73 @@ 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 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
Expand Down
10 changes: 5 additions & 5 deletions opentelemetry-proto/tests/json_serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Loading