-
Notifications
You must be signed in to change notification settings - Fork 504
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 "b3" trace header #2114
Comments
Thanks, @adriancole! I think this would be a very simple change to https://github.com/linkerd/linkerd/blob/master/linkerd/protocol/http/src/main/scala/io/buoyant/linkerd/protocol/ZipkinTracePropagator.scala Are you interested in working on this? |
yep can do if no one else in a timeout :P |
@adleong can you please assign this issue to me? |
Would like to have a discussion about the design around this enhancement:
@adriancole @adleong not sure I understand this request: "It would also be nice to support optionally writing this", is this covered by the changes described above or you mean to always write single header "b3", if asked by config, no matter if input was "b3" or historical "X-B3-"? Thanks! |
I'm not sure where the quoted points are from. I'm going to guess they were takeaways from digesting things. https://github.com/openzipkin/b3-propagation#http-encodings prioritizes "b3" over "X-B3-*" indeed that implies ignoring the latter if you read "b3" There's no way to know what the downstream is capable of, and it is more likely it supports "X-B3-*" vs not. It is a more portable choice to always write down "X-B3-" or dual propagate, vs only write "b3". In the brave library, we only write "X-B3-" even if we read "b3" unless a config change is made. Hope this helps. |
@adriancole I've quoted from your first comment on this issue, sorry for not being clear. But, you made it clear for me, will try implementing in Linkerd1 the same approach you're using in brave: be portable and always write "X-B3-" by default, if required by config write "b3". Thanks! |
thanks for the loopback and appreciate moving this forward. cheers!
…On Sun, Sep 22, 2019, 4:11 AM Tacalau Daniel Stefan < ***@***.***> wrote:
@adriancole <https://github.com/adriancole> I've quoted from your first
comment on this issue, sorry for not being clear.
But, you made it clear for me, will try implementing in Linkerd1 the same
approach you're using in brave: be portable and always write "X-B3-" by
default, if required by config write "b3". Thanks!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2114?email_source=notifications&email_token=AAAPVV233GTVXITZHAJDIE3QKZ5WLA5CNFSM4FSD57JKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7IY3CY#issuecomment-533826955>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAPVV64LEDWBWQIO62OEGDQKZ5WLANCNFSM4FSD57JA>
.
|
…ill propagate b3 single header the same way as x-b3- multi headers. If a request contains "b3" single header and "X-B3-" multi headers, the trace context from "b3" is preferred and "x-b3-" will be ignored. Added support for 128bit traceId. Signed-off-by: dst4096 <[email protected]>
…ill propagate b3 single header the same way as x-b3- multi headers. If a request contains "b3" single header and "X-B3-" multi headers, the trace context from "b3" is preferred and "x-b3-" will be ignored. Added support for 128bit traceId. Signed-off-by: dst4096 <[email protected]>
A fix for this issue #2329 |
…ill propagate b3 single header the same way as x-b3- multi headers. If a request contains "b3" single header and "X-B3-" multi headers, the trace context from "b3" is preferred and "x-b3-" will be ignored. Added support for 128bit traceId. Signed-off-by: dst4096 <[email protected]>
…ill propagate b3 single header the same way as x-b3- multi headers. If a request contains "b3" single header and "X-B3-" multi headers, the trace context from "b3" is preferred and "x-b3-" will be ignored. Added support for 128bit traceId. Signed-off-by: dst4096 <[email protected]>
…. Moved implementation of http and h2 ZipkinTracePropagator to a new common place base-http. Fixed setting of sampled/flags, computation of sampled. Updated tests. Support for 128bit trace. Signed-off-by: dst4096 <[email protected]>
…ill propagate b3 single header the same way as x-b3- multi headers. If a request contains b3 single header and X-B3- multi headers, the trace context from b3 is preferred and x-b3- will be ignored. Signed-off-by: dst4096 <[email protected]>
…ill propagate b3 single header the same way as x-b3- multi headers. If a request contains b3 single header and X-B3- multi headers, the trace context from b3 is preferred and x-b3- will be ignored. Signed-off-by: dst4096 <[email protected]>
…ill propagate b3 single header the same way as x-b3- multi headers. If a request contains b3 single header and X-B3- multi headers, the trace context from b3 is preferred and x-b3- will be ignored. Signed-off-by: dst4096 <[email protected]>
Let's support at least reading "b3" header from a single string, most commonly traceid-spanid-1
It would also be nice to support optionally writing this, especially in message providers or others with constrained environments.
Expected behavior
As discussed on openzipkin/b3-propagation#21 and first implemented here: https://github.com/openzipkin/brave/blob/master/brave/src/main/java/brave/propagation/B3SingleFormat.java https://github.com/openzipkin/brave/blob/master/brave/src/test/java/brave/propagation/B3SingleFormatTest.java we should be able to parse "b3" and ideally also write it depending on config.
Actual behavior
right now,
ZipkinTracePropagator
writes the historical "X-B3-" headers, which we should still support. However, if we see a header named "b3" we should read that instead.Steps to reproduce the behavior
Turn on tracing and see if
b3=80f198ee56343ba864fe8b2a57d3eff7-05e3ac9a4f6e3b90-1-e457b5a2e4d86bd1
results in the same context as:The text was updated successfully, but these errors were encountered: