adding note about propagation from apache reverse proxy #44
Conversation
codefromthecrypt
left a comment
codefromthecrypt
left a comment
There was a problem hiding this comment.
Looks good except that it is picking the wrong 16 chars and it is an error to not propagate down a span ID. X-B3-SpanId should be set with the truncated ID. I'm glad this got to point of sharing.. very nice!
A b3 single header variant would be good extra credit.
|
|
||
| ``` | ||
| RequestHeader setifempty X-B3-TraceId "expr=%{md5:%{reqenv:UNIQUE_ID}}" | ||
| RequestHeader edit X-B3-TraceId "^(.{0,16}).*$" "$1" |
There was a problem hiding this comment.
this is incorrect, it should be the last 16 chars, not the first. Also X-B3-SpanId should be set to the last 16 chars.
Doing this without setting the sampling flag will result in the first hop sampling, which is probably ok as it could be tricky to put sampling logic here :D.
OTOH an example of b3: 0 for /health path would be nice as it shows how to stop things from tracing that.
There was a problem hiding this comment.
@adriancole Can you please explain a bit on why "this is incorrect, it should be the last 16 chars, not the first"? The way I was thinking (but I wasn't thinking too much of it) is that MD5 generates a "uniformly random" hex string of 32 chars, so it doesn't matter if we take the first, middle or last 16 chars - as a matter of fact this trick was simply a way to generate a pseudorandom 16 char hex string.
There was a problem hiding this comment.
sure it is here. https://github.com/openzipkin/b3-propagation/blob/master/STATUS.md and also explained here https://github.com/openzipkin/zipkin/blob/master/zipkin/src/main/java/zipkin2/storage/StorageComponent.java#L106-L144
Seems yeah this should be in the top-level readme :D Offers welcome to fix that!
The main thing is that even if at your site, you feel truncating to 64bit is a constant, we shouldn't hint here picking the left hand side as that will conflict with other practice. I think many would use this with 128-bit IDs as almost all libraries at least truncate back on unsupported for the last several years. It isn't necessary to do that here, but it is interesting to show the option. When showing the option, we should be specific on how.
There was a problem hiding this comment.
Fair points, thanks for the explanation.
|
The spec says span ID is required unless one is only propagating sampling
so that should be there too.
…On Sun, 25 Oct 2020, 02:12 Adrian Cole, ***@***.***> wrote:
cc @adammichalik <https://github.com/adammichalik>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#44 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXOYAVJNX75K2TDLQ2BOULSMNUNNANCNFSM4S5YU7VQ>
.
|
Co-authored-by: Adrian Cole <[email protected]>
fixes openzipkin/zipkin#1479