Skip to content
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

Support Body Transformations #35783

Open
arkodg opened this issue Aug 21, 2024 · 14 comments
Open

Support Body Transformations #35783

arkodg opened this issue Aug 21, 2024 · 14 comments
Assignees
Labels
design proposal Needs design doc/proposal before implementation enhancement Feature requests. Not bugs or questions. help wanted Needs help!

Comments

@arkodg
Copy link
Contributor

arkodg commented Aug 21, 2024

Title: Support Body Transformations

Description:.
Envoy supports manipulating/transforming headers, would be great to also support transforming the request and response body to be able to

  • sanitize fields
  • support API conversions

Relevant Links:

Adding a list of other proxy implementations that support this

@arkodg arkodg added enhancement Feature requests. Not bugs or questions. triage Issue requires triage labels Aug 21, 2024
@arkodg
Copy link
Contributor Author

arkodg commented Aug 23, 2024

would be great if this feature/filter can also support copying/setting fields from the body into the header, allowing routing based on request body which is a AI LLM Gateway use case, more in this doc
cc @robscott

@wbpcode wbpcode added help wanted Needs help! and removed triage Issue requires triage labels Aug 23, 2024
@wbpcode
Copy link
Member

wbpcode commented Aug 23, 2024

Although it is not encouraged that to mutate request body because it will need to buffer whole body and break the streamlined processing. But I also admit that there are lots of related requirements.

So, SGTM. And I can help to review the design and to sponsor this new extension if someone wants to take this. (note: a design proposal is necessary first).

@wbpcode wbpcode added the design proposal Needs design doc/proposal before implementation label Aug 23, 2024
@mathetake
Copy link
Member

@wbpcode I will take a shot at this!

@mathetake
Copy link
Member

/assign

@kyessenov
Copy link
Contributor

Although it is not encouraged that to mutate request body because it will need to buffer whole body and break the streamlined processing. But I also admit that there are lots of related requirements.

So, SGTM. And I can help to review the design and to sponsor this new extension if someone wants to take this. (note: a design proposal is necessary first).

I think the technical challenge is to make mutations streaming. It's feasible as long as the body is structured. The buffering approach forces large connection buffers in multiplexed protocols, which is not scaleable for multi-tenant gateways.

@wbpcode
Copy link
Member

wbpcode commented Aug 23, 2024

Although it is not encouraged that to mutate request body because it will need to buffer whole body and break the streamlined processing. But I also admit that there are lots of related requirements.

So, SGTM. And I can help to review the design and to sponsor this new extension if someone wants to take this. (note: a design proposal is necessary first).

I think the technical challenge is to make mutations streaming. It's feasible as long as the body is structured. The buffering approach forces large connection buffers in multiplexed protocols, which is not scaleable for multi-tenant gateways.

According to my exp, in the scenarios where this feature is required, the body basically is a JSON.
It's almost impossible to make mutations streaming for that.

At least for now, I don't know how to make a general solution for long live stream which has unlimited body length. So, I am inclined to ignore them first.

@arkodg
Copy link
Contributor Author

arkodg commented Aug 23, 2024

+1 to @wbpcode's suggestion of keeping streaming, out of scope

@kyessenov
Copy link
Contributor

+1 to @wbpcode's suggestion of keeping streaming, out of scope

Yes, I didn't mean to require it. But we should be explicit about the limitations of a buffering approach (e.g. connection buffers must be at least the number of streams per connections \times max buffered bytes). E.g. 1MB JSONs with 100 H2 streams require up to 100MB connection buffers.

@mathetake
Copy link
Member

https://docs.google.com/document/d/1odMAistdE8OrJHqKvV4ou9vsZxB1Wepy0cxAyBJ-Bes/edit so @arkodg and I quickly wrote a simple proposal. @wbpcode could you take a look when you get a chance? Thanks in advance!

@tyxia
Copy link
Member

tyxia commented Oct 4, 2024

FWIW, ext_proc filter supports body mutation, as well as header and trailer

@kyessenov
Copy link
Contributor

CC @TAOXUY - I think you also need some body transformations and extractions.

Back to my early buffering point - I think it would be nice to have separate buffering and streaming modes. You don't need to buffer to sanitize JSON fields, and the only meaningful benefit of this over ext_proc is that it must perform significantly better (e.g. hundred of micros P50). overhead, which is hard to do when you fully buffer.

@TAOXUY
Copy link
Contributor

TAOXUY commented Oct 4, 2024

@missBerg
Copy link

missBerg commented Oct 7, 2024

I'm wondering what the negative effects of being able to do message transformation with a native filter are?

The positives I anticipate form this is ease of configuration for users, compared to developing their own ext proc, as well as reduced complexity of network calls invoking external processes.

@mathetake
Copy link
Member

@wbpcode could you take a look at the doc we shared above when you get a change? tia!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design proposal Needs design doc/proposal before implementation enhancement Feature requests. Not bugs or questions. help wanted Needs help!
Projects
None yet
Development

No branches or pull requests

7 participants