[release-1.29] Ambient Multicluster Telemetry #6799
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Include myself, Steven and Gustavo as owners of the experimental-ambient-multicluster-telemetry branch (istio#6772) * Include myself, Steven and Gustavo as owners of the experimental-ambient-multicluster-telemetry branch Signed-off-by: Mikhail Krinkin <[email protected]> * Use single match - creating multiple matches means that the later overrides the earlier Signed-off-by: Mikhail Krinkin <[email protected]> --------- Signed-off-by: Mikhail Krinkin <[email protected]> * Add Baggage metadata propagation (istio#6776) * Add Baggage metadata propagation Signed-off-by: Keith Mattix II <[email protected]> * clang-tidy Signed-off-by: Keith Mattix II <[email protected]> * Go back to old baggage impl Signed-off-by: Keith Mattix II <[email protected]> * Fix baggage format Signed-off-by: Keith Mattix II <[email protected]> * Actually use new baggage approach Signed-off-by: Keith Mattix II <[email protected]> --------- Signed-off-by: Keith Mattix II <[email protected]> * Introduce new filters discovering peer metadata from baggage header (istio#6771) * Introduce new filters discovering peer metadata from baggage header This a combination of two filters that have to be used together: - regular network filter (expected to be configured in connect_originate or inner_connect_originate listeners before TCP Proxy filter) - upstream network filter (expected to be configuration in all clusters that use HBONE or double-HBONE for endpoints) Those two filters together basically create a tunnel. The tunnel protocol just prepends a fixed size header to data stream coming from regular network filter to the upstream network filter, followed by the peer metadatra encoded as protobuf Any containing a protobuf Struct inside (I'm just re-using existing code from Istio proxy, that's why encoding is such as it is). The regular network filter only triggers when there is some data coming from upstream connection in response. It's not correct in general, but in waypoints we do know that we proxy an L7 protocol (http or gRPC), so we do expect a some data in reply. The regular network filter relies on TCP Proxy filter extracting response headers and saving them in the filter state. It then extracts and parses the baggage header from the saved headers. In all cases I explicitly communicate when no peer metadata has been discovered by sending some data downstream. This ensures that upstream network filter running downstream can always remove the prefix from the data stream and does not really need to guess if it's there or not. NOTE: We still do some checks to confirm that the prefix is there, but we cannot really rely on those checks for correctness in all the cases. The upstream network filter, as pointed out above, extracts the data sent by the regular network filter from the data stream, it parses the data and populates filter state based on that. Unlike the HTTP peer metadata filter, this one runs in the context of the upstream connection, so it populates the upstream filter state and not the regular one. I plan to add support to the HTTP peer metadata filter option for new upstream metadata discovery via upstream filter metadata, thus propagating it all the way to the istio stats filter. NOTE: None of those filters are yet generated by pilot and there are certainly some additional options to configure (e.g., maybe we can come up with a good way to transfer metadata via Envoy TLS instead of injecting it into the data stream directly - this way, in principle, we could avoid creating a custom upstream filter all together, if http peer metadata filter could get the peer metadata directly from connect_originate listener). All-in-all, it's not the final implementation. Signed-off-by: Mikhail Krinkin <[email protected]> * Fix BUILD formatting Signed-off-by: Mikhail Krinkin <[email protected]> * Fix formatting of C++ code Signed-off-by: Mikhail Krinkin <[email protected]> * Update HTTP peer_metadata filter to consume filter state set by upstream peer_metadata filter This basically taps the upstream peer metadata into the regular filter state consumed by the istio stats filter. http peer metadata filter also takes care of priorities between different discovery methods - we just need to put different discovery methods in the right order in the configuration. Signed-off-by: Mikhail Krinkin <[email protected]> * Populate peer principal in the upstream workload metadata as well Signed-off-by: Mikhail Krinkin <[email protected]> * Support propagating baggage header to upstream and additional safety checks for upstream network filter Signed-off-by: Mikhail Krinkin <[email protected]> * Only register UpstreamFilterState peer metadata discovery method for upstream peer discovery Signed-off-by: Mikhail Krinkin <[email protected]> * Move peer_metadata filter proto config in the same directory Signed-off-by: Mikhail Krinkin <[email protected]> * Fix typo Signed-off-by: Mikhail Krinkin <[email protected]> --------- Signed-off-by: Mikhail Krinkin <[email protected]> * Baggage discovery (istio#6779) * Add Baggage metadata propagation Signed-off-by: Keith Mattix II <[email protected]> * clang-tidy Signed-off-by: Keith Mattix II <[email protected]> * basics for baggage discovery downstream * removing unnecessary tests * reverting crazy claude changes in release-binary.sh * fixing tests, fixing baggage key tokens * removing comment * make lint * fixing unit tests for metadata_object * make lint * suggestions from PR * clarifying use of mappings for baggage and field access * make lint --------- Signed-off-by: Keith Mattix II <[email protected]> Co-authored-by: Keith Mattix II <[email protected]> * Add locality to proxy metadata (istio#6780) * Add locality to proxy metadata Signed-off-by: Keith Mattix II <[email protected]> * Clang-tidy Signed-off-by: Keith Mattix II <[email protected]> * Buildifier format Signed-off-by: Keith Mattix II <[email protected]> * Rebase and fix some bugs Signed-off-by: Keith Mattix II <[email protected]> --------- Signed-off-by: Keith Mattix II <[email protected]> * Drop app labels from baggage and propagate principal (istio#6791) * Drop app labels from baggage and propagate principal I think I confused folks a bit when I mentioned that app field is missing from the baggage - it wasn't. In fact, canonical name of the workload and app in ambient are the same thing, that's why baggage does not actually need an app label - it already has service.name that encodes what we need. I updated the design document, but it happened after I mentioned here and there that we need to add a missing field to the baggage. This change corrects implementation and that makes istio stats populate the app label correctly. The other field that has not been populated is principal. WorkloadMetadataObject contained that identity field that contained principle in principle, but the methods used to conver WorkloadMetadataObject to a protobuf Struct and back ignored that field and never populated it, so it got lost and istio stats never used it. We haven't noticed that before because in ambient we used xDS-based peer metadata discovery by default and it triggers a different code path that does not rely on the methods that convert protobuf Struct to WorkloadMetadataObject, and the code path used there didn't have the same issue. Signed-off-by: Mikhail Krinkin <[email protected]> * Keep backwards compatibility for app.service and app.version baggage fields Signed-off-by: Keith Mattix II <[email protected]> --------- Signed-off-by: Mikhail Krinkin <[email protected]> Signed-off-by: Keith Mattix II <[email protected]> Co-authored-by: Keith Mattix II <[email protected]> * Fix some test compilation errors Signed-off-by: Keith Mattix II <[email protected]> * Merge master branch and resolve merge conflicts properly (istio#6795) * Automator: update envoy@ in istio/proxy@master (istio#6777) * Automator: update envoy@ in istio/proxy@master (istio#6778) * Don't do workload discovery for cross-network traffic (istio#6767) * Get the implementation compiling * Add tests for cross-network peer metadata Signed-off-by: Keith Mattix II <[email protected]> * clang-tidy Signed-off-by: Keith Mattix II <[email protected]> * One more tidy Signed-off-by: Keith Mattix II <[email protected]> * Switch to debug for logging Signed-off-by: Keith Mattix II <[email protected]> --------- Signed-off-by: Keith Mattix II <[email protected]> * Automator: update envoy@ in istio/proxy@master (istio#6782) * Automator: update envoy@ in istio/proxy@master (istio#6784) * Automator: update go-control-plane in istio/proxy@master (istio#6786) * Automator: update envoy@ in istio/proxy@master (istio#6787) * Automator: update envoy@ in istio/proxy@master (istio#6788) * update x-network header key (istio#6790) Signed-off-by: Ian Rudie <[email protected]> * Automator: update envoy@ in istio/proxy@master (istio#6794) * Merge upstream/master and resolve merge conflicts Signed-off-by: Mikhail Krinkin <[email protected]> * Missed one Signed-off-by: Mikhail Krinkin <[email protected]> * Fixed a wrong one Signed-off-by: Mikhail Krinkin <[email protected]> --------- Signed-off-by: Keith Mattix II <[email protected]> Signed-off-by: Ian Rudie <[email protected]> Signed-off-by: Mikhail Krinkin <[email protected]> Co-authored-by: Istio Automation <[email protected]> Co-authored-by: Keith Mattix II <[email protected]> Co-authored-by: Ian Rudie <[email protected]> --------- Signed-off-by: Mikhail Krinkin <[email protected]> Signed-off-by: Keith Mattix II <[email protected]> Signed-off-by: Ian Rudie <[email protected]> Co-authored-by: Krinkin, Mike <[email protected]> Co-authored-by: Gustavo Meira <[email protected]> Co-authored-by: Istio Automation <[email protected]> Co-authored-by: Ian Rudie <[email protected]>
istio-testing
added
the
size/XXL
Contributor
Author
|
/retest |
Contributor
Author
|
/retest |
fjglira
approved these changes
Jan 28, 2026
fjglira
left a comment
fjglira
left a comment
There was a problem hiding this comment.
LGTM, approved by TOC during WG meeting last week
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry-pick of #6793 onto the release 1.29 branch