From e08ed0d823554a5934c75dc8310e5a52958fb5d0 Mon Sep 17 00:00:00 2001 From: james-prysm <90280386+james-prysm@users.noreply.github.com> Date: Mon, 24 Mar 2025 14:30:32 -0500 Subject: [PATCH] Deprecate broken and unused debug flags in favor of --pprof (#15083) * deprecating the trace and cpuprofile flags in favor of pprof * gaz * fixing change log title * added hidden tags --- beacon-chain/node/BUILD.bazel | 1 - beacon-chain/node/node.go | 2 - changelog/james-prysm_deprecate-non-pprof.md | 3 ++ cmd/beacon-chain/usage.go | 4 +- cmd/validator/main.go | 4 -- cmd/validator/usage.go | 9 ++++- runtime/debug/debug.go | 40 ++++---------------- validator/node/BUILD.bazel | 1 - validator/node/node.go | 2 - 9 files changed, 20 insertions(+), 46 deletions(-) create mode 100644 changelog/james-prysm_deprecate-non-pprof.md diff --git a/beacon-chain/node/BUILD.bazel b/beacon-chain/node/BUILD.bazel index d09e1df4dc..0f1f999a64 100644 --- a/beacon-chain/node/BUILD.bazel +++ b/beacon-chain/node/BUILD.bazel @@ -61,7 +61,6 @@ go_library( "//monitoring/prometheus:go_default_library", "//monitoring/tracing:go_default_library", "//runtime:go_default_library", - "//runtime/debug:go_default_library", "//runtime/prereqs:go_default_library", "//runtime/version:go_default_library", "@com_github_ethereum_go_ethereum//common:go_default_library", diff --git a/beacon-chain/node/node.go b/beacon-chain/node/node.go index a834c5af13..e80d3da3cf 100644 --- a/beacon-chain/node/node.go +++ b/beacon-chain/node/node.go @@ -64,7 +64,6 @@ import ( "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" "github.com/prysmaticlabs/prysm/v5/monitoring/prometheus" "github.com/prysmaticlabs/prysm/v5/runtime" - "github.com/prysmaticlabs/prysm/v5/runtime/debug" "github.com/prysmaticlabs/prysm/v5/runtime/prereqs" "github.com/prysmaticlabs/prysm/v5/runtime/version" "github.com/sirupsen/logrus" @@ -432,7 +431,6 @@ func (b *BeaconNode) Start() { defer signal.Stop(sigc) <-sigc log.Info("Got interrupt, shutting down...") - debug.Exit(b.cliCtx) // Ensure trace and CPU profile data are flushed. go b.Close() for i := 10; i > 0; i-- { <-sigc diff --git a/changelog/james-prysm_deprecate-non-pprof.md b/changelog/james-prysm_deprecate-non-pprof.md new file mode 100644 index 0000000000..47ea93af31 --- /dev/null +++ b/changelog/james-prysm_deprecate-non-pprof.md @@ -0,0 +1,3 @@ +### Deprecated + +- deprecates and removes usage of the `--trace` flag and`--cpuprofile` flag in favor of just using `--pprof` \ No newline at end of file diff --git a/cmd/beacon-chain/usage.go b/cmd/beacon-chain/usage.go index 196da04655..44b47cf327 100644 --- a/cmd/beacon-chain/usage.go +++ b/cmd/beacon-chain/usage.go @@ -211,6 +211,8 @@ var appHelpFlagGroups = []flagGroup{ Name: "deprecated", Flags: []cli.Flag{ cmd.BackupWebhookOutputDir, + debug.CPUProfileFlag, + debug.TraceFlag, }, }, { // Flags used in debugging Prysm. These are flags not usually run by end users. @@ -218,13 +220,11 @@ var appHelpFlagGroups = []flagGroup{ Flags: []cli.Flag{ cmd.MaxGoroutines, debug.BlockProfileRateFlag, - debug.CPUProfileFlag, debug.MemProfileRateFlag, debug.MutexProfileFractionFlag, debug.PProfAddrFlag, debug.PProfFlag, debug.PProfPortFlag, - debug.TraceFlag, flags.SetGCPercent, }, }, diff --git a/cmd/validator/main.go b/cmd/validator/main.go index 05f7b913e6..0dcb72dce7 100644 --- a/cmd/validator/main.go +++ b/cmd/validator/main.go @@ -199,10 +199,6 @@ func main() { return cmd.ValidateNoArgs(ctx) }, - After: func(ctx *cli.Context) error { - debug.Exit(ctx) - return nil - }, } defer func() { diff --git a/cmd/validator/usage.go b/cmd/validator/usage.go index 1341f797c0..5492f49ebc 100644 --- a/cmd/validator/usage.go +++ b/cmd/validator/usage.go @@ -85,8 +85,6 @@ var appHelpFlagGroups = []flagGroup{ debug.PProfAddrFlag, debug.PProfPortFlag, debug.MemProfileRateFlag, - debug.CPUProfileFlag, - debug.TraceFlag, debug.BlockProfileRateFlag, debug.MutexProfileFractionFlag, }, @@ -157,6 +155,13 @@ var appHelpFlagGroups = []flagGroup{ flags.InteropStartIndex, }, }, + { + Name: "deprecated", + Flags: []cli.Flag{ + debug.CPUProfileFlag, + debug.TraceFlag, + }, + }, } func init() { diff --git a/runtime/debug/debug.go b/runtime/debug/debug.go index 45c6994c5e..a91b643b94 100644 --- a/runtime/debug/debug.go +++ b/runtime/debug/debug.go @@ -78,15 +78,17 @@ var ( Name: "blockprofilerate", Usage: "Turns on block profiling with the given rate.", } - // CPUProfileFlag to specify where to write the CPU profile. + // deprecated: CPUProfileFlag CPUProfileFlag = &cli.StringFlag{ - Name: "cpuprofile", - Usage: "Writes CPU profile to the given file.", + Name: "cpuprofile", + Hidden: true, + Usage: "Do not use, deprecated", } - // TraceFlag to specify where to write the trace execution profile. + // deprecated: TraceFlag TraceFlag = &cli.StringFlag{ - Name: "trace", - Usage: "Writes execution trace to the given file.", + Name: "trace", + Hidden: true, + Usage: "Do not use, deprecated", } ) @@ -328,17 +330,6 @@ func Setup(ctx *cli.Context) error { if ctx.IsSet(MutexProfileFractionFlag.Name) { runtime.SetMutexProfileFraction(ctx.Int(MutexProfileFractionFlag.Name)) } - if traceFile := ctx.String(TraceFlag.Name); traceFile != "" { - if err := Handler.StartGoTrace(TraceFlag.Name); err != nil { - return err - } - } - if cpuFile := ctx.String(CPUProfileFlag.Name); cpuFile != "" { - if err := Handler.StartCPUProfile(cpuFile); err != nil { - return err - } - } - // pprof server if ctx.Bool(PProfFlag.Name) { address := fmt.Sprintf("%s:%d", ctx.String(PProfAddrFlag.Name), ctx.Int(PProfPortFlag.Name)) @@ -359,18 +350,3 @@ func startPProf(address string) { } }() } - -// Exit stops all running profiles, flushing their output to the -// respective file. -func Exit(ctx *cli.Context) { - if traceFile := ctx.String(TraceFlag.Name); traceFile != "" { - if err := Handler.StopGoTrace(); err != nil { - log.WithError(err).Error("Failed to stop go tracing") - } - } - if cpuFile := ctx.String(CPUProfileFlag.Name); cpuFile != "" { - if err := Handler.StopCPUProfile(); err != nil { - log.WithError(err).Error("Failed to stop CPU profiling") - } - } -} diff --git a/validator/node/BUILD.bazel b/validator/node/BUILD.bazel index 8a27ab47fa..af4c9fa675 100644 --- a/validator/node/BUILD.bazel +++ b/validator/node/BUILD.bazel @@ -47,7 +47,6 @@ go_library( "//monitoring/prometheus:go_default_library", "//monitoring/tracing:go_default_library", "//runtime:go_default_library", - "//runtime/debug:go_default_library", "//runtime/prereqs:go_default_library", "//runtime/version:go_default_library", "//validator/accounts/wallet:go_default_library", diff --git a/validator/node/node.go b/validator/node/node.go index 61e915a456..f7ad9bf56c 100644 --- a/validator/node/node.go +++ b/validator/node/node.go @@ -32,7 +32,6 @@ import ( "github.com/prysmaticlabs/prysm/v5/monitoring/prometheus" "github.com/prysmaticlabs/prysm/v5/monitoring/tracing" "github.com/prysmaticlabs/prysm/v5/runtime" - "github.com/prysmaticlabs/prysm/v5/runtime/debug" "github.com/prysmaticlabs/prysm/v5/runtime/prereqs" "github.com/prysmaticlabs/prysm/v5/runtime/version" "github.com/prysmaticlabs/prysm/v5/validator/accounts/wallet" @@ -154,7 +153,6 @@ func (c *ValidatorClient) Start() { defer signal.Stop(sigc) <-sigc log.Info("Got interrupt, shutting down...") - debug.Exit(c.cliCtx) // Ensure trace and CPU profile data are flushed. go c.Close() for i := 10; i > 0; i-- { <-sigc