opentelemetry-rust icon indicating copy to clipboard operation
opentelemetry-rust copied to clipboard

Serialization Fails for u64 Fields Due to Incoming String Values in OpenTelemetry Metrics

Open nikhilsinhaparseable opened this issue 10 months ago • 12 comments
trafficstars

I am using the opentelemetry-proto crate to serialize incoming OpenTelemetry metrics data. The crate defines certain fields (count, bucket_counts under HistogramDataPoint) as u64. However, the OpenTelemetry exporter sends these fields as strings, which leads to serialization failures.

This mismatch between the data types in the crate and the incoming data prevents successful serialization of metrics.

Sample JSON otel-metrics-histogram.json

Similar issue is found in the ExponentialHistogramDataPoint

nikhilsinhaparseable avatar Dec 30 '24 15:12 nikhilsinhaparseable

the OpenTelemetry exporter sends these fields as strings, which leads to serialization failures.

Which exporter are you referring to?

cijothomas avatar Dec 31 '24 07:12 cijothomas

I am using otel collector in opentelemetry-demo to send the otel logs, metrics and traces to Parseable.

nikhilsinhaparseable avatar Dec 31 '24 12:12 nikhilsinhaparseable

As per the proto-json conversion standard, u64 should be serialized to string - https://protobuf.dev/programming-guides/json/. So i believe, opentelemetry-proto is doing it correctly?

lalitb avatar Dec 31 '24 12:12 lalitb

I can see that the serialization to string is happening for the fields where below is available in the crate but this feature is not available for the mentioned fields.

#[cfg_attr( feature = "with-serde", serde( serialize_with = "crate::proto::serializers::serialize_u64_to_string", deserialize_with = "crate::proto::serializers::deserialize_string_to_u64" ) )]

nikhilsinhaparseable avatar Jan 01 '25 08:01 nikhilsinhaparseable

I can see that the serialization to string is happening for the fields where below is available in the crate but this feature is not available for the mentioned fields.

#[cfg_attr( feature = "with-serde", serde( serialize_with = "crate::proto::serializers::serialize_u64_to_string", deserialize_with = "crate::proto::serializers::deserialize_string_to_u64" ) )]

Yes, I did realize it afterwards, and have done changes locally to test before raising PR here - https://github.com/lalitb/opentelemetry-rust/pull/44/files

lalitb avatar Jan 01 '25 14:01 lalitb

@lalitb I see similar issue in SummaryDataPoint for the field quantile_values as the fields quantile and value fields in the ValueAtQuantile struct are f64 data type but the data from the collector is somethings like this - image

below is the sample JSON with summary metrics - otel-metrics-summary.json

nikhilsinhaparseable avatar Jan 02 '25 15:01 nikhilsinhaparseable

I see similar issue in SummaryDataPoint for the field quantile_values

@nikhilsinhaparseable Will include fix for this in #2491.

lalitb avatar Jan 03 '25 19:01 lalitb

Is this probably also #2434? It would be great to re-check if this fixes it once the PR goes in; there is some testing in integration tests that could then be properly roundtripped.

scottgerring avatar Mar 24 '25 10:03 scottgerring

@scottgerring i am able to work with the sum metric type

nikhilsinhaparseable avatar Mar 26 '25 09:03 nikhilsinhaparseable

@scottgerring may I know if you were able to work on this one?

nikhilsinhaparseable avatar Apr 12 '25 11:04 nikhilsinhaparseable

Hey @nikhilsinhaparseable i've not worked on this, but it looks like @lalitb might have? I note also that we have a ignored test that roundtrips using the pb models that should be useful for fixing this / asserting that it is fixed:

https://github.com/open-telemetry/opentelemetry-rust/blob/10cf02c4581b9368cad22a77fd29cc30009a7703/opentelemetry-otlp/tests/integration_test/tests/metrics_roundtrip.rs#L27-L49

scottgerring avatar Apr 14 '25 11:04 scottgerring

@scottgerring the issue i reported in my earlier comment is with Summary metric type where if quantileValues have floating point data, the serialization fails, the same has been discussed with @lalitb in the PR https://github.com/open-telemetry/opentelemetry-rust/pull/2491

nikhilsinhaparseable avatar Apr 15 '25 15:04 nikhilsinhaparseable