Merge pull request #1758 from OffchainLabs/pprof-metrics

Run pprof (by default enabled) on a separate port than metrics
OffchainLabs · Jul 17, 2023 · 82ec72f · 82ec72f
2 parents 41fd4df + 42d7842
commit 82ec72f
Show file tree

Hide file tree

Showing 8 changed files with 131 additions and 51 deletions.
diff --git a/cmd/daserver/daserver.go b/cmd/daserver/daserver.go
@@ -45,6 +45,8 @@ type DAServerConfig struct {
 
 	Metrics       bool                            `koanf:"metrics"`
 	MetricsServer genericconf.MetricsServerConfig `koanf:"metrics-server"`
+	PProf         bool                            `koanf:"pprof"`
+	PprofCfg      genericconf.PProf               `koanf:"pprof-cfg"`
 }
 
 var DefaultDAServerConfig = DAServerConfig{
@@ -60,6 +62,8 @@ var DefaultDAServerConfig = DAServerConfig{
 	ConfConfig:         genericconf.ConfConfigDefault,
 	Metrics:            false,
 	MetricsServer:      genericconf.MetricsServerConfigDefault,
+	PProf:              false,
+	PprofCfg:           genericconf.PProfDefault,
 	LogLevel:           3,
 }
 
@@ -89,6 +93,9 @@ func parseDAServer(args []string) (*DAServerConfig, error) {
 	f.Bool("metrics", DefaultDAServerConfig.Metrics, "enable metrics")
 	genericconf.MetricsServerAddOptions("metrics-server", f)
 
+	f.Bool("pprof", DefaultDAServerConfig.PProf, "enable pprof")
+	genericconf.PProfAddOptions("pprof-cfg", f)
+
 	f.Int("log-level", int(log.LvlInfo), "log level; 1: ERROR, 2: WARN, 3: INFO, 4: DEBUG, 5: TRACE")
 	das.DataAvailabilityConfigAddDaserverOptions("data-availability", f)
 	genericconf.ConfConfigAddOptions("conf", f)
@@ -135,6 +142,25 @@ func (c *L1ReaderCloser) String() string {
 	return "l1 reader closer"
 }
 
+// Checks metrics and PProf flag, runs them if enabled.
+// Note: they are separate so one can enable/disable them as they wish, the only
+// requirement is that they can't run on the same address and port.
+func startMetrics(cfg *DAServerConfig) error {
+	mAddr := fmt.Sprintf("%v:%v", cfg.MetricsServer.Addr, cfg.MetricsServer.Port)
+	pAddr := fmt.Sprintf("%v:%v", cfg.PprofCfg.Addr, cfg.PprofCfg.Port)
+	if cfg.Metrics && cfg.PProf && mAddr == pAddr {
+		return fmt.Errorf("metrics and pprof cannot be enabled on the same address:port: %s", mAddr)
+	}
+	if cfg.Metrics {
+		go metrics.CollectProcessMetrics(cfg.MetricsServer.UpdateInterval)
+		exp.Setup(fmt.Sprintf("%v:%v", cfg.MetricsServer.Addr, cfg.MetricsServer.Port))
+	}
+	if cfg.PProf {
+		genericconf.StartPprof(pAddr)
+	}
+	return nil
+}
+
 func startup() error {
 	// Some different defaults to DAS config in a node.
 	das.DefaultDataAvailabilityConfig.Enable = true
@@ -151,16 +177,8 @@ func startup() error {
 	glogger.Verbosity(log.Lvl(serverConfig.LogLevel))
 	log.Root().SetHandler(glogger)
 
-	if serverConfig.Metrics {
-		if len(serverConfig.MetricsServer.Addr) == 0 {
-			fmt.Printf("Metrics is enabled, but missing --metrics-server.addr")
-			return nil
-		}
-
-		go metrics.CollectProcessMetrics(serverConfig.MetricsServer.UpdateInterval)
-
-		address := fmt.Sprintf("%v:%v", serverConfig.MetricsServer.Addr, serverConfig.MetricsServer.Port)
-		exp.Setup(address)
+	if err := startMetrics(serverConfig); err != nil {
+		return err
 	}
 
 	sigint := make(chan os.Signal, 1)

diff --git a/cmd/genericconf/pprof.go b/cmd/genericconf/pprof.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net/http"
 
+	// Blank import pprof registers its HTTP handlers.
 	_ "net/http/pprof" // #nosec G108
 
 	"github.com/ethereum/go-ethereum/log"

diff --git a/cmd/genericconf/server.go b/cmd/genericconf/server.go
@@ -189,20 +189,32 @@ func AuthRPCConfigAddOptions(prefix string, f *flag.FlagSet) {
 type MetricsServerConfig struct {
 	Addr           string        `koanf:"addr"`
 	Port           int           `koanf:"port"`
-	Pprof          bool          `koanf:"pprof"`
 	UpdateInterval time.Duration `koanf:"update-interval"`
 }
 
 var MetricsServerConfigDefault = MetricsServerConfig{
 	Addr:           "127.0.0.1",
 	Port:           6070,
-	Pprof:          false,
 	UpdateInterval: 3 * time.Second,
 }
 
+type PProf struct {
+	Addr string `koanf:"addr"`
+	Port int    `koanf:"port"`
+}
+
+var PProfDefault = PProf{
+	Addr: "127.0.0.1",
+	Port: 6071,
+}
+
 func MetricsServerAddOptions(prefix string, f *flag.FlagSet) {
 	f.String(prefix+".addr", MetricsServerConfigDefault.Addr, "metrics server address")
 	f.Int(prefix+".port", MetricsServerConfigDefault.Port, "metrics server port")
-	f.Bool(prefix+".pprof", MetricsServerConfigDefault.Pprof, "enable profiling for Go")
 	f.Duration(prefix+".update-interval", MetricsServerConfigDefault.UpdateInterval, "metrics server update interval")
 }
+
+func PProfAddOptions(prefix string, f *flag.FlagSet) {
+	f.String(prefix+".addr", PProfDefault.Addr, "pprof server address")
+	f.Int(prefix+".port", PProfDefault.Port, "pprof server port")
+}
diff --git a/cmd/nitro-val/config.go b/cmd/nitro-val/config.go
@@ -30,6 +30,8 @@ type ValidationNodeConfig struct {
 	AuthRPC       genericconf.AuthRPCConfig       `koanf:"auth"`
 	Metrics       bool                            `koanf:"metrics"`
 	MetricsServer genericconf.MetricsServerConfig `koanf:"metrics-server"`
+	PProf         bool                            `koanf:"pprof"`
+	PprofCfg      genericconf.PProf               `koanf:"pprof-cfg"`
 	Workdir       string                          `koanf:"workdir" reload:"hot"`
 }
 
@@ -67,6 +69,8 @@ var ValidationNodeConfigDefault = ValidationNodeConfig{
 	AuthRPC:       genericconf.AuthRPCConfigDefault,
 	Metrics:       false,
 	MetricsServer: genericconf.MetricsServerConfigDefault,
+	PProf:         false,
+	PprofCfg:      genericconf.PProfDefault,
 	Workdir:       "",
 }
 
@@ -83,6 +87,8 @@ func ValidationNodeConfigAddOptions(f *flag.FlagSet) {
 	genericconf.AuthRPCConfigAddOptions("auth", f)
 	f.Bool("metrics", ValidationNodeConfigDefault.Metrics, "enable metrics")
 	genericconf.MetricsServerAddOptions("metrics-server", f)
+	f.Bool("pprof", ValidationNodeConfigDefault.PProf, "enable pprof")
+	genericconf.PProfAddOptions("pprof-cfg", f)
 	f.String("workdir", ValidationNodeConfigDefault.Workdir, "path used for purpose of resolving relative paths (ia. jwt secret file, log files), if empty then current working directory will be used.")
 }
 

diff --git a/cmd/nitro-val/nitro_val.go b/cmd/nitro-val/nitro_val.go
@@ -32,6 +32,25 @@ func main() {
 	os.Exit(mainImpl())
 }
 
+// Checks metrics and PProf flag, runs them if enabled.
+// Note: they are separate so one can enable/disable them as they wish, the only
+// requirement is that they can't run on the same address and port.
+func startMetrics(cfg *ValidationNodeConfig) error {
+	mAddr := fmt.Sprintf("%v:%v", cfg.MetricsServer.Addr, cfg.MetricsServer.Port)
+	pAddr := fmt.Sprintf("%v:%v", cfg.PprofCfg.Addr, cfg.PprofCfg.Port)
+	if cfg.Metrics && cfg.PProf && mAddr == pAddr {
+		return fmt.Errorf("metrics and pprof cannot be enabled on the same address:port: %s", mAddr)
+	}
+	if cfg.Metrics {
+		go metrics.CollectProcessMetrics(cfg.MetricsServer.UpdateInterval)
+		exp.Setup(fmt.Sprintf("%v:%v", cfg.MetricsServer.Addr, cfg.MetricsServer.Port))
+	}
+	if cfg.PProf {
+		genericconf.StartPprof(pAddr)
+	}
+	return nil
+}
+
 // Returns the exit code
 func mainImpl() int {
 	ctx, cancelFunc := context.WithCancel(context.Background())
@@ -96,20 +115,8 @@ func mainImpl() int {
 		log.Crit("failed to initialize geth stack", "err", err)
 	}
 
-	if nodeConfig.Metrics {
-		go metrics.CollectProcessMetrics(nodeConfig.MetricsServer.UpdateInterval)
-
-		if nodeConfig.MetricsServer.Addr != "" {
-			address := fmt.Sprintf("%v:%v", nodeConfig.MetricsServer.Addr, nodeConfig.MetricsServer.Port)
-			if nodeConfig.MetricsServer.Pprof {
-				genericconf.StartPprof(address)
-			} else {
-				exp.Setup(address)
-			}
-		}
-	} else if nodeConfig.MetricsServer.Pprof {
-		flag.Usage()
-		log.Error("--metrics must be enabled in order to use pprof with the metrics server")
+	if err := startMetrics(nodeConfig); err != nil {
+		log.Error("Starting metrics: %v", err)
 		return 1
 	}
 

diff --git a/cmd/nitro/nitro.go b/cmd/nitro/nitro.go
@@ -122,6 +122,25 @@ func main() {
 	os.Exit(mainImpl())
 }
 
+// Checks metrics and PProf flag, runs them if enabled.
+// Note: they are separate so one can enable/disable them as they wish, the only
+// requirement is that they can't run on the same address and port.
+func startMetrics(cfg *NodeConfig) error {
+	mAddr := fmt.Sprintf("%v:%v", cfg.MetricsServer.Addr, cfg.MetricsServer.Port)
+	pAddr := fmt.Sprintf("%v:%v", cfg.PprofCfg.Addr, cfg.PprofCfg.Port)
+	if cfg.Metrics && cfg.PProf && mAddr == pAddr {
+		return fmt.Errorf("metrics and pprof cannot be enabled on the same address:port: %s", mAddr)
+	}
+	if cfg.Metrics {
+		go metrics.CollectProcessMetrics(cfg.MetricsServer.UpdateInterval)
+		exp.Setup(fmt.Sprintf("%v:%v", cfg.MetricsServer.Addr, cfg.MetricsServer.Port))
+	}
+	if cfg.PProf {
+		genericconf.StartPprof(pAddr)
+	}
+	return nil
+}
+
 // Returns the exit code
 func mainImpl() int {
 	ctx, cancelFunc := context.WithCancel(context.Background())
@@ -350,6 +369,11 @@ func mainImpl() int {
 		}
 	}
 
+	if err := startMetrics(nodeConfig); err != nil {
+		log.Error("Starting metrics: %v", err)
+		return 1
+	}
+
 	chainDb, l2BlockChain, err := openInitializeChainDb(ctx, stack, nodeConfig, new(big.Int).SetUint64(nodeConfig.L2.ChainID), execution.DefaultCacheConfigFor(stack, &nodeConfig.Node.Caching), l1Client, rollupAddrs)
 	defer closeDb(chainDb, "chainDb")
 	if l2BlockChain != nil {
@@ -379,23 +403,6 @@ func mainImpl() int {
 		return 1
 	}
 
-	if nodeConfig.Metrics {
-		go metrics.CollectProcessMetrics(nodeConfig.MetricsServer.UpdateInterval)
-
-		if nodeConfig.MetricsServer.Addr != "" {
-			address := fmt.Sprintf("%v:%v", nodeConfig.MetricsServer.Addr, nodeConfig.MetricsServer.Port)
-			if nodeConfig.MetricsServer.Pprof {
-				genericconf.StartPprof(address)
-			} else {
-				exp.Setup(address)
-			}
-		}
-	} else if nodeConfig.MetricsServer.Pprof {
-		flag.Usage()
-		log.Error("--metrics must be enabled in order to use pprof with the metrics server")
-		return 1
-	}
-
 	fatalErrChan := make(chan error, 10)
 
 	var valNode *valnode.ValidationNode
@@ -525,6 +532,8 @@ type NodeConfig struct {
 	GraphQL       genericconf.GraphQLConfig       `koanf:"graphql"`
 	Metrics       bool                            `koanf:"metrics"`
 	MetricsServer genericconf.MetricsServerConfig `koanf:"metrics-server"`
+	PProf         bool                            `koanf:"pprof"`
+	PprofCfg      genericconf.PProf               `koanf:"pprof-cfg"`
 	Init          InitConfig                      `koanf:"init"`
 	Rpc           genericconf.RpcConfig           `koanf:"rpc"`
 }
@@ -542,6 +551,8 @@ var NodeConfigDefault = NodeConfig{
 	IPC:           genericconf.IPCConfigDefault,
 	Metrics:       false,
 	MetricsServer: genericconf.MetricsServerConfigDefault,
+	PProf:         false,
+	PprofCfg:      genericconf.PProfDefault,
 }
 
 func NodeConfigAddOptions(f *flag.FlagSet) {
@@ -561,6 +572,9 @@ func NodeConfigAddOptions(f *flag.FlagSet) {
 	genericconf.GraphQLConfigAddOptions("graphql", f)
 	f.Bool("metrics", NodeConfigDefault.Metrics, "enable metrics")
 	genericconf.MetricsServerAddOptions("metrics-server", f)
+	f.Bool("pprof", NodeConfigDefault.PProf, "enable pprof")
+	genericconf.PProfAddOptions("pprof-cfg", f)
+
 	InitConfigAddOptions("init", f)
 	genericconf.RpcConfigAddOptions("rpc", f)
 }

diff --git a/cmd/relay/relay.go b/cmd/relay/relay.go
@@ -37,6 +37,25 @@ func printSampleUsage(progname string) {
 	fmt.Printf("Sample usage:                  %s --node.feed.input.url=<L1 RPC> --chain.id=<L2 chain id> \n", progname)
 }
 
+// Checks metrics and PProf flag, runs them if enabled.
+// Note: they are separate so one can enable/disable them as they wish, the only
+// requirement is that they can't run on the same address and port.
+func startMetrics(cfg *relay.Config) error {
+	mAddr := fmt.Sprintf("%v:%v", cfg.MetricsServer.Addr, cfg.MetricsServer.Port)
+	pAddr := fmt.Sprintf("%v:%v", cfg.PprofCfg.Addr, cfg.PprofCfg.Port)
+	if cfg.Metrics && cfg.PProf && mAddr == pAddr {
+		return fmt.Errorf("metrics and pprof cannot be enabled on the same address:port: %s", mAddr)
+	}
+	if cfg.Metrics {
+		go metrics.CollectProcessMetrics(cfg.MetricsServer.UpdateInterval)
+		exp.Setup(fmt.Sprintf("%v:%v", cfg.MetricsServer.Addr, cfg.MetricsServer.Port))
+	}
+	if cfg.PProf {
+		genericconf.StartPprof(pAddr)
+	}
+	return nil
+}
+
 func startup() error {
 	ctx := context.Background()
 
@@ -68,16 +87,13 @@ func startup() error {
 	if err != nil {
 		return err
 	}
-	err = newRelay.Start(ctx)
-	if err != nil {
+
+	if err := startMetrics(relayConfig); err != nil {
 		return err
 	}
 
-	if relayConfig.Metrics && relayConfig.MetricsServer.Addr != "" {
-		go metrics.CollectProcessMetrics(relayConfig.MetricsServer.UpdateInterval)
-
-		address := fmt.Sprintf("%v:%v", relayConfig.MetricsServer.Addr, relayConfig.MetricsServer.Port)
-		exp.Setup(address)
+	if err := newRelay.Start(ctx); err != nil {
+		return err
 	}
 
 	select {

diff --git a/relay/relay.go b/relay/relay.go
@@ -146,6 +146,8 @@ type Config struct {
 	LogType       string                          `koanf:"log-type"`
 	Metrics       bool                            `koanf:"metrics"`
 	MetricsServer genericconf.MetricsServerConfig `koanf:"metrics-server"`
+	PProf         bool                            `koanf:"pprof"`
+	PprofCfg      genericconf.PProf               `koanf:"pprof-cfg"`
 	Node          NodeConfig                      `koanf:"node"`
 	Queue         int                             `koanf:"queue"`
 }
@@ -157,6 +159,8 @@ var ConfigDefault = Config{
 	LogType:       "plaintext",
 	Metrics:       false,
 	MetricsServer: genericconf.MetricsServerConfigDefault,
+	PProf:         false,
+	PprofCfg:      genericconf.PProfDefault,
 	Node:          NodeConfigDefault,
 	Queue:         1024,
 }
@@ -168,6 +172,8 @@ func ConfigAddOptions(f *flag.FlagSet) {
 	f.String("log-type", ConfigDefault.LogType, "log type")
 	f.Bool("metrics", ConfigDefault.Metrics, "enable metrics")
 	genericconf.MetricsServerAddOptions("metrics-server", f)
+	f.Bool("pprof", ConfigDefault.PProf, "enable pprof")
+	genericconf.PProfAddOptions("pprof-cfg", f)
 	NodeConfigAddOptions("node", f)
 	f.Int("queue", ConfigDefault.Queue, "size of relay queue")
 }