Skip to content

Commit

Permalink
Merge pull request #124446 from cockroachdb/blathers/backport-release…
Browse files Browse the repository at this point in the history
…-24.1-123015

release-24.1: sql: log session variable settings that are set in the connection string
  • Loading branch information
rafiss authored May 21, 2024
2 parents df782c7 + e377d12 commit 48a2035
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 48a2035

Please sign in to comment.