util/log: remove Safe in favor of redact.Safe

My desire to make this change is to break the dependency of `treewindow` on `util/log` (which is a part of the effort to clean up the dependencies of `execgen`). Release note: None Release justification: low risk change to clean up the dependencies a bit.
cockroachdb · Mar 3, 2022 · 8a77fe5 · 8a77fe5
1 parent 1e1d559
commit 8a77fe5
Show file tree

Hide file tree

Showing 99 changed files with 258 additions and 231 deletions.
diff --git a/pkg/ccl/changefeedccl/BUILD.bazel b/pkg/ccl/changefeedccl/BUILD.bazel
@@ -108,6 +108,7 @@ go_library(
         "@com_github_cockroachdb_apd_v3//:apd",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_cockroachdb_logtags//:logtags",
+        "@com_github_cockroachdb_redact//:redact",
         "@com_github_google_btree//:btree",
         "@com_github_linkedin_goavro_v2//:goavro",
         "@com_github_shopify_sarama//:sarama",

diff --git a/pkg/ccl/changefeedccl/changefeed_processors.go b/pkg/ccl/changefeedccl/changefeed_processors.go
@@ -47,6 +47,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/redact"
 )
 
 type changeAggregator struct {
@@ -1349,7 +1350,7 @@ func (cf *changeFrontier) noteAggregatorProgress(d rowenc.EncDatum) error {
 		if !resolved.Timestamp.IsEmpty() && resolved.Timestamp.Less(cf.highWaterAtStart) {
 			logcrash.ReportOrPanic(cf.Ctx, &cf.flowCtx.Cfg.Settings.SV,
 				`got a span level timestamp %s for %s that is less than the initial high-water %s`,
-				log.Safe(resolved.Timestamp), resolved.Span, log.Safe(cf.highWaterAtStart))
+				redact.Safe(resolved.Timestamp), resolved.Span, redact.Safe(cf.highWaterAtStart))
 			continue
 		}
 		if err := cf.forwardFrontier(resolved); err != nil {

diff --git a/pkg/ccl/kvccl/kvtenantccl/BUILD.bazel b/pkg/ccl/kvccl/kvtenantccl/BUILD.bazel
@@ -84,6 +84,7 @@ go_test(
         "//pkg/util/stop",
         "//pkg/util/tracing",
         "//pkg/util/uuid",
+        "@com_github_cockroachdb_redact//:redact",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
         "@org_golang_google_grpc//codes",

diff --git a/pkg/ccl/kvccl/kvtenantccl/tenant_trace_test.go b/pkg/ccl/kvccl/kvtenantccl/tenant_trace_test.go
@@ -26,6 +26,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
+	"github.com/cockroachdb/redact"
 	"github.com/stretchr/testify/require"
 )
 
@@ -57,7 +58,7 @@ func testTenantTracesAreRedactedImpl(t *testing.T, redactable bool) {
 				EvalKnobs: kvserverbase.BatchEvalTestingKnobs{
 					TestingEvalFilter: func(args kvserverbase.FilterArgs) *roachpb.Error {
 						log.Eventf(args.Ctx, "%v", sensitiveString)
-						log.Eventf(args.Ctx, "%v", log.Safe(visibleString))
+						log.Eventf(args.Ctx, "%v", redact.Safe(visibleString))
 						return nil
 					},
 				},

diff --git a/pkg/cli/mt_proxy.go b/pkg/cli/mt_proxy.go
@@ -23,6 +23,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/log/severity"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/redact"
 	"github.com/spf13/cobra"
 )
 
@@ -147,7 +148,7 @@ func waitForSignals(
 			}
 
 			log.Ops.Shoutf(ctx, severity.ERROR,
-				"received signal '%s' during shutdown, initiating hard shutdown", log.Safe(sig))
+				"received signal '%s' during shutdown, initiating hard shutdown", redact.Safe(sig))
 			panic("terminate")
 		case <-stopper.IsStopped():
 			const msgDone = "server shutdown completed"

diff --git a/pkg/cli/node.go b/pkg/cli/node.go
@@ -29,6 +29,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/retry"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/redact"
 	"github.com/spf13/cobra"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -398,7 +399,7 @@ func handleNodeDecommissionSelf(
 			cliflags.NodeDecommissionSelf.Name)
 	}
 
-	log.Infof(ctx, "%s node %d", log.Safe(command), localNodeID)
+	log.Infof(ctx, "%s node %d", redact.Safe(command), localNodeID)
 	return []roachpb.NodeID{localNodeID}, nil
 }
 

diff --git a/pkg/cli/start.go b/pkg/cli/start.go
@@ -888,7 +888,7 @@ func waitForShutdown(
 			// shutdown process.
 			log.Ops.Shoutf(shutdownCtx, severity.ERROR,
 				"received signal '%s' during shutdown, initiating hard shutdown%s",
-				log.Safe(sig), log.Safe(hardShutdownHint))
+				redact.Safe(sig), redact.Safe(hardShutdownHint))
 			handleSignalDuringShutdown(sig)
 			panic("unreachable")
 
@@ -1187,7 +1187,7 @@ func setupAndInitializeLoggingAndProfiling(
 				"- %s\n"+
 				"- %s",
 			build.MakeIssueURL(53404),
-			log.Safe(docs.URL("secure-a-cluster.html")),
+			redact.Safe(docs.URL("secure-a-cluster.html")),
 		)
 	}
 
@@ -1201,7 +1201,7 @@ func setupAndInitializeLoggingAndProfiling(
 				"For more information, see:\n\n" +
 				"- %s"
 			log.Shoutf(ctx, severity.WARNING, warningString,
-				log.Safe(docs.URL("cockroach-start.html#locality")))
+				redact.Safe(docs.URL("cockroach-start.html#locality")))
 		}
 	}
 

diff --git a/pkg/gossip/infostore.go b/pkg/gossip/infostore.go
@@ -28,6 +28,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/redact"
 )
 
 type stringMatcher interface {
@@ -253,7 +254,7 @@ func (is *infoStore) addInfo(key string, i *Info) error {
 		if highWaterStamp, ok := is.highWaterStamps[i.NodeID]; ok && highWaterStamp >= i.OrigStamp {
 			// Report both timestamps in the crash.
 			log.Fatalf(context.Background(),
-				"high water stamp %d >= %d", log.Safe(highWaterStamp), log.Safe(i.OrigStamp))
+				"high water stamp %d >= %d", redact.Safe(highWaterStamp), redact.Safe(i.OrigStamp))
 		}
 	}
 	// Update info map.

diff --git a/pkg/kv/kvclient/kvcoord/dist_sender.go b/pkg/kv/kvclient/kvcoord/dist_sender.go
@@ -609,7 +609,7 @@ func (ds *DistSender) getRoutingInfo(
 		}
 		if !containsFn(returnToken.Desc(), descKey) {
 			log.Fatalf(ctx, "programming error: range resolution returning non-matching descriptor: "+
-				"desc: %s, key: %s, reverse: %t", returnToken.Desc(), descKey, log.Safe(useReverseScan))
+				"desc: %s, key: %s, reverse: %t", returnToken.Desc(), descKey, redact.Safe(useReverseScan))
 		}
 	}
 
@@ -764,7 +764,7 @@ func (ds *DistSender) Send(
 		// We already verified above that the batch contains only scan requests of the same type.
 		// Such a batch should never need splitting.
 		log.Fatalf(ctx, "batch with MaxSpanRequestKeys=%d, TargetBytes=%d needs splitting",
-			log.Safe(ba.MaxSpanRequestKeys), log.Safe(ba.TargetBytes))
+			redact.Safe(ba.MaxSpanRequestKeys), redact.Safe(ba.TargetBytes))
 	}
 	var singleRplChunk [1]*roachpb.BatchResponse
 	rplChunks := singleRplChunk[:0:1]

diff --git a/pkg/kv/kvclient/rangefeed/BUILD.bazel b/pkg/kv/kvclient/rangefeed/BUILD.bazel
@@ -32,6 +32,7 @@ go_library(
         "//pkg/util/timeutil",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_cockroachdb_logtags//:logtags",
+        "@com_github_cockroachdb_redact//:redact",
     ],
 )
 

diff --git a/pkg/kv/kvclient/rangefeed/rangefeed.go b/pkg/kv/kvclient/rangefeed/rangefeed.go
@@ -31,6 +31,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/logtags"
+	"github.com/cockroachdb/redact"
 )
 
 //go:generate mockgen -destination=mocks_generated_test.go --package=rangefeed . DB
@@ -299,7 +300,7 @@ func (f *RangeFeed) run(ctx context.Context, frontier *span.Frontier) {
 		}
 		if err != nil && ctx.Err() == nil && restartLogEvery.ShouldLog() {
 			log.Warningf(ctx, "rangefeed failed %d times, restarting: %v",
-				log.Safe(i), err)
+				redact.Safe(i), err)
 		}
 		if ctx.Err() != nil {
 			log.VEventf(ctx, 1, "exiting rangefeed")

diff --git a/pkg/kv/kvserver/batcheval/BUILD.bazel b/pkg/kv/kvserver/batcheval/BUILD.bazel
@@ -86,6 +86,7 @@ go_library(
         "//pkg/util/tracing",
         "//pkg/util/uuid",
         "@com_github_cockroachdb_errors//:errors",
+        "@com_github_cockroachdb_redact//:redact",
         "@com_github_gogo_protobuf//types",
         "@com_github_kr_pretty//:pretty",
         "@org_golang_x_time//rate",

diff --git a/pkg/kv/kvserver/batcheval/cmd_push_txn.go b/pkg/kv/kvserver/batcheval/cmd_push_txn.go
@@ -24,6 +24,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/redact"
 )
 
 func init() {
@@ -268,10 +269,10 @@ func PushTxn(
 			s = "failed to push"
 		}
 		log.Infof(ctx, "%s %s (push type=%s) %s: %s (pushee last active: %s)",
-			args.PusherTxn.Short(), log.Safe(s),
-			log.Safe(pushType),
+			args.PusherTxn.Short(), redact.Safe(s),
+			redact.Safe(pushType),
 			args.PusheeTxn.Short(),
-			log.Safe(reason),
+			redact.Safe(reason),
 			reply.PusheeTxn.LastActive())
 	}
 

diff --git a/pkg/kv/kvserver/queue.go b/pkg/kv/kvserver/queue.go
@@ -33,6 +33,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/redact"
 )
 
 const (
@@ -568,7 +569,7 @@ func (bq *baseQueue) Async(
 	ctx context.Context, opName string, wait bool, fn func(ctx context.Context, h queueHelper),
 ) {
 	if log.V(3) {
-		log.InfofDepth(ctx, 2, "%s", log.Safe(opName))
+		log.InfofDepth(ctx, 2, "%s", redact.Safe(opName))
 	}
 	opName += " (" + bq.name + ")"
 	bgCtx := bq.AnnotateCtx(context.Background())
@@ -581,7 +582,7 @@ func (bq *baseQueue) Async(
 		func(ctx context.Context) {
 			fn(ctx, baseQueueHelper{bq})
 		}); err != nil && bq.addLogN.ShouldLog() {
-		log.Infof(ctx, "rate limited in %s: %s", log.Safe(opName), err)
+		log.Infof(ctx, "rate limited in %s: %s", redact.Safe(opName), err)
 	}
 }
 

diff --git a/pkg/kv/kvserver/raft.go b/pkg/kv/kvserver/raft.go
@@ -19,6 +19,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/redact"
 	"go.etcd.io/etcd/raft/v3"
 	"go.etcd.io/etcd/raft/v3/raftpb"
 )
@@ -120,29 +121,29 @@ func wrapNumbersAsSafe(v ...interface{}) {
 	for i := range v {
 		switch v[i].(type) {
 		case uint:
-			v[i] = log.Safe(v[i])
+			v[i] = redact.Safe(v[i])
 		case uint8:
-			v[i] = log.Safe(v[i])
+			v[i] = redact.Safe(v[i])
 		case uint16:
-			v[i] = log.Safe(v[i])
+			v[i] = redact.Safe(v[i])
 		case uint32:
-			v[i] = log.Safe(v[i])
+			v[i] = redact.Safe(v[i])
 		case uint64:
-			v[i] = log.Safe(v[i])
+			v[i] = redact.Safe(v[i])
 		case int:
-			v[i] = log.Safe(v[i])
+			v[i] = redact.Safe(v[i])
 		case int8:
-			v[i] = log.Safe(v[i])
+			v[i] = redact.Safe(v[i])
 		case int16:
-			v[i] = log.Safe(v[i])
+			v[i] = redact.Safe(v[i])
 		case int32:
-			v[i] = log.Safe(v[i])
+			v[i] = redact.Safe(v[i])
 		case int64:
-			v[i] = log.Safe(v[i])
+			v[i] = redact.Safe(v[i])
 		case float32:
-			v[i] = log.Safe(v[i])
+			v[i] = redact.Safe(v[i])
 		case float64:
-			v[i] = log.Safe(v[i])
+			v[i] = redact.Safe(v[i])
 		default:
 		}
 	}

diff --git a/pkg/kv/kvserver/raft_log_queue.go b/pkg/kv/kvserver/raft_log_queue.go
@@ -28,6 +28,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/redact"
 	"go.etcd.io/etcd/raft/v3"
 	"go.etcd.io/etcd/raft/v3/tracker"
 )
@@ -707,14 +708,14 @@ func (rlq *raftLogQueue) process(
 
 	// Can and should the raft logs be truncated?
 	if !decision.ShouldTruncate() {
-		log.VEventf(ctx, 3, "%s", log.Safe(decision.String()))
+		log.VEventf(ctx, 3, "%s", redact.Safe(decision.String()))
 		return false, nil
 	}
 
 	if n := decision.NumNewRaftSnapshots(); log.V(1) || n > 0 && rlq.logSnapshots.ShouldProcess(timeutil.Now()) {
-		log.Infof(ctx, "%v", log.Safe(decision.String()))
+		log.Infof(ctx, "%v", redact.Safe(decision.String()))
 	} else {
-		log.VEventf(ctx, 1, "%v", log.Safe(decision.String()))
+		log.VEventf(ctx, 1, "%v", redact.Safe(decision.String()))
 	}
 	b := &kv.Batch{}
 	truncRequest := &roachpb.TruncateLogRequest{

diff --git a/pkg/kv/kvserver/replica.go b/pkg/kv/kvserver/replica.go
@@ -1303,7 +1303,7 @@ func (r *Replica) assertStateRaftMuLockedReplicaMuRLocked(
 			pretty.Diff(diskState, r.mu.state))
 		r.mu.state.Desc, diskState.Desc = nil, nil
 		log.Fatalf(ctx, "on-disk and in-memory state diverged: %s",
-			log.Safe(pretty.Diff(diskState, r.mu.state)))
+			redact.Safe(pretty.Diff(diskState, r.mu.state)))
 	}
 	if r.isInitializedRLocked() {
 		if !r.startKey.Equal(r.mu.state.Desc.StartKey) {

diff --git a/pkg/kv/kvserver/replica_consistency.go b/pkg/kv/kvserver/replica_consistency.go
@@ -253,7 +253,7 @@ func (r *Replica) CheckConsistency(
 				if !haveDelta {
 					return resp, nil
 				}
-				log.Fatalf(ctx, "found a delta of %+v", log.Safe(delta))
+				log.Fatalf(ctx, "found a delta of %+v", redact.Safe(delta))
 			}
 		}
 

diff --git a/pkg/kv/kvserver/replica_proposal.go b/pkg/kv/kvserver/replica_proposal.go
@@ -35,6 +35,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/quotapool"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
+	"github.com/cockroachdb/redact"
 	"github.com/kr/pretty"
 	"golang.org/x/time/rate"
 )
@@ -226,21 +227,21 @@ func (r *Replica) leasePostApplyLocked(
 		switch {
 		case s2 < s1:
 			log.Fatalf(ctx, "lease sequence inversion, prevLease=%s, newLease=%s",
-				log.Safe(prevLease), log.Safe(newLease))
+				redact.Safe(prevLease), redact.Safe(newLease))
 		case s2 == s1:
 			// If the sequence numbers are the same, make sure they're actually
 			// the same lease. This can happen when callers are using
 			// leasePostApply for some of its side effects, like with
 			// splitPostApply. It can also happen during lease extensions.
 			if !prevLease.Equivalent(*newLease) {
 				log.Fatalf(ctx, "sequence identical for different leases, prevLease=%s, newLease=%s",
-					log.Safe(prevLease), log.Safe(newLease))
+					redact.Safe(prevLease), redact.Safe(newLease))
 			}
 		case s2 == s1+1:
 			// Lease sequence incremented by 1. Expected case.
 		case s2 > s1+1 && jumpOpt == assertNoLeaseJump:
 			log.Fatalf(ctx, "lease sequence jump, prevLease=%s, newLease=%s",
-				log.Safe(prevLease), log.Safe(newLease))
+				redact.Safe(prevLease), redact.Safe(newLease))
 		}
 	}
 

diff --git a/pkg/kv/kvserver/replica_raft.go b/pkg/kv/kvserver/replica_raft.go
@@ -38,6 +38,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/redact"
 	"go.etcd.io/etcd/raft/v3"
 	"go.etcd.io/etcd/raft/v3/raftpb"
 	"go.etcd.io/etcd/raft/v3/tracker"
@@ -1023,7 +1024,7 @@ func maybeFatalOnRaftReadyErr(ctx context.Context, expl string, err error) (remo
 	case errors.Is(err, apply.ErrRemoved):
 		return true
 	default:
-		log.FatalfDepth(ctx, 1, "%s: %+v", log.Safe(expl), err)
+		log.FatalfDepth(ctx, 1, "%s: %+v", redact.Safe(expl), err)
 		panic("unreachable")
 	}
 }

diff --git a/pkg/kv/kvserver/replica_tscache.go b/pkg/kv/kvserver/replica_tscache.go
@@ -22,6 +22,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
+	"github.com/cockroachdb/redact"
 )
 
 // addToTSCacheChecked adds the specified timestamp to the timestamp cache
@@ -346,7 +347,7 @@ func (r *Replica) applyTimestampCache(
 			if conflictingTxn != uuid.Nil {
 				conflictMsg = "conflicting txn: " + conflictingTxn.Short()
 			}
-			log.VEventf(ctx, 2, "bumped write timestamp to %s; %s", bumpedTS, log.Safe(conflictMsg))
+			log.VEventf(ctx, 2, "bumped write timestamp to %s; %s", bumpedTS, redact.Safe(conflictMsg))
 		}
 	}
 	return bumped

diff --git a/pkg/kv/kvserver/stateloader/BUILD.bazel b/pkg/kv/kvserver/stateloader/BUILD.bazel
@@ -20,6 +20,7 @@ go_library(
         "//pkg/util/log",
         "//pkg/util/protoutil",
         "@com_github_cockroachdb_errors//:errors",
+        "@com_github_cockroachdb_redact//:redact",
         "@io_etcd_go_etcd_raft_v3//raftpb",
     ],
 )