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

Include server address in server error logs #2500

Merged
merged 1 commit into from
Nov 3, 2023
Merged

Conversation

olix0r
Copy link
Member

@olix0r olix0r commented Nov 3, 2023

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.

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.
@olix0r olix0r requested a review from a team as a code owner November 3, 2023 01:58
@olix0r olix0r merged commit 26b718c into main Nov 3, 2023
11 checks passed
@olix0r olix0r deleted the ver/server-error-log branch November 3, 2023 17:30
adleong pushed a commit that referenced this pull request Nov 16, 2023
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.
@adleong adleong mentioned this pull request Nov 16, 2023
adleong added a commit that referenced this pull request Nov 16, 2023
* Include server address in server error logs (#2500)
* inbound: Fix gRPC response classification (#2496)
* Bump ahash to v0.8.5 (#2498)
* Allow BSD-2-Clause

---------

Co-authored-by: Oliver Gould <[email protected]>
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Nov 17, 2023
* Include server address in server error logs (linkerd/linkerd2-proxy#2500)
* dev: v42 (linkerd/linkerd2-proxy#2501)
* Separate tls::ServerName and identity::Id types (linkerd/linkerd2-proxy#2506)
* Use reference-counted strings in dns::Name (linkerd/linkerd2-proxy#2509)
* build(deps): bump tj-actions/changed-files from 39.2.0 to 40.1.1 (linkerd/linkerd2-proxy#2508)
* build(deps): bump actions/checkout from 4.1.0 to 4.1.1 (linkerd/linkerd2-proxy#2485)
* ci: Fix check-each workflow (linkerd/linkerd2-proxy#2511)
* ci: Turn off debuginfo in ci test builds (linkerd/linkerd2-proxy#2512)
* ci: Fix fuzzer listing (linkerd/linkerd2-proxy#2513)
* Use heap indirection to manage type signatures (linkerd/linkerd2-proxy#2510)
* dev: optimize image build (linkerd/linkerd2-proxy#2452)
* dev: Disable nightly install for now (linkerd/linkerd2-proxy#2515)
* meshtls: Extract TLS id verification out of TLS backends (linkerd/linkerd2-proxy#2507)
* Update DEFAULT_OUTBOUND_TCP_QUEUE_CAPACITY to 10K (linkerd/linkerd2-proxy#2521)
* admin: Add optional /debug/pprof/profile endpoint (linkerd/linkerd2-proxy#2516)

Signed-off-by: Oliver Gould <[email protected]>
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Nov 17, 2023
* Include server address in server error logs (linkerd/linkerd2-proxy#2500)
* dev: v42 (linkerd/linkerd2-proxy#2501)
* Separate tls::ServerName and identity::Id types (linkerd/linkerd2-proxy#2506)
* Use reference-counted strings in dns::Name (linkerd/linkerd2-proxy#2509)
* build(deps): bump tj-actions/changed-files from 39.2.0 to 40.1.1 (linkerd/linkerd2-proxy#2508)
* build(deps): bump actions/checkout from 4.1.0 to 4.1.1 (linkerd/linkerd2-proxy#2485)
* ci: Fix check-each workflow (linkerd/linkerd2-proxy#2511)
* ci: Turn off debuginfo in ci test builds (linkerd/linkerd2-proxy#2512)
* ci: Fix fuzzer listing (linkerd/linkerd2-proxy#2513)
* Use heap indirection to manage type signatures (linkerd/linkerd2-proxy#2510)
* dev: optimize image build (linkerd/linkerd2-proxy#2452)
* dev: Disable nightly install for now (linkerd/linkerd2-proxy#2515)
* meshtls: Extract TLS id verification out of TLS backends (linkerd/linkerd2-proxy#2507)
* Update DEFAULT_OUTBOUND_TCP_QUEUE_CAPACITY to 10K (linkerd/linkerd2-proxy#2521)
* admin: Add optional /debug/pprof/profile endpoint (linkerd/linkerd2-proxy#2516)

* proxy: Use debian12 distroless base image

Signed-off-by: Oliver Gould <[email protected]>
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.

3 participants