From eae73f0a33c5c2ec29fb1108034c04cc0650e504 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gianguido=20Sor=C3=A0?= Date: Wed, 24 Jan 2024 11:20:14 +0100 Subject: [PATCH] cmd/relay: bind debug and monitoring address separately Same principle as #2808. Refactor the binding functions to be usable across multiple commands. --- cmd/debug_tools.go | 13 ++++ cmd/debug_tools_internal_test.go | 110 +++++++++++++++++++++++++++++++ cmd/relay.go | 2 +- cmd/relay/relay.go | 53 +++++++++------ cmd/run.go | 3 +- 5 files changed, 157 insertions(+), 24 deletions(-) create mode 100644 cmd/debug_tools.go create mode 100644 cmd/debug_tools_internal_test.go diff --git a/cmd/debug_tools.go b/cmd/debug_tools.go new file mode 100644 index 000000000..98f80d53f --- /dev/null +++ b/cmd/debug_tools.go @@ -0,0 +1,13 @@ +// Copyright © 2022-2023 Obol Labs Inc. Licensed under the terms of a Business Source License 1.1 + +package cmd + +import ( + "github.com/spf13/cobra" +) + +// bindDebugMonitoringFlags binds Prometheus monitoring and debug address CLI flags. +func bindDebugMonitoringFlags(cmd *cobra.Command, monitorAddr, debugAddr *string, defaultMonitorAddr string) { + cmd.Flags().StringVar(monitorAddr, "monitoring-address", defaultMonitorAddr, "Listening address (ip and port) for the monitoring API (prometheus).") + cmd.Flags().StringVar(debugAddr, "debug-address", "", "Listening address (ip and port) for the pprof and QBFT debug API.") +} diff --git a/cmd/debug_tools_internal_test.go b/cmd/debug_tools_internal_test.go new file mode 100644 index 000000000..d214828d4 --- /dev/null +++ b/cmd/debug_tools_internal_test.go @@ -0,0 +1,110 @@ +// Copyright © 2022-2023 Obol Labs Inc. Licensed under the terms of a Business Source License 1.1 + +package cmd + +import ( + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" + + "github.com/obolnetwork/charon/app" +) + +func genTestCmd(t *testing.T, f func(config app.Config)) *cobra.Command { + t.Helper() + + var conf app.Config + + cmd := &cobra.Command{ + Use: "test", + Short: "test", + } + + cmd.Run = func(cmd *cobra.Command, args []string) { + f(conf) + } + + bindDebugMonitoringFlags(cmd, &conf.MonitoringAddr, &conf.DebugAddr, "") + + return cmd +} + +func Test_bindDebugMonitoringFlags(t *testing.T) { + cmd := &cobra.Command{ + Use: "testcmd", + } + + t.Run("both present", func(t *testing.T) { + var ( + mAddr = "127.0.0.1:9999" + dAddr = "127.0.0.1:8888" + ) + + cmd.ResetCommands() + + testCmd := genTestCmd(t, func(config app.Config) { + require.Equal(t, mAddr, config.MonitoringAddr) + require.Equal(t, dAddr, config.DebugAddr) + }) + + cmd.AddCommand(testCmd) + + cmd.SetArgs([]string{ + "test", + "--monitoring-address", + mAddr, + "--debug-address", + dAddr, + }) + + require.NoError(t, cmd.Execute()) + }) + + t.Run("only monitor", func(t *testing.T) { + var ( + mAddr = "127.0.0.1:9999" + dAddr = "" + ) + cmd.ResetCommands() + + testCmd := genTestCmd(t, func(config app.Config) { + require.Equal(t, mAddr, config.MonitoringAddr) + require.Equal(t, dAddr, config.DebugAddr) + }) + + cmd.AddCommand(testCmd) + + cmd.SetArgs([]string{ + "test", + "--monitoring-address", + mAddr, + }) + + require.NoError(t, cmd.Execute()) + }) + + t.Run("only debug", func(t *testing.T) { + var ( + mAddr = "" + dAddr = "127.0.0.1:8888" + ) + + cmd.ResetCommands() + + testCmd := genTestCmd(t, func(config app.Config) { + require.Equal(t, mAddr, config.MonitoringAddr) + require.Equal(t, dAddr, config.DebugAddr) + }) + + cmd.AddCommand(testCmd) + + cmd.SetArgs([]string{ + "test", + "--debug-address", + dAddr, + }) + + require.NoError(t, cmd.Execute()) + }) +} diff --git a/cmd/relay.go b/cmd/relay.go index de90682b9..e86d6e393 100644 --- a/cmd/relay.go +++ b/cmd/relay.go @@ -34,6 +34,7 @@ func newRelayCmd(runFunc func(context.Context, relay.Config) error) *cobra.Comma bindDataDirFlag(cmd.Flags(), &config.DataDir) bindRelayFlag(cmd, &config) + bindDebugMonitoringFlags(cmd, &config.MonitoringAddr, &config.DebugAddr, "") bindP2PFlags(cmd, &config.P2PConfig) bindLogFlags(cmd.Flags(), &config.LogConfig) bindLokiFlags(cmd.Flags(), &config.LogConfig) @@ -43,7 +44,6 @@ func newRelayCmd(runFunc func(context.Context, relay.Config) error) *cobra.Comma func bindRelayFlag(cmd *cobra.Command, config *relay.Config) { cmd.Flags().StringVar(&config.HTTPAddr, "http-address", "127.0.0.1:3640", "Listening address (ip and port) for the relay http server serving runtime ENR.") - cmd.Flags().StringVar(&config.MonitoringAddr, "monitoring-address", "127.0.0.1:3620", "Listening address (ip and port) for the prometheus and pprof monitoring http server.") cmd.Flags().BoolVar(&config.AutoP2PKey, "auto-p2pkey", true, "Automatically create a p2pkey (secp256k1 private key used for p2p authentication and ENR) if none found in data directory.") cmd.Flags().StringVar(&config.RelayLogLevel, "p2p-relay-loglevel", "", "Libp2p circuit relay log level. E.g., debug, info, warn, error.") diff --git a/cmd/relay/relay.go b/cmd/relay/relay.go index 4b8d16b81..d9f702464 100644 --- a/cmd/relay/relay.go +++ b/cmd/relay/relay.go @@ -34,6 +34,7 @@ type Config struct { DataDir string HTTPAddr string MonitoringAddr string + DebugAddr string P2PConfig p2p.Config LogConfig log.Config AutoP2PKey bool @@ -86,7 +87,7 @@ func Run(ctx context.Context, config Config) error { } // Start serving HTTP: ENR and monitoring. - serverErr := make(chan error, 2) // Buffer for 2 servers. + serverErr := make(chan error, 3) // Buffer for 3 servers. go func() { if config.HTTPAddr == "" { return @@ -99,27 +100,37 @@ func Run(ctx context.Context, config Config) error { serverErr <- server.ListenAndServe() }() - go func() { - if config.MonitoringAddr == "" { - return - } + if config.MonitoringAddr != "" { + go func() { + // Serve prometheus metrics wrapped with relay identifiers. + mux := http.NewServeMux() + mux.Handle("/metrics", promhttp.InstrumentMetricHandler( + promRegistry, promhttp.HandlerFor(promRegistry, promhttp.HandlerOpts{}), + )) + + log.Info(ctx, "Monitoring server started", z.Str("address", config.MonitoringAddr)) + server := http.Server{Addr: config.MonitoringAddr, Handler: mux, ReadHeaderTimeout: time.Second} + serverErr <- server.ListenAndServe() + }() + } - // Serve prometheus metrics wrapped with relay identifiers. - mux := http.NewServeMux() - mux.Handle("/metrics", promhttp.InstrumentMetricHandler( - promRegistry, promhttp.HandlerFor(promRegistry, promhttp.HandlerOpts{}), - )) - - // Copied from net/http/pprof/pprof.go - mux.HandleFunc("/debug/pprof/", pprof.Index) - mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) - mux.HandleFunc("/debug/pprof/profile", pprof.Profile) - mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol) - mux.HandleFunc("/debug/pprof/trace", pprof.Trace) - - server := http.Server{Addr: config.MonitoringAddr, Handler: mux, ReadHeaderTimeout: time.Second} - serverErr <- server.ListenAndServe() - }() + if config.DebugAddr != "" { + go func() { + debugMux := http.NewServeMux() + + // Copied from net/http/pprof/pprof.go + debugMux.HandleFunc("/debug/pprof/", pprof.Index) + debugMux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) + debugMux.HandleFunc("/debug/pprof/profile", pprof.Profile) + debugMux.HandleFunc("/debug/pprof/symbol", pprof.Symbol) + debugMux.HandleFunc("/debug/pprof/trace", pprof.Trace) + + log.Info(ctx, "Debug server started", z.Str("address", config.DebugAddr)) + + server := http.Server{Addr: config.DebugAddr, Handler: debugMux, ReadHeaderTimeout: time.Second} + serverErr <- server.ListenAndServe() + }() + } log.Info(ctx, "Relay started", z.Str("peer_name", p2p.PeerName(tcpNode.ID())), diff --git a/cmd/run.go b/cmd/run.go index 27b10b741..5da64eac1 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -44,6 +44,7 @@ func newRunCmd(runFunc func(context.Context, app.Config) error, unsafe bool) *co bindPrivKeyFlag(cmd, &conf.PrivKeyFile, &conf.PrivKeyLocking) bindRunFlags(cmd, &conf) + bindDebugMonitoringFlags(cmd, &conf.MonitoringAddr, &conf.DebugAddr, "127.0.0.1:3620") bindNoVerifyFlag(cmd.Flags(), &conf.NoVerify) bindP2PFlags(cmd, &conf.P2P) bindLogFlags(cmd.Flags(), &conf.Log) @@ -70,8 +71,6 @@ func bindRunFlags(cmd *cobra.Command, config *app.Config) { cmd.Flags().StringVar(&config.ManifestFile, "manifest-file", ".charon/cluster-manifest.pb", "The path to the cluster manifest file. If both cluster manifest and cluster lock files are provided, the cluster manifest file takes precedence.") cmd.Flags().StringSliceVar(&config.BeaconNodeAddrs, "beacon-node-endpoints", nil, "Comma separated list of one or more beacon node endpoint URLs.") cmd.Flags().StringVar(&config.ValidatorAPIAddr, "validator-api-address", "127.0.0.1:3600", "Listening address (ip and port) for validator-facing traffic proxying the beacon-node API.") - cmd.Flags().StringVar(&config.MonitoringAddr, "monitoring-address", "127.0.0.1:3620", "Listening address (ip and port) for the monitoring API (prometheus).") - cmd.Flags().StringVar(&config.DebugAddr, "debug-address", "", "Listening address (ip and port) for the pprof and QBFT debug API.") cmd.Flags().StringVar(&config.JaegerAddr, "jaeger-address", "", "Listening address for jaeger tracing.") cmd.Flags().StringVar(&config.JaegerService, "jaeger-service", "charon", "Service name used for jaeger tracing.") cmd.Flags().BoolVar(&config.SimnetBMock, "simnet-beacon-mock", false, "Enables an internal mock beacon node for running a simnet.")