From 5ff2ff2834d83a1a5a66a189a26717e55d396997 Mon Sep 17 00:00:00 2001 From: Arjun Mahishi Date: Thu, 28 Nov 2024 14:07:37 +0530 Subject: [PATCH] pkg/*: replace uses of debug.Stack() with debugutil.Stack() A previous commit introduced the debugutil.Stack() function which wraps the debug.Stack() output in "SafeStack" type to make sure that it does not get redacted. This commit replaces all the current uses of debug.Stack() with debugutil.Stack(). With one exception - there is still a call to debug.Stack() in pkg/util/log/clog.go. Changing this is not that straightforward. But it's ok keep it for now because this does not go through the redaction flow. Part of: CRDB-15292 Epic: CRDB-37533 Release note: None --- .../roachtestutil/mixedversion/BUILD.bazel | 1 + .../roachtest/roachtestutil/mixedversion/runner.go | 4 ++-- pkg/config/zonepb/BUILD.bazel | 1 + pkg/config/zonepb/zone_yaml.go | 6 +++--- pkg/kv/kvclient/kvcoord/BUILD.bazel | 1 + pkg/kv/kvclient/kvcoord/txn_coord_sender.go | 4 ++-- pkg/kv/kvserver/BUILD.bazel | 1 + pkg/kv/kvserver/concurrency/BUILD.bazel | 1 + pkg/kv/kvserver/concurrency/concurrency_manager.go | 4 ++-- pkg/kv/kvserver/intentresolver/BUILD.bazel | 1 + pkg/kv/kvserver/intentresolver/intent_resolver.go | 4 ++-- pkg/kv/kvserver/kvserverbase/BUILD.bazel | 1 + pkg/kv/kvserver/kvserverbase/syncing_write.go | 4 ++-- pkg/kv/kvserver/metrics.go | 8 ++++---- pkg/kv/kvserver/spanset/BUILD.bazel | 1 + pkg/kv/kvserver/spanset/spanset.go | 4 ++-- pkg/kv/kvserver/store_merge.go | 4 ++-- pkg/spanconfig/spanconfigsqlwatcher/BUILD.bazel | 1 + .../spanconfigsqlwatcher/zonesdecoder.go | 5 ++--- pkg/sql/catalog/lease/BUILD.bazel | 1 + pkg/sql/catalog/lease/lease_test.go | 6 +++--- pkg/sql/logictest/BUILD.bazel | 1 + pkg/sql/logictest/logic.go | 4 ++-- pkg/sql/types/BUILD.bazel | 1 + pkg/sql/types/types.go | 4 ++-- pkg/storage/fs/BUILD.bazel | 1 + pkg/storage/fs/file_registry_test.go | 4 ++-- pkg/util/ctxlog/BUILD.bazel | 5 ++++- pkg/util/ctxlog/context.go | 4 ++-- pkg/util/debugutil/BUILD.bazel | 6 +----- pkg/util/log/BUILD.bazel | 1 + pkg/util/log/clog.go | 8 ++++---- pkg/util/log/flags.go | 3 ++- pkg/util/log/log_entry.go | 3 ++- pkg/util/log/stderr_redirect.go | 6 +++--- pkg/util/stop/BUILD.bazel | 1 + pkg/util/stop/stopper.go | 6 +++--- pkg/util/tracing/BUILD.bazel | 1 + pkg/util/tracing/span.go | 14 +++++++------- pkg/util/tracing/tracer.go | 6 +++--- 40 files changed, 79 insertions(+), 63 deletions(-) diff --git a/pkg/cmd/roachtest/roachtestutil/mixedversion/BUILD.bazel b/pkg/cmd/roachtest/roachtestutil/mixedversion/BUILD.bazel index d591d17bd376..058142937bf1 100644 --- a/pkg/cmd/roachtest/roachtestutil/mixedversion/BUILD.bazel +++ b/pkg/cmd/roachtest/roachtestutil/mixedversion/BUILD.bazel @@ -30,6 +30,7 @@ go_library( "//pkg/roachprod/vm", "//pkg/testutils/release", "//pkg/util/ctxgroup", + "//pkg/util/debugutil", "//pkg/util/intsets", "//pkg/util/randutil", "//pkg/util/retry", diff --git a/pkg/cmd/roachtest/roachtestutil/mixedversion/runner.go b/pkg/cmd/roachtest/roachtestutil/mixedversion/runner.go index 5a5af126494f..78874bd36b12 100644 --- a/pkg/cmd/roachtest/roachtestutil/mixedversion/runner.go +++ b/pkg/cmd/roachtest/roachtestutil/mixedversion/runner.go @@ -13,7 +13,6 @@ import ( "os" "path/filepath" "regexp" - "runtime/debug" "strings" "sync" "sync/atomic" @@ -30,6 +29,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachprod/install" "github.com/cockroachdb/cockroach/pkg/roachprod/logger" "github.com/cockroachdb/cockroach/pkg/util/ctxgroup" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/syncutil" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/errors" @@ -868,7 +868,7 @@ func panicAsError(l *logger.Logger, f func() error) (retErr error) { // logPanicToErr logs the panic stack trace and returns an error with the // panic message. func logPanicToErr(l *logger.Logger, r interface{}) error { - l.Printf("panic stack trace:\n%s", string(debug.Stack())) + l.Printf("panic stack trace:\n%s", debugutil.Stack()) return fmt.Errorf("panic (stack trace above): %v", r) } diff --git a/pkg/config/zonepb/BUILD.bazel b/pkg/config/zonepb/BUILD.bazel index e1605f6bd347..e97c8321d7f9 100644 --- a/pkg/config/zonepb/BUILD.bazel +++ b/pkg/config/zonepb/BUILD.bazel @@ -18,6 +18,7 @@ go_library( "//pkg/sql/pgwire/pgcode", "//pkg/sql/pgwire/pgerror", "//pkg/sql/sem/tree", + "//pkg/util/debugutil", "//pkg/util/envutil", "//pkg/util/log", "@com_github_cockroachdb_errors//:errors", diff --git a/pkg/config/zonepb/zone_yaml.go b/pkg/config/zonepb/zone_yaml.go index 728f5bf76c3a..e3bb573c61f2 100644 --- a/pkg/config/zonepb/zone_yaml.go +++ b/pkg/config/zonepb/zone_yaml.go @@ -7,10 +7,10 @@ package zonepb import ( "fmt" - "runtime/debug" "sort" "strings" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/errors" "github.com/gogo/protobuf/proto" "gopkg.in/yaml.v2" @@ -50,13 +50,13 @@ var _ yaml.Unmarshaler = &ConstraintsConjunction{} // MarshalYAML implements yaml.Marshaler. func (c ConstraintsConjunction) MarshalYAML() (interface{}, error) { return nil, fmt.Errorf( - "MarshalYAML should never be called directly on Constraints (%v): %v", c, debug.Stack()) + "MarshalYAML should never be called directly on Constraints (%v): %v", c, debugutil.Stack()) } // UnmarshalYAML implements yaml.Marshaler. func (c *ConstraintsConjunction) UnmarshalYAML(unmarshal func(interface{}) error) error { return fmt.Errorf( - "UnmarshalYAML should never be called directly on Constraints: %v", debug.Stack()) + "UnmarshalYAML should never be called directly on Constraints: %v", debugutil.Stack()) } // ConstraintsList is an alias for a slice of Constraints that can be diff --git a/pkg/kv/kvclient/kvcoord/BUILD.bazel b/pkg/kv/kvclient/kvcoord/BUILD.bazel index f400e5c9fd59..8b926c105187 100644 --- a/pkg/kv/kvclient/kvcoord/BUILD.bazel +++ b/pkg/kv/kvclient/kvcoord/BUILD.bazel @@ -65,6 +65,7 @@ go_library( "//pkg/util/buildutil", "//pkg/util/circuit", "//pkg/util/ctxgroup", + "//pkg/util/debugutil", "//pkg/util/envutil", "//pkg/util/errorutil/unimplemented", "//pkg/util/future", diff --git a/pkg/kv/kvclient/kvcoord/txn_coord_sender.go b/pkg/kv/kvclient/kvcoord/txn_coord_sender.go index 500b41e7bbc3..6ca6becd9178 100644 --- a/pkg/kv/kvclient/kvcoord/txn_coord_sender.go +++ b/pkg/kv/kvclient/kvcoord/txn_coord_sender.go @@ -8,7 +8,6 @@ package kvcoord import ( "context" "math/rand" - "runtime/debug" "github.com/cockroachdb/cockroach/pkg/build" "github.com/cockroachdb/cockroach/pkg/kv" @@ -17,6 +16,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/storage/enginepb" "github.com/cockroachdb/cockroach/pkg/util/buildutil" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/envutil" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/log" @@ -734,7 +734,7 @@ func (tc *TxnCoordSender) maybeRejectClientLocked( // If the client is trying to do anything other than rollback, it is // unexpected for it to find the transaction already in a txnFinalized // state. This may be a bug, so log a stack trace. - stack := string(debug.Stack()) + stack := debugutil.Stack() log.Errorf(ctx, "%s. stack:\n%s", msg, stack) } reason := kvpb.TransactionStatusError_REASON_UNKNOWN diff --git a/pkg/kv/kvserver/BUILD.bazel b/pkg/kv/kvserver/BUILD.bazel index d932039f5fc3..3ba5bd59e5e8 100644 --- a/pkg/kv/kvserver/BUILD.bazel +++ b/pkg/kv/kvserver/BUILD.bazel @@ -207,6 +207,7 @@ go_library( "//pkg/util/buildutil", "//pkg/util/circuit", "//pkg/util/ctxgroup", + "//pkg/util/debugutil", "//pkg/util/encoding", "//pkg/util/envutil", "//pkg/util/errorutil", diff --git a/pkg/kv/kvserver/concurrency/BUILD.bazel b/pkg/kv/kvserver/concurrency/BUILD.bazel index 80ca7f517d55..871452eb7c09 100644 --- a/pkg/kv/kvserver/concurrency/BUILD.bazel +++ b/pkg/kv/kvserver/concurrency/BUILD.bazel @@ -32,6 +32,7 @@ go_library( "//pkg/storage/enginepb", "//pkg/util/buildutil", "//pkg/util/container/list", + "//pkg/util/debugutil", "//pkg/util/hlc", "//pkg/util/humanizeutil", "//pkg/util/log", diff --git a/pkg/kv/kvserver/concurrency/concurrency_manager.go b/pkg/kv/kvserver/concurrency/concurrency_manager.go index b81fe0bb9990..4036a9b5dc08 100644 --- a/pkg/kv/kvserver/concurrency/concurrency_manager.go +++ b/pkg/kv/kvserver/concurrency/concurrency_manager.go @@ -8,7 +8,6 @@ package concurrency import ( "context" "fmt" - "runtime/debug" "sync" "github.com/cockroachdb/cockroach/pkg/kv" @@ -22,6 +21,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/storage/enginepb" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/metric" @@ -302,7 +302,7 @@ func (m *managerImpl) sequenceReqWithGuard( panic(redact.Safe(fmt.Sprintf("must not be holding latches\n"+ "this is tracked in github.com/cockroachdb/cockroach/issues/77663; please comment if seen\n"+ "eval_kind=%d, holding_latches=%t, branch=%d, first_iteration=%t, stack=\n%s", - g.EvalKind, g.HoldingLatches(), branch, firstIteration, string(debug.Stack())))) + g.EvalKind, g.HoldingLatches(), branch, firstIteration, debugutil.Stack()))) } log.Event(ctx, "optimistic failed, so waiting for latches") g.lg, err = m.lm.WaitUntilAcquired(ctx, g.lg) diff --git a/pkg/kv/kvserver/intentresolver/BUILD.bazel b/pkg/kv/kvserver/intentresolver/BUILD.bazel index bd2662249493..90f3938029e0 100644 --- a/pkg/kv/kvserver/intentresolver/BUILD.bazel +++ b/pkg/kv/kvserver/intentresolver/BUILD.bazel @@ -21,6 +21,7 @@ go_library( "//pkg/settings", "//pkg/settings/cluster", "//pkg/storage/enginepb", + "//pkg/util/debugutil", "//pkg/util/hlc", "//pkg/util/log", "//pkg/util/metric", diff --git a/pkg/kv/kvserver/intentresolver/intent_resolver.go b/pkg/kv/kvserver/intentresolver/intent_resolver.go index e06507bc8f9a..16a8481a3980 100644 --- a/pkg/kv/kvserver/intentresolver/intent_resolver.go +++ b/pkg/kv/kvserver/intentresolver/intent_resolver.go @@ -9,7 +9,6 @@ import ( "bytes" "context" "math" - "runtime/debug" "sort" "time" @@ -24,6 +23,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/storage/enginepb" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/quotapool" @@ -1034,7 +1034,7 @@ func (ir *IntentResolver) resolveIntents( // if !build.IsRelease() && h == (kvpb.AdmissionHeader{}) && ir.everyAdmissionHeaderMissing.ShouldLog() { if false { log.Warningf(ctx, - "test-only warning: if you see this, please report to https://github.com/cockroachdb/cockroach/issues/112680. empty admission header provided by %s", string(debug.Stack())) + "test-only warning: if you see this, please report to https://github.com/cockroachdb/cockroach/issues/112680. empty admission header provided by %s", debugutil.Stack()) } // Send the requests ... if opts.sendImmediately { diff --git a/pkg/kv/kvserver/kvserverbase/BUILD.bazel b/pkg/kv/kvserver/kvserverbase/BUILD.bazel index d47c68418f70..a48cf8cb1388 100644 --- a/pkg/kv/kvserver/kvserverbase/BUILD.bazel +++ b/pkg/kv/kvserver/kvserverbase/BUILD.bazel @@ -21,6 +21,7 @@ go_library( "//pkg/roachpb", "//pkg/settings", "//pkg/settings/cluster", + "//pkg/util/debugutil", "//pkg/util/errorutil", "//pkg/util/hlc", "//pkg/util/log", diff --git a/pkg/kv/kvserver/kvserverbase/syncing_write.go b/pkg/kv/kvserver/kvserverbase/syncing_write.go index 95c46020be28..42597b5c0e08 100644 --- a/pkg/kv/kvserver/kvserverbase/syncing_write.go +++ b/pkg/kv/kvserver/kvserverbase/syncing_write.go @@ -8,12 +8,12 @@ package kvserverbase import ( "context" "os" - "runtime/debug" "strings" "time" "github.com/cockroachdb/cockroach/pkg/settings" "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/errors" @@ -53,7 +53,7 @@ func LimitBulkIOWrite(ctx context.Context, limiter *rate.Limiter, cost int) erro if d := timeutil.Since(begin); d > bulkIOWriteLimiterLongWait { log.Warningf(ctx, "bulk io write limiter took %s (>%s):\n%s", - d, bulkIOWriteLimiterLongWait, debug.Stack()) + d, bulkIOWriteLimiterLongWait, debugutil.Stack()) } return nil } diff --git a/pkg/kv/kvserver/metrics.go b/pkg/kv/kvserver/metrics.go index c7c8189d43b5..24db14f5fa10 100644 --- a/pkg/kv/kvserver/metrics.go +++ b/pkg/kv/kvserver/metrics.go @@ -8,7 +8,6 @@ package kvserver import ( "context" "fmt" - "runtime/debug" "sync/atomic" "time" @@ -24,6 +23,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/storage/disk" "github.com/cockroachdb/cockroach/pkg/storage/enginepb" "github.com/cockroachdb/cockroach/pkg/storage/fs" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/metric" "github.com/cockroachdb/cockroach/pkg/util/metric/aggmetric" @@ -3004,7 +3004,7 @@ type tenantMetricsRef struct { // in assertions on failure. _stack struct { syncutil.Mutex - string + debugutil.SafeStack } } @@ -3012,7 +3012,7 @@ func (ref *tenantMetricsRef) assert(ctx context.Context) { if atomic.LoadInt32(&ref._state) != 0 { ref._stack.Lock() defer ref._stack.Unlock() - log.FatalfDepth(ctx, 1, "tenantMetricsRef already finalized in:\n%s", ref._stack.string) + log.FatalfDepth(ctx, 1, "tenantMetricsRef already finalized in:\n%s", ref._stack.SafeStack) } } @@ -3162,7 +3162,7 @@ func (sm *TenantsStorageMetrics) releaseTenant(ctx context.Context, ref *tenantM return // unreachable } ref._stack.Lock() - ref._stack.string = string(debug.Stack()) + ref._stack.SafeStack = debugutil.Stack() ref._stack.Unlock() m.mu.Lock() defer m.mu.Unlock() diff --git a/pkg/kv/kvserver/spanset/BUILD.bazel b/pkg/kv/kvserver/spanset/BUILD.bazel index b6a028ac1184..77486b4f8a8c 100644 --- a/pkg/kv/kvserver/spanset/BUILD.bazel +++ b/pkg/kv/kvserver/spanset/BUILD.bazel @@ -14,6 +14,7 @@ go_library( "//pkg/roachpb", "//pkg/storage", "//pkg/storage/fs", + "//pkg/util/debugutil", "//pkg/util/hlc", "//pkg/util/log", "//pkg/util/protoutil", diff --git a/pkg/kv/kvserver/spanset/spanset.go b/pkg/kv/kvserver/spanset/spanset.go index 80550c915b1e..c1b9a7892bb8 100644 --- a/pkg/kv/kvserver/spanset/spanset.go +++ b/pkg/kv/kvserver/spanset/spanset.go @@ -8,12 +8,12 @@ package spanset import ( "context" "fmt" - "runtime/debug" "strings" "sync" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/errors" @@ -360,7 +360,7 @@ func (s *SpanSet) checkAllowed( } } - return errors.Errorf("cannot %s undeclared span %s\ndeclared:\n%s\nstack:\n%s", access, span, s, debug.Stack()) + return errors.Errorf("cannot %s undeclared span %s\ndeclared:\n%s\nstack:\n%s", access, span, s, debugutil.Stack()) } // contains returns whether s1 contains s2. Unlike Span.Contains, this function diff --git a/pkg/kv/kvserver/store_merge.go b/pkg/kv/kvserver/store_merge.go index e0623cb6aade..71a0ce320c97 100644 --- a/pkg/kv/kvserver/store_merge.go +++ b/pkg/kv/kvserver/store_merge.go @@ -8,10 +8,10 @@ package kvserver import ( "context" "runtime" - "runtime/debug" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/readsummary/rspb" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/errors" @@ -38,7 +38,7 @@ func (s *Store) maybeAssertNoHole(ctx context.Context, from, to roachpb.RKey) fu } goroutineStopped := make(chan struct{}) - caller := string(debug.Stack()) + caller := debugutil.Stack() if from.Equal(roachpb.RKeyMax) { // There will be a hole to the right of RKeyMax but it's just the end of // the addressable keyspace. diff --git a/pkg/spanconfig/spanconfigsqlwatcher/BUILD.bazel b/pkg/spanconfig/spanconfigsqlwatcher/BUILD.bazel index c156ee4e1cfb..05bfa66083af 100644 --- a/pkg/spanconfig/spanconfigsqlwatcher/BUILD.bazel +++ b/pkg/spanconfig/spanconfigsqlwatcher/BUILD.bazel @@ -28,6 +28,7 @@ go_library( "//pkg/sql/sem/tree", "//pkg/sql/types", "//pkg/util", + "//pkg/util/debugutil", "//pkg/util/hlc", "//pkg/util/log", "//pkg/util/log/logcrash", diff --git a/pkg/spanconfig/spanconfigsqlwatcher/zonesdecoder.go b/pkg/spanconfig/spanconfigsqlwatcher/zonesdecoder.go index 8f4bed8d4e8c..220ecf7403e6 100644 --- a/pkg/spanconfig/spanconfigsqlwatcher/zonesdecoder.go +++ b/pkg/spanconfig/spanconfigsqlwatcher/zonesdecoder.go @@ -6,8 +6,6 @@ package spanconfigsqlwatcher import ( - "runtime/debug" - "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" @@ -15,6 +13,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/rowenc" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/types" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/errors" ) @@ -40,7 +39,7 @@ func (zd *zonesDecoder) DecodePrimaryKey(key roachpb.Key) (descpb.ID, error) { types := []*types.T{tbl.PublicColumns()[0].GetType()} startKeyRow := make([]rowenc.EncDatum, 1) if _, err := rowenc.DecodeIndexKey(zd.codec, startKeyRow, nil /* colDirs */, key); err != nil { - return descpb.InvalidID, errors.NewAssertionErrorWithWrappedErrf(err, "failed to decode key in system.zones %v %v", key, string(debug.Stack())) + return descpb.InvalidID, errors.NewAssertionErrorWithWrappedErrf(err, "failed to decode key in system.zones %v %v", key, debugutil.Stack()) } if err := startKeyRow[0].EnsureDecoded(types[0], &zd.alloc); err != nil { return descpb.InvalidID, errors.NewAssertionErrorWithWrappedErrf(err, "failed to decode key in system.zones %v", key) diff --git a/pkg/sql/catalog/lease/BUILD.bazel b/pkg/sql/catalog/lease/BUILD.bazel index f46155ff28c2..1f0b94d53586 100644 --- a/pkg/sql/catalog/lease/BUILD.bazel +++ b/pkg/sql/catalog/lease/BUILD.bazel @@ -144,6 +144,7 @@ go_test( "//pkg/util/admission", "//pkg/util/allstacks", "//pkg/util/ctxgroup", + "//pkg/util/debugutil", "//pkg/util/encoding", "//pkg/util/hlc", "//pkg/util/leaktest", diff --git a/pkg/sql/catalog/lease/lease_test.go b/pkg/sql/catalog/lease/lease_test.go index 2cc05c6adf0e..d68819c26bcc 100644 --- a/pkg/sql/catalog/lease/lease_test.go +++ b/pkg/sql/catalog/lease/lease_test.go @@ -12,7 +12,6 @@ import ( "context" gosql "database/sql" "fmt" - "runtime/debug" "strings" "sync" "sync/atomic" @@ -64,6 +63,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" "github.com/cockroachdb/cockroach/pkg/util/allstacks" "github.com/cockroachdb/cockroach/pkg/util/ctxgroup" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/encoding" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/leaktest" @@ -936,7 +936,7 @@ func TestDescriptorRefreshOnRetry(t *testing.T) { RemoveOnceDereferenced: true, LeaseAcquiredEvent: func(desc catalog.Descriptor, _ error) { if desc.GetName() == "foo" { - log.Infof(ctx, "lease acquirer stack trace: %s", debug.Stack()) + log.Infof(ctx, "lease acquirer stack trace: %s", debugutil.Stack()) atomic.AddInt32(&fooAcquiredCount, 1) } }, @@ -1323,7 +1323,7 @@ func TestLeaseRenewedAutomatically(testingT *testing.T) { // see a block event dump a stack to aid in debugging. log.Infof(ctx, "Lease acquisition of ID %d resulted in a block event. Stack trace to follow:\n%s", - id, debug.Stack()) + id, debugutil.Stack()) }, }, } diff --git a/pkg/sql/logictest/BUILD.bazel b/pkg/sql/logictest/BUILD.bazel index 5bb6f18b8682..b6f964479cd6 100644 --- a/pkg/sql/logictest/BUILD.bazel +++ b/pkg/sql/logictest/BUILD.bazel @@ -89,6 +89,7 @@ go_library( "//pkg/testutils/serverutils", "//pkg/testutils/skip", "//pkg/testutils/sqlutils", + "//pkg/util/debugutil", "//pkg/util/envutil", "//pkg/util/log", "//pkg/util/metamorphic", diff --git a/pkg/sql/logictest/logic.go b/pkg/sql/logictest/logic.go index 226238bb1db3..afc1d1825b10 100644 --- a/pkg/sql/logictest/logic.go +++ b/pkg/sql/logictest/logic.go @@ -23,7 +23,6 @@ import ( "reflect" "regexp" "runtime" - "runtime/debug" "runtime/trace" "sort" "strconv" @@ -69,6 +68,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/envutil" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/metamorphic" @@ -4217,7 +4217,7 @@ func (t *logicTest) runFile(path string, config logictestbase.TestClusterConfig) defer func() { if r := recover(); r != nil { // Translate panics during the test to test errors. - t.Fatalf("panic: %v\n%s", r, string(debug.Stack())) + t.Fatalf("panic: %v\n%s", r, debugutil.Stack()) } }() diff --git a/pkg/sql/types/BUILD.bazel b/pkg/sql/types/BUILD.bazel index 6c1bc9bec4aa..532e1aa49225 100644 --- a/pkg/sql/types/BUILD.bazel +++ b/pkg/sql/types/BUILD.bazel @@ -22,6 +22,7 @@ go_library( "//pkg/sql/pgwire/pgcode", "//pkg/sql/pgwire/pgerror", "//pkg/sql/sem/catid", + "//pkg/util/debugutil", "//pkg/util/errorutil/unimplemented", "//pkg/util/protoutil", "@com_github_cockroachdb_errors//:errors", diff --git a/pkg/sql/types/types.go b/pkg/sql/types/types.go index acb65424e628..749c68da34d9 100644 --- a/pkg/sql/types/types.go +++ b/pkg/sql/types/types.go @@ -9,7 +9,6 @@ import ( "bytes" "fmt" "regexp" - "runtime/debug" "strings" "github.com/cockroachdb/cockroach/pkg/geo/geopb" @@ -18,6 +17,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/sem/catid" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" "github.com/cockroachdb/cockroach/pkg/util/protoutil" "github.com/cockroachdb/errors" @@ -2842,7 +2842,7 @@ func (t *T) EnumGetIdxOfPhysical(phys []byte) (int, error) { phys, t.TypeMeta.Name.FQName(true /* explicitCatalog */), t.TypeMeta.EnumData.debugString(), - debug.Stack(), + debugutil.Stack(), ) return 0, err } diff --git a/pkg/storage/fs/BUILD.bazel b/pkg/storage/fs/BUILD.bazel index 5891bf71a07c..6be281be5fb4 100644 --- a/pkg/storage/fs/BUILD.bazel +++ b/pkg/storage/fs/BUILD.bazel @@ -56,6 +56,7 @@ go_test( "//pkg/storage/enginepb", "//pkg/testutils/datapathutils", "//pkg/testutils/skip", + "//pkg/util/debugutil", "//pkg/util/leaktest", "//pkg/util/log", "//pkg/util/stop", diff --git a/pkg/storage/fs/file_registry_test.go b/pkg/storage/fs/file_registry_test.go index a63d4e555df4..ff9dcdcfb2c9 100644 --- a/pkg/storage/fs/file_registry_test.go +++ b/pkg/storage/fs/file_registry_test.go @@ -11,7 +11,6 @@ import ( "fmt" "io" "os" - "runtime/debug" "sort" "strings" "testing" @@ -20,6 +19,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/storage/enginepb" "github.com/cockroachdb/cockroach/pkg/testutils/datapathutils" "github.com/cockroachdb/cockroach/pkg/testutils/skip" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/datadriven" @@ -101,7 +101,7 @@ func TestFileRegistryOps(t *testing.T) { registry.writeMu.Lock() defer registry.writeMu.Unlock() if diff := pretty.Diff(registry.writeMu.mu.entries, expected); diff != nil { - t.Log(string(debug.Stack())) + t.Log(debugutil.Stack()) t.Fatalf("%s\n%v", strings.Join(diff, "\n"), registry.writeMu.mu.entries) } } diff --git a/pkg/util/ctxlog/BUILD.bazel b/pkg/util/ctxlog/BUILD.bazel index 1b94882aa493..384d5f66834e 100644 --- a/pkg/util/ctxlog/BUILD.bazel +++ b/pkg/util/ctxlog/BUILD.bazel @@ -5,7 +5,10 @@ go_library( srcs = ["context.go"], importpath = "github.com/cockroachdb/cockroach/pkg/util/ctxlog", visibility = ["//visibility:public"], - deps = ["//pkg/util/log"], + deps = [ + "//pkg/util/debugutil", + "//pkg/util/log", + ], ) go_test( diff --git a/pkg/util/ctxlog/context.go b/pkg/util/ctxlog/context.go index b8a891cbadba..797dfe232e16 100644 --- a/pkg/util/ctxlog/context.go +++ b/pkg/util/ctxlog/context.go @@ -7,8 +7,8 @@ package ctxlog import ( "context" - "runtime/debug" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/log" ) @@ -23,7 +23,7 @@ func wrap(ctx context.Context, cancel context.CancelFunc) (context.Context, cont } return ctx, func() { if log.V(2) { - log.InfofDepth(ctx, 1, "canceling context:\n%s", debug.Stack()) + log.InfofDepth(ctx, 1, "canceling context:\n%s", debugutil.Stack()) } else if log.V(1) { log.InfofDepth(ctx, 1, "canceling context") } diff --git a/pkg/util/debugutil/BUILD.bazel b/pkg/util/debugutil/BUILD.bazel index 4e0991522802..8f6f1a133ded 100644 --- a/pkg/util/debugutil/BUILD.bazel +++ b/pkg/util/debugutil/BUILD.bazel @@ -5,11 +5,7 @@ go_library( srcs = ["debugutil.go"], importpath = "github.com/cockroachdb/cockroach/pkg/util/debugutil", visibility = ["//visibility:public"], - deps = [ - "@com_github_cockroachdb_redact//:redact", - "@com_github_cockroachdb_redact//interfaces", - "@com_github_elastic_gosigar//:gosigar", - ], + deps = ["@com_github_elastic_gosigar//:gosigar"], ) go_test( diff --git a/pkg/util/log/BUILD.bazel b/pkg/util/log/BUILD.bazel index a026838f8517..c45fc31c9ad5 100644 --- a/pkg/util/log/BUILD.bazel +++ b/pkg/util/log/BUILD.bazel @@ -63,6 +63,7 @@ go_library( "//pkg/util", "//pkg/util/allstacks", "//pkg/util/caller", + "//pkg/util/debugutil", "//pkg/util/encoding/encodingtype", "//pkg/util/envutil", "//pkg/util/fileutil", diff --git a/pkg/util/log/clog.go b/pkg/util/log/clog.go index 60c00c671328..9418e46e34d8 100644 --- a/pkg/util/log/clog.go +++ b/pkg/util/log/clog.go @@ -10,13 +10,13 @@ package log import ( "context" - "runtime/debug" "sync" "sync/atomic" "time" "github.com/cockroachdb/cockroach/pkg/cli/exit" "github.com/cockroachdb/cockroach/pkg/util/allstacks" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/log/logpb" "github.com/cockroachdb/cockroach/pkg/util/log/severity" "github.com/cockroachdb/cockroach/pkg/util/syncutil" @@ -94,7 +94,7 @@ type loggingT struct { // active indicates that at least one event has been logged // to this logger already. active bool - firstUseStack string + firstUseStack debugutil.SafeStack // redactionPolicyManaged indicates whether we're running as part of a managed // service (sourced from COCKROACH_REDACTION_POLICY_MANAGED env var). Impacts @@ -289,7 +289,7 @@ func (l *loggerT) outputLogEntry(entry logEntry) { switch traceback { case tracebackSingle: - entry.stacks = debug.Stack() + entry.stacks = debugutil.Stack() case tracebackAll: entry.stacks = allstacks.Get() } @@ -446,7 +446,7 @@ func setActive() { defer logging.mu.Unlock() if !logging.mu.active { logging.mu.active = true - logging.mu.firstUseStack = string(debug.Stack()) + logging.mu.firstUseStack = debugutil.Stack() } } diff --git a/pkg/util/log/flags.go b/pkg/util/log/flags.go index a64e16f0f3df..cb088cfc11fb 100644 --- a/pkg/util/log/flags.go +++ b/pkg/util/log/flags.go @@ -13,6 +13,7 @@ import ( "strings" "sync/atomic" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/envutil" "github.com/cockroachdb/cockroach/pkg/util/log/channel" "github.com/cockroachdb/cockroach/pkg/util/log/logconfig" @@ -82,7 +83,7 @@ func init() { // // This is used to assert that configuration is performed // before logging has been used for the first time. -func IsActive() (active bool, firstUse string) { +func IsActive() (active bool, firstUse debugutil.SafeStack) { logging.mu.Lock() defer logging.mu.Unlock() return logging.mu.active, logging.mu.firstUseStack diff --git a/pkg/util/log/log_entry.go b/pkg/util/log/log_entry.go index d9210548693a..36c6b62572c2 100644 --- a/pkg/util/log/log_entry.go +++ b/pkg/util/log/log_entry.go @@ -16,6 +16,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/base/serverident" "github.com/cockroachdb/cockroach/pkg/build" "github.com/cockroachdb/cockroach/pkg/util/caller" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/envutil" "github.com/cockroachdb/cockroach/pkg/util/log/logpb" "github.com/cockroachdb/cockroach/pkg/util/log/severity" @@ -73,7 +74,7 @@ type logEntry struct { counter uint64 // The stack trace(s), when processing e.g. a fatal event. - stacks []byte + stacks debugutil.SafeStack // Whether the entry is structured or not. structured bool diff --git a/pkg/util/log/stderr_redirect.go b/pkg/util/log/stderr_redirect.go index cc5f2a4c2974..52a73cb5c47b 100644 --- a/pkg/util/log/stderr_redirect.go +++ b/pkg/util/log/stderr_redirect.go @@ -8,8 +8,8 @@ package log import ( "fmt" "os" - "runtime/debug" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/syncutil" "github.com/cockroachdb/errors" ) @@ -83,7 +83,7 @@ func (l *fileSink) takeOverInternalStderr(logger *loggerT) error { // Success: remember who called us, in case the next // caller comes along with the wrong call sequence. - takeOverStderrMu.previousStderrTakeover = string(debug.Stack()) + takeOverStderrMu.previousStderrTakeover = debugutil.Stack() return nil } @@ -139,5 +139,5 @@ var takeOverStderrMu struct { // previousStderrTakeover is the stack trace of the previous call to // takeOverStderrInternal(). This can be used to troubleshoot // invalid call sequences. - previousStderrTakeover string + previousStderrTakeover debugutil.SafeStack } diff --git a/pkg/util/stop/BUILD.bazel b/pkg/util/stop/BUILD.bazel index 637c3e55c78e..6e471916a2cb 100644 --- a/pkg/util/stop/BUILD.bazel +++ b/pkg/util/stop/BUILD.bazel @@ -7,6 +7,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/kv/kvpb", + "//pkg/util/debugutil", "//pkg/util/leaktest", "//pkg/util/log", "//pkg/util/log/logcrash", diff --git a/pkg/util/stop/stopper.go b/pkg/util/stop/stopper.go index efce58396c1f..99933c479c37 100644 --- a/pkg/util/stop/stopper.go +++ b/pkg/util/stop/stopper.go @@ -9,13 +9,13 @@ import ( "context" "fmt" "net/http" - "runtime/debug" "runtime/trace" "sync/atomic" "testing" "time" "github.com/cockroachdb/cockroach/pkg/kv/kvpb" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/log/logcrash" @@ -42,7 +42,7 @@ func register(s *Stopper) { trackedStoppers.Lock() defer trackedStoppers.Unlock() trackedStoppers.stoppers = append(trackedStoppers.stoppers, - stopperWithStack{s: s, createdAt: string(debug.Stack())}) + stopperWithStack{s: s, createdAt: debugutil.Stack()}) } func unregister(s *Stopper) { @@ -60,7 +60,7 @@ func unregister(s *Stopper) { type stopperWithStack struct { s *Stopper - createdAt string // stack from NewStopper() + createdAt debugutil.SafeStack // stack from NewStopper() } var trackedStoppers struct { diff --git a/pkg/util/tracing/BUILD.bazel b/pkg/util/tracing/BUILD.bazel index 75aca41a1eaf..2cbbc0023146 100644 --- a/pkg/util/tracing/BUILD.bazel +++ b/pkg/util/tracing/BUILD.bazel @@ -23,6 +23,7 @@ go_library( "//pkg/settings", "//pkg/util/allstacks", "//pkg/util/buildutil", + "//pkg/util/debugutil", "//pkg/util/envutil", "//pkg/util/iterutil", "//pkg/util/metamorphic", diff --git a/pkg/util/tracing/span.go b/pkg/util/tracing/span.go index ddb1128fc1ee..83a4f7642eec 100644 --- a/pkg/util/tracing/span.go +++ b/pkg/util/tracing/span.go @@ -7,11 +7,11 @@ package tracing import ( "fmt" - "runtime/debug" "strings" "sync/atomic" "time" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/protoutil" "github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb" "github.com/cockroachdb/errors" @@ -133,7 +133,7 @@ type Span struct { // finishStack is set if debugUseAfterFinish is set. It represents the stack // that called Finish(), in order to report it on further use. - finishStack string + finishStack debugutil.SafeStack } // IsNoop returns true if this span is a black hole - it doesn't correspond to a @@ -168,9 +168,9 @@ func (sp *Span) detectUseAfterFinish() bool { // In test builds, we panic on span use after Finish. This is in preparation // of span pooling, at which point use-after-Finish would become corruption. if alreadyFinished && sp.i.tracer.PanicOnUseAfterFinish() { - var finishStack string - if sp.finishStack == "" { - finishStack = "" + var finishStack debugutil.SafeStack + if sp.finishStack == nil { + finishStack = debugutil.SafeStack("") } else { finishStack = sp.finishStack } @@ -248,7 +248,7 @@ func (sp *Span) finishInternal() { return } if sp.Tracer().debugUseAfterFinish { - sp.finishStack = string(debug.Stack()) + sp.finishStack = debugutil.Stack() } atomic.StoreInt32(&sp.finished, 1) sp.i.Finish() @@ -741,7 +741,7 @@ func (sp *Span) reset( // detect use-after-Finish. Similarly, we do the write atomically to prevent // reorderings. atomic.StoreInt32(&sp.finished, 0) - sp.finishStack = "" + sp.finishStack = nil } // SetOtelStatus sets the status of the OpenTelemetry span (if any). diff --git a/pkg/util/tracing/tracer.go b/pkg/util/tracing/tracer.go index 3e9644928326..5092036ddead 100644 --- a/pkg/util/tracing/tracer.go +++ b/pkg/util/tracing/tracer.go @@ -10,7 +10,6 @@ import ( "fmt" "os" "regexp" - "runtime/debug" "strconv" "strings" "sync" @@ -20,6 +19,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings" "github.com/cockroachdb/cockroach/pkg/util/buildutil" + "github.com/cockroachdb/cockroach/pkg/util/debugutil" "github.com/cockroachdb/cockroach/pkg/util/envutil" "github.com/cockroachdb/cockroach/pkg/util/iterutil" "github.com/cockroachdb/cockroach/pkg/util/metamorphic" @@ -381,7 +381,7 @@ type Tracer struct { // stack is populated in NewTracer and is printed in assertions related to // mixing tracers. - stack string + stack debugutil.SafeStack // closed is set on Close(). _closed int32 // accessed atomically } @@ -617,7 +617,7 @@ func NewTracer() *Tracer { } t := &Tracer{ - stack: string(debug.Stack()), + stack: debugutil.Stack(), activeSpansRegistry: makeSpanRegistry(), // These might be overridden in NewTracerWithOpt. panicOnUseAfterFinish: panicOnUseAfterFinish,