Merged
Conversation
Member
adleong
commented
Nov 16, 2023
adleong
commented
- inbound: Fix gRPC response classification (inbound: Fix gRPC response classification #2496)
- Include server address in server error logs (Include server address in server error logs #2500)
When the proxy's TCP server encounters an error (usually due to one of the connections failing, we log the error and the client's address. The server's address was omitted because it varies based on context that is not known in this module: in some cases it's the actual server address on the socket, but when proxying a connection it may be determined by the value retrieved from the SO_ORIGINAL_DST socket option. To fix this, the server now requires that connection metadata be able to materialize an 'AddrPair' parameter that describes a client-server connection. The TCP listener impls are updated to satisfy this based on the appropriate metadata; and the TCP server consumes this type to include both client and server addresses in the relevant logs/contexts.
ecaaf39 changed the proxy's behavior with regard to creating [default response classifiers][default]: the defaults used to support detecting gRPC response (regardless of the request properties). To fix this, we modify the metrics module that uses responses classifiers to *require* them without inferring defaults. This enforces the intended usage pattern so that we do not silently and implicitly fall back to the default behavior. This change also updates the `NewClassify` module that inserts the response classifier request extension so that overrides are supported. We then can install a default classifier early in request processing and override it only if specified by a route configuration. To support this change, the http-metrics crate is updated to support querying response_total metrics without stringifying everything. [default]: ecaaf39#diff-372e8a8a57b1fad5d94f37d2f77fdc7a45bcf708782475424b75d671f99ea1a0L97-L103
* Bump ahash to v0.8.5 * Allow BSD-2-Clause
olix0r
approved these changes
Nov 16, 2023
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.