peer_metadata: set source workload in dynamic metadata for tracing #6693
Conversation
Signed-off-by: Petr McAllister <[email protected]>
|
😊 Welcome @PetrMc! This is either your first contribution to the Istio proxy repo, or it's been You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines Thanks for contributing! Courtesy of your friendly welcome wagon. |
istio-testing
added
the
size/XS
keithmattix
left a comment
keithmattix
left a comment
There was a problem hiding this comment.
We've got tests for stats; should we add some test cases for this?
| setFilterState(info, downstream, *result); | ||
| // For waypoint case, set dynamic metadata with SOURCE workload name for tracing | ||
| // ONLY set for downstream (client) discovery - not upstream (destination). | ||
| if (downstream && !result->workload_name_.empty()) { |
There was a problem hiding this comment.
Is there any reasonable way to expose all the info? Its kind of annoying to just have a subset.
In particular, the 'workload' may be useless info without 'source namespace' as well?
There was a problem hiding this comment.
is it already in filter state? with istio/api#3600 can we access it?
There was a problem hiding this comment.
Thanks @howardjohn! istio/api#3600 would help if Envoy's CustomTag supported formatters, but it currently only reads from dynamic metadata (CustomTag_Metadata), not filter state. Updated the PR to expose all peer metadata fields.
There was a problem hiding this comment.
value
(string) Custom tag value.The same format specifier as used for HTTP access logging applies here, however unknown specifier values are replaced with the empty string instead of -.
Used to specify what kind of custom tag.
which allows %FILTER_STATE(KEY:F:FIELD?):Z%
seems like it could work? I haven't tried
(I read the API 3 times and didn't realize this was possible until now)
There was a problem hiding this comment.
thanks John, closing the PR as there is no need for the proxy change. (and wow!)
Signed-off-by: Petr McAllister <[email protected]>
istio-testing
added
size/M
|
/test test-arm-arm64 |
|
@PetrMc: The following tests failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
5 participants
What this PR does / why we need it:
Enables waypoint proxies to identify source workload in distributed traces. The peer_metadata filter now sets dynamic metadata with the source workload name during downstream peer discovery. Istio control plane will configure waypoint trace tags to read this metadata. (will be a separate PR in https://github.com/istio/istio repo
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes # istio/istio#58348