Skip to content

Commit

Permalink
Merge pull request #138656 from rafiss/backport24.2-136897
Browse files Browse the repository at this point in the history
release-24.2: sql: do not redact names when formatting statement for logs
  • Loading branch information
rafiss authored Jan 10, 2025
2 parents 8f73351 + 0ab6a80 commit f22c1ba
Show file tree
Hide file tree
Showing 324 changed files with 621 additions and 800 deletions.
150 changes: 75 additions & 75 deletions docs/generated/eventlog.md

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions docs/generated/settings/settings-for-tenants.txt
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ sql.insights.anomaly_detection.memory_limit byte size 1.0 MiB the maximum amount
sql.insights.execution_insights_capacity integer 1000 the size of the per-node store of execution insights application
sql.insights.high_retry_count.threshold integer 10 the number of retries a slow statement must have undergone for its high retry count to be highlighted as a potential problem application
sql.insights.latency_threshold duration 100ms amount of time after which an executing statement is considered slow. Use 0 to disable. application
sql.log.redact_names.enabled boolean false if set, schema object identifers are redacted in SQL statements that appear in event logs application
sql.log.slow_query.experimental_full_table_scans.enabled boolean false when set to true, statements that perform a full table/index scan will be logged to the slow query log even if they do not meet the latency threshold. Must have the slow query log enabled for this setting to have any effect. application
sql.log.slow_query.internal_queries.enabled boolean false when set to true, internal queries which exceed the slow query log threshold are logged to a separate log. Must have the slow query log enabled for this setting to have any effect. application
sql.log.slow_query.latency_threshold duration 0s when set to non-zero, log statements whose service latency exceeds the threshold to a secondary logger on each node application
Expand Down
1 change: 1 addition & 0 deletions docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@
<tr><td><div id="setting-sql-insights-execution-insights-capacity" class="anchored"><code>sql.insights.execution_insights_capacity</code></div></td><td>integer</td><td><code>1000</code></td><td>the size of the per-node store of execution insights</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-insights-high-retry-count-threshold" class="anchored"><code>sql.insights.high_retry_count.threshold</code></div></td><td>integer</td><td><code>10</code></td><td>the number of retries a slow statement must have undergone for its high retry count to be highlighted as a potential problem</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-insights-latency-threshold" class="anchored"><code>sql.insights.latency_threshold</code></div></td><td>duration</td><td><code>100ms</code></td><td>amount of time after which an executing statement is considered slow. Use 0 to disable.</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-log-redact-names-enabled" class="anchored"><code>sql.log.redact_names.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>if set, schema object identifers are redacted in SQL statements that appear in event logs</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-log-slow-query-experimental-full-table-scans-enabled" class="anchored"><code>sql.log.slow_query.experimental_full_table_scans.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>when set to true, statements that perform a full table/index scan will be logged to the slow query log even if they do not meet the latency threshold. Must have the slow query log enabled for this setting to have any effect.</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-log-slow-query-internal-queries-enabled" class="anchored"><code>sql.log.slow_query.internal_queries.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>when set to true, internal queries which exceed the slow query log threshold are logged to a separate log. Must have the slow query log enabled for this setting to have any effect.</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-log-slow-query-latency-threshold" class="anchored"><code>sql.log.slow_query.latency_threshold</code></div></td><td>duration</td><td><code>0s</code></td><td>when set to non-zero, log statements whose service latency exceeds the threshold to a secondary logger on each node</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
Expand Down
6 changes: 2 additions & 4 deletions pkg/ccl/auditloggingccl/audit_logging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,22 +355,20 @@ func TestReducedAuditConfig(t *testing.T) {

// Run a query on the new connection. The new connection will cause the reduced audit config to be re-computed.
// The user now has a corresponding audit setting. We use a new query here to differentiate.
testRunner2.Exec(t, `GRANT SELECT ON TABLE u TO root`)
testRunner2.Exec(t, `ALTER TABLE u ADD COLUMN y STRING DEFAULT 'foo'`)

log.FlushFiles()

entries, err = log.FetchEntriesFromFiles(
0,
math.MaxInt64,
10000,
regexp.MustCompile(`GRANT SELECT ON TABLE ‹u› TO root`),
regexp.MustCompile(`ALTER TABLE u ADD COLUMN y STRING DEFAULT ‹'foo'›`),
log.WithMarkedSensitiveData,
)

if err != nil {
t.Fatal(err)
}

if len(entries) != 1 {
t.Fatal(errors.Newf("unexpected number of entries; expected: %d found: %d", 1, len(entries)))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ CREATE INDEX idx
PARTITION BY LIST (id) (PARTITION p1 VALUES IN (1));
EXPLAIN (DDL) rollback at post-commit stage 1 of 7;
----
Schema change plan for rolling back CREATE INDEX idx ON defaultdb.public.‹t› (‹id›, ‹name) STORING (money) PARTITION BY LIST (‹id›) (PARTITION ‹p1› VALUES IN (‹1›));
Schema change plan for rolling back CREATE INDEX idx ON defaultdb.public.t (id, name) STORING (money) PARTITION BY LIST (id) (PARTITION p1 VALUES IN (‹1›));
└── PostCommitNonRevertiblePhase
└── Stage 1 of 1 in PostCommitNonRevertiblePhase
├── 12 elements transitioning toward ABSENT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ CREATE INDEX idx
PARTITION BY LIST (id) (PARTITION p1 VALUES IN (1));
EXPLAIN (DDL) rollback at post-commit stage 2 of 7;
----
Schema change plan for rolling back CREATE INDEX idx ON defaultdb.public.‹t› (‹id›, ‹name) STORING (money) PARTITION BY LIST (‹id›) (PARTITION ‹p1› VALUES IN (‹1›));
Schema change plan for rolling back CREATE INDEX idx ON defaultdb.public.t (id, name) STORING (money) PARTITION BY LIST (id) (PARTITION p1 VALUES IN (‹1›));
└── PostCommitNonRevertiblePhase
├── Stage 1 of 2 in PostCommitNonRevertiblePhase
│ ├── 11 elements transitioning toward ABSENT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ CREATE INDEX idx
PARTITION BY LIST (id) (PARTITION p1 VALUES IN (1));
EXPLAIN (DDL) rollback at post-commit stage 3 of 7;
----
Schema change plan for rolling back CREATE INDEX idx ON defaultdb.public.‹t› (‹id›, ‹name) STORING (money) PARTITION BY LIST (‹id›) (PARTITION ‹p1› VALUES IN (‹1›));
Schema change plan for rolling back CREATE INDEX idx ON defaultdb.public.t (id, name) STORING (money) PARTITION BY LIST (id) (PARTITION p1 VALUES IN (‹1›));
└── PostCommitNonRevertiblePhase
├── Stage 1 of 2 in PostCommitNonRevertiblePhase
│ ├── 11 elements transitioning toward ABSENT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ CREATE INDEX idx
PARTITION BY LIST (id) (PARTITION p1 VALUES IN (1));
EXPLAIN (DDL) rollback at post-commit stage 4 of 7;
----
Schema change plan for rolling back CREATE INDEX idx ON defaultdb.public.‹t› (‹id›, ‹name) STORING (money) PARTITION BY LIST (‹id›) (PARTITION ‹p1› VALUES IN (‹1›));
Schema change plan for rolling back CREATE INDEX idx ON defaultdb.public.t (id, name) STORING (money) PARTITION BY LIST (id) (PARTITION p1 VALUES IN (‹1›));
└── PostCommitNonRevertiblePhase
├── Stage 1 of 2 in PostCommitNonRevertiblePhase
│ ├── 11 elements transitioning toward ABSENT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ CREATE INDEX idx
PARTITION BY LIST (id) (PARTITION p1 VALUES IN (1));
EXPLAIN (DDL) rollback at post-commit stage 5 of 7;
----
Schema change plan for rolling back CREATE INDEX idx ON defaultdb.public.‹t› (‹id›, ‹name) STORING (money) PARTITION BY LIST (‹id›) (PARTITION ‹p1› VALUES IN (‹1›));
Schema change plan for rolling back CREATE INDEX idx ON defaultdb.public.t (id, name) STORING (money) PARTITION BY LIST (id) (PARTITION p1 VALUES IN (‹1›));
└── PostCommitNonRevertiblePhase
├── Stage 1 of 2 in PostCommitNonRevertiblePhase
│ ├── 11 elements transitioning toward ABSENT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ CREATE INDEX idx
PARTITION BY LIST (id) (PARTITION p1 VALUES IN (1));
EXPLAIN (DDL) rollback at post-commit stage 6 of 7;
----
Schema change plan for rolling back CREATE INDEX idx ON defaultdb.public.‹t› (‹id›, ‹name) STORING (money) PARTITION BY LIST (‹id›) (PARTITION ‹p1› VALUES IN (‹1›));
Schema change plan for rolling back CREATE INDEX idx ON defaultdb.public.t (id, name) STORING (money) PARTITION BY LIST (id) (PARTITION p1 VALUES IN (‹1›));
└── PostCommitNonRevertiblePhase
├── Stage 1 of 2 in PostCommitNonRevertiblePhase
│ ├── 11 elements transitioning toward ABSENT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ CREATE INDEX idx
PARTITION BY LIST (id) (PARTITION p1 VALUES IN (1));
EXPLAIN (DDL) rollback at post-commit stage 7 of 7;
----
Schema change plan for rolling back CREATE INDEX idx ON defaultdb.public.‹t› (‹id›, ‹name) STORING (money) PARTITION BY LIST (‹id›) (PARTITION ‹p1› VALUES IN (‹1›));
Schema change plan for rolling back CREATE INDEX idx ON defaultdb.public.t (id, name) STORING (money) PARTITION BY LIST (id) (PARTITION p1 VALUES IN (‹1›));
└── PostCommitNonRevertiblePhase
├── Stage 1 of 2 in PostCommitNonRevertiblePhase
│ ├── 11 elements transitioning toward ABSENT
Expand Down
12 changes: 6 additions & 6 deletions pkg/sql/admin_audit_log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func TestAdminAuditLogTransaction(t *testing.T) {
db.Exec(t, `SET CLUSTER SETTING sql.log.admin_audit.enabled = true;`)
db.Exec(t, `
BEGIN;
CREATE TABLE t();
CREATE TABLE t(a INT8 PRIMARY KEY DEFAULT 2);
SELECT * FROM t;
SELECT 1;
COMMIT;
Expand All @@ -155,11 +155,11 @@ COMMIT;
},
{
"select-*-from-table-query",
`"EventType":"admin_query","Statement":"SELECT * FROM \"\"›.‹\"\"›.‹t›"`,
`"EventType":"admin_query","Statement":"SELECT * FROM \"\".\"\".t"`,
},
{
"create-table-query",
`"EventType":"admin_query","Statement":"CREATE TABLE defaultdb.public.‹t› ()"`,
`"EventType":"admin_query","Statement":"CREATE TABLE defaultdb.public.t (a INT8 PRIMARY KEY DEFAULT ‹2›)"`,
},
}

Expand Down Expand Up @@ -222,7 +222,7 @@ func TestAdminAuditLogMultipleTransactions(t *testing.T) {

if _, err := testuser.Exec(`
BEGIN;
CREATE TABLE t();
CREATE TABLE t(a INT8 PRIMARY KEY DEFAULT 2);
SELECT * FROM t;
SELECT 1;
COMMIT;
Expand All @@ -240,11 +240,11 @@ COMMIT;
},
{
"select-*-from-table-query",
`"EventType":"admin_query","Statement":"SELECT * FROM \"\"›.‹\"\"›.‹t›"`,
`"EventType":"admin_query","Statement":"SELECT * FROM \"\".\"\".t"`,
},
{
"create-table-query",
`"EventType":"admin_query","Statement":"CREATE TABLE defaultdb.public.‹t› ()"`,
`"EventType":"admin_query","Statement":"CREATE TABLE defaultdb.public.t (a INT8 PRIMARY KEY DEFAULT ‹2›)"`,
},
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/catalog/internal/validate/schema_changer_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func validateSchemaChangerState(d catalog.Descriptor, vea catalog.ValidationErro
statementRanks.Add(int(s.StatementRank))
if _, ok := statementsExpected[s.StatementRank]; !ok {
report(errors.Errorf("unexpected statement %d (%s)",
s.StatementRank, s.Statement.Statement))
s.StatementRank, s.Statement.RedactedStatement))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ func TestValidateSchemaChangerState(t *testing.T) {
RelevantStatements: []scpb.DescriptorState_Statement{
{
Statement: scpb.Statement{
Statement: "ALTER TABLE a RENAME TO b",
Statement: "ALTER TABLE a RENAME TO b",
RedactedStatement: "ALTER TABLE a RENAME TO b",
},
StatementRank: 0,
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/catalog/redact/redact.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func redactTableDescriptor(d *descpb.TableDescriptor) (errs []error) {
if scs := d.DeclarativeSchemaChangerState; scs != nil {
for i := range scs.RelevantStatements {
stmt := &scs.RelevantStatements[i]
stmt.Statement.Statement = stmt.Statement.RedactedStatement
stmt.Statement.Statement = stmt.Statement.RedactedStatement.StripMarkers()
}
for i := range scs.Targets {
t := &scs.Targets[i]
Expand Down Expand Up @@ -219,7 +219,7 @@ func redactFunctionDescriptor(desc *descpb.FunctionDescriptor) (errs []error) {
if scs := desc.DeclarativeSchemaChangerState; scs != nil {
for i := range scs.RelevantStatements {
stmt := &scs.RelevantStatements[i]
stmt.Statement.Statement = stmt.Statement.RedactedStatement
stmt.Statement.Statement = stmt.Statement.RedactedStatement.StripMarkers()
}
for i := range scs.Targets {
t := &scs.Targets[i]
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/conn_executor_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -2211,7 +2211,7 @@ func (ex *connExecutor) handleTxnRowsGuardrails(
*alreadyLogged = shouldLog
}
if shouldLog {
commonSQLEventDetails := ex.planner.getCommonSQLEventDetails(defaultRedactionOptions)
commonSQLEventDetails := ex.planner.getCommonSQLEventDetails()
var event logpb.EventPayload
if ex.executorType == executorTypeInternal {
if isRead {
Expand Down
30 changes: 3 additions & 27 deletions pkg/sql/event_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/isql"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scbuild"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scrun"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
Expand Down Expand Up @@ -156,33 +155,10 @@ type eventLogOptions struct {
// If verboseTraceLevel is non-zero, its value is used as value for
// the vmodule filter. See exec_log for an example use.
verboseTraceLevel log.Level

// Additional redaction options, if necessary.
rOpts redactionOptions
}

// redactionOptions contains instructions on how to redact the SQL
// events.
type redactionOptions struct {
omitSQLNameRedaction bool
}

func (ro *redactionOptions) toFlags() tree.FmtFlags {
if ro.omitSQLNameRedaction {
return tree.FmtOmitNameRedaction
}
return tree.FmtSimple
}

var defaultRedactionOptions = redactionOptions{
omitSQLNameRedaction: false,
}

func (p *planner) getCommonSQLEventDetails(opt redactionOptions) eventpb.CommonSQLEventDetails {
redactableStmt := formatStmtKeyAsRedactableString(
p.extendedEvalCtx.VirtualSchemas, p.stmt.AST,
p.extendedEvalCtx.Context.Annotations, opt.toFlags(), p,
)
func (p *planner) getCommonSQLEventDetails() eventpb.CommonSQLEventDetails {
redactableStmt := p.FormatAstAsRedactableString(p.stmt.AST, p.extendedEvalCtx.Context.Annotations)
commonSQLEventDetails := eventpb.CommonSQLEventDetails{
Statement: redactableStmt,
Tag: p.stmt.AST.StatementTag(),
Expand Down Expand Up @@ -210,7 +186,7 @@ func (p *planner) logEventsWithOptions(
p.extendedEvalCtx.ExecCfg, p.InternalSQLTxn(),
1+depth,
opts,
p.getCommonSQLEventDetails(opts.rOpts),
p.getCommonSQLEventDetails(),
entries...)
}

Expand Down
Loading

0 comments on commit f22c1ba

Please sign in to comment.