B3 header support #749
B3 header support #749
Conversation
|
|
|
@adriancole @mosesn this is the result of the last changes to crispywalrus#1 to add OpenZipkin support to finagle. |
| def handleSampled(headers: HeaderMap,value: String): Unit = | ||
| value match { | ||
| case "0" => headers.set(Header.Flags,"0") | ||
| case "d" => headers.set(Header.Flags,"1") |
There was a problem hiding this comment.
don't recall if in scala case falls through, but debug implies sampled. so you could also set the sampled header here
Codecov Report
@@ Coverage Diff @@
## develop #749 +/- ##
===========================================
- Coverage 69.31% 69.31% -0.01%
===========================================
Files 765 765
Lines 24397 24419 +22
Branches 1829 1811 -18
===========================================
+ Hits 16911 16925 +14
- Misses 7486 7494 +8
Continue to review full report at Codecov.
|
|
This approach seems reasonable. I'd like to see some unit tests before going forward. |
|
Hi @crispywalrus, how is testing going? Is there anything we can do to assist? |
crispywalrus
force-pushed
the
b3-header-support
branch
from
dd85c59 to
bacd35e
Compare
so should this remove the "b3" header, and if so how does it get added back in in downstream HTTP calls?
named `TraceContext`
+ break out the new convertions logic from the current handler + rename the parse field handlers to match their function not field
added a removal of the b3 header as internally span/trace management works on the X-B3 versions and it seems prudent to make sure there isn't a conflict
crispywalrus
force-pushed
the
b3-header-support
branch
from
bacd35e to
5545b86
Compare
|
@kevinoliver this adds some basic test that show a b3 header ends up being parsed into finagles X-B3 headers. I've chosen to remove the |
|
hey folks. can you help get this merged? @crispywalrus has responded and we are nearing a year on the original feature request. |
Problem OpenZipkin has created a new set of header to carry trace information. These headers are not compatible with the old ones and are needed to interoperate with services that use the new version of the header. Solution If a "b3" header exists in the request this extracts all the new information and populates finagles tracing header with the existing values. #749 Signed-off-by: Ian Bennett <[email protected]> Differential Revision: https://phabricator.twitter.biz/D334419
|
@enbnt why did this get closed? |
|
@crispywalrus because it has landed in 8721837 😄 |
|
Great! |
|
Thanks for the patience, everyone! |
Problem
OpenZipkin has created a new set of header to carry trace information. These headers are not compatible with the old ones and are needed to interoperate with services that use the new version of the header.
Solution
If a "b3" header exists in the request this extracts all the new information and populates finagles tracing header with the existing values.