Skip to content

ext_proc: Implement STREAMED body processing mode#17069

Merged
htuch merged 15 commits intoenvoyproxy:mainfrom
gbrail:streaming-1
Jul 22, 2021
Merged

ext_proc: Implement STREAMED body processing mode#17069
htuch merged 15 commits intoenvoyproxy:mainfrom
gbrail:streaming-1

Conversation

@gbrail
Copy link
Copy Markdown
Contributor

@gbrail gbrail commented Jun 21, 2021

Commit Message: Implement STREAMED body processing mode in ext_proc

Additional Description: Implement this mode, which allows an external processor to receive the body one chunk at a time and examine or modify the body while it is being processed.

Risk Level: Medium. The mode is only engaged if used. However an external processor must be coded carefully, particularly if it expects to stream both request and response bodies simultaneously, since each will proceed in its own ordering.

Testing: New unit and integration tests.

Docs Changes: Updated proto docs to reflect that the mode is now supported.

Release Notes: External processing servers may now use the STREAMED processing mode. In this mode, chunks of the body are forwarded to the external processing server when they arrive. Depending on how the upstream system is implemented, request body chunks may be delivered before or after the response headers, and request and response body chunks may be interleaved if the upstream system delivers them that way. An external processor should be carefully coded so that it does not assume that a particular ordering will be implemented.

gbrail added 7 commits June 21, 2021 19:04
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
envoyproxy/api-shepherds assignee is @htuch
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #17069 was synchronize by gbrail.

see: more, trace.

@gbrail gbrail marked this pull request as ready for review June 22, 2021 08:29
@gbrail gbrail requested a review from snowp as a code owner June 22, 2021 08:29
@gbrail
Copy link
Copy Markdown
Contributor Author

gbrail commented Jun 22, 2021

@ikepolinsky

Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is super cool.
/wait

@htuch
Copy link
Copy Markdown
Member

htuch commented Jun 28, 2021

@ikepolinsky would appreciate your take on this PR; since you will be writing the fuzzing against this, it's probably a good idea to make sure it's testable and matches your understanding.

@gbrail
Copy link
Copy Markdown
Contributor Author

gbrail commented Jul 1, 2021

Thanks for your comments -- I have some improvements I'm working on but I have one test failure to address before they're ready.

gbrail added 3 commits July 2, 2021 01:32
Clarify "streaming integration test" by using standard assert macros and
with some comments.

Fix bugs that happen when streamed message bodies are prematurely closed
or have their processing mode changed.

Reduce the amount of watermarking the filter does in streamed mode.

Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the delay, holiday week.
/wait

gbrail added 3 commits July 20, 2021 21:02
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this reads well to me now, the only two things before shipping are nits and maybe getting some better docs on the thread around HTTP filter chain resumption while chunks are out for processing.

gbrail added 2 commits July 21, 2021 22:42
Signed-off-by: Gregory Brail <gregbrail@google.com>
Signed-off-by: Gregory Brail <gregbrail@google.com>
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@htuch htuch merged commit 271e674 into envoyproxy:main Jul 22, 2021
@gbrail gbrail deleted the streaming-1 branch July 22, 2021 17:22
@gbrail
Copy link
Copy Markdown
Contributor Author

gbrail commented Jul 22, 2021

The original discussion about how to do this was covered in #16760 and with this PR, that issue can be closed.

leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
Implement this mode, which allows an external processor to receive the body one chunk at a time and examine or modify the body while it is being processed.

Risk Level: Medium. The mode is only engaged if used. However an external processor must be coded carefully, particularly if it expects to stream both request and response bodies simultaneously, since each will proceed in its own ordering.

Testing: New unit and integration tests.

Docs Changes: Updated proto docs to reflect that the mode is now supported.

Release Notes: External processing servers may now use the STREAMED processing mode. In this mode, chunks of the body are forwarded to the external processing server when they arrive. Depending on how the upstream system is implemented, request body chunks may be delivered before or after the response headers, and request and response body chunks may be interleaved if the upstream system delivers them that way. An external processor should be carefully coded so that it does not assume that a particular ordering will be implemented.

Signed-off-by: Gregory Brail <gregbrail@google.com>
@machaval
Copy link
Copy Markdown

@gbrail do you have a working example of Streamed mode working I'm not able to make it work even on the most basic example... only on buffered mode

@gbrail
Copy link
Copy Markdown
Contributor Author

gbrail commented Dec 11, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants