Expose trace IDs in hex format #270
Conversation
There was a problem hiding this comment.
Since the spec requires the trace id to be 32 characters and the span id to be 16 characters the integer_to_binary function can't be used -- or at least can't be used without also adding padding to the beginning of the string. My suggested change is using io_lib:format to get a string of the correct length that is in hex.
| @@ -69,6 +71,14 @@ trace_id(#span_ctx{trace_id=TraceId }) -> | |||
| span_id(#span_ctx{span_id=SpanId }) -> | |||
| SpanId. | |||
|
|
|||
| -spec hex_trace_id(opentelemetry:span_ctx()) -> opentelemetry:hex_trace_id(). | |||
| hex_trace_id(#span_ctx{trace_id=TraceId }) -> | |||
| string:lowercase(integer_to_binary(TraceId, 16)). | |||
There was a problem hiding this comment.
| string:lowercase(integer_to_binary(TraceId, 16)). | |
| iolist_to_binary(io_lib:format("~32.16.0b", [TraceId])). |
There was a problem hiding this comment.
Good catch. I forgot that completely. https://github.com/open-telemetry/opentelemetry-erlang/compare/a19f1a44c1e63f969b87d34fe1e690994cb8767e..a97f5b7f3a034057a542bcde7be1a925ef169184
There was a problem hiding this comment.
Also added a test for that.
|
|
||
| -spec hex_span_id(opentelemetry:span_ctx()) -> opentelemetry:hex_span_id(). | ||
| hex_span_id(#span_ctx{span_id=SpanId }) -> | ||
| string:lowercase(integer_to_binary(SpanId, 16)). |
There was a problem hiding this comment.
| string:lowercase(integer_to_binary(SpanId, 16)). | |
| iolist_to_binary(io_lib:format("~16.16.0b", [SpanId])). |
There was a problem hiding this comment.
Codecov Report
@@ Coverage Diff @@
## main #270 +/- ##
==========================================
+ Coverage 36.42% 36.50% +0.08%
==========================================
Files 41 41
Lines 3165 3169 +4
==========================================
+ Hits 1153 1157 +4
Misses 2012 2012
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Thanks! My initial thought was an api like |
According to the [spec][1] > The API MUST allow retrieving the TraceId and SpanId in the following forms: > > Hex - returns the lowercase hex-encoded TraceId (result MUST be a 32-hex-character lowercase string) or SpanId (result MUST be a 16-hex-character lowercase string). > Binary - returns the binary representation of the TraceId (result MUST be a 16-byte array) or SpanId (result MUST be an 8-byte array). > > The API SHOULD NOT expose details about how they are internally stored. This commit takes care of the Hex part. [1]: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#retrieving-the-traceid-and-spanid)
According to the [spec][1]
This commit takes care of the Hex part.
[1]: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#retrieving-the-traceid-and-spanid)