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

Support 128bit trace IDs #858

Closed
7 tasks done
yurishkuro opened this issue Jun 1, 2018 · 13 comments
Closed
7 tasks done

Support 128bit trace IDs #858

yurishkuro opened this issue Jun 1, 2018 · 13 comments
Labels
help wanted Features that maintainers are willing to accept but do not have cycles to implement meta-issue An tracking issue that requires work in other repos

Comments

@yurishkuro
Copy link
Member

yurishkuro commented Jun 1, 2018

Currently Jaeger clients generate 64bit trace IDs by default. Organizations with large traffic volume may run into ID collision and need 128bit trace IDs (also required by w3c trace context #855). Jaeger backend and some clients already support 128bit IDs. This issue is to track support across all clients.

Requirements

  • By default the clients must still generate 64bit IDs, to maintain compatibility with existing code in production, especially in the applications that have not yet upgraded to the newer clients.
  • 128bit IDs for new traces must be opt-in via tracer configuration.
  • However, if a client receives 128bit trace IDs, it should propagate it further as is, without truncating to 64bit. The risk of receiving side not being able to parse it is an organizational challenge (below).

Work tracker

Org Upgrade Process

The proposal for organizations wishing to upgrade to 128bit IDs is to follow this process:

  1. make sure that all applications / services are upgraded to use Jaeger clients that can read 128bit IDs, but on trace creation still generate 64bit.
  2. once all applications are upgraded, start rolling out a configuration change that instructs Jaeger clients to generate 128bit IDs for new traces.
@yurishkuro yurishkuro added help wanted Features that maintainers are willing to accept but do not have cycles to implement meta-issue An tracking issue that requires work in other repos client-libs labels Jun 1, 2018
@isaachier
Copy link
Contributor

Don't forget C++, although this is already done.

@yurishkuro
Copy link
Member Author

Does C++ implementation meet the requirements above?

@isaachier
Copy link
Contributor

Ya this "feature" ended up delaying the initial release of the C++ client because it would generate 128 bit by default. After debugging that, I added the option to generate 128 bit trace IDs, but set it off by default. The propagation inherently works because I didn't add any other logic to support 64 bit trace IDs (no truncating, etc.).

Must use option flags to use high bits: https://github.com/jaegertracing/jaeger-client-cpp/blob/master/src/jaegertracing/Tracer.cpp#L77-L79.

@Falco20019
Copy link

C# Version will create new 64 bit trace ids stating v0.1.1 and accept 64 and 128 bit trace ids. v0.2.0 will add configurability. We will have a look at how C++ solved it and check if that’s a solution we also want to use.

@Falco20019
Copy link

Just added configurability in jaegertracing/jaeger-client-csharp#94 by proposing the following changes to the API:

  • Added environment variable JAEGER_TRACEID_128BIT
  • Added WithTraceId128Bit method to Configuration
  • Added WithTraceId128Bit method to Tracer.Builder
  • Adde UseTraceId128Bit property to Tracer

Go and C++ have done it through their options context and do not offer it through Configuration (either code or environment) yet. It would be great to discuss if we like to have this in configuration and if, what env-var to use.

@yurishkuro
Copy link
Member Author

I guess we should think about the objective here. Many tracing systems today do 128bit+ IDs by default. I would like Jaeger, in the future, to also default to 128bit, and make the 64it configuration a legacy thing. Given that, should it really be controlled via Configuration class, rather than more verbose/explicit tracer builder?

@isaachier
Copy link
Contributor

I guess I don't mind either way. The issue with the Go/C++ client is that the default is 64 bits and any change to those options might be considered a breaking API change. C# had no option until now, so it should probably hide the option a little better, but defaulting to 64 bit is a requirement until all clients have this support. My crossdock tests broke when I tried to default to 128 bits in C++ because the Python/Java clients did not preserve the trace ID from C++ and effectively dropped the first 64 bits.

@yurishkuro
Copy link
Member Author

yurishkuro commented Jul 18, 2018

but defaulting to 64 bit is a requirement

Actually, when we create brand new clients it does not have to be. But if we have users already then yes.

@Falco20019
Copy link

Right now this would force you to use TraceBuilder instead of Configuration if you want 128bit for whatever reason. So I thought it wouldn’t hurt to st least offer it until it is the standard. On that case the env-var would become obsolete since it does nothing and could be removed

@isaachier
Copy link
Contributor

Not unless all the clients involved support 128 bits. That's exactly my point about crossdock testing. The clients that support it expect other languages to propagate the same trace ID. 64 bit clients won't do that, breaking the trace.

@Falco20019
Copy link

Yes, default until every client supports it 64Bit of course. Setting the configuration would also be an explicit action involving the user to know that it only works if the client supports it. For example if he only uses C# and C++

@jpkrohling
Copy link
Contributor

@Tanejaankush looks like this might be related to the C# client? Could you please open an issue against that repo? https://github.com/jaegertracing/jaeger-client-csharp

@Tanejaankush
Copy link

Tanejaankush commented Apr 5, 2019

@Tanejaankush looks like this might be related to the C# client? Could you please open an issue against that repo? https://github.com/jaegertracing/jaeger-client-csharp

Okay raised jaegertracing/jaeger-client-csharp#143

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Features that maintainers are willing to accept but do not have cycles to implement meta-issue An tracking issue that requires work in other repos
Projects
None yet
Development

No branches or pull requests

5 participants