Merge pull request #127221 from cockroachdb/blathers/backport-release…

…-24.1-127195

release-24.1: schematelemetry: don't redact object ID or validation error in logs

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",
     ],
 )
 

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
 	})

pkg/sql/catalog/schematelemetry/schema_telemetry_test.go

@@ -186,8 +186,15 @@ 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)
+
+	// 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)
 }