Revert "Revert "util/log: more misc cleanups""

pkg/ccl/changefeedccl/BUILD.bazel

@@ -75,6 +75,7 @@ go_library(
         "//pkg/util/humanizeutil",
         "//pkg/util/json",
         "//pkg/util/log",
+        "//pkg/util/log/logcrash",
         "//pkg/util/metric",
         "//pkg/util/mon",
         "//pkg/util/protoutil",

pkg/ccl/changefeedccl/changefeed_processors.go

@@ -31,6 +31,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
 	"github.com/cockroachdb/cockroach/pkg/util/span"
@@ -715,7 +716,7 @@ func (cf *changeFrontier) noteResolvedSpan(d rowenc.EncDatum) error {
 	// job progress update closure, but it currently doesn't pass along the info
 	// we'd need to do it that way.
 	if !resolved.Timestamp.IsEmpty() && resolved.Timestamp.Less(cf.highWaterAtStart) {
-		log.ReportOrPanic(cf.Ctx, &cf.flowCtx.Cfg.Settings.SV,
+		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))
 		return nil

pkg/cli/BUILD.bazel

@@ -144,6 +144,7 @@ go_library(
         "//pkg/util/iterutil",
         "//pkg/util/keysutil",
         "//pkg/util/log",
+        "//pkg/util/log/logcrash",
         "//pkg/util/log/logflags",
         "//pkg/util/log/logpb",
         "//pkg/util/log/severity",

pkg/cli/cli.go

@@ -23,6 +23,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/build"
 	"github.com/cockroachdb/cockroach/pkg/cli/exit"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
 	"github.com/cockroachdb/cockroach/pkg/util/log/logflags"
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 	// intentionally not all the workloads in pkg/ccl/workloadccl/allccl
@@ -58,12 +59,12 @@ func Main() {
 
 	cmdName := commandName(os.Args[1:])
 
-	log.SetupCrashReporter(
+	logcrash.SetupCrashReporter(
 		context.Background(),
 		cmdName,
 	)
 
-	defer log.RecoverAndReportPanic(context.Background(), &serverCfg.Settings.SV)
+	defer logcrash.RecoverAndReportPanic(context.Background(), &serverCfg.Settings.SV)
 
 	err := Run(os.Args[1:])
 

pkg/cli/cpuprofile.go

@@ -25,6 +25,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 )
 
@@ -76,7 +77,7 @@ func initCPUProfile(ctx context.Context, dir string, st *cluster.Settings) {
 	// TODO(knz,tbg): The caller of initCPUProfile() also defines a stopper;
 	// arguably this code would be better served by stopper.RunAsyncTask().
 	go func() {
-		defer log.RecoverAndReportPanic(ctx, &serverCfg.Settings.SV)
+		defer logcrash.RecoverAndReportPanic(ctx, &serverCfg.Settings.SV)
 
 		ctx := context.Background()
 

pkg/cli/start.go

@@ -46,6 +46,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/humanizeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
 	"github.com/cockroachdb/cockroach/pkg/util/log/logflags"
 	"github.com/cockroachdb/cockroach/pkg/util/log/severity"
 	"github.com/cockroachdb/cockroach/pkg/util/sdnotify"
@@ -526,7 +527,7 @@ If problems persist, please see %s.`
 				// actually been started successfully. If there's no server,
 				// we won't know enough to decide whether reporting is
 				// permitted.
-				log.RecoverAndReportPanic(ctx, &s.ClusterSettings().SV)
+				logcrash.RecoverAndReportPanic(ctx, &s.ClusterSettings().SV)
 			}
 		}()
 		// When the start up goroutine completes, so can the start up span

pkg/jobs/BUILD.bazel

@@ -46,6 +46,7 @@ go_library(
         "//pkg/util/envutil",
         "//pkg/util/hlc",
         "//pkg/util/log",
+        "//pkg/util/log/logcrash",
         "//pkg/util/metric",
         "//pkg/util/protoutil",
         "//pkg/util/retry",

pkg/jobs/adopt.go

@@ -21,6 +21,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
 	"github.com/cockroachdb/errors"
 )
@@ -259,7 +260,7 @@ func (r *Registry) runJob(
 		// assertion errors, which shouldn't cause the test to panic. For now,
 		// comment this out.
 		// if errors.HasAssertionFailure(err) {
-		// 	log.ReportOrPanic(ctx, nil, err.Error())
+		// 	logcrash.ReportOrPanic(ctx, nil, err.Error())
 		// }
 		log.Errorf(ctx, "job %d: adoption completed with error %v", *job.ID(), err)
 	}
@@ -308,7 +309,7 @@ RETURNING id, status`,
 				log.Infof(ctx, "job %d, session id: %s canceled: the job is now reverting",
 					id, s.ID())
 			default:
-				log.ReportOrPanic(ctx, nil, "unexpected job status")
+				logcrash.ReportOrPanic(ctx, nil, "unexpected job status")
 			}
 		}
 		return nil

pkg/jobs/deprecated.go

@@ -375,7 +375,7 @@ func (r *Registry) deprecatedResume(
 				// assertion errors, which shouldn't cause the test to panic. For now,
 				// comment this out.
 				// if errors.HasAssertionFailure(err) {
-				// 	log.ReportOrPanic(ctx, nil, err.Error())
+				// 	logcrash.ReportOrPanic(ctx, nil, err.Error())
 				// }
 				log.Errorf(ctx, "job %d: adoption completed with error %v", *job.ID(), err)
 			}

pkg/server/BUILD.bazel

@@ -135,6 +135,7 @@ go_library(
         "//pkg/util/httputil",
         "//pkg/util/humanizeutil",
         "//pkg/util/log",
+        "//pkg/util/log/logcrash",
         "//pkg/util/log/logpb",
         "//pkg/util/log/severity",
         "//pkg/util/metric",

pkg/server/admin.go

@@ -51,6 +51,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/contextutil"
 	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
@@ -1322,7 +1323,7 @@ func (s *adminServer) Cluster(
 
 	return &serverpb.ClusterResponse{
 		ClusterID:         clusterID.String(),
-		ReportingEnabled:  log.DiagnosticsReportingEnabled.Get(&s.server.st.SV),
+		ReportingEnabled:  logcrash.DiagnosticsReportingEnabled.Get(&s.server.st.SV),
 		EnterpriseEnabled: enterpriseEnabled,
 	}, nil
 }

pkg/server/debug/logspy.go

@@ -160,7 +160,7 @@ func (spy *logSpy) run(ctx context.Context, w io.Writer, opts logSpyOptions) (er
 		if err == nil {
 			if dropped := atomic.LoadInt32(&countDropped); dropped > 0 {
 				entry := log.MakeEntry(
-					ctx, severity.WARNING, nil /* LogCounter */, 0 /* depth */, false, /* redactable */
+					ctx, severity.WARNING, 0 /* depth */, false, /* redactable */
 					"%d messages were dropped", log.Safe(dropped))
 				err = log.FormatEntry(entry, w) // modify return value
 			}
@@ -175,7 +175,7 @@ func (spy *logSpy) run(ctx context.Context, w io.Writer, opts logSpyOptions) (er
 
 	{
 		entry := log.MakeEntry(
-			ctx, severity.INFO, nil /* LogCounter */, 0 /* depth */, false, /* redactable */
+			ctx, severity.INFO, 0 /* depth */, false, /* redactable */
 			"intercepting logs with options %+v", opts)
 		entries <- entry
 	}
@@ -212,8 +212,8 @@ func (spy *logSpy) run(ctx context.Context, w io.Writer, opts logSpyOptions) (er
 	for {
 		select {
 		case <-done:
-
 			return
+
 		case entry := <-entries:
 			if err := log.FormatEntry(entry, w); err != nil {
 				return errors.Wrapf(err, "while writing entry %v", entry)
@@ -225,6 +225,7 @@ func (spy *logSpy) run(ctx context.Context, w io.Writer, opts logSpyOptions) (er
 			if done == nil {
 				done = ctx.Done()
 			}
+
 		case <-flushTimer.C:
 			flushTimer.Read = true
 			flushTimer.Reset(flushInterval)

pkg/server/updates.go

@@ -38,6 +38,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/cloudinfo"
 	"github.com/cockroachdb/cockroach/pkg/util/httputil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
@@ -80,7 +81,7 @@ type versionInfo struct {
 // phones home to check for updates and report usage.
 func (s *Server) PeriodicallyCheckForUpdates(ctx context.Context) {
 	s.stopper.RunWorker(ctx, func(ctx context.Context) {
-		defer log.RecoverAndReportNonfatalPanic(ctx, &s.st.SV)
+		defer logcrash.RecoverAndReportNonfatalPanic(ctx, &s.st.SV)
 		nextUpdateCheck := s.startTime
 		nextDiagnosticReport := s.startTime
 
@@ -120,7 +121,7 @@ func (s *Server) maybeCheckForUpdates(
 
 	// if diagnostics reporting is disabled, we should assume that means that the
 	// user doesn't want us phoning home for new-version checks either.
-	if !log.DiagnosticsReportingEnabled.Get(&s.st.SV) {
+	if !logcrash.DiagnosticsReportingEnabled.Get(&s.st.SV) {
 		return now.Add(updateCheckFrequency)
 	}
 
@@ -251,7 +252,7 @@ func (s *Server) maybeReportDiagnostics(ctx context.Context, now, scheduled time
 	// TODO(dt): we should allow tuning the reset and report intervals separately.
 	// Consider something like rand.Float() > resetFreq/reportFreq here to sample
 	// stat reset periods for reporting.
-	if log.DiagnosticsReportingEnabled.Get(&s.st.SV) {
+	if logcrash.DiagnosticsReportingEnabled.Get(&s.st.SV) {
 		s.ReportDiagnostics(ctx)
 	}
 

pkg/sql/BUILD.bazel

@@ -341,6 +341,7 @@ go_library(
         "//pkg/util/humanizeutil",
         "//pkg/util/json",
         "//pkg/util/log",
+        "//pkg/util/log/logcrash",
         "//pkg/util/log/severity",
         "//pkg/util/metric",
         "//pkg/util/mon",

pkg/sql/catalog/lease/BUILD.bazel

@@ -28,6 +28,7 @@ go_library(
         "//pkg/sql/sqlutil",
         "//pkg/util/hlc",
         "//pkg/util/log",
+        "//pkg/util/log/logcrash",
         "//pkg/util/quotapool",
         "//pkg/util/retry",
         "//pkg/util/stop",

pkg/sql/catalog/lease/lease.go

@@ -41,6 +41,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
 	"github.com/cockroachdb/cockroach/pkg/util/quotapool"
 	"github.com/cockroachdb/cockroach/pkg/util/retry"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
@@ -1846,7 +1847,7 @@ func (m *Manager) watchForRangefeedUpdates(
 		}
 		var descriptor descpb.Descriptor
 		if err := ev.Value.GetProto(&descriptor); err != nil {
-			log.ReportOrPanic(ctx, &m.storage.settings.SV,
+			logcrash.ReportOrPanic(ctx, &m.storage.settings.SV,
 				"%s: unable to unmarshal descriptor %v", ev.Key, ev.Value)
 			return
 		}

pkg/sql/colexec/colbuilder/BUILD.bazel

@@ -28,6 +28,7 @@ go_library(
         "//pkg/sql/types",
         "//pkg/util",
         "//pkg/util/log",
+        "//pkg/util/log/logcrash",
         "//pkg/util/mon",
         "//vendor/github.com/cockroachdb/errors",
     ],

pkg/sql/colexec/colbuilder/execplan.go

@@ -35,6 +35,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
 	"github.com/cockroachdb/errors"
 )
@@ -628,7 +629,7 @@ func NewColOperator(
 			result.OpMonitors = result.OpMonitors[:0]
 		}
 		if panicErr != nil {
-			colexecerror.InternalError(log.PanicAsError(0, panicErr))
+			colexecerror.InternalError(logcrash.PanicAsError(0, panicErr))
 		}
 	}()
 	spec := args.Spec

pkg/sql/colflow/colrpc/BUILD.bazel

@@ -20,6 +20,7 @@ go_library(
         "//pkg/sql/execinfrapb",
         "//pkg/sql/types",
         "//pkg/util/log",
+        "//pkg/util/log/logcrash",
         "//pkg/util/timeutil",
         "//vendor/github.com/apache/arrow/go/arrow/array",
         "//vendor/github.com/cockroachdb/errors",

pkg/sql/colflow/colrpc/inbox.go

@@ -26,6 +26,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/logtags"
 )
@@ -260,7 +261,7 @@ func (i *Inbox) Next(ctx context.Context) coldata.Batch {
 		// during normal termination.
 		if err := recover(); err != nil {
 			i.close()
-			colexecerror.InternalError(log.PanicAsError(0, err))
+			colexecerror.InternalError(logcrash.PanicAsError(0, err))
 		}
 	}()
 

pkg/sql/conn_executor.go

@@ -48,6 +48,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/fsm"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
 	"github.com/cockroachdb/cockroach/pkg/util/log/severity"
 	"github.com/cockroachdb/cockroach/pkg/util/metric"
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
@@ -771,7 +772,7 @@ const (
 
 func (ex *connExecutor) closeWrapper(ctx context.Context, recovered interface{}) {
 	if recovered != nil {
-		panicErr := log.PanicAsError(1, recovered)
+		panicErr := logcrash.PanicAsError(1, recovered)
 
 		// If there's a statement currently being executed, we'll report
 		// on it.
@@ -788,7 +789,7 @@ func (ex *connExecutor) closeWrapper(ctx context.Context, recovered interface{})
 		}
 
 		// Report the panic to telemetry in any case.
-		log.ReportPanic(ctx, &ex.server.cfg.Settings.SV, panicErr, 1 /* depth */)
+		logcrash.ReportPanic(ctx, &ex.server.cfg.Settings.SV, panicErr, 1 /* depth */)
 
 		// Close the executor before propagating the panic further.
 		ex.close(ctx, panicClose)
@@ -2695,7 +2696,7 @@ func statementFromCtx(ctx context.Context) tree.Statement {
 
 func init() {
 	// Register a function to include the anonymized statement in crash reports.
-	log.RegisterTagFn("statement", func(ctx context.Context) string {
+	logcrash.RegisterTagFn("statement", func(ctx context.Context) string {
 		stmt := statementFromCtx(ctx)
 		if stmt == nil {
 			return ""

pkg/sql/execinfra/BUILD.bazel

@@ -50,6 +50,7 @@ go_library(
         "//pkg/storage/fs",
         "//pkg/util",
         "//pkg/util/log",
+        "//pkg/util/log/logcrash",
         "//pkg/util/metric",
         "//pkg/util/mon",
         "//pkg/util/optional",

pkg/sql/execinfra/base.go

@@ -21,6 +21,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/rowenc"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
@@ -394,7 +395,7 @@ func (rb *rowSourceBase) consumerDone() {
 func (rb *rowSourceBase) consumerClosed(name string) {
 	status := ConsumerStatus(atomic.LoadUint32((*uint32)(&rb.ConsumerStatus)))
 	if status == ConsumerClosed {
-		log.ReportOrPanic(context.Background(), nil, "%s already closed", log.Safe(name))
+		logcrash.ReportOrPanic(context.Background(), nil, "%s already closed", log.Safe(name))
 	}
 	atomic.StoreUint32((*uint32)(&rb.ConsumerStatus), uint32(ConsumerClosed))
 }