Use existing opentracing context #2016
Conversation
We wanted linkerd to continue a tracing context created outside (in our load balancers basically) instead of creating a new context and using their own headers when propagating it. To solve it, we have added a condition in the method that parses headers to look for a span header, and if present, parse that information and use it to create the tracing context. Also, we have added the headers to propagate outside linkerd, to make life easier for applications that want to use opentracing libraries. We have validated this in our environment, using linkerd as service mesh as part of a DC/OS installation. We have seen the same traceId in zipkin with this change for both LB and linkerd. Fixes #[1447]
|
Thanks for this, @jmalvarezf! I agree with you that Linkerd is long overdue to play more nicely with other tracing systems like OpenTracing. Unfortunately, there are a few complications. So far, Linkerd has always used its own With your change, Linkerd will read the Even switching the precedence so that Linkerd prefers the The complexity compounds even further if you want to support other tracing propogation headers such as jaeger's The TLDR is that I think this needs to be something configurable so that users can decide which tracing systems they want Linkerd to participate in and which they want Linkerd to ignore. Would you be willing to hold off on this PR while I put together a Linkerd trace propagation plugin interface? This would then allow you to resubmit your change as a trace propagation plugin. I hope this context is useful, please let me know what you think! |
|
Hi @adleong, Yes, I think it makes total sense! Thank you! |
|
@jmalvarezf I'll leave it up to you. Either way we can close this one once the replacement is ready. |
|
Please see #2025 and let me know what you think. |
|
I'll try to adapt this to the interface you propose during the following month when I have enough time and I'll let you know. Thank you! |
|
@jmalvarezf the new interface has merged. Please let me know if you need any help with this new interface! |
| } | ||
|
|
||
| object Trace { | ||
| val Key = Prefix + "trace" | ||
| val OpentracingSpanHeader = "x-b3-spanid"; |
There was a problem hiding this comment.
this has nothing to do with opentracing by the way. OpenTracing includes no library support for B3 unless something changed recently. census does however. B3 is defined here https://github.com/openzipkin/b3-propagation
|
You are right. This is not opentracing but zipkin. Ill close this and start working on the interface already merged. Thanks |
|
cheers!
|
Hi,
As we wanted to have the ability for linkerd to propagate the opentracing context that was originated outside linkerd, and it wasn't clearly in your roadmap, we went ahead and modified linkerd to do this.
I'm a scala newbie, so the code may be less than optimal. Any change you feel would fit better, feel more than free to tell me. The change consists basically in adding an if when getting tracing context to get opentracing headers, and to propagate the context in the same headers in the next step.
This is very important for us to be able to have the full trace for a request, as it is originated outside the platform that is running linkerd as service mesh.
We have tested this in our deployment and it's working as expected.
This should fix #1447 with standard opentracing headers
Thanks!