Improvents on telemetry calls

Author: lucaslopezf

baseapp/abci.go

@@ -177,8 +177,7 @@ func (app *BaseApp) Query(_ context.Context, req *abci.RequestQuery) (resp *abci
 
 	telemetry.IncrCounter(1, "query", "count")
 	telemetry.IncrCounter(1, "query", req.Path)
-	start := telemetry.Now()
-	defer telemetry.MeasureSince(start, req.Path)
+	defer telemetry.MeasureSince(time.Now(), req.Path)
 
 	if req.Path == QueryPathBroadcastTx {
 		return sdkerrors.QueryResult(errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "can't route a broadcast tx message"), app.trace), nil

server/start.go

@@ -533,10 +533,6 @@ func startAPIServer(
 }
 
 func startTelemetry(cfg serverconfig.Config) (*telemetry.Metrics, error) {
-	if !cfg.Telemetry.Enabled {
-		return nil, nil
-	}
-
 	return telemetry.New(cfg.Telemetry)
 }
 

telemetry/metrics.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
-	"sync/atomic"
 	"time"
 
 	"github.com/hashicorp/go-metrics"
@@ -17,17 +16,11 @@ import (
 
 // globalTelemetryEnabled is a private variable that stores the telemetry enabled state.
 // It is set on initialization and does not change for the lifetime of the program.
-var globalTelemetryEnabled atomic.Bool
+var globalTelemetryEnabled bool
 
-// initTelemetry sets the global variable based on the configuration.
-// It is called only once, at startup, to set the telemetry enabled state.
-func initTelemetry(enabled bool) {
-	globalTelemetryEnabled.Store(enabled)
-}
-
-// isTelemetryEnabled provides controlled access to check if telemetry is enabled.
-func isTelemetryEnabled() bool {
-	return globalTelemetryEnabled.Load()
+// IsTelemetryEnabled provides controlled access to check if telemetry is enabled.
+func IsTelemetryEnabled() bool {
+	return globalTelemetryEnabled
 }
 
 // globalLabels defines the set of global labels that will be applied to all
@@ -111,7 +104,7 @@ type GatherResponse struct {
 
 // New creates a new instance of Metrics
 func New(cfg Config) (_ *Metrics, rerr error) {
-	initTelemetry(cfg.Enabled)
+	globalTelemetryEnabled = cfg.Enabled
 	if !cfg.Enabled {
 		return nil, nil
 	}

telemetry/wrapper.go

@@ -24,7 +24,7 @@ func NewLabel(name, value string) metrics.Label {
 // metric for a module with a given set of keys. If any global labels are defined,
 // they will be added to the module label.
 func ModuleMeasureSince(module string, start time.Time, keys ...string) {
-	if !isTelemetryEnabled() {
+	if !IsTelemetryEnabled() {
 		return
 	}
 
@@ -39,7 +39,7 @@ func ModuleMeasureSince(module string, start time.Time, keys ...string) {
 // module with a given set of keys. If any global labels are defined, they will
 // be added to the module label.
 func ModuleSetGauge(module string, val float32, keys ...string) {
-	if !isTelemetryEnabled() {
+	if !IsTelemetryEnabled() {
 		return
 	}
 
@@ -53,7 +53,7 @@ func ModuleSetGauge(module string, val float32, keys ...string) {
 // IncrCounter provides a wrapper functionality for emitting a counter metric with
 // global labels (if any).
 func IncrCounter(val float32, keys ...string) {
-	if !isTelemetryEnabled() {
+	if !IsTelemetryEnabled() {
 		return
 	}
 
@@ -63,7 +63,7 @@ func IncrCounter(val float32, keys ...string) {
 // IncrCounterWithLabels provides a wrapper functionality for emitting a counter
 // metric with global labels (if any) along with the provided labels.
 func IncrCounterWithLabels(keys []string, val float32, labels []metrics.Label) {
-	if !isTelemetryEnabled() {
+	if !IsTelemetryEnabled() {
 		return
 	}
 
@@ -73,7 +73,7 @@ func IncrCounterWithLabels(keys []string, val float32, labels []metrics.Label) {
 // SetGauge provides a wrapper functionality for emitting a gauge metric with
 // global labels (if any).
 func SetGauge(val float32, keys ...string) {
-	if !isTelemetryEnabled() {
+	if !IsTelemetryEnabled() {
 		return
 	}
 
@@ -83,7 +83,7 @@ func SetGauge(val float32, keys ...string) {
 // SetGaugeWithLabels provides a wrapper functionality for emitting a gauge
 // metric with global labels (if any) along with the provided labels.
 func SetGaugeWithLabels(keys []string, val float32, labels []metrics.Label) {
-	if !isTelemetryEnabled() {
+	if !IsTelemetryEnabled() {
 		return
 	}
 
@@ -93,7 +93,7 @@ func SetGaugeWithLabels(keys []string, val float32, labels []metrics.Label) {
 // MeasureSince provides a wrapper functionality for emitting a a time measure
 // metric with global labels (if any).
 func MeasureSince(start time.Time, keys ...string) {
-	if !isTelemetryEnabled() {
+	if !IsTelemetryEnabled() {
 		return
 	}
 
@@ -102,7 +102,7 @@ func MeasureSince(start time.Time, keys ...string) {
 
 // Now return the current time if telemetry is enabled or a zero time if it's not
 func Now() time.Time {
-	if !isTelemetryEnabled() {
+	if !IsTelemetryEnabled() {
 		return time.Time{}
 	}
 

telemetry/wrapper_test.go

@@ -5,50 +5,46 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/suite"
+	"github.com/stretchr/testify/assert"
 )
 
-// TelemetrySuite is a struct that holds the setup for the telemetry tests.
-// It includes a mutex to ensure that tests that depend on the global state
-// do not run in parallel, which can cause race conditions and unpredictable results.
-type TelemetrySuite struct {
-	suite.Suite
-	mu sync.Mutex
+var mu sync.Mutex
+
+func initTelemetry(v bool) {
+	globalTelemetryEnabled = v
 }
 
-// SetupTest is called before each test to reset the global state to a known disabled state.
-// This ensures each test starts with the telemetry disabled
-func (suite *TelemetrySuite) SetupTest() {
+// Reset the global state to a known disabled state before each test.
+func setupTest(t *testing.T) {
+	mu.Lock() // Ensure no other test can modify global state at the same time.
+	defer mu.Unlock()
 	initTelemetry(false)
 }
 
 // TestNow tests the Now function when telemetry is enabled and disabled.
-func (suite *TelemetrySuite) TestNow() {
-	suite.mu.Lock()
-	defer suite.mu.Unlock()
+func TestNow(t *testing.T) {
+	setupTest(t) // Locks the mutex to avoid race condition.
 
 	initTelemetry(true)
 	telemetryTime := Now()
-	suite.NotEqual(time.Time{}, telemetryTime, "Now() should not return zero time when telemetry is enabled")
+	assert.NotEqual(t, time.Time{}, telemetryTime, "Now() should not return zero time when telemetry is enabled")
+
+	setupTest(t) // Reset the global state and lock the mutex again.
 
 	initTelemetry(false)
 	telemetryTime = Now()
-	suite.Equal(time.Time{}, telemetryTime, "Now() should return zero time when telemetry is disabled")
+	assert.Equal(t, time.Time{}, telemetryTime, "Now() should return zero time when telemetry is disabled")
 }
 
-// TestIsTelemetryEnabled tests the isTelemetryEnabled function.
-func (suite *TelemetrySuite) TestIsTelemetryEnabled() {
-	suite.mu.Lock()
-	defer suite.mu.Unlock()
+// TestIsTelemetryEnabled tests the IsTelemetryEnabled function.
+func TestIsTelemetryEnabled(t *testing.T) {
+	setupTest(t) // Locks the mutex to avoid race condition.
 
 	initTelemetry(true)
-	suite.True(isTelemetryEnabled(), "isTelemetryEnabled() should return true when globalTelemetryEnabled is set to true")
+	assert.True(t, IsTelemetryEnabled(), "IsTelemetryEnabled() should return true when globalTelemetryEnabled is set to true")
 
-	initTelemetry(false)
-	suite.False(isTelemetryEnabled(), "isTelemetryEnabled() should return false when globalTelemetryEnabled is set to false")
-}
+	setupTest(t) // Reset the global state and lock the mutex again.
 
-// TestTelemetrySuite initiates the test suite.
-func TestTelemetrySuite(t *testing.T) {
-	suite.Run(t, new(TelemetrySuite))
+	initTelemetry(false)
+	assert.False(t, IsTelemetryEnabled(), "IsTelemetryEnabled() should return false when globalTelemetryEnabled is set to false")
 }

x/crisis/abci.go

@@ -11,8 +11,7 @@ import (
 
 // check all registered invariants
 func EndBlocker(ctx context.Context, k keeper.Keeper) {
-	start := telemetry.Now()
-	defer telemetry.ModuleMeasureSince(types.ModuleName, start, telemetry.MetricKeyEndBlocker)
+	defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyEndBlocker)
 
 	sdkCtx := sdk.UnwrapSDKContext(ctx)
 	if k.InvCheckPeriod() == 0 || sdkCtx.BlockHeight()%int64(k.InvCheckPeriod()) != 0 {

x/crisis/module.go

@@ -117,12 +117,11 @@ func (am AppModule) ValidateGenesis(bz json.RawMessage) error {
 
 // InitGenesis performs genesis initialization for the crisis module.
 func (am AppModule) InitGenesis(ctx context.Context, data json.RawMessage) error {
-	start := telemetry.Now()
 	var genesisState types.GenesisState
 	if err := am.cdc.UnmarshalJSON(data, &genesisState); err != nil {
 		return err
 	}
-	telemetry.MeasureSince(start, "InitGenesis", "crisis", "unmarshal")
+	telemetry.MeasureSince(telemetry.Now(), "InitGenesis", "crisis", "unmarshal")
 
 	am.keeper.InitGenesis(ctx, &genesisState)
 	if !am.skipGenesisInvariants {

x/distribution/keeper/abci.go

@@ -11,8 +11,7 @@ import (
 // and distribute rewards for the previous block.
 // TODO: use context.Context after including the comet service
 func (k Keeper) BeginBlocker(ctx sdk.Context) error {
-	start := telemetry.Now()
-	defer telemetry.ModuleMeasureSince(types.ModuleName, start, telemetry.MetricKeyBeginBlocker)
+	defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyBeginBlocker)
 
 	// determine the total power signing the block
 	var previousTotalPower int64

x/evidence/keeper/abci.go

@@ -14,8 +14,7 @@ import (
 // BeginBlocker iterates through and handles any newly discovered evidence of
 // misbehavior submitted by CometBFT. Currently, only equivocation is handled.
 func (k Keeper) BeginBlocker(ctx context.Context) error {
-	start := telemetry.Now()
-	defer telemetry.ModuleMeasureSince(types.ModuleName, start, telemetry.MetricKeyBeginBlocker)
+	defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyBeginBlocker)
 
 	bi := sdk.UnwrapSDKContext(ctx).CometInfo()
 

x/gov/keeper/abci.go

@@ -21,8 +21,7 @@ import (
 
 // EndBlocker is called every block.
 func (k Keeper) EndBlocker(ctx context.Context) error {
-	start := telemetry.Now()
-	defer telemetry.ModuleMeasureSince(types.ModuleName, start, telemetry.MetricKeyEndBlocker)
+	defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyEndBlocker)
 
 	logger := k.Logger()
 	// delete dead proposals from store and returns theirs deposits.