-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[network]Add ip address family info to error details upon socket connect failure #34286
Conversation
Signed-off-by: Renjie Tang <renjietang@google.com>
@@ -1037,7 +1037,11 @@ void ClientConnectionImpl::connect() { | |||
} else { | |||
immediate_error_event_ = ConnectionEvent::RemoteClose; | |||
connecting_ = false; | |||
setFailureReason(absl::StrCat("immediate connect error: ", errorDetails(result.errno_))); | |||
setFailureReason(absl::StrCat( | |||
"immediate connect error: ", errorDetails(result.errno_), "remote address family:", |
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.
Probably want a space or some other delimiter at the start of "remote address family:"
or it and the remote error will run together.
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.
Can you give an example where this clarifies an error?
Recently we've seen a lot of socket errors of NO_ROUTE_TO_HOST emitted here. It took us quite some time to hypothesize that the socket is trying ipv6 connection when the network doesn't support it. With the addr family logged here, it'd be very clear that this is a ipv4/v6 connectivity issue. |
I recommend switching this to log the result of |
Signed-off-by: Renjie Tang <renjietang@google.com>
Thanks for catching that. Done using string instead. |
Approved by @zuercher, merging. |
Commit Message: Add ip address family info to error details upon socket connect failure
Additional Description: This helps reveal more info on why the socket connection fails immediately
Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a