ext_proc: add support for receiving typed dynamic metadata#45654
ext_proc: add support for receiving typed dynamic metadata#45654toddmgreer wants to merge 7 commits into
Conversation
Allow the external processor to return typed dynamic metadata to Envoy via the `typed_dynamic_metadata` field in `ProcessingResponse`. This metadata is populated in the stream info if the namespace is configured in `metadata_options.receiving_namespaces.typed`. Also added unit and integration tests to verify the functionality. Signed-off-by: Todd Greer <toddmgreer@gmail.com>
Signed-off-by: Todd Greer <toddmgreer@gmail.com>
toddmgreer
requested review from
mattklein123,
tyxia,
yanavlasov and
yanjunxiang-google
as code owners
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for receiving typed dynamic metadata from external processing servers in the ext_proc filter. It updates the relevant protobuf definitions, implements the processing logic in the filter, and adds comprehensive integration and unit tests. The review feedback suggests two optimizations: avoiding a redundant map lookup in ext_proc.cc by using the iterator's value directly, and eliminating an unnecessary string copy in the integration test helper by passing the map key directly to Http::LowerCaseString.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| std::string key_prefix = md_entry.first; | ||
| envoy::extensions::filters::http::set_metadata::v3::Metadata typed_md_val; | ||
| if (md_entry.second.UnpackTo(&typed_md_val)) { | ||
| headers.addCopy(Http::LowerCaseString(key_prefix), typed_md_val.metadata_namespace()); | ||
| } |
There was a problem hiding this comment.
Avoid an unnecessary string copy of md_entry.first into key_prefix by passing md_entry.first directly to Http::LowerCaseString.
envoy::extensions::filters::http::set_metadata::v3::Metadata typed_md_val;
if (md_entry.second.UnpackTo(&typed_md_val)) {
headers.addCopy(Http::LowerCaseString(md_entry.first), typed_md_val.metadata_namespace());
}
Avoid a redundant map lookup by using context_key.second.struct_value() directly instead of response_metadata.at(context_key.first).struct_value(). Since we are already iterating over the map, context_key.second already holds the value corresponding to context_key.first. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: toddmgreer <tgreer@google.com>
Avoid an unnecessary string copy of md_entry.first into key_prefix by passing md_entry.first directly to Http::LowerCaseString in the typed metadata loop. TAG=agy CONV=2c07b7d8-d1c9-4dbf-887e-321764aaf112 Signed-off-by: Todd Greer <toddmgreer@gmail.com>
|
gemini-code-assist's comments are addressed. |
|
HI, @toddmgreer thanks for adding this support. Could you please add some high level description why such typed dynamic metadata support is needed? |
|
Please fix the CI error and merge main. |
Add a comment explaining that typed dynamic metadata (typed_dynamic_metadata) should be preferred over untyped dynamic metadata (dynamic_metadata) because it is more efficient and more type-safe. Signed-off-by: Todd Greer <toddmgreer@gmail.com> TAG=agy CONV=ec28f592-9ffb-4881-bdc9-6147e3ebc45b
Signed-off-by: Todd Greer <toddmgreer@gmail.com> TAG=agy CONV=ec28f592-9ffb-4881-bdc9-6147e3ebc45b
Signed-off-by: Todd Greer <toddmgreer@gmail.com> TAG=agy CONV=ec28f592-9ffb-4881-bdc9-6147e3ebc45b
|
4 participants
Commit Message:
Allow the external processor to return typed dynamic metadata to Envoy via the
typed_dynamic_metadatafield inProcessingResponse. This metadata is populated in the stream info if the namespace is configured inmetadata_options.receiving_namespaces.typed.Also added unit and integration tests to verify the functionality.
Additional Description:
I used Gemini generative AI. I understand this code, but this is my first ext_proc PR, so I could have points of confusion.
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]