-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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(tracing): Add instana header support in propagation #13915
base: master
Are you sure you want to change the base?
Conversation
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.
Thank you so much for this great contribution! A couple of notes:
- For consistency, this should be supported by the Zipkin plugin as well, which requires a small schema update similar to what was done for OpenTelemetry in this PR, and an integration test in
spec/03-plugins/34-zipkin/zipkin_spec.lua
. - The test in
spec/01-unit/26-observability/02-propagation_strategies_spec.lua
should be updated as well to include this new header format
In addition to my previous comment, this needs a rebase. |
e28fb70
to
0c0f894
Compare
|
||
trace_id = from_hex(trace_id_raw) or nil | ||
span_id = from_hex(span_id_raw) or nil | ||
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
b21ca04
to
e26f7ca
Compare
834a15d
to
ee041d6
Compare
808c492
to
592015b
Compare
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.
just a few minor comments, then it looks good to me.
return | ||
end | ||
|
||
if trace_id_raw then |
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.
this if
is not needed, we know type
is string by now, so it cannot be nil
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.
updated
return | ||
end | ||
|
||
if span_id_raw then |
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.
same as above
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.
updated
|
||
local level_id_raw = headers["x-instana-l"] | ||
|
||
if level_id_raw then |
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.
same
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.
We probably still need this one since level id can be nil.
592015b
to
d670630
Compare
Summary
Checklist
changelog/unreleased/kong
orskip-changelog
label added on PR if changelog is unnecessary. README.mdIssue reference
Fix #12494