[EXPORTER] Convert uint64_t attribute values exceeding INT64_MAX to s… - #862
Merged
Conversation
…tring per OTel spec (#4090) 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> Co-authored-by: Doug Barker <3782873+dbarker@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…tring per OTel spec (open-telemetry#4090)
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:
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:
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 open-telemetry#2053
Fixes # (issue)
Changes
Please provide a brief description of the changes here.
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes