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

parmesant · 2025-12-11T07:20:52Z

Fixes #2487
Design discussion issue (if applicable) #

Changes

This PR adds to the changes made by @lalitb in his PR #2491

For metric type Summary, quantileValues.quantile gets treated as u64 and not f64 when its value is 1

Also, flags comes optionally in the payload hence added default to it (and some other fields)

Merge requirement checklist

CONTRIBUTING guidelines followed
Unit tests added/updated (if applicable)
Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
Changes in public API reviewed (if applicable)

In metric type summary, `quantileValue` with `quantile` = 1 gets treated as u64

linux-foundation-easycla · 2025-12-11T07:21:00Z

The committers listed above are authorized under a signed CLA.

✅ login: lalitb / name: Lalit Kumar Bhasin (6635302)

codecov · 2025-12-11T07:23:45Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.5%. Comparing base (2b63b75) to head (6635302).

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #3284   +/-   ##
=====================================
  Coverage   80.5%   80.5%           
=====================================
  Files        129     129           
  Lines      23294   23294           
=====================================
  Hits       18756   18756           
  Misses      4538    4538

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR fixes serialization and deserialization issues for metric fields in the OpenTelemetry Protocol (OTLP) JSON format. It addresses the requirement that certain numeric fields (u64 counts, bucket counts) must be serialized as strings in JSON, and adds proper handling for special floating-point values (NaN, Infinity, -Infinity) that cannot be directly represented in standard JSON.

Key changes:

Added custom serializers/deserializers for u64 count and bucket_counts fields to convert between u64 and string representation
Implemented special handling for f64 fields that may contain NaN, Infinity, or -Infinity values
Added default attribute to optional fields (flags, sum, count) to handle cases where they may be missing from JSON payloads

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.

File	Description
`opentelemetry-proto/tests/json_serde.rs`	Updated existing test expectations to reflect u64 values serialized as strings; added comprehensive test cases for NaN handling in Summary metrics
`opentelemetry-proto/tests/grpc_build.rs`	Added field attribute configurations for timestamps, counts, bucket counts, and special f64 handling to ensure proper serialization/deserialization code generation
`opentelemetry-proto/src/proto/tonic/opentelemetry.proto.metrics.v1.rs`	Applied serde attributes to metric data point fields (HistogramDataPoint, ExponentialHistogramDataPoint, SummaryDataPoint) for proper string/numeric conversions and NaN handling
`opentelemetry-proto/src/proto.rs`	Implemented new serializer/deserializer functions for vector u64-to-string conversions and special f64 handling for NaN/Infinity values

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-18T16:18:16Z

opentelemetry-proto/tests/grpc_build.rs

+        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\"))]",
+    );


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 · 2025-12-18T16:18:17Z

opentelemetry-proto/tests/grpc_build.rs

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


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 · 2025-12-18T16:18:17Z

opentelemetry-proto/src/proto.rs

+        let str_vec: Vec<String> = vec.iter().map(|&num| num.to_string()).collect();
+        serializer.collect_seq(str_vec)
+    }
+


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 · 2025-12-18T16:18:17Z

opentelemetry-proto/src/proto.rs

+    }
+
+    deserializer.deserialize_any(F64Visitor)
+   }


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 · 2025-12-18T16:18:18Z

opentelemetry-proto/tests/grpc_build.rs

+    }
+
+    // special serializer and deserializer for metrics bucket counts
+    // OTLP/JSON format may uses string for bucket counts


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

Copilot · 2025-12-18T16:18:18Z

opentelemetry-proto/src/proto.rs

+    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();


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 · 2025-12-18T16:18:18Z

opentelemetry-proto/src/proto.rs

+            .collect()
+    }
+
+


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 · 2025-12-18T16:18:19Z

opentelemetry-proto/src/proto.rs

+        {
+            Ok(value as f64)
+        }
+


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 · 2025-12-18T16:18:19Z

opentelemetry-proto/tests/grpc_build.rs

    }

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


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

Copilot · 2025-12-18T16:18:19Z

opentelemetry-proto/src/proto.rs

    use serde::de::{self, MapAccess, Visitor};
    use serde::ser::{SerializeMap, SerializeSeq, SerializeStruct};
    use serde::{Deserialize, Deserializer, Serialize, Serializer};
+    use core::f64;


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;

lalitb · 2025-12-22T04:10:36Z

@parmesant - thanks for the PR. Can you go through the comments from copilot if they make sense.

lalitb and others added 7 commits December 10, 2025 20:19

initial commit

e7e3b39

fix

eefc117

add NaN serialization/deserialization

da5ed50

add tests for NaN

8d57a93

add test for quantile

18b7c71

Fix for deserialization issue

196d926

In metric type summary, `quantileValue` with `quantile` = 1 gets treated as u64

add defaults

45fb828

parmesant requested a review from a team as a code owner December 11, 2025 07:20

parmesant changed the title ~~Fix metrics u64 serialization~~ fix: metrics u64 deserialization Dec 11, 2025

parmesant changed the title ~~fix: metrics u64 deserialization~~ fix: serialization/deserialization for u64, f64, and "NaN" float fields in Metrics Dec 11, 2025

lalitb self-assigned this Dec 18, 2025

lalitb requested a review from Copilot December 18, 2025 16:11

Copilot started reviewing on behalf of lalitb December 18, 2025 16:12 View session

Copilot AI reviewed Dec 18, 2025

View reviewed changes

Merge branch 'main' into fix-metrics-u64-serialization

6635302

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

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

Are you sure you want to change the base?

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

Conversation

parmesant commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Merge requirement checklist

Uh oh!

linux-foundation-easycla bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

lalitb commented Dec 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

parmesant commented Dec 11, 2025 •

edited

Loading

linux-foundation-easycla bot commented Dec 11, 2025 •

edited

Loading

codecov bot commented Dec 11, 2025 •

edited

Loading