-
Notifications
You must be signed in to change notification settings - Fork 410
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.
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
[EXPORTER] Support empty arrays in OtlpRecordable
attributes
#2166
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very clean PR, thanks.
Please expand the unit tests to cover every data types beside std::vector<int64_t>
.
This will help to catch bugs if somehow the fix for other alternatives gets lost, with bad merges.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2166 +/- ##
==========================================
- Coverage 87.17% 87.15% -0.02%
==========================================
Files 166 166
Lines 4777 4777
==========================================
- Hits 4164 4163 -1
- Misses 613 614 +1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the fix.
Changes
This PR changes behavior for empty array attribute values in
OtlpRecordable
. Empty array attribute values did not set theAnyValue
field toArrayValue
.However, following the spec it seems we should forward empty arrays.
https://github.com/open-telemetry/opentelemetry-specification/blob/ce2c5946faa9c961ea01d89c7aed83621865b67b/specification/common/README.md?plain=1#L37-L39
Invoking
mutable_array_value()
before looping over the values sets theAnyValue
variant toArrayValue
regardless of the number of items in the iterator.