fix(otlp-transformer): correctly handle Uint8Array attribute values when serializing to JSON#6348
Conversation
…hen serializing to JSON
…alues when serializing to JSON
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6348 +/- ##
==========================================
- Coverage 95.68% 95.48% -0.21%
==========================================
Files 313 363 +50
Lines 9667 11564 +1897
Branches 2226 2669 +443
==========================================
+ Hits 9250 11042 +1792
- Misses 417 522 +105
🚀 New features to boost your workflow:
|
|
note: something was wrong with the coverage upload, so I pushed a commit so that the browser/webworker tests also publish the results to |
raphael-theriault-swi
left a comment
There was a problem hiding this comment.
Much prefer this approach to the options !
| export interface OtlpEncodingOptions { | ||
| /** Convert trace and span IDs to hex strings. */ | ||
| useHex?: boolean; | ||
| /** Convert HrTime to 2 part 64 bit values. */ | ||
| useLongBits?: boolean; | ||
| } |
There was a problem hiding this comment.
Is this still used anywhere after the changes ?
There was a problem hiding this comment.
it is used by createExpectedSpanJson function in test/trace.test.ts to create the expected spans within the test. Tests are using different combinations of the options so (without knowing much) I guess is better to keep it. We could move the type to the test file.
There was a problem hiding this comment.
yep, that's the reason why I kept it for now. Once I get to working on the custom proto serializer, I'm hoping that I can further simplify the tests and get rid of that too.
Co-authored-by: Colin Ihrig <cjihrig@gmail.com>
|
Thanks for working on this @pichlermarc :) |
Which problem is this PR solving?
Fixes #6190
Uint8Arrays (includingBuffers) are currently not handled properly when they get exported as part of a log record body or attribute value. The incorrect handling results inbytesValuecontaining an array of numbers instead of the data as a base64-encoded string - this causes the collector to reject the whole request.Semi-related changes:
Encoderobject was created. In the past, this was public API; nowadays however, it's an implementation detail. As it was getting difficult to track which of the four options apply to which format in testing, I opted to remove this intermediate options step and pass theEncoderdirectly.Type of change
How Has This Been Tested?