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

Jaeger does not strip 0 from trace/span IDs anlylonger #4661

Closed
tobiasstadler opened this issue Jan 29, 2021 · 3 comments · Fixed by #4671
Closed

Jaeger does not strip 0 from trace/span IDs anlylonger #4661

tobiasstadler opened this issue Jan 29, 2021 · 3 comments · Fixed by #4671

Comments

@tobiasstadler
Copy link
Contributor

tobiasstadler commented Jan 29, 2021

In #3849 I implemented support for stripping leading 0s from trace/span ids when they where created by Jaeger (Fixed #3837). Currently Jaeger is in the process of switching to full length ids (see jaegertracing/jaeger#1657).

Should the apm-server also switch to full length ids?

@tobiasstadler
Copy link
Contributor Author

One could also introduce a configuration property in the jaeger configuration or add a new processor which does the stripping.

@axw
Copy link
Member

axw commented Feb 1, 2021

@tobiasstadler thanks for raising this!

I was thinking about this recently, in a different context: distributed tracing across a mix of Jaeger clients, Elastic APM agents, and OpenTelemetry SDKs. The latter two will already record full trace IDs, whereas Jaeger trace IDs might be trimmed. In the case of inconsistent IDs, distributed tracing would fail to work across them. Given that only the Java client for Jaeger supports W3C TraceContext (jaegertracing/jaeger#855) anyway, this hasn't been a problem so far, hence why we haven't done anything about it yet.

I think it would be a good idea to switch to full length IDs. I would prefer to just drop support for trimming and not add configuration. Jaeger support is still labelled as experimental to allow for changes like this. Would that pose a problem for you?

@tobiasstadler
Copy link
Contributor Author

@axw That is fine for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants