-
Notifications
You must be signed in to change notification settings - Fork 5.2k
http: add sse_to_metadata filter for stream parsing #42762
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
|
Tagging @JuniorHsu and @KBaichoo for review 🙏 |
happy to help review and be one of the owner :) |
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
|
/retest |
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
|
@KBaichoo added you as maintainer for now but let me know if we should change that 😁 |
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
|
@adisuissa Looking for some guidance here please |
api/envoy/extensions/filters/http/sse_to_metadata/v3/sse_to_metadata.proto
Outdated
Show resolved
Hide resolved
api/envoy/extensions/sse_content_parsers/json/v3/json_content_parser.proto
Outdated
Show resolved
Hide resolved
api/envoy/extensions/filters/http/sse_to_metadata/v3/sse_to_metadata.proto
Outdated
Show resolved
Hide resolved
api/envoy/extensions/filters/http/sse_to_metadata/v3/sse_to_metadata.proto
Outdated
Show resolved
Hide resolved
api/envoy/extensions/filters/http/sse_to_metadata/v3/sse_to_metadata.proto
Outdated
Show resolved
Hide resolved
api/envoy/extensions/filters/http/sse_to_metadata/v3/sse_to_metadata.proto
Show resolved
Hide resolved
It's been a while since I've seen this, but the script may require the extension to be referenced (so maybe if the doc error is fixed, then it will keep it). |
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
|
/retest |
|
@PeterL328, wondering if you are willing to split the SSE parser out to another PR, and I will take a look, but up to you. |
@botengyao sure i can do that. I'll move the common/sse part and related tests (unit + fuzzer) to another PR. This PR is getting larger so a smaller one will be better to review anyways 😄 |
|
Created SSE parser util PR After SSE parser diff lands, I will remove the related files/changes in this diff. |
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
Signed-off-by: Peter Leng <[email protected]>
|
@adisuissa looks like CI issue relating to proto are fixed. |
| // appears early in the stream (e.g., extracting a model name from the first SSE event). | ||
| // | ||
| // If false (default), continue processing the entire stream. Later matches will overwrite | ||
| // earlier ones (unless preserve_existing_metadata_value is true), effectively picking the LAST occurrence. |
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.
Please add a reference to preserve_existing_metadata_value.
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.
At leave this should be under /api/envoy/extensions/http/sse_content_parsers/....
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.
sg
| // earlier ones (unless preserve_existing_metadata_value is true), effectively picking the LAST occurrence. | ||
| // | ||
| // Note: This applies to the first matching rule. on_missing and on_error do not trigger stopping. | ||
| bool stop_processing_on_first_match = 2; |
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.
2 high-level questions:
- is this field part of the json-content parsing (what I consider as output) or should it be part of the SSE parsing (what I consider as input)? - My first thought was this is related to the matched rules on the json-content parsing side, but the phrase in the comment ("extracting a model name from the first sse event") implied that it is related to the SSE events rather than the matched rules.
- Thinking out loud: I'm no expert in the use-cases of this, but I wonder if it makes sense to configure a number here (stop_processing_after_matches) which will be optional to set (WKT type). - Note that I'm not saying to go this way, I just want to make sure that this was considered, and if needed in the future, then could be done now.
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.
Thanks for feedback.
-
For SSE-to-metadata filter is designed to be content agnostic, and stopping criteria should be purely based on content so let's update the comment here to be better here.
-
I thought about this a bit. I have two thoughts here:
First I think I agree with you that even though most cases I would imagine would want to extract the first occurrence or last occurrence but we should keep API flexible in case we want extract after n occurrence. Let's go with something like UInt32Value stop_processing_after_matches where 0 means last, 1 means first and later N.
Second I feel it's confusing now that stop_processing_on_first_match is applied to all the rules. Previously when we didn't use j2m rules we had per rule stop_processing_on_first_match which is more fine grained and clearer. So I think we can add a wrapper around j2m rule but have a stop_processing_after_matches on each rule.
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.
Thanks for this contribution. To be honest, I think this could be supported by add SSE support at transform filter. For the AI token extraction, it should be easier. But anyway, this have got sponsor, so I will not block this if you guys do want this. It's not big deal to have overlap between different filters. :)
| // appears early in the stream (e.g., extracting a model name from the first SSE event). | ||
| // | ||
| // If false (default), continue processing the entire stream. Later matches will overwrite | ||
| // earlier ones (unless preserve_existing_metadata_value is true), effectively picking the LAST occurrence. |
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.
At leave this should be under /api/envoy/extensions/http/sse_content_parsers/....
|
/wait for API review |
Commit Message: Add sse_to_metadata HTTP filter with pluggable content parsers
Additional Description:
This PR introduces a new HTTP filter for extracting values from Server-Sent Events (SSE) response streams and writing them into dynamic metadata. The filter uses a pluggable content parser architecture that separates SSE protocol handling from content-specific parsing logic, making it extensible for future parser types (XML, plaintext, regex, etc.).
The primary use case is extracting token usage information from LLM streaming responses (e.g., OpenAI, Anthropic) for logging, observability and rate-limit token reporting.
Key Architecture
This layered design allows future parsers (XML, plaintext, regex) to be added without modifying the filter.
Key Features
Risk Level: Low (new filter)
Testing:
Docs Changes: Added comprehensive documentation for sse_to_metadata filter and JSON content parser
Release Notes: http: Added
envoy.filters.http.sse_to_metadatafilter for extracting values from Server-Sent Events (SSE) streams and writing them to dynamic metadata. Supports pluggable content parsers with initial JSON parser implementation. Useful for token-based rate limiting, logging and observability of LLM streaming APIs.Platform Specific Features: N/A
Fixes #29758