From 5f1259c736dc88e987d90d2b829e4f1caeed22ba Mon Sep 17 00:00:00 2001 From: Andy Pliszka Date: Mon, 16 Sep 2024 16:04:38 -0400 Subject: [PATCH 01/12] feat: adds metrics server address to config and sets localhost defaults * uses metrics-listen-addr for metrics server * sets default debug server api-listen-addr to 127.0.0.1:5183 * sets default metrics server metrics-listen-addr to 127.0.0.1:9100 --- cmd/config.go | 28 +++++++++++++++------------- examples/config_EXAMPLE.yaml | 3 ++- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index 253580507..ced242be8 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -488,24 +488,26 @@ func DefaultConfig(memo string) *Config { // GlobalConfig describes any global relayer settings type GlobalConfig struct { - APIListenPort string `yaml:"api-listen-addr" json:"api-listen-addr"` - Timeout string `yaml:"timeout" json:"timeout"` - Memo string `yaml:"memo" json:"memo"` - LightCacheSize int `yaml:"light-cache-size" json:"light-cache-size"` - LogLevel string `yaml:"log-level" json:"log-level"` - ICS20MemoLimit int `yaml:"ics20-memo-limit" json:"ics20-memo-limit"` - MaxReceiverSize int `yaml:"max-receiver-size" json:"max-receiver-size"` + APIListenPort string `yaml:"api-listen-addr" json:"api-listen-addr"` + MetricsListenPort string `yaml:"metrics-listen-addr" json:"metrics-listen-addr"` + Timeout string `yaml:"timeout" json:"timeout"` + Memo string `yaml:"memo" json:"memo"` + LightCacheSize int `yaml:"light-cache-size" json:"light-cache-size"` + LogLevel string `yaml:"log-level" json:"log-level"` + ICS20MemoLimit int `yaml:"ics20-memo-limit" json:"ics20-memo-limit"` + MaxReceiverSize int `yaml:"max-receiver-size" json:"max-receiver-size"` } // newDefaultGlobalConfig returns a global config with defaults set func newDefaultGlobalConfig(memo string) GlobalConfig { return GlobalConfig{ - APIListenPort: ":5183", - Timeout: "10s", - LightCacheSize: 20, - Memo: memo, - LogLevel: "info", - MaxReceiverSize: 150, + APIListenPort: "127.0.0.1:5183", + MetricsListenPort: "127.0.0.1:9100", + Timeout: "10s", + LightCacheSize: 20, + Memo: memo, + LogLevel: "info", + MaxReceiverSize: 150, } } diff --git a/examples/config_EXAMPLE.yaml b/examples/config_EXAMPLE.yaml index 900d9d608..7552b18fc 100644 --- a/examples/config_EXAMPLE.yaml +++ b/examples/config_EXAMPLE.yaml @@ -1,5 +1,6 @@ global: - api-listen-addr: :5183 + api-listen-addr: 127.0.0.1:5183 + metrics-listen-addr: 127.0.0.1:9100 timeout: 10s memo: "" light-cache-size: 20 From b3779b4636cb2fb047ce38af1a8d089474af3195 Mon Sep 17 00:00:00 2001 From: Andy Pliszka Date: Mon, 16 Sep 2024 16:10:11 -0400 Subject: [PATCH 02/12] refactor: separates debug and metrics server * moves metrics to separate server --- cmd/flags.go | 27 +++++++++++++++ cmd/start.go | 44 ++++++++++++++++++++++-- internal/relaydebug/debugserver.go | 10 +----- internal/relayermetrics/metricsserver.go | 41 ++++++++++++++++++++++ 4 files changed, 110 insertions(+), 12 deletions(-) create mode 100644 internal/relayermetrics/metricsserver.go diff --git a/cmd/flags.go b/cmd/flags.go index a0c42d3ef..f6ecd2d47 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -40,6 +40,8 @@ const ( flagVersion = "version" flagEnableDebugServer = "enable-debug-server" flagDebugAddr = "debug-addr" + flagEnableMetricsServer = "enable-metrics-server" + flagMetricsAddr = "metrics-addr" flagOverwriteConfig = "overwrite" flagLimit = "limit" flagHeight = "height" @@ -443,6 +445,31 @@ func debugServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command { return cmd } +func metricsServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command { + cmd.Flags().String( + flagMetricsAddr, + "", + "address to use for metrics server. By default, "+ + "will be the metrics-listen-addr parameter in the global config.", + ) + + if err := v.BindPFlag(flagMetricsAddr, cmd.Flags().Lookup(flagMetricsAddr)); err != nil { + panic(err) + } + + cmd.Flags().Bool( + flagEnableMetricsServer, + false, + "enables metrics server. By default, the metrics server is disabled due to security concerns.", + ) + + if err := v.BindPFlag(flagEnableMetricsServer, cmd.Flags().Lookup(flagEnableMetricsServer)); err != nil { + panic(err) + } + + return cmd +} + func processorFlag(v *viper.Viper, cmd *cobra.Command) *cobra.Command { cmd.Flags().StringP(flagProcessor, "p", relayer.ProcessorEvents, "which relayer processor to use") if err := v.BindPFlag(flagProcessor, cmd.Flags().Lookup(flagProcessor)); err != nil { diff --git a/cmd/start.go b/cmd/start.go index 8e1d38d5d..b9f258f82 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -24,6 +24,7 @@ import ( "strings" "github.com/cosmos/relayer/v2/internal/relaydebug" + "github.com/cosmos/relayer/v2/internal/relayermetrics" "github.com/cosmos/relayer/v2/relayer" "github.com/cosmos/relayer/v2/relayer/chains/cosmos" "github.com/cosmos/relayer/v2/relayer/processor" @@ -92,10 +93,9 @@ $ %s start demo-path2 --max-tx-size 10`, appName, appName, appName, appName)), return err } - var prometheusMetrics *processor.PrometheusMetrics - debugAddr := a.config.Global.APIListenPort + // debug server debugAddrFlag, err := cmd.Flags().GetString(flagDebugAddr) if err != nil { return err @@ -126,8 +126,45 @@ $ %s start demo-path2 --max-tx-size 10`, appName, appName, appName, appName)), } log := a.log.With(zap.String("sys", "debughttp")) log.Info("Debug server listening", zap.String("addr", debugAddr)) + relaydebug.StartDebugServer(cmd.Context(), log, ln) + } + + // metrics server + var prometheusMetrics *processor.PrometheusMetrics + + metricsAddr := a.config.Global.MetricsListenPort + + metricsAddrFlag, err := cmd.Flags().GetString(flagMetricsAddr) + if err != nil { + return err + } + + if metricsAddrFlag != "" { + metricsAddr = metricsAddrFlag + } + + flagEnableMetricsServer, err := cmd.Flags().GetBool(flagEnableMetricsServer) + if err != nil { + return err + } + + if flagEnableMetricsServer == false || metricsAddr == "" { + a.log.Info("Skipping metrics server due to empty metrics address flag") + } else { + ln, err := net.Listen("tcp", metricsAddr) + if err != nil { + a.log.Error( + "Failed to listen on metrics address. If you have another relayer process open, use --" + + flagMetricsAddr + + " to pick a different address.", + ) + + return fmt.Errorf("failed to listen on metrics address %q: %w", debugAddr, err) + } + log := a.log.With(zap.String("sys", "metricshttp")) + log.Info("Metrics server listening", zap.String("addr", metricsAddr)) prometheusMetrics = processor.NewPrometheusMetrics() - relaydebug.StartDebugServer(cmd.Context(), log, ln, prometheusMetrics.Registry) + relayermetrics.StartMetricsServer(cmd.Context(), log, ln, prometheusMetrics.Registry) for _, chain := range chains { if ccp, ok := chain.ChainProvider.(*cosmos.CosmosProvider); ok { ccp.SetMetrics(prometheusMetrics) @@ -195,6 +232,7 @@ $ %s start demo-path2 --max-tx-size 10`, appName, appName, appName, appName)), cmd = updateTimeFlags(a.viper, cmd) cmd = strategyFlag(a.viper, cmd) cmd = debugServerFlags(a.viper, cmd) + cmd = metricsServerFlags(a.viper, cmd) cmd = processorFlag(a.viper, cmd) cmd = initBlockFlag(a.viper, cmd) cmd = flushIntervalFlag(a.viper, cmd) diff --git a/internal/relaydebug/debugserver.go b/internal/relaydebug/debugserver.go index d72721a71..2ae8aae63 100644 --- a/internal/relaydebug/debugserver.go +++ b/internal/relaydebug/debugserver.go @@ -6,8 +6,6 @@ import ( "net/http" "net/http/pprof" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promhttp" "go.uber.org/zap" ) @@ -15,7 +13,7 @@ import ( // accepting connections on the given listener. // Any HTTP logging will be written at info level to the given logger. // The server will be forcefully shut down when ctx finishes. -func StartDebugServer(ctx context.Context, log *zap.Logger, ln net.Listener, registry *prometheus.Registry) { +func StartDebugServer(ctx context.Context, log *zap.Logger, ln net.Listener) { // Although we could just import net/http/pprof and rely on the default global server, // we may want many instances of this in test, // and we will probably want more endpoints as time goes on, @@ -33,12 +31,6 @@ func StartDebugServer(ctx context.Context, log *zap.Logger, ln net.Listener, reg // so operators don't see a mysterious 404 page. mux.Handle("/", http.RedirectHandler("/debug/pprof", http.StatusSeeOther)) - // Serve default prometheus metrics - mux.Handle("/metrics", promhttp.Handler()) - - // Serve relayer metrics - mux.Handle("/relayer/metrics", promhttp.HandlerFor(registry, promhttp.HandlerOpts{})) - srv := &http.Server{ Handler: mux, ErrorLog: zap.NewStdLog(log), diff --git a/internal/relayermetrics/metricsserver.go b/internal/relayermetrics/metricsserver.go new file mode 100644 index 000000000..74873a039 --- /dev/null +++ b/internal/relayermetrics/metricsserver.go @@ -0,0 +1,41 @@ +package relayermetrics + +import ( + "context" + "net" + "net/http" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promhttp" + "go.uber.org/zap" +) + +// StartMetricsServer starts a metrics server in a background goroutine, +// accepting connections on the given listener. +// Any HTTP logging will be written at info level to the given logger. +// The server will be forcefully shut down when ctx finishes. +func StartMetricsServer(ctx context.Context, log *zap.Logger, ln net.Listener, registry *prometheus.Registry) { + // Set up new mux identical to the default mux configuration in net/http/pprof. + mux := http.NewServeMux() + + // Serve default prometheus metrics + mux.Handle("/metrics", promhttp.Handler()) + + // Serve relayer metrics + mux.Handle("/relayer/metrics", promhttp.HandlerFor(registry, promhttp.HandlerOpts{})) + + srv := &http.Server{ + Handler: mux, + ErrorLog: zap.NewStdLog(log), + BaseContext: func(net.Listener) context.Context { + return ctx + }, + } + + go srv.Serve(ln) + + go func() { + <-ctx.Done() + srv.Close() + }() +} From 16a7bc852ee58a22210a2317b71cbfc9bdc7318c Mon Sep 17 00:00:00 2001 From: Andy Pliszka Date: Tue, 17 Sep 2024 12:35:41 -0400 Subject: [PATCH 03/12] feat: changes the default metrics port to 5184 --- cmd/config.go | 2 +- examples/config_EXAMPLE.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index ced242be8..2344a9d63 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -502,7 +502,7 @@ type GlobalConfig struct { func newDefaultGlobalConfig(memo string) GlobalConfig { return GlobalConfig{ APIListenPort: "127.0.0.1:5183", - MetricsListenPort: "127.0.0.1:9100", + MetricsListenPort: "127.0.0.1:5184", Timeout: "10s", LightCacheSize: 20, Memo: memo, diff --git a/examples/config_EXAMPLE.yaml b/examples/config_EXAMPLE.yaml index 7552b18fc..e3a67f79c 100644 --- a/examples/config_EXAMPLE.yaml +++ b/examples/config_EXAMPLE.yaml @@ -1,6 +1,6 @@ global: api-listen-addr: 127.0.0.1:5183 - metrics-listen-addr: 127.0.0.1:9100 + metrics-listen-addr: 127.0.0.1:5184 timeout: 10s memo: "" light-cache-size: 20 From 155aa60873515d05068a6a01101b7f2448bbec23 Mon Sep 17 00:00:00 2001 From: Andy Pliszka Date: Thu, 19 Sep 2024 10:19:13 -0400 Subject: [PATCH 04/12] refactor: removes address flags for debug and metrics server * removes address flags so config file is the source of truth for addresses * adds tests for flags to enable debug and metrics servers * adds tests for config file settings for addresses for debug and metrics servers * adjusts log messages --- cmd/flags.go | 24 --- cmd/start.go | 139 +++++++------ cmd/start_test.go | 245 +++++++++++++++++++++++ internal/relaydebug/debugserver.go | 2 + internal/relayermetrics/metricsserver.go | 2 + internal/relayertest/system.go | 14 ++ 6 files changed, 331 insertions(+), 95 deletions(-) create mode 100644 cmd/start_test.go diff --git a/cmd/flags.go b/cmd/flags.go index f6ecd2d47..b52d9674e 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -39,9 +39,7 @@ const ( flagOrder = "order" flagVersion = "version" flagEnableDebugServer = "enable-debug-server" - flagDebugAddr = "debug-addr" flagEnableMetricsServer = "enable-metrics-server" - flagMetricsAddr = "metrics-addr" flagOverwriteConfig = "overwrite" flagLimit = "limit" flagHeight = "height" @@ -421,17 +419,6 @@ func dstPortFlag(v *viper.Viper, cmd *cobra.Command) *cobra.Command { } func debugServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command { - cmd.Flags().String( - flagDebugAddr, - "", - "address to use for debug and metrics server. By default, "+ - "will be the api-listen-addr parameter in the global config.", - ) - - if err := v.BindPFlag(flagDebugAddr, cmd.Flags().Lookup(flagDebugAddr)); err != nil { - panic(err) - } - cmd.Flags().Bool( flagEnableDebugServer, false, @@ -446,17 +433,6 @@ func debugServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command { } func metricsServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command { - cmd.Flags().String( - flagMetricsAddr, - "", - "address to use for metrics server. By default, "+ - "will be the metrics-listen-addr parameter in the global config.", - ) - - if err := v.BindPFlag(flagMetricsAddr, cmd.Flags().Lookup(flagMetricsAddr)); err != nil { - panic(err) - } - cmd.Flags().Bool( flagEnableMetricsServer, false, diff --git a/cmd/start.go b/cmd/start.go index b9f258f82..ff26dfacb 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -93,85 +93,16 @@ $ %s start demo-path2 --max-tx-size 10`, appName, appName, appName, appName)), return err } - debugAddr := a.config.Global.APIListenPort - - // debug server - debugAddrFlag, err := cmd.Flags().GetString(flagDebugAddr) + err = setupDebugServer(cmd, a, err) if err != nil { return err } - if debugAddrFlag != "" { - debugAddr = debugAddrFlag - } - - flagEnableDebugServer, err := cmd.Flags().GetBool(flagEnableDebugServer) + prometheusMetrics, err := setupMetricsServer(cmd, a, err, chains) if err != nil { return err } - if flagEnableDebugServer == false || debugAddr == "" { - a.log.Info("Skipping debug server due to empty debug address flag") - } else { - a.log.Warn("SECURITY WARNING! Debug server is enabled. It should only be used for non-production deployments.") - ln, err := net.Listen("tcp", debugAddr) - if err != nil { - a.log.Error( - "Failed to listen on debug address. If you have another relayer process open, use --" + - flagDebugAddr + - " to pick a different address.", - ) - - return fmt.Errorf("failed to listen on debug address %q: %w", debugAddr, err) - } - log := a.log.With(zap.String("sys", "debughttp")) - log.Info("Debug server listening", zap.String("addr", debugAddr)) - relaydebug.StartDebugServer(cmd.Context(), log, ln) - } - - // metrics server - var prometheusMetrics *processor.PrometheusMetrics - - metricsAddr := a.config.Global.MetricsListenPort - - metricsAddrFlag, err := cmd.Flags().GetString(flagMetricsAddr) - if err != nil { - return err - } - - if metricsAddrFlag != "" { - metricsAddr = metricsAddrFlag - } - - flagEnableMetricsServer, err := cmd.Flags().GetBool(flagEnableMetricsServer) - if err != nil { - return err - } - - if flagEnableMetricsServer == false || metricsAddr == "" { - a.log.Info("Skipping metrics server due to empty metrics address flag") - } else { - ln, err := net.Listen("tcp", metricsAddr) - if err != nil { - a.log.Error( - "Failed to listen on metrics address. If you have another relayer process open, use --" + - flagMetricsAddr + - " to pick a different address.", - ) - - return fmt.Errorf("failed to listen on metrics address %q: %w", debugAddr, err) - } - log := a.log.With(zap.String("sys", "metricshttp")) - log.Info("Metrics server listening", zap.String("addr", metricsAddr)) - prometheusMetrics = processor.NewPrometheusMetrics() - relayermetrics.StartMetricsServer(cmd.Context(), log, ln, prometheusMetrics.Registry) - for _, chain := range chains { - if ccp, ok := chain.ChainProvider.(*cosmos.CosmosProvider); ok { - ccp.SetMetrics(prometheusMetrics) - } - } - } - processorType, err := cmd.Flags().GetString(flagProcessor) if err != nil { return err @@ -240,3 +171,69 @@ $ %s start demo-path2 --max-tx-size 10`, appName, appName, appName, appName)), cmd = stuckPacketFlags(a.viper, cmd) return cmd } + +func setupMetricsServer(cmd *cobra.Command, a *appState, err error, chains map[string]*relayer.Chain) (*processor.PrometheusMetrics, error) { + var prometheusMetrics *processor.PrometheusMetrics + + metricsAddr := a.config.Global.MetricsListenPort + + flagEnableMetricsServer, err := cmd.Flags().GetBool(flagEnableMetricsServer) + if err != nil { + return nil, err + } + + if flagEnableMetricsServer == false || metricsAddr == "" { + a.log.Warn("Disabled metrics server due to missing metrics-listen-addr setting in config file.") + } else { + a.log.Info("Metrics server is enabled.") + ln, err := net.Listen("tcp", metricsAddr) + if err != nil { + a.log.Error( + "Failed to listen on metrics address. If you have another relayer process open, use --" + + metricsAddr + + " to pick a different address or port.", + ) + + return nil, fmt.Errorf("failed to listen on metrics address %q: %w", metricsAddr, err) + } + log := a.log.With(zap.String("sys", "metricshttp")) + log.Info("Metrics server listening", zap.String("addr", metricsAddr)) + prometheusMetrics = processor.NewPrometheusMetrics() + relayermetrics.StartMetricsServer(cmd.Context(), log, ln, prometheusMetrics.Registry) + for _, chain := range chains { + if ccp, ok := chain.ChainProvider.(*cosmos.CosmosProvider); ok { + ccp.SetMetrics(prometheusMetrics) + } + } + } + return prometheusMetrics, nil +} + +func setupDebugServer(cmd *cobra.Command, a *appState, err error) error { + debugAddr := a.config.Global.APIListenPort + + flagEnableDebugServer, err := cmd.Flags().GetBool(flagEnableDebugServer) + if err != nil { + return err + } + + if flagEnableDebugServer == false || debugAddr == "" { + a.log.Warn("Disabled debug server due to missing api-listen-addr setting in config file.") + } else { + a.log.Warn("SECURITY WARNING! Debug server is enabled. It should only be used with caution and proper security.") + ln, err := net.Listen("tcp", debugAddr) + if err != nil { + a.log.Error( + "Failed to listen on debug address. If you have another relayer process open, use --" + + debugAddr + + " to pick a different address or port.", + ) + + return fmt.Errorf("failed to listen on debug address %q: %w", debugAddr, err) + } + log := a.log.With(zap.String("sys", "debughttp")) + log.Info("Debug server listening", zap.String("addr", debugAddr)) + relaydebug.StartDebugServer(cmd.Context(), log, ln) + } + return nil +} diff --git a/cmd/start_test.go b/cmd/start_test.go new file mode 100644 index 000000000..df7f1c982 --- /dev/null +++ b/cmd/start_test.go @@ -0,0 +1,245 @@ +package cmd_test + +import ( + "fmt" + "net" + "net/http" + "os" + "strings" + "testing" + + "github.com/cosmos/relayer/v2/cmd" + "github.com/cosmos/relayer/v2/internal/relaydebug" + "github.com/cosmos/relayer/v2/internal/relayermetrics" + "github.com/cosmos/relayer/v2/internal/relayertest" + "github.com/cosmos/relayer/v2/relayer/chains/cosmos" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" +) + +func TestMetricsServerFlag(t *testing.T) { + t.Parallel() + + tests := []struct { + args []string + wantedRunning bool + }{ + { + []string{"start"}, + false, + }, + { + []string{"start", "--enable-metrics-server"}, + true, + }, + } + + for _, tt := range tests { + t.Run(strings.Join(tt.args, " "), func(t *testing.T) { + sys := setupRelayer(t) + logs, logger := setupLogger() + + sys.MustRunWithLogger(t, logger, tt.args...) + + if tt.wantedRunning == true { + requireRunningMetricsServer(t, logs, relayermetrics.MetricsServerPort) + } else { + requireDisabledMetricsServer(t, logs, relayermetrics.MetricsServerPort) + } + }) + } +} + +func TestMetricsServerConfig(t *testing.T) { + t.Parallel() + + tests := []struct { + args []string + newSetting string + wantedPort int + serverRunning bool + }{ + { + []string{"start"}, + "", + 0, + false, + }, + { + []string{"start", "--enable-metrics-server"}, + "metrics-listen-addr: 127.0.0.1:6184", + 6184, + true, + }, + } + + for _, tt := range tests { + t.Run(strings.Join(tt.args, " "), func(t *testing.T) { + sys := setupRelayer(t) + + updateConfig(t, sys, "metrics-listen-addr: 127.0.0.1:5184", tt.newSetting) + + logs, logger := setupLogger() + + sys.MustRunWithLogger(t, logger, tt.args...) + + if tt.serverRunning == true { + requireRunningMetricsServer(t, logs, tt.wantedPort) + } else { + requireDisabledMetricsServer(t, logs, tt.wantedPort) + } + }) + } +} + +func TestDebugServerFlag(t *testing.T) { + t.Parallel() + + tests := []struct { + args []string + wantedRunning bool + }{ + { + []string{"start"}, + false, + }, + { + []string{"start", "--enable-debug-server"}, + true, + }, + } + + for _, tt := range tests { + t.Run(strings.Join(tt.args, " "), func(t *testing.T) { + sys := setupRelayer(t) + logs, logger := setupLogger() + + sys.MustRunWithLogger(t, logger, tt.args...) + + if tt.wantedRunning == true { + requireRunningDebugServer(t, logs, relaydebug.DebugServerPort) + } else { + requireDisabledDebugServer(t, logs, relaydebug.DebugServerPort) + } + }) + } +} + +func TestDebugServerConfig(t *testing.T) { + t.Parallel() + + tests := []struct { + args []string + newSetting string + wantedPort int + wantedRunning bool + }{ + { + []string{"start"}, + "", + 0, + false, + }, + { + []string{"start", "--enable-debug-server"}, + "api-listen-addr: 127.0.0.1:6183", + 6183, + true, + }, + } + + for _, tt := range tests { + t.Run(strings.Join(tt.args, " "), func(t *testing.T) { + sys := setupRelayer(t) + + updateConfig(t, sys, "api-listen-addr: 127.0.0.1:5183", tt.newSetting) + + logs, logger := setupLogger() + + sys.MustRunWithLogger(t, logger, tt.args...) + + if tt.wantedRunning == true { + requireRunningDebugServer(t, logs, tt.wantedPort) + } else { + requireDisabledDebugServer(t, logs, tt.wantedPort) + } + }) + } +} + +func requireDisabledMetricsServer(t *testing.T, logs *observer.ObservedLogs, port int) { + conn, err := net.Dial("tcp", fmt.Sprintf("127.0.0.1:%d", port)) + if conn != nil { + defer conn.Close() + } + require.Error(t, err, "Server should be disabled") + require.Len(t, logs.FilterMessage("Disabled debug server due to missing api-listen-addr setting in config file.").All(), 1) +} + +func requireRunningMetricsServer(t *testing.T, logs *observer.ObservedLogs, port int) { + conn, err := net.Dial("tcp", fmt.Sprintf("127.0.0.1:%d", port)) + if conn != nil { + defer conn.Close() + } + require.NoError(t, err, "Metrics server should be running") + res, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/metrics", port)) + require.NoError(t, err) + require.Equal(t, res.StatusCode, 200) + require.Len(t, logs.FilterMessage("Metrics server listening").All(), 1) + require.Len(t, logs.FilterMessage("Metrics server is enabled.").All(), 1) +} + +func requireDisabledDebugServer(t *testing.T, logs *observer.ObservedLogs, port int) { + conn, err := net.Dial("tcp", fmt.Sprintf("127.0.0.1:%d", port)) + if conn != nil { + defer conn.Close() + } + require.Error(t, err, "Server should be disabled") + require.Len(t, logs.FilterMessage("Disabled debug server due to missing api-listen-addr setting in config file.").All(), 1) +} + +func requireRunningDebugServer(t *testing.T, logs *observer.ObservedLogs, port int) { + conn, err := net.Dial("tcp", fmt.Sprintf("127.0.0.1:%d", port)) + if conn != nil { + defer conn.Close() + } + require.NoError(t, err, "Server should be running") + res, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/debug/pprof/goroutine", port)) + require.NoError(t, err) + require.Equal(t, res.StatusCode, 200) + require.Len(t, logs.FilterMessage("Debug server listening").All(), 1) + require.Len(t, logs.FilterMessage("SECURITY WARNING! Debug server is enabled. It should only be used with caution and proper security.").All(), 1) +} + +func setupLogger() (*observer.ObservedLogs, *zap.Logger) { + observedZapCore, observedLogs := observer.New(zap.InfoLevel) + observedLogger := zap.New(observedZapCore) + return observedLogs, observedLogger +} + +func setupRelayer(t *testing.T) *relayertest.System { + sys := relayertest.NewSystem(t) + + _ = sys.MustRun(t, "config", "init") + + sys.MustAddChain(t, "testChain", cmd.ProviderConfigWrapper{ + Type: "cosmos", + Value: cosmos.CosmosProviderConfig{ + ChainID: "testcosmos", + KeyringBackend: "test", + Timeout: "10s", + }, + }) + return sys +} + +func updateConfig(t *testing.T, sys *relayertest.System, oldSetting string, newSetting string) { + configFile := fmt.Sprintf("%s/config/config.yaml", sys.HomeDir) + data, err := os.ReadFile(configFile) + require.NoError(t, err) + + newConfig := strings.Replace(string(data), oldSetting, newSetting, 1) + + os.WriteFile(configFile, []byte(newConfig), 0644) +} diff --git a/internal/relaydebug/debugserver.go b/internal/relaydebug/debugserver.go index 2ae8aae63..3eed17342 100644 --- a/internal/relaydebug/debugserver.go +++ b/internal/relaydebug/debugserver.go @@ -9,6 +9,8 @@ import ( "go.uber.org/zap" ) +const DebugServerPort = 5183 + // StartDebugServer starts a debug server in a background goroutine, // accepting connections on the given listener. // Any HTTP logging will be written at info level to the given logger. diff --git a/internal/relayermetrics/metricsserver.go b/internal/relayermetrics/metricsserver.go index 74873a039..fc2183fe4 100644 --- a/internal/relayermetrics/metricsserver.go +++ b/internal/relayermetrics/metricsserver.go @@ -10,6 +10,8 @@ import ( "go.uber.org/zap" ) +const MetricsServerPort = 5184 + // StartMetricsServer starts a metrics server in a background goroutine, // accepting connections on the given listener. // Any HTTP logging will be written at info level to the given logger. diff --git a/internal/relayertest/system.go b/internal/relayertest/system.go index 59daad624..11467cd9b 100644 --- a/internal/relayertest/system.go +++ b/internal/relayertest/system.go @@ -106,6 +106,20 @@ func (s *System) MustRunWithInput(t *testing.T, in io.Reader, args ...string) Ru return res } +func (s *System) MustRunWithLogger(t *testing.T, logger *zap.Logger, args ...string) RunResult { + t.Helper() + + res := s.RunWithInput(logger, bytes.NewReader(nil), args...) + if res.Err != nil { + t.Logf("Error executing %v: %v", args, res.Err) + t.Logf("Stdout: %q", res.Stdout.String()) + t.Logf("Stderr: %q", res.Stderr.String()) + t.FailNow() + } + + return res +} + // MustAddChain serializes pcw to disk and calls "chains add --file". func (s *System) MustAddChain(t *testing.T, chainName string, pcw cmd.ProviderConfigWrapper) { t.Helper() From ffa1ef39d5c8ed3ee306ee5d10f8a96c666fd85e Mon Sep 17 00:00:00 2001 From: Andy Pliszka Date: Thu, 19 Sep 2024 11:34:16 -0400 Subject: [PATCH 05/12] refactor: renames api-listen-addr to debug-listen-addr --- cmd/config.go | 4 +-- cmd/config_test.go | 56 ++++++++++++++++++++++++++++++++++++ cmd/start.go | 4 +-- cmd/start_test.go | 8 +++--- examples/config_EXAMPLE.yaml | 2 +- 5 files changed, 65 insertions(+), 9 deletions(-) create mode 100644 cmd/config_test.go diff --git a/cmd/config.go b/cmd/config.go index 2344a9d63..3f3c9c8c2 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -488,7 +488,7 @@ func DefaultConfig(memo string) *Config { // GlobalConfig describes any global relayer settings type GlobalConfig struct { - APIListenPort string `yaml:"api-listen-addr" json:"api-listen-addr"` + DebugListenPort string `yaml:"debug-listen-addr" json:"debug-listen-addr"` MetricsListenPort string `yaml:"metrics-listen-addr" json:"metrics-listen-addr"` Timeout string `yaml:"timeout" json:"timeout"` Memo string `yaml:"memo" json:"memo"` @@ -501,7 +501,7 @@ type GlobalConfig struct { // newDefaultGlobalConfig returns a global config with defaults set func newDefaultGlobalConfig(memo string) GlobalConfig { return GlobalConfig{ - APIListenPort: "127.0.0.1:5183", + DebugListenPort: "127.0.0.1:5183", MetricsListenPort: "127.0.0.1:5184", Timeout: "10s", LightCacheSize: 20, diff --git a/cmd/config_test.go b/cmd/config_test.go new file mode 100644 index 000000000..767ac4624 --- /dev/null +++ b/cmd/config_test.go @@ -0,0 +1,56 @@ +package cmd_test + +import ( + "fmt" + "os" + "testing" + + "github.com/cosmos/relayer/v2/cmd" + "github.com/cosmos/relayer/v2/internal/relayertest" + "github.com/cosmos/relayer/v2/relayer/chains/cosmos" + "github.com/stretchr/testify/require" +) + +func TestDefaultConfig(t *testing.T) { + t.Parallel() + + sys := relayertest.NewSystem(t) + + _ = sys.MustRun(t, "config", "init") + + sys.MustAddChain(t, "testChain", cmd.ProviderConfigWrapper{ + Type: "cosmos", + Value: cosmos.CosmosProviderConfig{ + ChainID: "testcosmos", + KeyringBackend: "test", + Timeout: "10s", + }, + }) + + tests := []struct { + setting string + wantedPresent bool + }{ + { + "debug-listen-addr: 127.0.0.1:5183", + true, + }, + { + "metrics-listen-addr: 127.0.0.1:5184", + true, + }, + } + + for _, tt := range tests { + t.Run(tt.setting, func(t *testing.T) { + sys := setupRelayer(t) + + configFile := fmt.Sprintf("%s/config/config.yaml", sys.HomeDir) + data, err := os.ReadFile(configFile) + require.NoError(t, err) + config := string(data) + + require.Contains(t, config, tt.setting) + }) + } +} diff --git a/cmd/start.go b/cmd/start.go index ff26dfacb..c233109f0 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -210,7 +210,7 @@ func setupMetricsServer(cmd *cobra.Command, a *appState, err error, chains map[s } func setupDebugServer(cmd *cobra.Command, a *appState, err error) error { - debugAddr := a.config.Global.APIListenPort + debugAddr := a.config.Global.DebugListenPort flagEnableDebugServer, err := cmd.Flags().GetBool(flagEnableDebugServer) if err != nil { @@ -218,7 +218,7 @@ func setupDebugServer(cmd *cobra.Command, a *appState, err error) error { } if flagEnableDebugServer == false || debugAddr == "" { - a.log.Warn("Disabled debug server due to missing api-listen-addr setting in config file.") + a.log.Warn("Disabled debug server due to missing debug-listen-addr setting in config file.") } else { a.log.Warn("SECURITY WARNING! Debug server is enabled. It should only be used with caution and proper security.") ln, err := net.Listen("tcp", debugAddr) diff --git a/cmd/start_test.go b/cmd/start_test.go index df7f1c982..1301e82b0 100644 --- a/cmd/start_test.go +++ b/cmd/start_test.go @@ -143,7 +143,7 @@ func TestDebugServerConfig(t *testing.T) { }, { []string{"start", "--enable-debug-server"}, - "api-listen-addr: 127.0.0.1:6183", + "debug-listen-addr: 127.0.0.1:6183", 6183, true, }, @@ -153,7 +153,7 @@ func TestDebugServerConfig(t *testing.T) { t.Run(strings.Join(tt.args, " "), func(t *testing.T) { sys := setupRelayer(t) - updateConfig(t, sys, "api-listen-addr: 127.0.0.1:5183", tt.newSetting) + updateConfig(t, sys, "debug-listen-addr: 127.0.0.1:5183", tt.newSetting) logs, logger := setupLogger() @@ -174,7 +174,7 @@ func requireDisabledMetricsServer(t *testing.T, logs *observer.ObservedLogs, por defer conn.Close() } require.Error(t, err, "Server should be disabled") - require.Len(t, logs.FilterMessage("Disabled debug server due to missing api-listen-addr setting in config file.").All(), 1) + require.Len(t, logs.FilterMessage("Disabled debug server due to missing debug-listen-addr setting in config file.").All(), 1) } func requireRunningMetricsServer(t *testing.T, logs *observer.ObservedLogs, port int) { @@ -196,7 +196,7 @@ func requireDisabledDebugServer(t *testing.T, logs *observer.ObservedLogs, port defer conn.Close() } require.Error(t, err, "Server should be disabled") - require.Len(t, logs.FilterMessage("Disabled debug server due to missing api-listen-addr setting in config file.").All(), 1) + require.Len(t, logs.FilterMessage("Disabled debug server due to missing debug-listen-addr setting in config file.").All(), 1) } func requireRunningDebugServer(t *testing.T, logs *observer.ObservedLogs, port int) { diff --git a/examples/config_EXAMPLE.yaml b/examples/config_EXAMPLE.yaml index e3a67f79c..ff78a5145 100644 --- a/examples/config_EXAMPLE.yaml +++ b/examples/config_EXAMPLE.yaml @@ -1,5 +1,5 @@ global: - api-listen-addr: 127.0.0.1:5183 + debug-listen-addr: 127.0.0.1:5183 metrics-listen-addr: 127.0.0.1:5184 timeout: 10s memo: "" From fd2fc8506b5f1af908d89ec8c1a334f60ccc4c84 Mon Sep 17 00:00:00 2001 From: Andy Pliszka Date: Thu, 19 Sep 2024 13:52:23 -0400 Subject: [PATCH 06/12] feat: restores cli flags to set addresses for debug and metrics servers * restores address flags * renames `--debug-addr` to `--debug-listen-addr` to be consistent with config file setting `debug-listen-addr:` * renames `--metrics-addr` to `--metrics-listen-addr` to be consistent with config file setting `metrics-listen-addr:` * updates docs --- README.md | 2 +- cmd/flags.go | 24 ++++++++++++++++++++++ cmd/start.go | 42 ++++++++++++++++++++++++++++----------- cmd/start_test.go | 30 ++++++++++++++++++++++++---- docs/advanced_usage.md | 6 ++++-- docs/troubleshooting.md | 6 +++--- interchaintest/relayer.go | 2 +- 7 files changed, 89 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 3c9d2e5df..095fcd323 100644 --- a/README.md +++ b/README.md @@ -193,7 +193,7 @@ Additional information on how IBC works can be found [here](https://ibc.cosmos.n $ rly start ``` - >When running multiple instances of `rly start`, you will need to use the `--debug-addr` flag and provide an address:port. You can also pass an empty string `''` to turn off this feature or pass `localhost:0` to randomly select a port. + >When running multiple instances of `rly start`, you will need to use the `--debug-listen-addr` flag and provide an address:port. You can also pass an empty string `''` to turn off this feature or pass `localhost:0` to randomly select a port. --- [[TROUBLESHOOTING](docs/troubleshooting.md)] diff --git a/cmd/flags.go b/cmd/flags.go index b52d9674e..500b2287f 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -39,7 +39,9 @@ const ( flagOrder = "order" flagVersion = "version" flagEnableDebugServer = "enable-debug-server" + flagDebugListenAddr = "debug-listen-addr" flagEnableMetricsServer = "enable-metrics-server" + flagMetricsListenAddr = "metrics-listen-addr" flagOverwriteConfig = "overwrite" flagLimit = "limit" flagHeight = "height" @@ -419,6 +421,17 @@ func dstPortFlag(v *viper.Viper, cmd *cobra.Command) *cobra.Command { } func debugServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command { + cmd.Flags().String( + flagDebugListenAddr, + "", + "address to use for debug and metrics server. By default, "+ + "will be the debug-listen-addr parameter in the global config.", + ) + + if err := v.BindPFlag(flagDebugListenAddr, cmd.Flags().Lookup(flagDebugListenAddr)); err != nil { + panic(err) + } + cmd.Flags().Bool( flagEnableDebugServer, false, @@ -433,6 +446,17 @@ func debugServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command { } func metricsServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command { + cmd.Flags().String( + flagMetricsListenAddr, + "", + "address to use for metrics server. By default, "+ + "will be the metrics-listen-addr parameter in the global config.", + ) + + if err := v.BindPFlag(flagMetricsListenAddr, cmd.Flags().Lookup(flagMetricsListenAddr)); err != nil { + panic(err) + } + cmd.Flags().Bool( flagEnableMetricsServer, false, diff --git a/cmd/start.go b/cmd/start.go index c233109f0..8fe471d73 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -175,29 +175,38 @@ $ %s start demo-path2 --max-tx-size 10`, appName, appName, appName, appName)), func setupMetricsServer(cmd *cobra.Command, a *appState, err error, chains map[string]*relayer.Chain) (*processor.PrometheusMetrics, error) { var prometheusMetrics *processor.PrometheusMetrics - metricsAddr := a.config.Global.MetricsListenPort + metricsListenAddr := a.config.Global.MetricsListenPort + + metricsListenAddrFlag, err := cmd.Flags().GetString(flagMetricsListenAddr) + if err != nil { + return nil, err + } + + if metricsListenAddrFlag != "" { + metricsListenAddr = metricsListenAddrFlag + } flagEnableMetricsServer, err := cmd.Flags().GetBool(flagEnableMetricsServer) if err != nil { return nil, err } - if flagEnableMetricsServer == false || metricsAddr == "" { + if flagEnableMetricsServer == false || metricsListenAddr == "" { a.log.Warn("Disabled metrics server due to missing metrics-listen-addr setting in config file.") } else { a.log.Info("Metrics server is enabled.") - ln, err := net.Listen("tcp", metricsAddr) + ln, err := net.Listen("tcp", metricsListenAddr) if err != nil { a.log.Error( "Failed to listen on metrics address. If you have another relayer process open, use --" + - metricsAddr + + metricsListenAddr + " to pick a different address or port.", ) - return nil, fmt.Errorf("failed to listen on metrics address %q: %w", metricsAddr, err) + return nil, fmt.Errorf("failed to listen on metrics address %q: %w", metricsListenAddr, err) } log := a.log.With(zap.String("sys", "metricshttp")) - log.Info("Metrics server listening", zap.String("addr", metricsAddr)) + log.Info("Metrics server listening", zap.String("addr", metricsListenAddr)) prometheusMetrics = processor.NewPrometheusMetrics() relayermetrics.StartMetricsServer(cmd.Context(), log, ln, prometheusMetrics.Registry) for _, chain := range chains { @@ -210,29 +219,38 @@ func setupMetricsServer(cmd *cobra.Command, a *appState, err error, chains map[s } func setupDebugServer(cmd *cobra.Command, a *appState, err error) error { - debugAddr := a.config.Global.DebugListenPort + debugListenAddr := a.config.Global.DebugListenPort + + debugListenAddrFlag, err := cmd.Flags().GetString(flagDebugListenAddr) + if err != nil { + return err + } + + if debugListenAddrFlag != "" { + debugListenAddr = debugListenAddrFlag + } flagEnableDebugServer, err := cmd.Flags().GetBool(flagEnableDebugServer) if err != nil { return err } - if flagEnableDebugServer == false || debugAddr == "" { + if flagEnableDebugServer == false || debugListenAddr == "" { a.log.Warn("Disabled debug server due to missing debug-listen-addr setting in config file.") } else { a.log.Warn("SECURITY WARNING! Debug server is enabled. It should only be used with caution and proper security.") - ln, err := net.Listen("tcp", debugAddr) + ln, err := net.Listen("tcp", debugListenAddr) if err != nil { a.log.Error( "Failed to listen on debug address. If you have another relayer process open, use --" + - debugAddr + + debugListenAddr + " to pick a different address or port.", ) - return fmt.Errorf("failed to listen on debug address %q: %w", debugAddr, err) + return fmt.Errorf("failed to listen on debug address %q: %w", debugListenAddr, err) } log := a.log.With(zap.String("sys", "debughttp")) - log.Info("Debug server listening", zap.String("addr", debugAddr)) + log.Info("Debug server listening", zap.String("addr", debugListenAddr)) relaydebug.StartDebugServer(cmd.Context(), log, ln) } return nil diff --git a/cmd/start_test.go b/cmd/start_test.go index 1301e82b0..f3520efaf 100644 --- a/cmd/start_test.go +++ b/cmd/start_test.go @@ -23,14 +23,22 @@ func TestMetricsServerFlag(t *testing.T) { tests := []struct { args []string + wantedPort int wantedRunning bool }{ { []string{"start"}, + 0, false, }, { []string{"start", "--enable-metrics-server"}, + relayermetrics.MetricsServerPort, + true, + }, + { + []string{"start", "--enable-metrics-server", "--metrics-listen-addr", "127.0.0.1:7778"}, + 7778, true, }, } @@ -43,9 +51,9 @@ func TestMetricsServerFlag(t *testing.T) { sys.MustRunWithLogger(t, logger, tt.args...) if tt.wantedRunning == true { - requireRunningMetricsServer(t, logs, relayermetrics.MetricsServerPort) + requireRunningMetricsServer(t, logs, tt.wantedPort) } else { - requireDisabledMetricsServer(t, logs, relayermetrics.MetricsServerPort) + requireDisabledMetricsServer(t, logs, tt.wantedPort) } }) } @@ -72,6 +80,12 @@ func TestMetricsServerConfig(t *testing.T) { 6184, true, }, + { + []string{"start", "--enable-metrics-server", "--metrics-listen-addr", "127.0.0.1:7184"}, + "", + 7184, + true, + }, } for _, tt := range tests { @@ -98,14 +112,22 @@ func TestDebugServerFlag(t *testing.T) { tests := []struct { args []string + wantedPort int wantedRunning bool }{ { []string{"start"}, + 0, false, }, { []string{"start", "--enable-debug-server"}, + relaydebug.DebugServerPort, + true, + }, + { + []string{"start", "--enable-debug-server", "--debug-listen-addr", "127.0.0.1:7777"}, + 7777, true, }, } @@ -118,9 +140,9 @@ func TestDebugServerFlag(t *testing.T) { sys.MustRunWithLogger(t, logger, tt.args...) if tt.wantedRunning == true { - requireRunningDebugServer(t, logs, relaydebug.DebugServerPort) + requireRunningDebugServer(t, logs, tt.wantedPort) } else { - requireDisabledDebugServer(t, logs, relaydebug.DebugServerPort) + requireDisabledDebugServer(t, logs, tt.wantedPort) } }) } diff --git a/docs/advanced_usage.md b/docs/advanced_usage.md index def6c6b96..91fc94f0b 100644 --- a/docs/advanced_usage.md +++ b/docs/advanced_usage.md @@ -4,8 +4,10 @@ **Prometheus exporter** -If you started `rly` with the default `--debug-addr` argument, -you can use `http://$IP:5183/relayer/metrics` as a target for your prometheus scraper. +If you started `rly` with `--enable-metrics-server` argument, +you can use `http://127.0.0.1:5184/relayer/metrics` as a target for your prometheus scraper. + +You can use `--metrics-listen-addr $IP:7777` to customize the address and port where the metrics server. Exported metrics: diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 1988a16fa..af7c5ca21 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -65,11 +65,11 @@ $ rly query clients-expiration ## Inspect Go runtime debug data -If you started `rly` with the default `--debug-addr` argument, -you can open `http://localhost:7597` in your browser to explore details from the Go runtime. +If you started `rly` with `--enable-debug-server` argument, +you can open `http://127.0.0.1:5183` in your browser to explore details from the Go runtime. If you need active assistance from the Relayer development team regarding an unresponsive Relayer instance, -it will be helpful to provide the output from `http://localhost:7597/debug/pprof/goroutine?debug=2` at a minimum. +it will be helpful to provide the output from `http://127.0.0.1:7597/debug/pprof/goroutine?debug=2` at a minimum.
diff --git a/interchaintest/relayer.go b/interchaintest/relayer.go index 0aa4b1945..2ece6b085 100644 --- a/interchaintest/relayer.go +++ b/interchaintest/relayer.go @@ -290,7 +290,7 @@ func (r *Relayer) start(ctx context.Context, remainingArgs ...string) { // Start the debug server on a random port. // It won't be reachable without introspecting the output, // but this will allow catching any possible data races around the debug server. - args := append([]string{"start", "--debug-addr", "localhost:0"}, remainingArgs...) + args := append([]string{"start", "--enable-debug-server", "--debug-listen-addr", "localhost:0", "--enable-metrics-server", "--metrics-listen-addr", "localhost:0"}, remainingArgs...) res := r.Sys().RunC(ctx, r.log(), args...) if res.Err != nil { r.errCh <- res.Err From 57923f4c75770729cd162ae3a42238703b7f2704 Mon Sep 17 00:00:00 2001 From: Andy Pliszka Date: Thu, 19 Sep 2024 16:50:47 -0400 Subject: [PATCH 07/12] feat: improves error reporting --- cmd/flags.go | 6 ++-- cmd/start.go | 17 ++++++---- cmd/start_test.go | 83 ++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 82 insertions(+), 24 deletions(-) diff --git a/cmd/flags.go b/cmd/flags.go index 500b2287f..aa78be2eb 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -425,7 +425,8 @@ func debugServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command { flagDebugListenAddr, "", "address to use for debug and metrics server. By default, "+ - "will be the debug-listen-addr parameter in the global config.", + "will be the debug-listen-addr parameter in the global config. "+ + "Make sure to enable debug server using --enable-debug-server flag.", ) if err := v.BindPFlag(flagDebugListenAddr, cmd.Flags().Lookup(flagDebugListenAddr)); err != nil { @@ -450,7 +451,8 @@ func metricsServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command { flagMetricsListenAddr, "", "address to use for metrics server. By default, "+ - "will be the metrics-listen-addr parameter in the global config.", + "will be the metrics-listen-addr parameter in the global config. "+ + "Make sure to enable metrics server using --enable-metrics-server flag.", ) if err := v.BindPFlag(flagMetricsListenAddr, cmd.Flags().Lookup(flagMetricsListenAddr)); err != nil { diff --git a/cmd/start.go b/cmd/start.go index 8fe471d73..a289adbd6 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -191,10 +191,12 @@ func setupMetricsServer(cmd *cobra.Command, a *appState, err error, chains map[s return nil, err } - if flagEnableMetricsServer == false || metricsListenAddr == "" { - a.log.Warn("Disabled metrics server due to missing metrics-listen-addr setting in config file.") + if flagEnableMetricsServer == false { + a.log.Info("Metrics server is disabled. You can enable it using --enable-metrics-server flag.") + } else if metricsListenAddr == "" { + a.log.Warn("Disabled metrics server due to missing metrics-listen-addr setting in config file or --metrics-listen-addr flag.") } else { - a.log.Info("Metrics server is enabled.") + a.log.Info("Metrics server is enabled") ln, err := net.Listen("tcp", metricsListenAddr) if err != nil { a.log.Error( @@ -235,10 +237,13 @@ func setupDebugServer(cmd *cobra.Command, a *appState, err error) error { return err } - if flagEnableDebugServer == false || debugListenAddr == "" { - a.log.Warn("Disabled debug server due to missing debug-listen-addr setting in config file.") + if flagEnableDebugServer == false { + a.log.Info("Debug server is disabled. You can enable it using --enable-debug-server flag.") + } else if debugListenAddr == "" { + a.log.Warn("Disabled debug server due to missing debug-listen-addr setting in config file or --debug-listen-addr flag.") } else { - a.log.Warn("SECURITY WARNING! Debug server is enabled. It should only be used with caution and proper security.") + a.log.Info("Debug server is enabled") + a.log.Warn("SECURITY WARNING! Debug server should only be run with caution and proper security in place.") ln, err := net.Listen("tcp", debugListenAddr) if err != nil { a.log.Error( diff --git a/cmd/start_test.go b/cmd/start_test.go index f3520efaf..90c4a97dc 100644 --- a/cmd/start_test.go +++ b/cmd/start_test.go @@ -22,24 +22,34 @@ func TestMetricsServerFlag(t *testing.T) { t.Parallel() tests := []struct { - args []string - wantedPort int - wantedRunning bool + args []string + wantedPort int + wantedRunning bool + wantedMessages []string }{ { []string{"start"}, 0, false, + []string{"Metrics server is disabled. You can enable it using --enable-metrics-server flag."}, }, { []string{"start", "--enable-metrics-server"}, relayermetrics.MetricsServerPort, true, + []string{"Metrics server is enabled", "Metrics server listening"}, }, { []string{"start", "--enable-metrics-server", "--metrics-listen-addr", "127.0.0.1:7778"}, 7778, true, + []string{"Metrics server is enabled", "Metrics server listening"}, + }, + { + []string{"start", "--metrics-listen-addr", "127.0.0.1:7778"}, + 0, + false, + []string{"Metrics server is disabled. You can enable it using --enable-metrics-server flag."}, }, } @@ -55,6 +65,7 @@ func TestMetricsServerFlag(t *testing.T) { } else { requireDisabledMetricsServer(t, logs, tt.wantedPort) } + requireMessages(t, logs, tt.wantedMessages) }) } } @@ -63,28 +74,32 @@ func TestMetricsServerConfig(t *testing.T) { t.Parallel() tests := []struct { - args []string - newSetting string - wantedPort int - serverRunning bool + args []string + newSetting string + wantedPort int + serverRunning bool + wantedMessages []string }{ { []string{"start"}, "", 0, false, + []string{"Metrics server is disabled. You can enable it using --enable-metrics-server flag."}, }, { []string{"start", "--enable-metrics-server"}, "metrics-listen-addr: 127.0.0.1:6184", 6184, true, + []string{"Metrics server is enabled", "Metrics server listening"}, }, { []string{"start", "--enable-metrics-server", "--metrics-listen-addr", "127.0.0.1:7184"}, "", 7184, true, + []string{"Metrics server is enabled", "Metrics server listening"}, }, } @@ -103,32 +118,50 @@ func TestMetricsServerConfig(t *testing.T) { } else { requireDisabledMetricsServer(t, logs, tt.wantedPort) } + requireMessages(t, logs, tt.wantedMessages) }) } } +func TestMissingMetricsListenAddr(t *testing.T) { + sys := setupRelayer(t) + + logs, logger := setupLogger() + + updateConfig(t, sys, "metrics-listen-addr: 127.0.0.1:5184", "") + + sys.MustRunWithLogger(t, logger, []string{"start", "--enable-metrics-server"}...) + + requireDisabledMetricsServer(t, logs, 0) + requireMessage(t, logs, "Disabled metrics server due to missing metrics-listen-addr setting in config file or --metrics-listen-addr flag.") +} + func TestDebugServerFlag(t *testing.T) { t.Parallel() tests := []struct { - args []string - wantedPort int - wantedRunning bool + args []string + wantedPort int + wantedRunning bool + wantedMessages []string }{ { []string{"start"}, 0, false, + []string{"Debug server is disabled. You can enable it using --enable-debug-server flag."}, }, { []string{"start", "--enable-debug-server"}, relaydebug.DebugServerPort, true, + []string{"Debug server is enabled", "SECURITY WARNING! Debug server should only be run with caution and proper security in place."}, }, { []string{"start", "--enable-debug-server", "--debug-listen-addr", "127.0.0.1:7777"}, 7777, true, + []string{"Debug server is enabled", "SECURITY WARNING! Debug server should only be run with caution and proper security in place."}, }, } @@ -144,10 +177,24 @@ func TestDebugServerFlag(t *testing.T) { } else { requireDisabledDebugServer(t, logs, tt.wantedPort) } + requireMessages(t, logs, tt.wantedMessages) }) } } +func TestMissingDebugListenAddr(t *testing.T) { + sys := setupRelayer(t) + + logs, logger := setupLogger() + + updateConfig(t, sys, "debug-listen-addr: 127.0.0.1:5183", "") + + sys.MustRunWithLogger(t, logger, []string{"start", "--enable-debug-server"}...) + + requireDisabledMetricsServer(t, logs, 0) + requireMessage(t, logs, "Disabled debug server due to missing debug-listen-addr setting in config file or --debug-listen-addr flag.") +} + func TestDebugServerConfig(t *testing.T) { t.Parallel() @@ -196,7 +243,6 @@ func requireDisabledMetricsServer(t *testing.T, logs *observer.ObservedLogs, por defer conn.Close() } require.Error(t, err, "Server should be disabled") - require.Len(t, logs.FilterMessage("Disabled debug server due to missing debug-listen-addr setting in config file.").All(), 1) } func requireRunningMetricsServer(t *testing.T, logs *observer.ObservedLogs, port int) { @@ -208,8 +254,6 @@ func requireRunningMetricsServer(t *testing.T, logs *observer.ObservedLogs, port res, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/metrics", port)) require.NoError(t, err) require.Equal(t, res.StatusCode, 200) - require.Len(t, logs.FilterMessage("Metrics server listening").All(), 1) - require.Len(t, logs.FilterMessage("Metrics server is enabled.").All(), 1) } func requireDisabledDebugServer(t *testing.T, logs *observer.ObservedLogs, port int) { @@ -218,7 +262,6 @@ func requireDisabledDebugServer(t *testing.T, logs *observer.ObservedLogs, port defer conn.Close() } require.Error(t, err, "Server should be disabled") - require.Len(t, logs.FilterMessage("Disabled debug server due to missing debug-listen-addr setting in config file.").All(), 1) } func requireRunningDebugServer(t *testing.T, logs *observer.ObservedLogs, port int) { @@ -230,8 +273,16 @@ func requireRunningDebugServer(t *testing.T, logs *observer.ObservedLogs, port i res, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/debug/pprof/goroutine", port)) require.NoError(t, err) require.Equal(t, res.StatusCode, 200) - require.Len(t, logs.FilterMessage("Debug server listening").All(), 1) - require.Len(t, logs.FilterMessage("SECURITY WARNING! Debug server is enabled. It should only be used with caution and proper security.").All(), 1) +} + +func requireMessages(t *testing.T, logs *observer.ObservedLogs, messages []string) { + for _, message := range messages { + requireMessage(t, logs, message) + } +} + +func requireMessage(t *testing.T, logs *observer.ObservedLogs, message string) { + require.Len(t, logs.FilterMessage(message).All(), 1, fmt.Sprintf("Expected message '%s' not found in logs", message), logs.All()) } func setupLogger() (*observer.ObservedLogs, *zap.Logger) { From b0e25257d1a2ae038b89e864e0534d2e53494ab4 Mon Sep 17 00:00:00 2001 From: Andy Pliszka Date: Fri, 20 Sep 2024 09:38:50 -0400 Subject: [PATCH 08/12] chore: code cleanup --- README.md | 2 +- cmd/flags.go | 2 +- cmd/start.go | 12 ++---------- cmd/start_test.go | 30 +++++++++++++++--------------- 4 files changed, 19 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 095fcd323..88e296a93 100644 --- a/README.md +++ b/README.md @@ -193,7 +193,7 @@ Additional information on how IBC works can be found [here](https://ibc.cosmos.n $ rly start ``` - >When running multiple instances of `rly start`, you will need to use the `--debug-listen-addr` flag and provide an address:port. You can also pass an empty string `''` to turn off this feature or pass `localhost:0` to randomly select a port. + >When running multiple instances of `rly start`, you will need to use the `--enable-debug-server` and `--debug-listen-addr` flag and provide an address:port. You can also pass an empty string `''` to turn off this feature or pass `localhost:0` to randomly select a port. --- [[TROUBLESHOOTING](docs/troubleshooting.md)] diff --git a/cmd/flags.go b/cmd/flags.go index aa78be2eb..c87c90c5b 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -424,7 +424,7 @@ func debugServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command { cmd.Flags().String( flagDebugListenAddr, "", - "address to use for debug and metrics server. By default, "+ + "address to use for debug server. By default, "+ "will be the debug-listen-addr parameter in the global config. "+ "Make sure to enable debug server using --enable-debug-server flag.", ) diff --git a/cmd/start.go b/cmd/start.go index a289adbd6..f36374d3c 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -199,11 +199,7 @@ func setupMetricsServer(cmd *cobra.Command, a *appState, err error, chains map[s a.log.Info("Metrics server is enabled") ln, err := net.Listen("tcp", metricsListenAddr) if err != nil { - a.log.Error( - "Failed to listen on metrics address. If you have another relayer process open, use --" + - metricsListenAddr + - " to pick a different address or port.", - ) + a.log.Error(fmt.Sprintf("Failed to start metrics server. You can change the address and port using metrics-listen-addr config settingh or --metrics-listen-flag.")) return nil, fmt.Errorf("failed to listen on metrics address %q: %w", metricsListenAddr, err) } @@ -246,11 +242,7 @@ func setupDebugServer(cmd *cobra.Command, a *appState, err error) error { a.log.Warn("SECURITY WARNING! Debug server should only be run with caution and proper security in place.") ln, err := net.Listen("tcp", debugListenAddr) if err != nil { - a.log.Error( - "Failed to listen on debug address. If you have another relayer process open, use --" + - debugListenAddr + - " to pick a different address or port.", - ) + a.log.Error(fmt.Sprintf("Failed to start debug server. You can change the address and port using debug-listen-addr config settingh or --debug-listen-flag.")) return fmt.Errorf("failed to listen on debug address %q: %w", debugListenAddr, err) } diff --git a/cmd/start_test.go b/cmd/start_test.go index 90c4a97dc..bd89dff11 100644 --- a/cmd/start_test.go +++ b/cmd/start_test.go @@ -18,7 +18,7 @@ import ( "go.uber.org/zap/zaptest/observer" ) -func TestMetricsServerFlag(t *testing.T) { +func TestMetricsServerFlags(t *testing.T) { t.Parallel() tests := []struct { @@ -136,7 +136,7 @@ func TestMissingMetricsListenAddr(t *testing.T) { requireMessage(t, logs, "Disabled metrics server due to missing metrics-listen-addr setting in config file or --metrics-listen-addr flag.") } -func TestDebugServerFlag(t *testing.T) { +func TestDebugServerFlags(t *testing.T) { t.Parallel() tests := []struct { @@ -182,19 +182,6 @@ func TestDebugServerFlag(t *testing.T) { } } -func TestMissingDebugListenAddr(t *testing.T) { - sys := setupRelayer(t) - - logs, logger := setupLogger() - - updateConfig(t, sys, "debug-listen-addr: 127.0.0.1:5183", "") - - sys.MustRunWithLogger(t, logger, []string{"start", "--enable-debug-server"}...) - - requireDisabledMetricsServer(t, logs, 0) - requireMessage(t, logs, "Disabled debug server due to missing debug-listen-addr setting in config file or --debug-listen-addr flag.") -} - func TestDebugServerConfig(t *testing.T) { t.Parallel() @@ -237,6 +224,19 @@ func TestDebugServerConfig(t *testing.T) { } } +func TestMissingDebugListenAddr(t *testing.T) { + sys := setupRelayer(t) + + logs, logger := setupLogger() + + updateConfig(t, sys, "debug-listen-addr: 127.0.0.1:5183", "") + + sys.MustRunWithLogger(t, logger, []string{"start", "--enable-debug-server"}...) + + requireDisabledMetricsServer(t, logs, 0) + requireMessage(t, logs, "Disabled debug server due to missing debug-listen-addr setting in config file or --debug-listen-addr flag.") +} + func requireDisabledMetricsServer(t *testing.T, logs *observer.ObservedLogs, port int) { conn, err := net.Dial("tcp", fmt.Sprintf("127.0.0.1:%d", port)) if conn != nil { From b73998ff3dc97b0260649368e2310c6ce822c5d7 Mon Sep 17 00:00:00 2001 From: Andy Pliszka Date: Sat, 5 Oct 2024 00:04:11 -0400 Subject: [PATCH 09/12] feat: rollbacks --debug-addr flag --- cmd/flags.go | 13 ++++ cmd/start.go | 14 +++- cmd/start_test.go | 118 ++++++++++++++++++++++++++------- internal/relayertest/system.go | 1 - 4 files changed, 121 insertions(+), 25 deletions(-) diff --git a/cmd/flags.go b/cmd/flags.go index c87c90c5b..00524fa8e 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -38,6 +38,7 @@ const ( flagDstPort = "dst-port" flagOrder = "order" flagVersion = "version" + flagDebugAddr = "debug-addr" flagEnableDebugServer = "enable-debug-server" flagDebugListenAddr = "debug-listen-addr" flagEnableMetricsServer = "enable-metrics-server" @@ -421,6 +422,18 @@ func dstPortFlag(v *viper.Viper, cmd *cobra.Command) *cobra.Command { } func debugServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command { + cmd.Flags().String( + flagDebugAddr, + "", + "address to use for debug server. By default, "+ + "will be the debug-listen-addr parameter in the global config. "+ + "DEPRECATED: Use --debug-listen-addr flag.", + ) + + if err := v.BindPFlag(flagDebugAddr, cmd.Flags().Lookup(flagDebugAddr)); err != nil { + panic(err) + } + cmd.Flags().String( flagDebugListenAddr, "", diff --git a/cmd/start.go b/cmd/start.go index f36374d3c..2b2af23ee 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -219,11 +219,21 @@ func setupMetricsServer(cmd *cobra.Command, a *appState, err error, chains map[s func setupDebugServer(cmd *cobra.Command, a *appState, err error) error { debugListenAddr := a.config.Global.DebugListenPort + debugAddrFlag, err := cmd.Flags().GetString(flagDebugAddr) + if err != nil { + return err + } + debugListenAddrFlag, err := cmd.Flags().GetString(flagDebugListenAddr) if err != nil { return err } + if debugAddrFlag != "" { + debugListenAddr = debugAddrFlag + a.log.Warn("DEPRECATED: --debug-addr flag is deprecated use --enable-debug-server and --debug-listen-addr instead.") + } + if debugListenAddrFlag != "" { debugListenAddr = debugListenAddrFlag } @@ -233,7 +243,9 @@ func setupDebugServer(cmd *cobra.Command, a *appState, err error) error { return err } - if flagEnableDebugServer == false { + enableDebugServer := flagEnableDebugServer == true || debugAddrFlag != "" + + if enableDebugServer == false { a.log.Info("Debug server is disabled. You can enable it using --enable-debug-server flag.") } else if debugListenAddr == "" { a.log.Warn("Disabled debug server due to missing debug-listen-addr setting in config file or --debug-listen-addr flag.") diff --git a/cmd/start_test.go b/cmd/start_test.go index bd89dff11..aec0e5f7e 100644 --- a/cmd/start_test.go +++ b/cmd/start_test.go @@ -1,6 +1,7 @@ package cmd_test import ( + "errors" "fmt" "net" "net/http" @@ -22,43 +23,67 @@ func TestMetricsServerFlags(t *testing.T) { t.Parallel() tests := []struct { + description string args []string wantedPort int wantedRunning bool wantedMessages []string + err error }{ { + "should start relayer without metrics server given no flags", []string{"start"}, 0, false, []string{"Metrics server is disabled. You can enable it using --enable-metrics-server flag."}, + nil, }, { + "should start relayer with metrics server running on default port", []string{"start", "--enable-metrics-server"}, relayermetrics.MetricsServerPort, true, []string{"Metrics server is enabled", "Metrics server listening"}, + nil, }, { + "should start relayer with metrics server running on custom port", []string{"start", "--enable-metrics-server", "--metrics-listen-addr", "127.0.0.1:7778"}, 7778, true, []string{"Metrics server is enabled", "Metrics server listening"}, + nil, }, { + "should start relayer without metrics server when metrics server is not enabled", []string{"start", "--metrics-listen-addr", "127.0.0.1:7778"}, 0, false, []string{"Metrics server is disabled. You can enable it using --enable-metrics-server flag."}, + nil, + }, + { + "should not start relayer and report an error when address is not provided", + []string{"start", "--metrics-listen-addr"}, + 0, + false, + nil, + errors.New("flag needs an argument: --metrics-listen-addr"), }, } for _, tt := range tests { - t.Run(strings.Join(tt.args, " "), func(t *testing.T) { + t.Run(tt.description, func(t *testing.T) { sys := setupRelayer(t) logs, logger := setupLogger() - sys.MustRunWithLogger(t, logger, tt.args...) + result := sys.MustRunWithLogger(t, logger, tt.args...) + + if tt.err != nil { + require.Error(t, result.Err, tt.err) + } else { + require.NoError(t, result.Err) + } if tt.wantedRunning == true { requireRunningMetricsServer(t, logs, tt.wantedPort) @@ -74,6 +99,7 @@ func TestMetricsServerConfig(t *testing.T) { t.Parallel() tests := []struct { + description string args []string newSetting string wantedPort int @@ -81,13 +107,7 @@ func TestMetricsServerConfig(t *testing.T) { wantedMessages []string }{ { - []string{"start"}, - "", - 0, - false, - []string{"Metrics server is disabled. You can enable it using --enable-metrics-server flag."}, - }, - { + "should starts relayer on custom address and port provided in config file", []string{"start", "--enable-metrics-server"}, "metrics-listen-addr: 127.0.0.1:6184", 6184, @@ -95,6 +115,7 @@ func TestMetricsServerConfig(t *testing.T) { []string{"Metrics server is enabled", "Metrics server listening"}, }, { + "should starts relayer on custom address provided via flag", []string{"start", "--enable-metrics-server", "--metrics-listen-addr", "127.0.0.1:7184"}, "", 7184, @@ -104,7 +125,7 @@ func TestMetricsServerConfig(t *testing.T) { } for _, tt := range tests { - t.Run(strings.Join(tt.args, " "), func(t *testing.T) { + t.Run(tt.description, func(t *testing.T) { sys := setupRelayer(t) updateConfig(t, sys, "metrics-listen-addr: 127.0.0.1:5184", tt.newSetting) @@ -140,37 +161,78 @@ func TestDebugServerFlags(t *testing.T) { t.Parallel() tests := []struct { + description string args []string wantedPort int wantedRunning bool wantedMessages []string + err error }{ { + "should start relayer without debug server", []string{"start"}, 0, false, []string{"Debug server is disabled. You can enable it using --enable-debug-server flag."}, + nil, + }, + { + "should not start the relayer and report an error when address is missing", + []string{"start", "--debug-addr"}, + 0, + false, + nil, + errors.New("flag needs an argument: --debug-addr"), + }, + { + "should start relayer with debug server given --debug-addr flag and address", + []string{"start", "--debug-addr", "127.0.0.1:7777"}, + 7777, + true, + []string{ + "Debug server is enabled", "SECURITY WARNING! Debug server should only be run with caution and proper security in place.", + "DEPRECATED: --debug-addr flag is deprecated use --enable-debug-server and --debug-listen-addr instead.", + }, + nil, }, { + "should start relayer with debug server given --enable-debug-server flag", []string{"start", "--enable-debug-server"}, relaydebug.DebugServerPort, true, []string{"Debug server is enabled", "SECURITY WARNING! Debug server should only be run with caution and proper security in place."}, + nil, }, { - []string{"start", "--enable-debug-server", "--debug-listen-addr", "127.0.0.1:7777"}, - 7777, + "should start relayer with debug server given --enable-debug-server flag and an address", + []string{"start", "--enable-debug-server", "--debug-listen-addr", "127.0.0.1:7779"}, + 7779, true, []string{"Debug server is enabled", "SECURITY WARNING! Debug server should only be run with caution and proper security in place."}, + nil, + }, + { + "should not start relayer and report an error when address is missing", + []string{"start", "--enable-debug-server", "--debug-listen-addr"}, + 0, + false, + nil, + errors.New("flag needs an argument: --debug-listen-addr"), }, } for _, tt := range tests { - t.Run(strings.Join(tt.args, " "), func(t *testing.T) { + t.Run(tt.description, func(t *testing.T) { sys := setupRelayer(t) logs, logger := setupLogger() - sys.MustRunWithLogger(t, logger, tt.args...) + result := sys.MustRunWithLogger(t, logger, tt.args...) + + if tt.err != nil { + require.Error(t, result.Err, tt.err) + } else { + require.NoError(t, result.Err) + } if tt.wantedRunning == true { requireRunningDebugServer(t, logs, tt.wantedPort) @@ -186,27 +248,37 @@ func TestDebugServerConfig(t *testing.T) { t.Parallel() tests := []struct { + description string args []string newSetting string wantedPort int wantedRunning bool }{ { - []string{"start"}, - "", - 0, - false, - }, - { + "should start debug server on custom address and port set in config file", []string{"start", "--enable-debug-server"}, "debug-listen-addr: 127.0.0.1:6183", 6183, true, }, + { + "should start debug server on custom address and port set via flag", + []string{"start", "--enable-debug-server", "--debug-listen-addr", "127.0.0.1:7183"}, + "debug-listen-addr: 127.0.0.1:6183", + 7183, + true, + }, + { + "should start debug server on custom address and port set via legacy flag", + []string{"start", "--enable-debug-server", "--debug-addr", "127.0.0.1:9183"}, + "debug-listen-addr: 127.0.0.1:6183", + 9183, + true, + }, } for _, tt := range tests { - t.Run(strings.Join(tt.args, " "), func(t *testing.T) { + t.Run(tt.description, func(t *testing.T) { sys := setupRelayer(t) updateConfig(t, sys, "debug-listen-addr: 127.0.0.1:5183", tt.newSetting) @@ -250,7 +322,7 @@ func requireRunningMetricsServer(t *testing.T, logs *observer.ObservedLogs, port if conn != nil { defer conn.Close() } - require.NoError(t, err, "Metrics server should be running") + require.NoError(t, err, fmt.Sprintf("Metrics server should be running on port %d", port)) res, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/metrics", port)) require.NoError(t, err) require.Equal(t, res.StatusCode, 200) @@ -269,7 +341,7 @@ func requireRunningDebugServer(t *testing.T, logs *observer.ObservedLogs, port i if conn != nil { defer conn.Close() } - require.NoError(t, err, "Server should be running") + require.NoError(t, err, fmt.Sprintf("Server should be running on port %d", port)) res, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/debug/pprof/goroutine", port)) require.NoError(t, err) require.Equal(t, res.StatusCode, 200) diff --git a/internal/relayertest/system.go b/internal/relayertest/system.go index 11467cd9b..88a95943b 100644 --- a/internal/relayertest/system.go +++ b/internal/relayertest/system.go @@ -114,7 +114,6 @@ func (s *System) MustRunWithLogger(t *testing.T, logger *zap.Logger, args ...str t.Logf("Error executing %v: %v", args, res.Err) t.Logf("Stdout: %q", res.Stdout.String()) t.Logf("Stderr: %q", res.Stderr.String()) - t.FailNow() } return res From 172fa92a6e6604b21889d7b06fc58e468d03efd0 Mon Sep 17 00:00:00 2001 From: Andy Pliszka Date: Sat, 5 Oct 2024 01:05:28 -0400 Subject: [PATCH 10/12] feat: deprecates api-listen-addr --- cmd/config.go | 2 ++ cmd/config_test.go | 10 +++++++++- cmd/start.go | 7 +++++++ cmd/start_test.go | 9 ++++++++- 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index 3f3c9c8c2..a211c85a9 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -488,6 +488,7 @@ func DefaultConfig(memo string) *Config { // GlobalConfig describes any global relayer settings type GlobalConfig struct { + ApiListenPort string `yaml:"api-listen-addr,omitempty" json:"api-listen-addr,omitempty"` DebugListenPort string `yaml:"debug-listen-addr" json:"debug-listen-addr"` MetricsListenPort string `yaml:"metrics-listen-addr" json:"metrics-listen-addr"` Timeout string `yaml:"timeout" json:"timeout"` @@ -501,6 +502,7 @@ type GlobalConfig struct { // newDefaultGlobalConfig returns a global config with defaults set func newDefaultGlobalConfig(memo string) GlobalConfig { return GlobalConfig{ + ApiListenPort: "", DebugListenPort: "127.0.0.1:5183", MetricsListenPort: "127.0.0.1:5184", Timeout: "10s", diff --git a/cmd/config_test.go b/cmd/config_test.go index 767ac4624..226ee3aa3 100644 --- a/cmd/config_test.go +++ b/cmd/config_test.go @@ -39,6 +39,10 @@ func TestDefaultConfig(t *testing.T) { "metrics-listen-addr: 127.0.0.1:5184", true, }, + { + "api-listen-addr: 127.0.0.1:5184", + false, + }, } for _, tt := range tests { @@ -50,7 +54,11 @@ func TestDefaultConfig(t *testing.T) { require.NoError(t, err) config := string(data) - require.Contains(t, config, tt.setting) + if tt.wantedPresent { + require.Contains(t, config, tt.setting) + } else { + require.NotContains(t, config, tt.setting) + } }) } } diff --git a/cmd/start.go b/cmd/start.go index 2b2af23ee..8ec4d0d22 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -219,6 +219,13 @@ func setupMetricsServer(cmd *cobra.Command, a *appState, err error, chains map[s func setupDebugServer(cmd *cobra.Command, a *appState, err error) error { debugListenAddr := a.config.Global.DebugListenPort + if debugListenAddr == "" { + debugListenAddr = a.config.Global.ApiListenPort + if debugListenAddr != "" { + a.log.Warn("DEPRECATED: api-listen-addr config setting is deprecated use debug-listen-addr instead.") + } + } + debugAddrFlag, err := cmd.Flags().GetString(flagDebugAddr) if err != nil { return err diff --git a/cmd/start_test.go b/cmd/start_test.go index aec0e5f7e..c92e301e4 100644 --- a/cmd/start_test.go +++ b/cmd/start_test.go @@ -269,12 +269,19 @@ func TestDebugServerConfig(t *testing.T) { true, }, { - "should start debug server on custom address and port set via legacy flag", + "should start debug server on custom address and port set via deprecated flag", []string{"start", "--enable-debug-server", "--debug-addr", "127.0.0.1:9183"}, "debug-listen-addr: 127.0.0.1:6183", 9183, true, }, + { + "should start debug server on custom address and port set via deprecated config", + []string{"start", "--enable-debug-server"}, + "api-listen-addr: 127.0.0.1:10183", + 10183, + true, + }, } for _, tt := range tests { From a1b3899941f91bec7004e3b911e0d1a49ebb62f1 Mon Sep 17 00:00:00 2001 From: Andy Pliszka Date: Sat, 5 Oct 2024 01:26:38 -0400 Subject: [PATCH 11/12] fix: closes body --- cmd/start_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/start_test.go b/cmd/start_test.go index c92e301e4..5aa427be6 100644 --- a/cmd/start_test.go +++ b/cmd/start_test.go @@ -332,6 +332,7 @@ func requireRunningMetricsServer(t *testing.T, logs *observer.ObservedLogs, port require.NoError(t, err, fmt.Sprintf("Metrics server should be running on port %d", port)) res, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/metrics", port)) require.NoError(t, err) + defer res.Body.Close() require.Equal(t, res.StatusCode, 200) } @@ -351,6 +352,7 @@ func requireRunningDebugServer(t *testing.T, logs *observer.ObservedLogs, port i require.NoError(t, err, fmt.Sprintf("Server should be running on port %d", port)) res, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/debug/pprof/goroutine", port)) require.NoError(t, err) + defer res.Body.Close() require.Equal(t, res.StatusCode, 200) } From 56008aa25d1d4f730786e773593e2b0ee98d798c Mon Sep 17 00:00:00 2001 From: Andy Pliszka Date: Wed, 23 Oct 2024 13:44:09 -0400 Subject: [PATCH 12/12] fix: reformat log messages to follow styleguide --- cmd/start.go | 18 +++++++++--------- cmd/start_test.go | 18 +++++++++--------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/cmd/start.go b/cmd/start.go index 8ec4d0d22..3d74a2637 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -192,14 +192,14 @@ func setupMetricsServer(cmd *cobra.Command, a *appState, err error, chains map[s } if flagEnableMetricsServer == false { - a.log.Info("Metrics server is disabled. You can enable it using --enable-metrics-server flag.") + a.log.Info("Metrics server is disabled you can enable it using --enable-metrics-server flag") } else if metricsListenAddr == "" { - a.log.Warn("Disabled metrics server due to missing metrics-listen-addr setting in config file or --metrics-listen-addr flag.") + a.log.Warn("Disabled metrics server due to missing metrics-listen-addr setting in config file or --metrics-listen-addr flag") } else { a.log.Info("Metrics server is enabled") ln, err := net.Listen("tcp", metricsListenAddr) if err != nil { - a.log.Error(fmt.Sprintf("Failed to start metrics server. You can change the address and port using metrics-listen-addr config settingh or --metrics-listen-flag.")) + a.log.Error(fmt.Sprintf("Failed to start metrics server you can change the address and port using metrics-listen-addr config settingh or --metrics-listen-flag")) return nil, fmt.Errorf("failed to listen on metrics address %q: %w", metricsListenAddr, err) } @@ -222,7 +222,7 @@ func setupDebugServer(cmd *cobra.Command, a *appState, err error) error { if debugListenAddr == "" { debugListenAddr = a.config.Global.ApiListenPort if debugListenAddr != "" { - a.log.Warn("DEPRECATED: api-listen-addr config setting is deprecated use debug-listen-addr instead.") + a.log.Warn("DEPRECATED: api-listen-addr config setting is deprecated use debug-listen-addr instead") } } @@ -238,7 +238,7 @@ func setupDebugServer(cmd *cobra.Command, a *appState, err error) error { if debugAddrFlag != "" { debugListenAddr = debugAddrFlag - a.log.Warn("DEPRECATED: --debug-addr flag is deprecated use --enable-debug-server and --debug-listen-addr instead.") + a.log.Warn("DEPRECATED: --debug-addr flag is deprecated use --enable-debug-server and --debug-listen-addr instead") } if debugListenAddrFlag != "" { @@ -253,15 +253,15 @@ func setupDebugServer(cmd *cobra.Command, a *appState, err error) error { enableDebugServer := flagEnableDebugServer == true || debugAddrFlag != "" if enableDebugServer == false { - a.log.Info("Debug server is disabled. You can enable it using --enable-debug-server flag.") + a.log.Info("Debug server is disabled you can enable it using --enable-debug-server flag") } else if debugListenAddr == "" { - a.log.Warn("Disabled debug server due to missing debug-listen-addr setting in config file or --debug-listen-addr flag.") + a.log.Warn("Disabled debug server due to missing debug-listen-addr setting in config file or --debug-listen-addr flag") } else { a.log.Info("Debug server is enabled") - a.log.Warn("SECURITY WARNING! Debug server should only be run with caution and proper security in place.") + a.log.Warn("SECURITY WARNING! Debug server should only be run with caution and proper security in place") ln, err := net.Listen("tcp", debugListenAddr) if err != nil { - a.log.Error(fmt.Sprintf("Failed to start debug server. You can change the address and port using debug-listen-addr config settingh or --debug-listen-flag.")) + a.log.Error(fmt.Sprintf("Failed to start debug server you can change the address and port using debug-listen-addr config settingh or --debug-listen-flag")) return fmt.Errorf("failed to listen on debug address %q: %w", debugListenAddr, err) } diff --git a/cmd/start_test.go b/cmd/start_test.go index 5aa427be6..e6cd30bf6 100644 --- a/cmd/start_test.go +++ b/cmd/start_test.go @@ -35,7 +35,7 @@ func TestMetricsServerFlags(t *testing.T) { []string{"start"}, 0, false, - []string{"Metrics server is disabled. You can enable it using --enable-metrics-server flag."}, + []string{"Metrics server is disabled you can enable it using --enable-metrics-server flag"}, nil, }, { @@ -59,7 +59,7 @@ func TestMetricsServerFlags(t *testing.T) { []string{"start", "--metrics-listen-addr", "127.0.0.1:7778"}, 0, false, - []string{"Metrics server is disabled. You can enable it using --enable-metrics-server flag."}, + []string{"Metrics server is disabled you can enable it using --enable-metrics-server flag"}, nil, }, { @@ -154,7 +154,7 @@ func TestMissingMetricsListenAddr(t *testing.T) { sys.MustRunWithLogger(t, logger, []string{"start", "--enable-metrics-server"}...) requireDisabledMetricsServer(t, logs, 0) - requireMessage(t, logs, "Disabled metrics server due to missing metrics-listen-addr setting in config file or --metrics-listen-addr flag.") + requireMessage(t, logs, "Disabled metrics server due to missing metrics-listen-addr setting in config file or --metrics-listen-addr flag") } func TestDebugServerFlags(t *testing.T) { @@ -173,7 +173,7 @@ func TestDebugServerFlags(t *testing.T) { []string{"start"}, 0, false, - []string{"Debug server is disabled. You can enable it using --enable-debug-server flag."}, + []string{"Debug server is disabled you can enable it using --enable-debug-server flag"}, nil, }, { @@ -190,8 +190,8 @@ func TestDebugServerFlags(t *testing.T) { 7777, true, []string{ - "Debug server is enabled", "SECURITY WARNING! Debug server should only be run with caution and proper security in place.", - "DEPRECATED: --debug-addr flag is deprecated use --enable-debug-server and --debug-listen-addr instead.", + "Debug server is enabled", "SECURITY WARNING! Debug server should only be run with caution and proper security in place", + "DEPRECATED: --debug-addr flag is deprecated use --enable-debug-server and --debug-listen-addr instead", }, nil, }, @@ -200,7 +200,7 @@ func TestDebugServerFlags(t *testing.T) { []string{"start", "--enable-debug-server"}, relaydebug.DebugServerPort, true, - []string{"Debug server is enabled", "SECURITY WARNING! Debug server should only be run with caution and proper security in place."}, + []string{"Debug server is enabled", "SECURITY WARNING! Debug server should only be run with caution and proper security in place"}, nil, }, { @@ -208,7 +208,7 @@ func TestDebugServerFlags(t *testing.T) { []string{"start", "--enable-debug-server", "--debug-listen-addr", "127.0.0.1:7779"}, 7779, true, - []string{"Debug server is enabled", "SECURITY WARNING! Debug server should only be run with caution and proper security in place."}, + []string{"Debug server is enabled", "SECURITY WARNING! Debug server should only be run with caution and proper security in place"}, nil, }, { @@ -313,7 +313,7 @@ func TestMissingDebugListenAddr(t *testing.T) { sys.MustRunWithLogger(t, logger, []string{"start", "--enable-debug-server"}...) requireDisabledMetricsServer(t, logs, 0) - requireMessage(t, logs, "Disabled debug server due to missing debug-listen-addr setting in config file or --debug-listen-addr flag.") + requireMessage(t, logs, "Disabled debug server due to missing debug-listen-addr setting in config file or --debug-listen-addr flag") } func requireDisabledMetricsServer(t *testing.T, logs *observer.ObservedLogs, port int) {