Skip to content

Commit

Permalink
sql: log session variable settings that are set in the connection string
Browse files Browse the repository at this point in the history
This commit logs session variables passed on the connection url. The goal is to
provide more information on trouble shooting issues. The output could be
redacted for security purpose.

Fixes: #122934

Release note: None
  • Loading branch information
lyang24 committed Apr 24, 2024
1 parent 205565c commit e377d12
Show file tree
Hide file tree
Showing 7 changed files with 361 additions and 287 deletions.
28 changes: 28 additions & 0 deletions pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"net/url"
"reflect"
"regexp"
"sort"
"strings"
"time"

Expand Down Expand Up @@ -2200,6 +2201,33 @@ type queryMeta struct {
// configuration values in SET ... TO DEFAULT (or RESET ...) statements.
type SessionDefaults map[string]string

// SafeFormat implements the redact.SafeFormatter interface.
// An example output for SessionDefaults SafeFormat:
// [disallow_full_table_scans=‹true›; database=‹test›; statement_timeout=‹250ms›]
func (sd SessionDefaults) SafeFormat(s redact.SafePrinter, _ rune) {
s.Printf("[")
addSemiColon := false
// Iterate through map in alphabetical order.
sortedKeys := make([]string, 0, len(sd))
for k := range sd {
sortedKeys = append(sortedKeys, k)
}
sort.Strings(sortedKeys)
for _, k := range sortedKeys {
if addSemiColon {
s.Print(redact.SafeString("; "))
}
s.Printf("%s=%s", redact.SafeString(k), sd[k])
addSemiColon = true
}
s.Printf("]")
}

// String implements the fmt.Stringer interface.
func (sd SessionDefaults) String() string {
return redact.StringWithoutMarkers(sd)
}

// SessionArgs contains arguments for serving a client connection.
type SessionArgs struct {
User username.SQLUsername
Expand Down
14 changes: 14 additions & 0 deletions pkg/sql/exec_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/redact"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -98,3 +99,16 @@ func TestMaybeHashAppName(t *testing.T) {
})
}
}

// TestSessionDefaultsSafeFormat tests the redacted output of SessionDefaults.
func TestSessionDefaultsSafeFormat(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

session := SessionDefaults(make(map[string]string))
session["database"] = "test"
session["statement_timeout"] = "250ms"
session["disallow_full_table_scans"] = "true"
require.Contains(t, redact.Sprint(session), "database=‹test›")
require.Contains(t, redact.Sprint(session).Redact(), "statement_timeout=‹×›")
}
6 changes: 6 additions & 0 deletions pkg/sql/pgwire/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/ring"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/redact"
"github.com/lib/pq/oid"
)

Expand Down Expand Up @@ -230,6 +231,11 @@ func (c *conn) processCommands(
// Signal the connection was established to the authenticator.
ac.AuthOK(ctx)
ac.LogAuthOK(ctx)
ac.LogAuthInfof(ctx, redact.Sprintf(
"session created with SessionDefaults=%s and CustomOptions=%s",
c.sessionArgs.SessionDefaults,
c.sessionArgs.CustomOptionSessionDefaults,
))

// We count the connection establish latency until we are ready to
// serve a SQL query. It includes the time it takes to authenticate and
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/pgwire/pre_serve_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ func parseClientProvidedSessionParameters(
telemetry.Inc(sqltelemetry.CockroachShellCounter)
}
}

return args, nil
}

Expand Down
210 changes: 108 additions & 102 deletions pkg/sql/pgwire/testdata/auth/conn_log

Large diffs are not rendered by default.

118 changes: 62 additions & 56 deletions pkg/sql/pgwire/testdata/auth/identity_map

Large diffs are not rendered by default.

271 changes: 143 additions & 128 deletions pkg/sql/pgwire/testdata/auth/scram

Large diffs are not rendered by default.

0 comments on commit e377d12

Please sign in to comment.