-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
feat(zipkin): support b3 req header #3551
Conversation
Fix apache#1125 Signed-off-by: spacewander <[email protected]>
apisix/plugins/zipkin.lua
Outdated
err, trace_id, request_span_id, sampled, parent_span_id = parse_b3(b3) | ||
|
||
if err then | ||
core.log.warn("invalid b3 header: ", b3, ", ignored: ", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use error
is better here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@membphis
Raise error
in a request may be considered a bug of APISIX (as it will provide 500 and backtrack).
Maybe we can return 400 to reject it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think 400
is better, since an invalid header is always caused by client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
400
is better, since an invalid header is always caused by client.
agree this too.
Raise error in a request may be considered a bug of APISIX (as it will provide 500 and backtrack).
I means core.log.error(****)
^_^
Signed-off-by: spacewander <[email protected]>
@Yiyiyimu Chaos Test failed, do you have time to look at this PR? |
Fix #1125
Signed-off-by: spacewander [email protected]
What this PR does / why we need it:
Pre-submission checklist: