Skip to content

fix: serialization/deserialization for u64, f64, and "NaN" float fields in Metrics #3284

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.

Sign up for GitHub

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

Open
parmesant wants to merge 8 commits into
base: main
Choose a base branch
from
+420 −5
Open

fix: serialization/deserialization for u64, f64, and "NaN" float fields in Metrics #3284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e7e3b39
initial commit
lalitb Dec 31, 2024
eefc117
fix
lalitb Jan 7, 2025
da5ed50
add NaN serialization/deserialization
lalitb Jan 7, 2025
8d57a93
add tests for NaN
lalitb Jan 28, 2025
18b7c71
add test for quantile
lalitb Jan 29, 2025
196d926
Fix for deserialization issue
parmesant Dec 11, 2025
45fb828
add defaults
parmesant Dec 11, 2025
6635302
Merge branch 'main' into fix-metrics-u64-serialization
lalitb Dec 22, 2025
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
84 changes: 84 additions & 0 deletions opentelemetry-proto/src/proto.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ pub(crate) mod serializers {
use serde::de::{self, MapAccess, Visitor};
use serde::ser::{SerializeMap, SerializeSeq, SerializeStruct};
use serde::{Deserialize, Deserializer, Serialize, Serializer};
use core::f64;
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary import statement. The use core::f64; import is not needed since f64 is a primitive type in Rust and its associated constants (NAN, INFINITY, NEG_INFINITY) can be accessed directly without importing the module. This import can be removed.

Suggested change
use core::f64;

Copilot uses AI. Check for mistakes.
use std::fmt;

// hex string <-> bytes conversion
Expand Down Expand Up @@ -213,6 +214,89 @@ 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();
Comment on lines +217 to +221
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function parameter should use slice instead of vector reference. The parameter vec: &Vec<u64> should be value: &[u64] to follow Rust best practices and match the existing serialize_vec_u64_to_string function pattern at line 178. Using &[u64] is more idiomatic and flexible as it accepts both vectors and slices.

Suggested change
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();
pub fn serialize_vec_u64_to_strings<S>(value: &[u64], serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
let str_vec: Vec<String> = value.iter().map(|&num| num.to_string()).collect();

Copilot uses AI. Check for mistakes.
serializer.collect_seq(str_vec)
}

Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing whitespace on this line. The line ends with spaces after the closing brace, which should be removed for consistency with coding standards.

Suggested change

Copilot uses AI. Check for mistakes.
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()
}


Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary blank line. This blank line creates excessive vertical spacing (two consecutive blank lines). Remove this line to maintain consistent spacing throughout the file.

Suggested change

Copilot uses AI. Check for mistakes.
// 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_u64<E>(self, value: u64) -> Result<f64, E>
where
E: de::Error,
{
Ok(value as f64)
}

Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deserializer should handle additional numeric types. The visit_u64 method is implemented but visit_i64 is missing. If the JSON contains integer values like 0 or 1 for quantile values, they might be deserialized as i64 depending on the JSON parser, which would cause deserialization to fail. Consider adding a visit_i64 method similar to visit_u64.

Suggested change
fn visit_i64<E>(self, value: i64) -> Result<f64, E>
where
E: de::Error,
{
Ok(value as f64)
}

Copilot uses AI. Check for mistakes.
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)
}
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing whitespace on this line. The line ends with a space after the closing brace, which should be removed for consistency with coding standards.

Suggested change
}
}

Copilot uses AI. Check for mistakes.
}

#[cfg(feature = "gen-tonic-messages")]
Expand Down
95 changes: 95 additions & 0 deletions opentelemetry-proto/src/proto/tonic/opentelemetry.proto.metrics.v1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,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 @@ -476,6 +483,13 @@ pub struct HistogramDataPoint {
/// is when the length of bucket_counts is 0, then the length of explicit_bounds
/// must also be 0.
#[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 @@ -541,17 +555,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 @@ -589,6 +624,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 @@ -676,15 +718,37 @@ 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(
default,
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 @@ -695,6 +759,10 @@ pub struct SummaryDataPoint {
/// doing so. This is specifically to enforce compatibility w/ OpenMetrics,
/// see: <https://github.com/prometheus/OpenMetrics/blob/v1.0.0/specification/OpenMetrics.md#summary>
#[prost(double, tag = "5")]
#[cfg_attr(
feature = "with-serde",
serde(default)
)]
pub sum: f64,
/// (Optional) list of values at different quantiles of the distribution calculated
/// from the current snapshot. The quantiles must be strictly increasing.
Expand All @@ -703,6 +771,10 @@ pub struct SummaryDataPoint {
/// Flags that apply to this specific data point. See DataPointFlags
/// for the available flags and their meaning.
#[prost(uint32, tag = "8")]
#[cfg_attr(
feature = "with-serde",
serde(default)
)]
pub flags: u32,
}
/// Nested message and enum types in `SummaryDataPoint`.
Expand All @@ -724,11 +796,27 @@ 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(
default,
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(
default,
serialize_with = "crate::proto::serializers::serialize_f64_special",
deserialize_with = "crate::proto::serializers::deserialize_f64_special"
)
)]
pub value: f64,
}
}
Expand All @@ -753,6 +841,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,23 @@ 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\"))]")
Expand All @@ -123,6 +131,51 @@ fn build_tonic() {
.field_attribute(path, "#[cfg_attr(feature = \"with-serde\", serde(serialize_with = \"crate::proto::serializers::serialize_vec_u64_to_string\", deserialize_with = \"crate::proto::serializers::deserialize_vec_string_to_vec_u64\"))]")
}

// special serializer and deserializer for metrics count
// OTLP/JSON format may uses string for count
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammatical error in comment. The phrase "may uses" is grammatically incorrect and should be "may use" (without the 's').

Copilot uses AI. Check for mistakes.
// 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
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammatical error in comment. The phrase "may uses" is grammatically incorrect and should be "may use" (without the 's').

Copilot uses AI. Check for mistakes.
// 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\"))]",
);
Comment on lines +160 to +163
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indentation in field_attribute call. Lines 160-163 have inconsistent indentation compared to the similar block above at lines 145-148. The path and the attribute string should be indented consistently with other field_attribute calls in the file.

Copilot uses AI. Check for mistakes.
}

// 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\"))]",
);
Comment on lines +174 to +176
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indentation in field_attribute call. Lines 173-176 have inconsistent indentation compared to the field_attribute call at lines 145-148. The path and the attribute string should be indented consistently with other field_attribute calls in the file.

Suggested change
path,
"#[cfg_attr(feature = \"with-serde\", serde(serialize_with = \"crate::proto::serializers::serialize_f64_special\", deserialize_with = \"crate::proto::serializers::deserialize_f64_special\"))]",
);
path,
"#[cfg_attr(feature = \"with-serde\", serde(serialize_with = \"crate::proto::serializers::serialize_f64_special\", deserialize_with = \"crate::proto::serializers::deserialize_f64_special\"))]",
);

Copilot uses AI. Check for mistakes.
}

// special serializer and deserializer for value
// The Value::value field must be hidden
builder = builder
Expand Down
Loading
Loading