config: rename prof_laddr to pprof_laddr and move it to rpc (#5315)

* config: rename prof_laddr to pprof_laddr and move it to rpc

also, remove `/unsafe_start_cpu_profiler`, `/unsafe_stop_cpu_profiler`
and `/unsafe_write_heap_profile` in favor of pprof server functionality.

Closes #5303

* update changelog

* log start

CHANGELOG_PENDING.md

@@ -8,6 +8,10 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 
 - [crypto/secp256k1] \#5280 `secp256k1` has been removed from the Tendermint repo. (@marbar3778)
 
+- CLI/RPC/Config
+  - [config] \#5315 Rename `prof_laddr` to `pprof_laddr` and move it to `rpc` section (@melekes)
+  - [rpc] \#5315 Remove `/unsafe_start_cpu_profiler`, `/unsafe_stop_cpu_profiler` and `/unsafe_write_heap_profile`. Please use pprof functionality instead (@melekes)
+
 ## FEATURES
 
 - [privval] \#5239 Add `chainID` to requests from client. (@marbar3778)

UPGRADING.md

@@ -46,7 +46,7 @@ Merkle tree built from:
 - `BeginBlock#Events`.
 
 Merkle hashes of empty trees previously returned nothing, but now return the hash of an empty input,
-to conform with RFC-6962. This mainly affects `Header#DataHash`, `Header#LastResultsHash`, and 
+to conform with RFC-6962. This mainly affects `Header#DataHash`, `Header#LastResultsHash`, and
 `Header#EvidenceHash`, which are often empty. Non-empty hashes can also be affected, e.g. if their
 inputs depend on other (empty) Merkle hashes, giving different results.
 
@@ -144,6 +144,13 @@ All requests are now accompanied by the chainID from the network.
 This is a optional field and can be ignored by key management systems. It
 is recommended to check the chainID if using the same key management system for multiple chains.
 
+### RPC
+
+`/unsafe_start_cpu_profiler`, `/unsafe_stop_cpu_profiler` and
+`/unsafe_write_heap_profile` were removed. Please use pprof server, which can
+be enabled through `--rpc.pprof_laddr=X` flag or `pprof_laddr=X` config setting
+in the rpc section.
+
 ## v0.33.4
 
 ### Go API

cmd/tendermint/commands/run_node.go

@@ -58,6 +58,7 @@ func AddNodeFlags(cmd *cobra.Command) {
 		config.RPC.GRPCListenAddress,
 		"GRPC listen address (BroadcastTx only). Port required")
 	cmd.Flags().Bool("rpc.unsafe", config.RPC.Unsafe, "Enabled unsafe rpc methods")
+	cmd.Flags().String("rpc.pprof_laddr", config.RPC.PprofListenAddress, "pprof listen address (https://golang.org/pkg/net/http/pprof)")
 
 	// p2p flags
 	cmd.Flags().String(

config/config.go

@@ -210,9 +210,6 @@ type BaseConfig struct { //nolint: maligned
 	// Mechanism to connect to the ABCI application: socket | grpc
 	ABCI string `mapstructure:"abci"`
 
-	// TCP or UNIX socket address for the profiling server to listen on
-	ProfListenAddress string `mapstructure:"prof_laddr"`
-
 	// If true, query the ABCI app on connecting to a new peer
 	// so the app can decide if we should keep the connection or not
 	FilterPeers bool `mapstructure:"filter_peers"` // false
@@ -230,7 +227,6 @@ func DefaultBaseConfig() BaseConfig {
 		ABCI:               "socket",
 		LogLevel:           DefaultPackageLogLevels(),
 		LogFormat:          LogFormatPlain,
-		ProfListenAddress:  "",
 		FastSyncMode:       true,
 		FilterPeers:        false,
 		DBBackend:          "goleveldb",
@@ -383,6 +379,9 @@ type RPCConfig struct {
 	// NOTE: both tls_cert_file and tls_key_file must be present for Tendermint to create HTTPS server.
 	// Otherwise, HTTP server is run.
 	TLSKeyFile string `mapstructure:"tls_key_file"`
+
+	// pprof listen address (https://golang.org/pkg/net/http/pprof)
+	PprofListenAddress string `mapstructure:"pprof_laddr"`
 }
 
 // DefaultRPCConfig returns a default configuration for the RPC server

config/toml.go

@@ -139,9 +139,6 @@ node_key_file = "{{ js .BaseConfig.NodeKey }}"
 # Mechanism to connect to the ABCI application: socket | grpc
 abci = "{{ .BaseConfig.ABCI }}"
 
-# TCP or UNIX socket address for the profiling server to listen on
-prof_laddr = "{{ .BaseConfig.ProfListenAddress }}"
-
 # If true, query the ABCI app on connecting to a new peer
 # so the app can decide if we should keep the connection or not
 filter_peers = {{ .BaseConfig.FilterPeers }}
@@ -232,6 +229,9 @@ tls_cert_file = "{{ .RPC.TLSCertFile }}"
 # Otherwise, HTTP server is run.
 tls_key_file = "{{ .RPC.TLSKeyFile }}"
 
+# pprof listen address (https://golang.org/pkg/net/http/pprof)
+pprof_laddr = "{{ .RPC.PprofListenAddress }}"
+
 #######################################################
 ###           P2P Configuration Options             ###
 #######################################################
@@ -363,7 +363,7 @@ temp_dir = "{{ .StateSync.TempDir }}"
 # Fast Sync version to use:
 #   1) "v0" (default) - the legacy fast sync implementation
 #   2) "v1" - refactor of v0 version for better testability
-#   2) "v2" - complete redesign of v0, optimized for testability & readability 
+#   2) "v2" - complete redesign of v0, optimized for testability & readability
 version = "{{ .FastSync.Version }}"
 
 #######################################################
@@ -404,7 +404,7 @@ peer_query_maj23_sleep_duration = "{{ .Consensus.PeerQueryMaj23SleepDuration }}"
 [tx_index]
 
 # What indexer to use for transactions
-# 
+#
 # The application will set which txs to index. In some cases a node operator will be able
 # to decide which txs to index based on configuration set in the application.
 #

docs/tendermint-core/configuration.md

@@ -92,9 +92,6 @@ node_key_file = "config/node_key.json"
 # Mechanism to connect to the ABCI application: socket | grpc
 abci = "socket"
 
-# TCP or UNIX socket address for the profiling server to listen on
-prof_laddr = ""
-
 # If true, query the ABCI app on connecting to a new peer
 # so the app can decide if we should keep the connection or not
 filter_peers = false
@@ -185,6 +182,9 @@ tls_cert_file = ""
 # Otherwise, HTTP server is run.
 tls_key_file = ""
 
+# pprof listen address (https://golang.org/pkg/net/http/pprof)
+pprof_laddr = ""
+
 #######################################################
 ###           P2P Configuration Options             ###
 #######################################################
@@ -316,7 +316,7 @@ temp_dir = ""
 # Fast Sync version to use:
 #   1) "v0" (default) - the legacy fast sync implementation
 #   2) "v1" - refactor of v0 version for better testability
-#   2) "v2" - complete redesign of v0, optimized for testability & readability 
+#   2) "v2" - complete redesign of v0, optimized for testability & readability
 version = "v0"
 
 #######################################################
@@ -357,7 +357,7 @@ peer_query_maj23_sleep_duration = "2s"
 [tx_index]
 
 # What indexer to use for transactions
-# 
+#
 # The application will set which txs to index. In some cases a node operator will be able
 # to decide which txs to index based on configuration set in the application.
 #

node/node.go

@@ -787,9 +787,10 @@ func NewNode(config *cfg.Config,
 		pexReactor = createPEXReactorAndAddToSwitch(addrBook, config, sw, logger)
 	}
 
-	if config.ProfListenAddress != "" {
+	if config.RPC.PprofListenAddress != "" {
 		go func() {
-			logger.Error("Profile server", "err", http.ListenAndServe(config.ProfListenAddress, nil))
+			logger.Info("Starting pprof server", "laddr", config.RPC.PprofListenAddress)
+			logger.Error("pprof server error", "err", http.ListenAndServe(config.RPC.PprofListenAddress, nil))
 		}()
 	}
 

rpc/core/dev.go

@@ -1,9 +1,6 @@
 package core
 
 import (
-	"os"
-	"runtime/pprof"
-
 	ctypes "github.com/tendermint/tendermint/rpc/core/types"
 	rpctypes "github.com/tendermint/tendermint/rpc/jsonrpc/types"
 )
@@ -13,44 +10,3 @@ func UnsafeFlushMempool(ctx *rpctypes.Context) (*ctypes.ResultUnsafeFlushMempool
 	env.Mempool.Flush()
 	return &ctypes.ResultUnsafeFlushMempool{}, nil
 }
-
-var profFile *os.File
-
-// UnsafeStartCPUProfiler starts a pprof profiler using the given filename.
-func UnsafeStartCPUProfiler(ctx *rpctypes.Context, filename string) (*ctypes.ResultUnsafeProfile, error) {
-	var err error
-	profFile, err = os.Create(filename)
-	if err != nil {
-		return nil, err
-	}
-	err = pprof.StartCPUProfile(profFile)
-	if err != nil {
-		return nil, err
-	}
-	return &ctypes.ResultUnsafeProfile{}, nil
-}
-
-// UnsafeStopCPUProfiler stops the running pprof profiler.
-func UnsafeStopCPUProfiler(ctx *rpctypes.Context) (*ctypes.ResultUnsafeProfile, error) {
-	pprof.StopCPUProfile()
-	if err := profFile.Close(); err != nil {
-		return nil, err
-	}
-	return &ctypes.ResultUnsafeProfile{}, nil
-}
-
-// UnsafeWriteHeapProfile dumps a heap profile to the given filename.
-func UnsafeWriteHeapProfile(ctx *rpctypes.Context, filename string) (*ctypes.ResultUnsafeProfile, error) {
-	memProfFile, err := os.Create(filename)
-	if err != nil {
-		return nil, err
-	}
-	if err := pprof.WriteHeapProfile(memProfFile); err != nil {
-		return nil, err
-	}
-	if err := memProfFile.Close(); err != nil {
-		return nil, err
-	}
-
-	return &ctypes.ResultUnsafeProfile{}, nil
-}

rpc/core/doc.go

@@ -25,7 +25,6 @@ Available endpoints:
 /health
 /unconfirmed_txs
 /unsafe_flush_mempool
-/unsafe_stop_cpu_profiler
 /validators
 
 Endpoints that require arguments:
@@ -40,8 +39,6 @@ Endpoints that require arguments:
 /dial_persistent_peers?persistent_peers=_
 /subscribe?event=_
 /tx?hash=_&prove=_
-/unsafe_start_cpu_profiler?filename=_
-/unsafe_write_heap_profile?filename=_
 /unsubscribe?event=_
 ```
 */

rpc/core/routes.go

@@ -6,6 +6,7 @@ import (
 
 // TODO: better system than "unsafe" prefix
 
+// Routes is a map of available routes.
 var Routes = map[string]*rpc.RPCFunc{
 	// subscribe/unsubscribe are reserved for websocket events.
 	"subscribe":       rpc.NewWSRPCFunc(Subscribe, "query"),
@@ -45,14 +46,10 @@ var Routes = map[string]*rpc.RPCFunc{
 	"broadcast_evidence": rpc.NewRPCFunc(BroadcastEvidence, "evidence"),
 }
 
+// AddUnsafeRoutes adds unsafe routes.
 func AddUnsafeRoutes() {
 	// control API
 	Routes["dial_seeds"] = rpc.NewRPCFunc(UnsafeDialSeeds, "seeds")
 	Routes["dial_peers"] = rpc.NewRPCFunc(UnsafeDialPeers, "peers,persistent")
 	Routes["unsafe_flush_mempool"] = rpc.NewRPCFunc(UnsafeFlushMempool, "")
-
-	// profiler API
-	Routes["unsafe_start_cpu_profiler"] = rpc.NewRPCFunc(UnsafeStartCPUProfiler, "filename")
-	Routes["unsafe_stop_cpu_profiler"] = rpc.NewRPCFunc(UnsafeStopCPUProfiler, "")
-	Routes["unsafe_write_heap_profile"] = rpc.NewRPCFunc(UnsafeWriteHeapProfile, "filename")
 }