Skip to content
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

Consider using byte arrays for trace and span IDs #12

Closed
iredelmeier opened this issue Nov 27, 2019 · 7 comments
Closed

Consider using byte arrays for trace and span IDs #12

iredelmeier opened this issue Nov 27, 2019 · 7 comments

Comments

@iredelmeier
Copy link
Member

See open-telemetry/opentelemetry-go#183 for a similar issue.

@jtescher
Copy link
Member

jtescher commented Dec 5, 2019

Could be nice to see a before / after here to evaluate ergonomics and conversion (e.g. uuid)

Looks like currently to/from would be something like:

let trace_id: u128 = 0x4bf9_2f35_77b3_4da6_a3ce_929d_0e0e_4736;

// From str
assert_eq!(u128::from_str_radix("4bf92f3577b34da6a3ce929d0e0e4736", 16), Ok(trace_id));

// To str
assert_eq!(format!("{:032x}", trace_id).as_str(), "4bf92f3577b34da6a3ce929d0e0e4736");

// From / to uuid
assert_eq!(uuid::Uuid::from_u128(trace_id).as_u128(), trace_id);

@MikeGoldsmith
Copy link
Member

MikeGoldsmith commented Dec 5, 2019

It's worth mentioning that B3 headers support 128 bit trace IDs where most other propagators are limited to 64 bits. The full trace ID needs to be preserved if received and then propagated via B3 headers but can be truncated to 64 bits (lower significant bits) for other propagators.

Some implementations use a byte array (like Go) to manage a single 64 or 128 bit id, others maintain the original trace ID as a string and a uint64 for the truncated 64 bit part (eg .NET).

@jtescher
Copy link
Member

jtescher commented Dec 5, 2019

@MikeGoldsmith great point, is there a list of propagators that otel impls should support? Seems 64 bit traces are typically older versions?

@MikeGoldsmith
Copy link
Member

I think B3 should be good enough for now. Vendor specific propagators can come later, and as long as we have both 128 and 64 bit IDs, we should be good 👍

@jtescher
Copy link
Member

jtescher commented Dec 5, 2019

Hm well we currently support b3 the same way that the go impl does, and it looks like they require 128 bits? could be 0 padded but that works with u128 just the same? https://github.com/open-telemetry/opentelemetry-go/blob/a9756528bada550d81d668a2536a4e0788faa5c4/api/core/span_context.go#L88-L90

@iredelmeier
Copy link
Member Author

B3 and W3C Trace-Context, which is 128-bit and should be the default.

Monkeyanator pushed a commit to Monkeyanator/opentelemetry-rust that referenced this issue Mar 3, 2020
@jtescher
Copy link
Member

jtescher commented Apr 5, 2020

Closing this for now, can re-open if there is renewed interest.

@jtescher jtescher closed this as completed Apr 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants