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 10 commits into
base: main
Choose a base branch
from
+427 −9
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
10 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
a8eae89
address copilot comments
parmesant Dec 23, 2025
9c32633
Merge branch 'main' into fix-metrics-u64-serialization
lalitb Jan 5, 2026
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
93 changes: 90 additions & 3 deletions opentelemetry-proto/src/proto.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,7 @@ pub(crate) mod serializers {
where
S: Serializer,
{
let s = value.iter()
.map(|v| v.to_string())
.collect::<Vec<_>>();
let s = value.iter().map(|v| v.to_string()).collect::<Vec<_>>();
let mut sq = serializer.serialize_seq(Some(s.len()))?;
for v in value {
sq.serialize_element(&v.to_string())?;
Expand Down Expand Up @@ -213,6 +211,95 @@ 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: &[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_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_i64<E>(self, value: i64) -> Result<f64, E>
where
E: de::Error,
{
Ok(value as f64)
}

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
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
55 changes: 54 additions & 1 deletion opentelemetry-proto/tests/grpc_build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,19 +97,27 @@ fn build_tonic() {
}

// special serializer and deserializer for timestamp
// OTLP/JSON format may uses string for timestamp
// OTLP/JSON format may use string for timestamp
// 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 use 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 use 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
Loading
Loading