spacewander commented on code in PR #8099:
URL: https://github.com/apache/apisix/pull/8099#discussion_r999020747
##########
apisix/plugins/zipkin.lua:
##########
@@ -163,9 +165,9 @@ function _M.rewrite(plugin_conf, ctx)
-- pass the trace ids even the sample is rejected
-- see https://github.com/openzipkin/b3-propagation#why-send-
-- trace-ids-with-a-reject-sampling-decision
- core.request.set_header(ctx, "x-b3-traceid", trace_id)
- core.request.set_header(ctx, "x-b3-parentspanid", parent_span_id)
- core.request.set_header(ctx, "x-b3-spanid", request_span_id)
+ core.request.set_header(ctx, "x-b3-traceid", trace_id or
to_hex(rand_bytes(16)))
Review Comment:
Yes, I am wrong about the root span.
After some investigation, I figure out that:
Currently, APISIX handles root span like this:
1. check if need to sample or not
2. if not, set some upstream header and go to the next plugin
3. generate root span
The correct behavior should be:
1. check if need to sample or not
2. generate root span
3. set some upstream header (as the sampling should only affect the tracing
data report)
In step 2 of APISIX, the root span is missing so we have to create a fake
span ID in this PR.
Maybe we need to do some bigger changes to solve this problem?
For example, update
https://github.com/apache/apisix/blob/490e0ba995fb0bfe4c1b22eec424f58ebef39446/apisix/plugins/zipkin/codec.lua#L101
https://github.com/apache/apisix/blob/490e0ba995fb0bfe4c1b22eec424f58ebef39446/apisix/plugins/zipkin/reporter.lua#L58
to make it work correctly.
Otherwise, the generated fake span in this PR doesn't have a real span
associated with it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
