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

http2: propagates reset events to CodecEventCallbacks when sending RST_STREAM #37784

Merged
merged 5 commits into from
Jan 9, 2025

Conversation

birenroy
Copy link
Contributor

@birenroy birenroy commented Dec 20, 2024

We have found this patch useful in our testing.

The delta between the first and second commit demonstrates test coverage for the new code.

The new behavior can be disabled with the runtime guard envoy.reloadable_features.http2_propagate_reset_events.

Commit Message: propagates reset events to CodecEventCallbacks when sending RST_STREAM
Additional Description:
Risk Level: low
Testing: ran unit and integration tests locally
Docs Changes:
Release Notes:
Platform Specific Features:
Runtime guard: envoy.reloadable_features.http2_propagate_reset_events

@birenroy
Copy link
Contributor Author

@phlax
Copy link
Member

phlax commented Dec 23, 2024

note: envoy maintainers will be mostly on vacation until 6th jan

@birenroy
Copy link
Contributor Author

birenroy commented Jan 6, 2025

Happy new year!

@RyanTheOptimist
Copy link
Contributor

/assign @alyssawilk
since I'm still OOO

Copy link
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

Apologies for missing this yesterday. I'd be inclined to runtime guard but otherwise LGTM

Copy link

CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).

🐱

Caused by: #37784 was synchronize by birenroy.

see: more, trace.

@birenroy
Copy link
Contributor Author

birenroy commented Jan 8, 2025

Apologies for missing this yesterday. I'd be inclined to runtime guard but otherwise LGTM

I added runtime guard envoy.reloadable_features.http2_propagate_reset_events. Please take a look.

Signed-off-by: Biren Roy <[email protected]>
Copy link
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

LGTM. As ryan's back I'll give him until EoD if he wants to do a pass, else merge tomorrow.

Copy link
Contributor

@RyanTheOptimist RyanTheOptimist left a comment

Choose a reason for hiding this comment

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

LGTM!

@RyanTheOptimist RyanTheOptimist merged commit e294469 into envoyproxy:main Jan 9, 2025
25 checks passed
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.

4 participants