The Wayback Machine - https://web.archive.org/web/20201115063547/https://github.com/twitter/finagle/issues/719
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

implement "b3 single" header format #719

Open
adriancole opened this issue Aug 29, 2018 · 4 comments
Open

implement "b3 single" header format #719

adriancole opened this issue Aug 29, 2018 · 4 comments

Comments

@adriancole
Copy link
Contributor

@adriancole adriancole commented Aug 29, 2018

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, we write 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:

X-B3-TraceId: 80f198ee56343ba864fe8b2a57d3eff7
X-B3-ParentSpanId: 05e3ac9a4f6e3b90
X-B3-SpanId: e457b5a2e4d86bd1
X-B3-Sampled: 1
@mosesn
Copy link
Contributor

@mosesn mosesn commented Aug 29, 2018

Thanks for filing a ticket @adriancole, do you know if there's someone from the zipkin community who uses finagle and would be interested in tackling this? I think it would be a good first issue.

@adriancole
Copy link
Contributor Author

@adriancole adriancole commented Aug 30, 2018

@stevesoundcloud @kristofa @dgarson @lawrencefinn @chemicL @crispywalrus @teodor-pripoae @oskarblom @LarryFinn @umichyiwan sorry for the spam, but are any of you interested in this, or helping with it? If so, Moses can help with stewarding here and I can answer any questions etc.

@adriancole
Copy link
Contributor Author

@adriancole adriancole commented Aug 30, 2018

ps this is related linkerd/linkerd#2114

@crispywalrus
Copy link
Contributor

@crispywalrus crispywalrus commented Aug 30, 2018

I've started a PR crispywalrus#1 to address this ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.