Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add separate flag for metrics #1504

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `--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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good, as discussed in our call we will support the old flags for now and mark them deprecated in the CLI output and the docs here.


---
[[TROUBLESHOOTING](docs/troubleshooting.md)]
Expand Down
28 changes: 15 additions & 13 deletions cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
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"`
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"`
Comment on lines +491 to +498
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed we will support the old config schema and mark it deprecated for one release

}

// 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,
DebugListenPort: "127.0.0.1:5183",
MetricsListenPort: "127.0.0.1:5184",
Timeout: "10s",
LightCacheSize: 20,
Memo: memo,
LogLevel: "info",
MaxReceiverSize: 150,
}
}

Expand Down
56 changes: 56 additions & 0 deletions cmd/config_test.go
Original file line number Diff line number Diff line change
@@ -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,
},
}
Comment on lines +33 to +42
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add test case to support the old config as well until we remove them


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)
})
}
}
39 changes: 34 additions & 5 deletions cmd/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ const (
flagOrder = "order"
flagVersion = "version"
flagEnableDebugServer = "enable-debug-server"
flagDebugAddr = "debug-addr"
flagDebugListenAddr = "debug-listen-addr"
flagEnableMetricsServer = "enable-metrics-server"
flagMetricsListenAddr = "metrics-listen-addr"
Comment on lines +42 to +44
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

flagOverwriteConfig = "overwrite"
flagLimit = "limit"
flagHeight = "height"
Expand Down Expand Up @@ -420,13 +422,14 @@ func dstPortFlag(v *viper.Viper, cmd *cobra.Command) *cobra.Command {

func debugServerFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command {
cmd.Flags().String(
flagDebugAddr,
flagDebugListenAddr,
"",
"address to use for debug and metrics server. By default, "+
"will be the api-listen-addr parameter in the global config.",
"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.",
)

if err := v.BindPFlag(flagDebugAddr, cmd.Flags().Lookup(flagDebugAddr)); err != nil {
if err := v.BindPFlag(flagDebugListenAddr, cmd.Flags().Lookup(flagDebugListenAddr)); err != nil {
panic(err)
}

Expand All @@ -443,6 +446,32 @@ 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(
flagMetricsListenAddr,
"",
"address to use for metrics server. By default, "+
"will be the metrics-listen-addr parameter in the global config. "+
"Make sure to enable metrics server using --enable-metrics-server flag.",
)
Comment on lines +453 to +456
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure we add a comment stating we will deprecate the old flags in next release for the new ones


if err := v.BindPFlag(flagMetricsListenAddr, cmd.Flags().Lookup(flagMetricsListenAddr)); 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 {
Expand Down
120 changes: 85 additions & 35 deletions cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -92,49 +93,16 @@ $ %s start demo-path2 --max-tx-size 10`, appName, appName, appName, appName)),
return err
}

var prometheusMetrics *processor.PrometheusMetrics

debugAddr := a.config.Global.APIListenPort

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))
prometheusMetrics = processor.NewPrometheusMetrics()
relaydebug.StartDebugServer(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
Expand Down Expand Up @@ -195,10 +163,92 @@ $ %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)
cmd = memoFlag(a.viper, cmd)
cmd = stuckPacketFlags(a.viper, cmd)
return cmd
}

func setupMetricsServer(cmd *cobra.Command, a *appState, err error, chains map[string]*relayer.Chain) (*processor.PrometheusMetrics, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea to break out the functionality into smaller methods!

var prometheusMetrics *processor.PrometheusMetrics

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 {
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")
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."))

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", metricsListenAddr))
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 {
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 {
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.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(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)
}
log := a.log.With(zap.String("sys", "debughttp"))
log.Info("Debug server listening", zap.String("addr", debugListenAddr))
relaydebug.StartDebugServer(cmd.Context(), log, ln)
}
return nil
}
Loading
Loading