From a190ab76d88f02fa03ae4019f4b0e1ba8b1fd9c6 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Wed, 20 Nov 2024 15:06:43 +0100 Subject: [PATCH 1/4] fix metrics by passing a callback --- server/v2/api/telemetry/metrics.go | 4 ++++ server/v2/api/telemetry/server.go | 10 +++++++++- simapp/v2/simdv2/cmd/commands.go | 3 ++- telemetry/metrics.go | 5 +++++ telemetry/wrapper.go | 1 + 5 files changed, 21 insertions(+), 2 deletions(-) diff --git a/server/v2/api/telemetry/metrics.go b/server/v2/api/telemetry/metrics.go index 3dd9e3d55b94..b1b5592a4bfb 100644 --- a/server/v2/api/telemetry/metrics.go +++ b/server/v2/api/telemetry/metrics.go @@ -59,6 +59,10 @@ type GatherResponse struct { // NewMetrics creates a new instance of Metrics func NewMetrics(cfg *Config) (*Metrics, error) { + if !cfg.Enable { + return nil, nil + } + if numGlobalLabels := len(cfg.GlobalLabels); numGlobalLabels > 0 { parsedGlobalLabels := make([]metrics.Label, numGlobalLabels) for i, gl := range cfg.GlobalLabels { diff --git a/server/v2/api/telemetry/server.go b/server/v2/api/telemetry/server.go index 7e3e066010b6..dd852dfa4a12 100644 --- a/server/v2/api/telemetry/server.go +++ b/server/v2/api/telemetry/server.go @@ -28,7 +28,7 @@ type Server[T transaction.Tx] struct { } // New creates a new telemetry server. -func New[T transaction.Tx](cfg server.ConfigMap, logger log.Logger) (*Server[T], error) { +func New[T transaction.Tx](cfg server.ConfigMap, logger log.Logger, setTelemetryEnabled func(bool)) (*Server[T], error) { srv := &Server[T]{} serverCfg := srv.Config().(*Config) if len(cfg) > 0 { @@ -39,6 +39,14 @@ func New[T transaction.Tx](cfg server.ConfigMap, logger log.Logger) (*Server[T], srv.config = serverCfg srv.logger = logger.With(log.ModuleKey, srv.Name()) + if setTelemetryEnabled == nil { + panic("setTelemetryEnabled must be provided") + } + + if srv.config.Enable { + setTelemetryEnabled(true) + } + metrics, err := NewMetrics(srv.config) if err != nil { return nil, fmt.Errorf("failed to initialize metrics: %w", err) diff --git a/simapp/v2/simdv2/cmd/commands.go b/simapp/v2/simdv2/cmd/commands.go index 64c91f85c12a..70b704db6a75 100644 --- a/simapp/v2/simdv2/cmd/commands.go +++ b/simapp/v2/simdv2/cmd/commands.go @@ -25,6 +25,7 @@ import ( "github.com/cosmos/cosmos-sdk/client/debug" "github.com/cosmos/cosmos-sdk/client/keys" "github.com/cosmos/cosmos-sdk/client/rpc" + sdktelemetry "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" "github.com/cosmos/cosmos-sdk/x/genutil" @@ -117,7 +118,7 @@ func InitRootCmd[T transaction.Tx]( } } - telemetryServer, err := telemetry.New[T](deps.GlobalConfig, logger) + telemetryServer, err := telemetry.New[T](deps.GlobalConfig, logger, sdktelemetry.SetTelemetryEnabled) if err != nil { return nil, err } diff --git a/telemetry/metrics.go b/telemetry/metrics.go index 175261408aae..004e8d2f1a1d 100644 --- a/telemetry/metrics.go +++ b/telemetry/metrics.go @@ -24,6 +24,11 @@ func IsTelemetryEnabled() bool { return globalTelemetryEnabled } +// SetTelemetryEnabled allows for the global telemetry enabled state to be set. +func SetTelemetryEnabled(enabled bool) { + globalTelemetryEnabled = enabled +} + // globalLabels defines the set of global labels that will be applied to all // metrics emitted using the telemetry package function wrappers. var globalLabels = []metrics.Label{} diff --git a/telemetry/wrapper.go b/telemetry/wrapper.go index 4362c46a4413..f6281680824e 100644 --- a/telemetry/wrapper.go +++ b/telemetry/wrapper.go @@ -62,6 +62,7 @@ func IncrCounter(val float32, keys ...string) { // metric with global labels (if any) along with the provided labels. func IncrCounterWithLabels(keys []string, val float32, labels []metrics.Label) { if !IsTelemetryEnabled() { + return } From ff7807d5e2bbcadaef8a76a8f329223fd0878301 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Wed, 20 Nov 2024 15:07:41 +0100 Subject: [PATCH 2/4] rename it --- server/v2/api/telemetry/server.go | 8 ++++---- simapp/v2/simdv2/cmd/commands.go | 2 +- telemetry/metrics.go | 4 ++-- telemetry/wrapper.go | 1 - 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/server/v2/api/telemetry/server.go b/server/v2/api/telemetry/server.go index dd852dfa4a12..f612964a8476 100644 --- a/server/v2/api/telemetry/server.go +++ b/server/v2/api/telemetry/server.go @@ -28,7 +28,7 @@ type Server[T transaction.Tx] struct { } // New creates a new telemetry server. -func New[T transaction.Tx](cfg server.ConfigMap, logger log.Logger, setTelemetryEnabled func(bool)) (*Server[T], error) { +func New[T transaction.Tx](cfg server.ConfigMap, logger log.Logger, enableTelemetry func()) (*Server[T], error) { srv := &Server[T]{} serverCfg := srv.Config().(*Config) if len(cfg) > 0 { @@ -39,12 +39,12 @@ func New[T transaction.Tx](cfg server.ConfigMap, logger log.Logger, setTelemetry srv.config = serverCfg srv.logger = logger.With(log.ModuleKey, srv.Name()) - if setTelemetryEnabled == nil { - panic("setTelemetryEnabled must be provided") + if enableTelemetry == nil { + panic("enableTelemetry must be provided") } if srv.config.Enable { - setTelemetryEnabled(true) + enableTelemetry() } metrics, err := NewMetrics(srv.config) diff --git a/simapp/v2/simdv2/cmd/commands.go b/simapp/v2/simdv2/cmd/commands.go index 70b704db6a75..c7d482f3ef0a 100644 --- a/simapp/v2/simdv2/cmd/commands.go +++ b/simapp/v2/simdv2/cmd/commands.go @@ -118,7 +118,7 @@ func InitRootCmd[T transaction.Tx]( } } - telemetryServer, err := telemetry.New[T](deps.GlobalConfig, logger, sdktelemetry.SetTelemetryEnabled) + telemetryServer, err := telemetry.New[T](deps.GlobalConfig, logger, sdktelemetry.EnableTelemetry) if err != nil { return nil, err } diff --git a/telemetry/metrics.go b/telemetry/metrics.go index 004e8d2f1a1d..0282653765bc 100644 --- a/telemetry/metrics.go +++ b/telemetry/metrics.go @@ -25,8 +25,8 @@ func IsTelemetryEnabled() bool { } // SetTelemetryEnabled allows for the global telemetry enabled state to be set. -func SetTelemetryEnabled(enabled bool) { - globalTelemetryEnabled = enabled +func EnableTelemetry() { + globalTelemetryEnabled = true } // globalLabels defines the set of global labels that will be applied to all diff --git a/telemetry/wrapper.go b/telemetry/wrapper.go index f6281680824e..4362c46a4413 100644 --- a/telemetry/wrapper.go +++ b/telemetry/wrapper.go @@ -62,7 +62,6 @@ func IncrCounter(val float32, keys ...string) { // metric with global labels (if any) along with the provided labels. func IncrCounterWithLabels(keys []string, val float32, labels []metrics.Label) { if !IsTelemetryEnabled() { - return } From a1f6f0824dba401e3827ad048d9681d8f7fd9159 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Wed, 20 Nov 2024 15:50:16 +0100 Subject: [PATCH 3/4] address comments --- server/v2/api/telemetry/metrics.go | 3 --- telemetry/metrics.go | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/server/v2/api/telemetry/metrics.go b/server/v2/api/telemetry/metrics.go index b1b5592a4bfb..966f0393a575 100644 --- a/server/v2/api/telemetry/metrics.go +++ b/server/v2/api/telemetry/metrics.go @@ -59,9 +59,6 @@ type GatherResponse struct { // NewMetrics creates a new instance of Metrics func NewMetrics(cfg *Config) (*Metrics, error) { - if !cfg.Enable { - return nil, nil - } if numGlobalLabels := len(cfg.GlobalLabels); numGlobalLabels > 0 { parsedGlobalLabels := make([]metrics.Label, numGlobalLabels) diff --git a/telemetry/metrics.go b/telemetry/metrics.go index 0282653765bc..67ace50c53ec 100644 --- a/telemetry/metrics.go +++ b/telemetry/metrics.go @@ -24,7 +24,7 @@ func IsTelemetryEnabled() bool { return globalTelemetryEnabled } -// SetTelemetryEnabled allows for the global telemetry enabled state to be set. +// EnableTelemetry allows for the global telemetry enabled state to be set. func EnableTelemetry() { globalTelemetryEnabled = true } From d01cb11312cb0a4a7bb3359b32247c44889eceb0 Mon Sep 17 00:00:00 2001 From: marbar3778 Date: Wed, 20 Nov 2024 16:46:07 +0100 Subject: [PATCH 4/4] linting --- math/dec_test.go | 4 ++-- server/v2/api/telemetry/metrics.go | 1 - server/v2/cometbft/abci.go | 9 +++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/math/dec_test.go b/math/dec_test.go index 316b6aaa63dc..6e84359b98cb 100644 --- a/math/dec_test.go +++ b/math/dec_test.go @@ -450,8 +450,8 @@ func (s *decimalTestSuite) TestApproxRoot() { expected math.LegacyDec }{ {math.LegacyNewDecFromInt(math.NewInt(2)), 0, math.LegacyOneDec()}, // 2 ^ 0 => 1.0 - {math.LegacyNewDecWithPrec(4, 2), 0, math.LegacyOneDec()}, // 0.04 ^ 0 => 1.0 - {math.LegacyNewDec(0), 1, math.LegacyNewDec(0)}, // 0 ^ 1 => 0 + {math.LegacyNewDecWithPrec(4, 2), 0, math.LegacyOneDec()}, // 0.04 ^ 0 => 1.0 + {math.LegacyNewDec(0), 1, math.LegacyNewDec(0)}, // 0 ^ 1 => 0 {math.LegacyOneDec(), 10, math.LegacyOneDec()}, // 1.0 ^ (0.1) => 1.0 {math.LegacyNewDecWithPrec(25, 2), 2, math.LegacyNewDecWithPrec(5, 1)}, // 0.25 ^ (0.5) => 0.5 {math.LegacyNewDecWithPrec(4, 2), 2, math.LegacyNewDecWithPrec(2, 1)}, // 0.04 ^ (0.5) => 0.2 diff --git a/server/v2/api/telemetry/metrics.go b/server/v2/api/telemetry/metrics.go index 966f0393a575..3dd9e3d55b94 100644 --- a/server/v2/api/telemetry/metrics.go +++ b/server/v2/api/telemetry/metrics.go @@ -59,7 +59,6 @@ type GatherResponse struct { // NewMetrics creates a new instance of Metrics func NewMetrics(cfg *Config) (*Metrics, error) { - if numGlobalLabels := len(cfg.GlobalLabels); numGlobalLabels > 0 { parsedGlobalLabels := make([]metrics.Label, numGlobalLabels) for i, gl := range cfg.GlobalLabels { diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index e8c72f6ef343..11c953c34e5c 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -11,9 +11,6 @@ import ( abci "github.com/cometbft/cometbft/abci/types" abciproto "github.com/cometbft/cometbft/api/cometbft/abci/v1" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" - sdk "github.com/cosmos/cosmos-sdk/types" - txtypes "github.com/cosmos/cosmos-sdk/types/tx" gogoproto "github.com/cosmos/gogoproto/proto" protoreflect "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/reflect/protoregistry" @@ -37,6 +34,10 @@ import ( "cosmossdk.io/server/v2/streaming" "cosmossdk.io/store/v2/snapshots" consensustypes "cosmossdk.io/x/consensus/types" + + codectypes "github.com/cosmos/cosmos-sdk/codec/types" + sdk "github.com/cosmos/cosmos-sdk/types" + txtypes "github.com/cosmos/cosmos-sdk/types/tx" ) const ( @@ -288,7 +289,7 @@ func (c *Consensus[T]) maybeRunGRPCQuery(ctx context.Context, req *abci.QueryReq txResult, _, err := c.app.Simulate(ctx, tx) if err != nil { - return nil, true, fmt.Errorf("%v with gas used: '%d'", err, txResult.GasUsed) + return nil, true, fmt.Errorf("%w with gas used: '%d'", err, txResult.GasUsed) } msgResponses := make([]*codectypes.Any, 0, len(txResult.Resp))