Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

SpanContext cannot handle 128-bit traceId #295

Closed
justindsmith opened this issue Sep 11, 2018 · 11 comments · Fixed by #361
Closed

SpanContext cannot handle 128-bit traceId #295

justindsmith opened this issue Sep 11, 2018 · 11 comments · Fixed by #361

Comments

@justindsmith
Copy link

Requirement - what kind of business use case are you trying to solve?

We have a mix of tracing instrumentation libraries in our environment, some of which generate 128-bit trace ids. Those trace ids cannot be handled by the jaeger-client-node library and we end up with disconnected traces.

Problem - what in Jaeger blocks you from solving the requirement?

The SpanContext class is attempting to parse the traceId as a 64-bit number, which means the 128-bit number is not handled properly and the trace ends up disconnected.

https://github.com/jaegertracing/jaeger-client-node/blob/master/src/span_context.js#L69-L74

Proposal - what do you suggest to solve the problem or improve the existing situation?

The trace id is a stored as a buffer, can we just use a 128-bit buffer? I'm not sure the ramifications of that though.

@justindsmith
Copy link
Author

For just a little more context, we're using the b3 headers and the b3 propagation spec claims that traceId can be 64-bit or 128-bit.

https://github.com/openzipkin/b3-propagation

@yurishkuro
Copy link
Member

The trace id is a stored as a buffer, can we just use a 128-bit buffer?

Yes. Just nobody got around to it. The only limitation is that we should have a config flag that explicitly enables 128bit, because otherwise the behavior is not backwards compatible with existing deployments, and we won't be doing a major version bump just for this feature.

@yurishkuro
Copy link
Member

for the reference, you can see how this is implemented in Go client.

@15168326318
Copy link

node.js Support 128bit trace ids ???? like Java!

@15168326318
Copy link

node.js does not Support 128bit trace ids,now!
Trace ids will be generated repeatedly, very serious problem.How do you solve that?

@yurishkuro
Copy link
Member

If this is important to you, please consider creating a pul request with implementation. I have linked this to the master ticket.

@mwieczorek
Copy link

Hi, 128bit traceid is also used by default in istio from 1.1 (istio/istio#10732).
Is anybody working on that? (I'm just asking in case I'd like to give it a shot)

@mwieczorek
Copy link

I've also seen that https://github.com/broofa/node-int64 is getting depracated.

Do you have any suggestion what can be a replacement? Maybe https://github.com/no2chem/bigint-buffer?

@yurishkuro
Copy link
Member

I don't know Node.js ecosystem that well, but a general preference is to avoid dependencies altogether. If Node has a notion of a "byte buffer" and it's efficient, could we use that? We don't necessarily need the "int64" semantics (except perhaps when converting to Thrift)

@Bessonov
Copy link

Bessonov commented Aug 2, 2019

@mwieczorek native BigInt is supported from node v10.4.0.

@yurishkuro if you support node v8, then the support of v8 LTS ends by the end of this year after reschedule.

@yurishkuro
Copy link
Member

btw, I am now not sure about the premise of this ticket, given that 128bit IDs are not supported by the client yet, there's an open PR #361

rafabene added a commit to rafabene/microservices4demo that referenced this issue Sep 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants