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: Conditionally emit metrics based on enablement #19903

Merged
merged 17 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i

### Improvements

* (telemetry) [#19903](https://github.com/cosmos/cosmos-sdk/pull/19903) Conditionally emit metrics based on enablement.
* **Introduction of `Now` Function**: Added a new function called `Now` to the telemetry package. It returns the current system time if telemetry is enabled, or a zero time if telemetry is not enabled.
* **Atomic Global Variable**: Implemented an atomic global variable to manage the state of telemetry's enablement. This ensures thread safety for the telemetry state.
* **Conditional Telemetry Emission**: All telemetry functions have been updated to emit metrics only when telemetry is enabled. They perform a check with `isTelemetryEnabled()` and return early if telemetry is disabled, minimizing unnecessary operations and overhead.
* (types) [#19869](https://github.com/cosmos/cosmos-sdk/pull/19869) Removed `Any` type from `codec/types` and replaced it with an alias for `cosmos/gogoproto/types/any`.
* (server) [#19854](https://github.com/cosmos/cosmos-sdk/pull/19854) Add customizability to start command.
* Add `StartCmdOptions` in `server.AddCommands` instead of `servertypes.ModuleInitFlags`. To set custom flags set them in the `StartCmdOptions` struct on the `AddFlags` field.
Expand Down
4 changes: 0 additions & 4 deletions server/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
10 changes: 10 additions & 0 deletions telemetry/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ import (
"github.com/prometheus/common/expfmt"
)

// 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 bool

// IsTelemetryEnabled provides controlled access to check if telemetry is enabled.
func IsTelemetryEnabled() bool {
return globalTelemetryEnabled
}
Comment on lines +17 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

The introduction of globalTelemetryEnabled and the IsTelemetryEnabled() function are well-implemented. However, consider changing globalTelemetryEnabled from an atomic type to a regular boolean since it is not modified after initialization.

- var globalTelemetryEnabled bool
+ var globalTelemetryEnabled atomic.Bool

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
// 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 bool
// IsTelemetryEnabled provides controlled access to check if telemetry is enabled.
func IsTelemetryEnabled() bool {
return globalTelemetryEnabled
}
// 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
// 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
// metrics emitted using the telemetry package function wrappers.
var globalLabels = []metrics.Label{}
Expand Down Expand Up @@ -95,6 +104,7 @@ type GatherResponse struct {

// New creates a new instance of Metrics
func New(cfg Config) (_ *Metrics, rerr error) {
globalTelemetryEnabled = cfg.Enabled
if !cfg.Enabled {
return nil, nil
}
Expand Down
37 changes: 37 additions & 0 deletions telemetry/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ 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() {
return
}

metrics.MeasureSinceWithLabels(
keys,
start.UTC(),
Expand All @@ -35,6 +39,10 @@ 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() {
return
}

metrics.SetGaugeWithLabels(
keys,
val,
Expand All @@ -45,29 +53,58 @@ 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() {
return
}

metrics.IncrCounterWithLabels(keys, val, globalLabels)
}

// 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() {
return
}

metrics.IncrCounterWithLabels(keys, val, append(labels, globalLabels...))
}

// SetGauge provides a wrapper functionality for emitting a gauge metric with
// global labels (if any).
func SetGauge(val float32, keys ...string) {
if !IsTelemetryEnabled() {
return
}

metrics.SetGaugeWithLabels(keys, val, globalLabels)
}

// 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() {
return
}

metrics.SetGaugeWithLabels(keys, val, append(labels, globalLabels...))
}

// 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() {
return
}

metrics.MeasureSinceWithLabels(keys, start.UTC(), globalLabels)
}

// Now return the current time if telemetry is enabled or a zero time if it's not
func Now() time.Time {
if !IsTelemetryEnabled() {
return time.Time{}
}

return time.Now()
}
51 changes: 51 additions & 0 deletions telemetry/wrapper_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package telemetry

import (
"sync"
"testing"
"time"

"github.com/stretchr/testify/assert"
)

var mu sync.Mutex

func initTelemetry(v bool) {
globalTelemetryEnabled = v
}

// Reset the global state to a known disabled state before each test.
func setupTest(t *testing.T) {
t.Helper()
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 TestNow(t *testing.T) {
setupTest(t) // Locks the mutex to avoid race condition.

initTelemetry(true)
telemetryTime := Now()
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()
assert.Equal(t, time.Time{}, telemetryTime, "Now() should return zero time when telemetry is disabled")
}

// TestIsTelemetryEnabled tests the IsTelemetryEnabled function.
func TestIsTelemetryEnabled(t *testing.T) {
setupTest(t) // Locks the mutex to avoid race condition.

initTelemetry(true)
assert.True(t, IsTelemetryEnabled(), "IsTelemetryEnabled() should return true when globalTelemetryEnabled is set to true")

setupTest(t) // Reset the global state and lock the mutex again.

initTelemetry(false)
assert.False(t, IsTelemetryEnabled(), "IsTelemetryEnabled() should return false when globalTelemetryEnabled is set to false")
}
3 changes: 1 addition & 2 deletions x/circuit/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"encoding/json"
"fmt"
"time"

gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime"
"google.golang.org/grpc"
Expand Down Expand Up @@ -93,7 +92,7 @@ func (am AppModule) ValidateGenesis(bz json.RawMessage) error {

// InitGenesis performs genesis initialization for the circuit module.
func (am AppModule) InitGenesis(ctx context.Context, data json.RawMessage) error {
start := time.Now()
start := telemetry.Now()
Copy link
Contributor

Choose a reason for hiding this comment

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

The change from using time.Now() to telemetry.Now() in the InitGenesis function aligns with the PR's objective to optimize telemetry operations. However, it would be beneficial to add a comment explaining the reason for this change, enhancing code maintainability.

+	// Using telemetry.Now() to optimize performance when telemetry is disabled
	start := telemetry.Now()

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
start := telemetry.Now()
// Using telemetry.Now() to optimize performance when telemetry is disabled
start := telemetry.Now()

var genesisState types.GenesisState
if err := am.cdc.UnmarshalJSON(data, &genesisState); err != nil {
return err
Expand Down
3 changes: 1 addition & 2 deletions x/crisis/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package crisis

import (
"context"
"time"

"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -12,7 +11,7 @@ import (

// check all registered invariants
func EndBlocker(ctx context.Context, k keeper.Keeper) {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), 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 {
Expand Down
4 changes: 1 addition & 3 deletions x/crisis/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"encoding/json"
"fmt"
"time"

"github.com/spf13/cobra"
"google.golang.org/grpc"
Expand Down Expand Up @@ -118,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 := time.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 {
Expand Down
4 changes: 1 addition & 3 deletions x/distribution/keeper/abci.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package keeper

import (
"time"

"cosmossdk.io/x/distribution/types"

"github.com/cosmos/cosmos-sdk/telemetry"
Expand All @@ -13,7 +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 {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker)
defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyBeginBlocker)

// determine the total power signing the block
var previousTotalPower int64
Expand Down
3 changes: 1 addition & 2 deletions x/evidence/keeper/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package keeper
import (
"context"
"fmt"
"time"

"cosmossdk.io/core/comet"
"cosmossdk.io/x/evidence/types"
Expand All @@ -15,7 +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 {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker)
defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyBeginBlocker)

bi := sdk.UnwrapSDKContext(ctx).CometInfo()

Expand Down
2 changes: 1 addition & 1 deletion x/gov/keeper/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (

// EndBlocker is called every block.
func (k Keeper) EndBlocker(ctx context.Context) error {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker)
defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyEndBlocker)

logger := k.Logger()
// delete dead proposals from store and returns theirs deposits.
Expand Down
3 changes: 1 addition & 2 deletions x/mint/keeper/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package keeper

import (
"context"
"time"

"cosmossdk.io/core/event"
"cosmossdk.io/x/mint/types"
Expand All @@ -13,7 +12,7 @@ import (

// BeginBlocker mints new tokens for the previous block.
func (k Keeper) BeginBlocker(ctx context.Context, ic types.InflationCalculationFn) error {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker)
defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyBeginBlocker)

// fetch stored minter & params
minter, err := k.Minter.Get(ctx)
Expand Down
3 changes: 1 addition & 2 deletions x/slashing/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package slashing

import (
"context"
"time"

"cosmossdk.io/x/slashing/keeper"
"cosmossdk.io/x/slashing/types"
Expand All @@ -14,7 +13,7 @@ import (
// BeginBlocker check for infraction evidence or downtime of validators
// on every begin block
func BeginBlocker(ctx context.Context, k keeper.Keeper) error {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker)
defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyBeginBlocker)

// Iterate over all the validators which *should* have signed this block
// store whether or not they have actually signed it and slash/unbond any
Expand Down
6 changes: 3 additions & 3 deletions x/staking/keeper/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package keeper

import (
"context"
"time"

"cosmossdk.io/core/appmodule"
"cosmossdk.io/x/staking/types"
Expand All @@ -13,12 +12,13 @@ import (
// BeginBlocker will persist the current header and validator set as a historical entry
// and prune the oldest entry based on the HistoricalEntries parameter
func (k *Keeper) BeginBlocker(ctx context.Context) error {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker)
defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyBeginBlocker)
return k.TrackHistoricalInfo(ctx)
}

// EndBlocker called at every block, update validator set
func (k *Keeper) EndBlocker(ctx context.Context) ([]appmodule.ValidatorUpdate, error) {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker)
start := telemetry.Now()
defer telemetry.ModuleMeasureSince(types.ModuleName, start, telemetry.MetricKeyEndBlocker)
return k.BlockValidatorUpdates(ctx)
}
3 changes: 1 addition & 2 deletions x/upgrade/keeper/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
"time"

storetypes "cosmossdk.io/store/types"
"cosmossdk.io/x/upgrade/types"
Expand All @@ -22,7 +21,7 @@ import (
// a migration to be executed if needed upon this switch (migration defined in the new binary)
// skipUpgradeHeightArray is a set of block heights for which the upgrade must be skipped
func (k Keeper) PreBlocker(ctx context.Context) error {
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker)
defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyBeginBlocker)

blockHeight := k.environment.HeaderService.GetHeaderInfo(ctx).Height
plan, err := k.GetUpgradePlan(ctx)
Expand Down
Loading