-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
quic: add listener config option to attach connection debug visitors #34036
Conversation
Add a way to configure a quic connection debug visitor factory that will be used to attach a debug visitor to all quic connections on the listener. Adds an interface for this new type of factory. Signed-off-by: Will Lampert <wlampert@google.com>
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
/assign @RyanTheOptimist |
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.
Looks great! How plausible would it be to add an integration test to quic_http_integration_test.cc
? That'd be a good way to confirm that the plumbing all works as expected.
source/common/quic/envoy_quic_connection_debug_visitor_factory_interface.h
Show resolved
Hide resolved
/wait |
…extension. Additionally, extend debug visitor constructor to take in a reference to the connection's StreamInfo. Signed-off-by: Will Lampert <wlampert@google.com>
Signed-off-by: Will Lampert <wlampert@google.com>
Signed-off-by: Will Lampert <wlampert@google.com>
Signed-off-by: Will Lampert <wlampert@google.com>
/retest |
Looks like an MSAN failure:
|
Signed-off-by: Will Lampert <wlampert@google.com>
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.
Nice! This looks great.
@wbpcode friendly ping for the API review. |
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.
/lgtm api
This looks like it's causing flakes - Will can you triage? |
Commit Message: Add a way to configure a quic connection debug visitor factory that will be used to attach a debug visitor to all quic connections on the listener. Adds an interface for this new type of factory.
Additional Description:
Risk Level: Low
Testing: Added new tests and modified existing tests in /test/common/quic. Also performed manual testing on a real machine and sent traffic to it using quic_client.
Docs Changes: Update envoy.config.listener.v3.quic_config.proto inline.
Release Notes: N/A
Platform Specific Features: N/A
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]