From 8792789588263fce7736be6d347f2cafedb28d78 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Mon, 18 May 2026 21:31:21 +0800 Subject: [PATCH] [EXPORTER] Convert uint64_t attribute values exceeding INT64_MAX to string per OTel spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per OpenTelemetry attribute type-mapping spec (https://opentelemetry.io/ docs/specs/otel/common/attribute-type-mapping/#integer-values), integer values outside the 64-bit signed range MUST be encoded as AnyValue's string_value using decimal representation, not wrapped to a negative int64 via narrowing. Pre-PR behavior at the four uint64_t conversion sites in otlp_populate_attribute_utils.cc was to implicitly narrow (suppressed via NOLINT). For uint64_t values exceeding INT64_MAX, this produced negative int_value on the wire — a long-standing spec violation that predates this PR. This PR fixes the violation via a file-local helper SetUint64Value that dispatches to set_int_value or set_string_value based on the value's range. All four uint64_t sites in both PopulateAnyValue overloads now follow the spec. Spec-compliance verified by three new tests in otlp_recordable_test.cc: - UINT64_MAX-equivalent value -> string_value with decimal representation - INT64_MAX boundary value -> int_value (encoding split is val > INT64_MAX) - Mixed-value array -> per-element dispatch (int + string in same array) Side effect: the four bugprone-narrowing-conversions warnings clang-tidy v22 was reporting are eliminated (the static_cast is now in a guarded branch, suppressing the diagnostic). Ratchet drops accordingly: * abiv1-preview warning_limit lowered from 389 to 385 * abiv2-preview warning_limit lowered from 395 to 391 Behavior change: existing callers relying on the wrap-to-negative behavior for uint64 > INT64_MAX will now see string_value on the wire instead of negative int_value. This was never spec-conformant; the new behavior aligns OTLP output with the protocol specification. Part of #2053 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- .github/workflows/clang-tidy.yaml | 4 +- CHANGELOG.md | 3 ++ .../otlp/src/otlp_populate_attribute_utils.cc | 31 +++++++++---- exporters/otlp/test/otlp_recordable_test.cc | 45 +++++++++++++++++++ 4 files changed, 73 insertions(+), 10 deletions(-) diff --git a/.github/workflows/clang-tidy.yaml b/.github/workflows/clang-tidy.yaml index b43f15a0ae..6ddb9d6dde 100644 --- a/.github/workflows/clang-tidy.yaml +++ b/.github/workflows/clang-tidy.yaml @@ -17,9 +17,9 @@ jobs: matrix: include: - cmake_options: all-options-abiv1-preview - warning_limit: 389 + warning_limit: 385 - cmake_options: all-options-abiv2-preview - warning_limit: 395 + warning_limit: 391 env: CC: /usr/bin/clang-22 CXX: /usr/bin/clang++-22 diff --git a/CHANGELOG.md b/CHANGELOG.md index 21bbc377fa..c755a88e66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,9 @@ Increment the: * [CODE HEALTH] Fix IWYU Clang22 warnings [#4083](https://github.com/open-telemetry/opentelemetry-cpp/pull/4083) +* [EXPORTER] Spec-compliant uint64_t attribute encoding in OTLP + [#4090](https://github.com/open-telemetry/opentelemetry-cpp/pull/4090) + * [CODE HEALTH] Remove unused alias declarations [#4091](https://github.com/open-telemetry/opentelemetry-cpp/pull/4091) diff --git a/exporters/otlp/src/otlp_populate_attribute_utils.cc b/exporters/otlp/src/otlp_populate_attribute_utils.cc index 75e47998ab..36ccdd6365 100644 --- a/exporters/otlp/src/otlp_populate_attribute_utils.cc +++ b/exporters/otlp/src/otlp_populate_attribute_utils.cc @@ -6,6 +6,7 @@ #endif #include +#include #include #include #include @@ -41,6 +42,24 @@ namespace otlp const int kAttributeValueSize = 16; const int kOwnedAttributeValueSize = 15; +namespace +{ +// Per OpenTelemetry spec, uint64_t attribute values exceeding INT64_MAX must be +// encoded as a decimal string rather than wrapping to a negative int64 via narrowing. +// https://opentelemetry.io/docs/specs/otel/common/attribute-type-mapping/#integer-values +inline void SetUint64Value(opentelemetry::proto::common::v1::AnyValue *proto_value, uint64_t val) +{ + if (val <= static_cast(std::numeric_limits::max())) + { + proto_value->set_int_value(static_cast(val)); + } + else + { + proto_value->set_string_value(std::to_string(val)); + } +} +} // namespace + void OtlpPopulateAttributeUtils::PopulateAnyValue( opentelemetry::proto::common::v1::AnyValue *proto_value, const opentelemetry::common::AttributeValue &value, @@ -75,8 +94,7 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue( } else if (nostd::holds_alternative(value)) { - proto_value->set_int_value( - nostd::get(value)); // NOLINT(cppcoreguidelines-narrowing-conversions) + SetUint64Value(proto_value, nostd::get(value)); } else if (nostd::holds_alternative(value)) { @@ -168,8 +186,7 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue( auto array_value = proto_value->mutable_array_value(); for (const auto &val : nostd::get>(value)) { - array_value->add_values()->set_int_value( - val); // NOLINT(cppcoreguidelines-narrowing-conversions) + SetUint64Value(array_value->add_values(), val); } } else if (nostd::holds_alternative>(value)) @@ -235,8 +252,7 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue( } else if (nostd::holds_alternative(value)) { - proto_value->set_int_value( - nostd::get(value)); // NOLINT(cppcoreguidelines-narrowing-conversions) + SetUint64Value(proto_value, nostd::get(value)); } else if (nostd::holds_alternative(value)) { @@ -313,8 +329,7 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue( auto array_value = proto_value->mutable_array_value(); for (const auto &val : nostd::get>(value)) { - array_value->add_values()->set_int_value( - val); // NOLINT(cppcoreguidelines-narrowing-conversions) + SetUint64Value(array_value->add_values(), val); } } else if (nostd::holds_alternative>(value)) diff --git a/exporters/otlp/test/otlp_recordable_test.cc b/exporters/otlp/test/otlp_recordable_test.cc index a695b41f1e..14efe93124 100644 --- a/exporters/otlp/test/otlp_recordable_test.cc +++ b/exporters/otlp/test/otlp_recordable_test.cc @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -611,6 +612,50 @@ TYPED_TEST(IntAttributeTest, SetIntArrayAttribute) } } +// Per OpenTelemetry spec, uint64_t attribute values exceeding INT64_MAX must be +// encoded as a decimal string rather than wrapping to a negative int64. +// https://opentelemetry.io/docs/specs/otel/common/attribute-type-mapping/#integer-values +TEST(OtlpRecordable, SetUint64OverflowAsStringPerSpec) +{ + const uint64_t overflow_val = static_cast(std::numeric_limits::max()) + 1U; + common::AttributeValue val(overflow_val); + OtlpRecordable rec; + rec.SetAttribute("u64_overflow", val); + EXPECT_EQ(rec.span().attributes(0).value().value_case(), + opentelemetry::proto::common::v1::AnyValue::kStringValue); + EXPECT_EQ(rec.span().attributes(0).value().string_value(), std::to_string(overflow_val)); +} + +TEST(OtlpRecordable, SetUint64BoundaryAsIntPerSpec) +{ + // INT64_MAX boundary still fits int_value (encoding split is val > INT64_MAX). + const uint64_t boundary = static_cast(std::numeric_limits::max()); + common::AttributeValue val(boundary); + OtlpRecordable rec; + rec.SetAttribute("u64_boundary", val); + EXPECT_EQ(rec.span().attributes(0).value().value_case(), + opentelemetry::proto::common::v1::AnyValue::kIntValue); + EXPECT_EQ(rec.span().attributes(0).value().int_value(), std::numeric_limits::max()); +} + +TEST(OtlpRecordable, SetUint64ArrayOverflowAsStringPerSpec) +{ + const uint64_t overflow_val = static_cast(std::numeric_limits::max()) + 1U; + const uint64_t in_range_val = 42; + const uint64_t arr[] = {in_range_val, overflow_val}; + nostd::span arr_span(arr, 2); + common::AttributeValue val(arr_span); + OtlpRecordable rec; + rec.SetAttribute("u64_arr_mixed", val); + const auto &array_v = rec.span().attributes(0).value().array_value(); + ASSERT_EQ(array_v.values_size(), 2); + EXPECT_EQ(array_v.values(0).value_case(), opentelemetry::proto::common::v1::AnyValue::kIntValue); + EXPECT_EQ(array_v.values(0).int_value(), static_cast(in_range_val)); + EXPECT_EQ(array_v.values(1).value_case(), + opentelemetry::proto::common::v1::AnyValue::kStringValue); + EXPECT_EQ(array_v.values(1).string_value(), std::to_string(overflow_val)); +} + TEST(OtlpRecordableTest, TestCollectionLimits) { // Initialize recordable with strict limits: