The problem/use-case that the feature addresses

When accepting a client fails, it is hard to know from the Redis log which client has a problem, especially in a system with many micro services.

Some examples:

15:S Error accepting a client connection: error:1417C086:SSL 
routines:tls_process_client_certificate:certificate verify failed
17:S Error accepting a client connection: Connection reset by peer
17:S Error accepting a client connection: Connection timed out
17:S Error accepting a client connection: (null)
17:S Error accepting a client connection: (null) (conn: fd=314)

Description of the feature

We'd like to log a few more things about the client and the reason for the error.

  • Client's source IP (most important)
  • Client name, if available (probably not?)
  • Connection state (CONN_STATE_*)
  • Connection type (TCP or TLS based on port, in case the client attempt to use TLS on the TCP port or vice versa)
  • Certificate expire date if the error is "certificate verify failed" or other details about what failed in certificate verification.

Comment From: itamarhaber

+1

Comment From: madolson

I would use client ID instead of client name, since it is globally unique and can't be "sensitive" in any way. FD is "alright" but can be re-used so sometimes a bit confusing in the logs.

Comment From: yossigo

+1 for improved logging, and for logging the client ID.

We should also add the local socket name, which can be helpful when binding multiple addresses. Regarding certificate validation, I think it's tricky because that can fail for a long list of reasons, and we generally don't control that and leave it for OpenSSL. So, the best we can do is provide the library's error.

Comment From: zuiderkwast

Thanks for the great response!

How about including addr=127.0.0.1:40378 laddr=127.0.0.1:6379 (like in CLIENT INFO) in these error log entries?

The local port helps identifying clients who try TLS over non-TLS port and vice versa.

The fd has just been created and has not been seen by anyone, so I don't think there is much value in including it. The client id same (it's just a unique integer, also not yet seen by anyone). And client name doesn't exist yet, since accept errors happen before the client can send any commands.

I'll open a PR.