From 4286f1a45deef3601a0152ca1f2dc5abf42173d4 Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Mon, 15 Jul 2024 20:31:41 +0000 Subject: [PATCH 1/2] schematelemetry: don't redact object ID or validation error in logs Release note: None --- pkg/sql/catalog/schematelemetry/BUILD.bazel | 1 + .../schematelemetry/schema_telemetry_job.go | 16 ++++++++++++++-- .../schematelemetry/schema_telemetry_test.go | 2 +- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/pkg/sql/catalog/schematelemetry/BUILD.bazel b/pkg/sql/catalog/schematelemetry/BUILD.bazel index c40b5af72002..b72d4ccbdad2 100644 --- a/pkg/sql/catalog/schematelemetry/BUILD.bazel +++ b/pkg/sql/catalog/schematelemetry/BUILD.bazel @@ -36,6 +36,7 @@ go_library( "//pkg/util/metric", "//pkg/util/uuid", "@com_github_cockroachdb_errors//:errors", + "@com_github_cockroachdb_redact//:redact", ], ) diff --git a/pkg/sql/catalog/schematelemetry/schema_telemetry_job.go b/pkg/sql/catalog/schematelemetry/schema_telemetry_job.go index d715cfe9c58a..9fbec4c296df 100644 --- a/pkg/sql/catalog/schematelemetry/schema_telemetry_job.go +++ b/pkg/sql/catalog/schematelemetry/schema_telemetry_job.go @@ -30,6 +30,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/metric" "github.com/cockroachdb/cockroach/pkg/util/uuid" "github.com/cockroachdb/errors" + "github.com/cockroachdb/redact" ) type Metrics struct { @@ -116,7 +117,11 @@ func processInvalidObjects( return err } - rows, err := txn.QueryIteratorEx(ctx, "sql-telemetry-invalid-objects", txn.KV(), sessiondata.NodeUserSessionDataOverride, `SELECT id, error FROM "".crdb_internal.invalid_objects LIMIT $1`, maxRecords) + rows, err := txn.QueryIteratorEx( + ctx, "sql-telemetry-invalid-objects", txn.KV(), sessiondata.NodeUserSessionDataOverride, + `SELECT id, error_redactable FROM "".crdb_internal.invalid_objects LIMIT $1`, + maxRecords, + ) if err != nil { return err } @@ -148,10 +153,17 @@ func processInvalidObjects( return errors.AssertionFailedf("expected err to be string (was %T)", row[1]) } - log.Warningf(ctx, "found invalid object with ID %d: %q", descID, validationErr) + // IDs are always non-sensitive, and the validationErr is written to the + // table with redact.Sprint, so it's a RedactableString. + log.Warningf(ctx, "found invalid object with ID %d: %s", + redact.SafeInt(*descID), redact.RedactableString(*validationErr), + ) } metrics.InvalidObjects.Update(count) + if count == 0 { + log.Infof(ctx, "schema telemetry job found no invalid objects") + } return nil }) diff --git a/pkg/sql/catalog/schematelemetry/schema_telemetry_test.go b/pkg/sql/catalog/schematelemetry/schema_telemetry_test.go index e5a4b06407fa..9510b4f760ab 100644 --- a/pkg/sql/catalog/schematelemetry/schema_telemetry_test.go +++ b/pkg/sql/catalog/schematelemetry/schema_telemetry_test.go @@ -186,7 +186,7 @@ UPDATE system.namespace SET id = %d WHERE id = %d; // Ensure that a log line is emitted for each invalid object, with a loose // enforcement of the log structure. - errorRE := regexp.MustCompile(`found invalid object with ID \d+: ".+"`) + errorRE := regexp.MustCompile(`found invalid object with ID \d+: .+`) entries, err := log.FetchEntriesFromFiles(0, math.MaxInt64, 1000, errorRE, log.SelectEditMode(false, false)) require.NoError(t, err) require.Len(t, entries, 9) From 6ade33ed1971f36bdff1c9ae8d0ecb04dffb1b83 Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Wed, 17 Jul 2024 15:05:05 -0400 Subject: [PATCH 2/2] schematelemetry: test that logs are redacted correctly Release note: None --- pkg/sql/catalog/schematelemetry/schema_telemetry_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/sql/catalog/schematelemetry/schema_telemetry_test.go b/pkg/sql/catalog/schematelemetry/schema_telemetry_test.go index 9510b4f760ab..99cb1a4d8d91 100644 --- a/pkg/sql/catalog/schematelemetry/schema_telemetry_test.go +++ b/pkg/sql/catalog/schematelemetry/schema_telemetry_test.go @@ -190,4 +190,11 @@ UPDATE system.namespace SET id = %d WHERE id = %d; entries, err := log.FetchEntriesFromFiles(0, math.MaxInt64, 1000, errorRE, log.SelectEditMode(false, false)) require.NoError(t, err) require.Len(t, entries, 9) + + // Verify that the log entries have redaction markers applied by checking one + // of the specific error messages. + errorRE = regexp.MustCompile(`found invalid object with ID \d+: relation ‹"nojob"›`) + entries, err = log.FetchEntriesFromFiles(0, math.MaxInt64, 1000, errorRE, log.SelectEditMode(false, true /* keepRedactable */)) + require.NoError(t, err) + require.Len(t, entries, 1) }