Merge #127195

127195: schematelemetry: don't redact object ID or validation error in logs r=rafiss a=rafiss fixes: #126584 Release note: None Co-authored-by: Rafi Shamim <[email protected]>
cockroachdb · Jul 16, 2024 · 7bc3aa5 · 7bc3aa5
2 parents 34290f5 + e31692d
commit 7bc3aa5
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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)