ccl/sqlproxyccl: add context and tag with remote addr/db #58009
Conversation
|
This change is |
When logging from the proxy and the related code, it is hard to figure out the connection to which the logging applies. This PR now passes a context to the frontend admitter and backend dialer so it can be used to decorate the log output. The proxy itself adds a remote addr tag and database tag once the startup message is available. Using the log functions will show a prefix like this one (after db tag): `[remote-addr=‹127.0.0.1:60820›,db=‹defaultdb_29›]` This also changes the signature of the BackendDialer to pass the incmoing connection. This isn't currently used but can be useful in the cases where the backend setup or reporting depends on the incoming connection. Release note: none
darinpp
force-pushed
the
sqlproxyccl-context
branch
from
6a2ae0c
to
6b59ea5
Compare
There was a problem hiding this comment.
I think it'd be preferable to do logging in the sqlproxy wrapper using the managed-service logging package. @chrisseto recently put out a PR to add distributed tracing to the logs: https://reviewable.io/reviews/cockroachlabs/managed-service/4485#-.
This is the future of logging in the managed service. We don't use the CRDB log packages there, so I think we should avoid them here as well, since this should be a reusable library pulling in a minimum of dependencies. Instead, we should add logging/tracing in the FrontEndAdmitter and BackendDialer callbacks. Let's chat more about it, since there's a lot at work here that you might not yet know about.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @andy-kimball, @darinpp, @imjching, and @spaskob)
pkg/ccl/cliccl/mtproxy.go, line 160 at r1 (raw file):
}, BackendDialer: func( ctx context.Context, incoming net.Conn, msg *pgproto3.StartupMessage,
I don't think we should add incoming until we actually need it. The principle: keep it simple to start, don't add API complexity until we need it.
When logging from the proxy and the related code, it is
hard to figure out the connection to which the logging applies.
This PR now passes a context to the frontend admitter and
backend dialer so it can be used to decorate the log output.
The proxy itself adds a remote addr tag and database tag
once the startup message is available. Using the log
functions will show a prefix like this one (after db tag):
[remote-addr=‹127.0.0.1:60820›,db=‹defaultdb_29›]This also changes the signature of the BackendDialer to
pass the incoming connection. This isn't currently used
but can be useful in the cases where the backend setup
or reporting depends on the incoming connection.
Release note: none