-
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: deferred logging of roundtrip response time #23648
Conversation
Make the following into shared pointers so they can be used in deferred logging that can be executed after a stream is destroyed: - request header map - response header map - response trailer map Signed-off-by: Paul Sohn <paulsohn@google.com>
These are the stream-level items currently passed to access loggers (other than the stream info itself). For deferred logging (i.e. after the stream itself might be destroyed), we need shared pointers to these items. Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
We need this because when we defer logging, the old streaminfo owned by the filter manager may be destroyed by the time we want to log using it. Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
To use RequestDecoder in a later commit. Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
Avoids re-logging on a zero-byte ack. Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
77458ac
to
d6ee6ec
Compare
Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
When waiting for access logs, flush pending client acks by running dispatcher every 25ms (default ack timer). Signed-off-by: Paul Sohn <paulsohn@google.com>
/retest |
Retrying Azure Pipelines: |
Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
/retest |
Retrying Azure Pipelines: |
Signed-off-by: Paul Sohn <paulsohn@google.com>
/retest |
Retrying Azure Pipelines: |
/retest |
Retrying Azure Pipelines: |
Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
/retest |
Retrying Azure Pipelines: |
Signed-off-by: Paul Sohn <paulsohn@google.com>
Signed-off-by: Paul Sohn <paulsohn@google.com>
/retest |
Retrying Azure Pipelines: |
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.
Woohoo!
/** | ||
* @return List of shared pointers to access loggers for this stream. | ||
*/ | ||
virtual std::list<AccessLog::InstanceSharedPtr> accessLogHandlers() PURE; |
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.
A drive-by comment:
Should this return a reference to the list?
Additionally, should this be const?
Looking at the uses of this method below I think the answer to the above 2 questions may be yes, and may avoid unnecessary copies.
Commit Message: For QUIC, defer access logging to when the final ack is received from downstream. Additional Description: This PR implements QuicAckListenerInterface which allows QUIC streams to listen for acks. Here, we use the ack listener to record a "roundtrip response time" that is analogous to the full response time or time-to-last-byte as experienced by a downstream client. We also defer access logging to the ack listener in order to record this metric in the stream info and make it available to access logs. The stream info is copied into the ack listener so that it can be used for logging even after its originating stream is destroyed. Risk Level: Medium Testing: Integration tests. Docs Changes: N/A Release Notes: added Platform Specific Features: N/A Runtime guard: envoy_reloadable_features_quic_defer_logging_to_ack_listener defaults to true. Signed-off-by: Paul Sohn <paulsohn@google.com>
Commit Message: [QUIC only] Log retransmitted packets and bytes. Additional Description: Adds to the deferred logging implementation from #23648 and implements the existing OnPacketRetransmitted function. Risk Level: Low Testing: Integration test Docs Changes: N/A Release Notes: added Signed-off-by: Paul Sohn <paulsohn@google.com>
Commit Message: [QUIC only] Log retransmitted packets and bytes. Additional Description: Adds to the deferred logging implementation from envoyproxy#23648 and implements the existing OnPacketRetransmitted function. Risk Level: Low Testing: Integration test Docs Changes: N/A Release Notes: added Signed-off-by: Paul Sohn <paulsohn@google.com> Signed-off-by: asheryer <asheryer@amazon.com>
Commit Message: [QUIC only] Log retransmitted packets and bytes. Additional Description: Adds to the deferred logging implementation from envoyproxy#23648 and implements the existing OnPacketRetransmitted function. Risk Level: Low Testing: Integration test Docs Changes: N/A Release Notes: added Signed-off-by: Paul Sohn <paulsohn@google.com> Signed-off-by: Ryan Eskin <ryan.eskin89@protonmail.com>
Commit Message: For QUIC, defer access logging to when the final ack is received from downstream.
Additional Description:
This PR implements QuicAckListenerInterface which allows QUIC streams to listen for acks. Here, we use the ack listener to record a "roundtrip response time" that is analogous to the full response time or time-to-last-byte as experienced by a downstream client. We also defer access logging to the ack listener in order to record this metric in the stream info and make it available to access logs. The stream info is copied into the ack listener so that it can be used for logging even after its originating stream is destroyed.
Risk Level: Medium
Testing: Integration tests.
Docs Changes: N/A
Release Notes: added
Platform Specific Features: N/A
Runtime guard:
envoy_reloadable_features_quic_defer_logging_to_ack_listener
defaults to true.