From e173ed8e84ad8e348460dd2229732ddc5cc855da Mon Sep 17 00:00:00 2001 From: Nicholas Chung <84615041+chungnicholas@users.noreply.github.com> Date: Mon, 12 Jun 2023 12:28:30 +0800 Subject: [PATCH] Code Refactoring: Refactor telemetry to begin and end block (#316) * Remove hardcoded telemetry from modules * Add telemetry to begin/endblock * Remove test labels --- types/module/module.go | 6 ++++++ x/capability/module.go | 4 ---- x/crisis/abci.go | 6 ------ x/distribution/abci.go | 6 ------ x/evidence/abci.go | 4 ---- x/gov/abci.go | 6 +----- x/mint/abci.go | 4 ---- x/slashing/abci.go | 6 ------ x/staking/abci.go | 12 ++---------- x/upgrade/abci.go | 6 +----- 10 files changed, 10 insertions(+), 50 deletions(-) diff --git a/types/module/module.go b/types/module/module.go index 95e35f3546e4..158442bdc317 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -31,7 +31,9 @@ package module import ( "encoding/json" "fmt" + "github.com/cosmos/cosmos-sdk/telemetry" "sort" + "time" "cosmossdk.io/core/appmodule" abci "github.com/cometbft/cometbft/abci/types" @@ -559,7 +561,9 @@ func (m *Manager) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) abci.R for _, moduleName := range m.OrderBeginBlockers { module, ok := m.Modules[moduleName].(BeginBlockAppModule) if ok { + startTime := time.Now() module.BeginBlock(ctx, req) + telemetry.ModuleMeasureSince(moduleName, startTime, telemetry.MetricKeyBeginBlocker) } } @@ -580,7 +584,9 @@ func (m *Manager) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo if !ok { continue } + startTime := time.Now() moduleValUpdates := module.EndBlock(ctx, req) + telemetry.ModuleMeasureSince(moduleName, startTime, telemetry.MetricKeyEndBlocker) // use these validator updates if provided, the module manager assumes // only one module will update the validator set diff --git a/x/capability/module.go b/x/capability/module.go index b511f2041dbd..4ae7fe276d6a 100644 --- a/x/capability/module.go +++ b/x/capability/module.go @@ -3,7 +3,6 @@ package capability import ( "encoding/json" "fmt" - "time" gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime" "github.com/spf13/cobra" @@ -19,7 +18,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" cdctypes "github.com/cosmos/cosmos-sdk/codec/types" store "github.com/cosmos/cosmos-sdk/store/types" - "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" simtypes "github.com/cosmos/cosmos-sdk/types/simulation" @@ -148,8 +146,6 @@ func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlocker calls InitMemStore to assert that the memory store is initialized. // It's safe to run multiple times. func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { - defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) - am.keeper.InitMemStore(ctx) if am.sealKeeper && !am.keeper.IsSealed() { diff --git a/x/crisis/abci.go b/x/crisis/abci.go index fa1b932b8f63..482a069b5056 100644 --- a/x/crisis/abci.go +++ b/x/crisis/abci.go @@ -1,18 +1,12 @@ package crisis import ( - "time" - - "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/crisis/keeper" - "github.com/cosmos/cosmos-sdk/x/crisis/types" ) // check all registered invariants func EndBlocker(ctx sdk.Context, k keeper.Keeper) { - defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker) - if k.InvCheckPeriod() == 0 || ctx.BlockHeight()%int64(k.InvCheckPeriod()) != 0 { // skip running the invariant check return diff --git a/x/distribution/abci.go b/x/distribution/abci.go index 320f3961feaa..aace65e20a14 100644 --- a/x/distribution/abci.go +++ b/x/distribution/abci.go @@ -1,21 +1,15 @@ package distribution import ( - "time" - abci "github.com/cometbft/cometbft/abci/types" - "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/distribution/keeper" - "github.com/cosmos/cosmos-sdk/x/distribution/types" ) // BeginBlocker sets the proposer for determining distribution during endblock // and distribute rewards for the previous block. func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) { - defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) - // determine the total power signing the block var previousTotalPower int64 for _, voteInfo := range req.LastCommitInfo.GetVotes() { diff --git a/x/evidence/abci.go b/x/evidence/abci.go index 740b3a162ccd..ccf2d9d49aa9 100644 --- a/x/evidence/abci.go +++ b/x/evidence/abci.go @@ -2,11 +2,9 @@ package evidence import ( "fmt" - "time" abci "github.com/cometbft/cometbft/abci/types" - "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/evidence/keeper" "github.com/cosmos/cosmos-sdk/x/evidence/types" @@ -15,8 +13,6 @@ import ( // BeginBlocker iterates through and handles any newly discovered evidence of // misbehavior submitted by Tendermint. Currently, only equivocation is handled. func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) { - defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) - for _, tmEvidence := range req.ByzantineValidators { switch tmEvidence.Type { // It's still ongoing discussion how should we treat and slash attacks with diff --git a/x/gov/abci.go b/x/gov/abci.go index fd0df2981779..cb53ec75cf44 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -2,9 +2,7 @@ package gov import ( "fmt" - "time" - "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/gov/keeper" "github.com/cosmos/cosmos-sdk/x/gov/types" @@ -12,9 +10,7 @@ import ( ) // EndBlocker called every block, process inflation, update validator set. -func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) { - defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker) - +func EndBlocker(ctx sdk.Context, keeper keeper.Keeper) { logger := keeper.Logger(ctx) // delete dead proposals from store and returns theirs deposits. diff --git a/x/mint/abci.go b/x/mint/abci.go index f8f0c8ce411f..f0bb235595c1 100644 --- a/x/mint/abci.go +++ b/x/mint/abci.go @@ -1,8 +1,6 @@ package mint import ( - "time" - "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/mint/keeper" @@ -11,8 +9,6 @@ import ( // BeginBlocker mints new tokens for the previous block. func BeginBlocker(ctx sdk.Context, k keeper.Keeper, ic types.InflationCalculationFn) { - defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) - // fetch stored minter & params minter := k.GetMinter(ctx) params := k.GetParams(ctx) diff --git a/x/slashing/abci.go b/x/slashing/abci.go index adbe0db78f9c..ae55f190e99a 100644 --- a/x/slashing/abci.go +++ b/x/slashing/abci.go @@ -1,21 +1,15 @@ package slashing import ( - "time" - abci "github.com/cometbft/cometbft/abci/types" - "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/slashing/keeper" - "github.com/cosmos/cosmos-sdk/x/slashing/types" ) // BeginBlocker check for infraction evidence or downtime of validators // on every begin block func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) { - defer telemetry.ModuleMeasureSince(types.ModuleName, time.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 // which have missed too many blocks in a row (downtime slashing) diff --git a/x/staking/abci.go b/x/staking/abci.go index 1912beb99747..f698561a3fb4 100644 --- a/x/staking/abci.go +++ b/x/staking/abci.go @@ -1,27 +1,19 @@ package staking import ( - "time" - abci "github.com/cometbft/cometbft/abci/types" - "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/staking/keeper" - "github.com/cosmos/cosmos-sdk/x/staking/types" ) // BeginBlocker will persist the current header and validator set as a historical entry // and prune the oldest entry based on the HistoricalEntries parameter -func BeginBlocker(ctx sdk.Context, k *keeper.Keeper) { - defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) - +func BeginBlocker(ctx sdk.Context, k keeper.Keeper) { k.TrackHistoricalInfo(ctx) } // Called every block, update validator set -func EndBlocker(ctx sdk.Context, k *keeper.Keeper) []abci.ValidatorUpdate { - defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker) - +func EndBlocker(ctx sdk.Context, k keeper.Keeper) []abci.ValidatorUpdate { return k.BlockValidatorUpdates(ctx) } diff --git a/x/upgrade/abci.go b/x/upgrade/abci.go index 188e0cd02b5b..bbd083a45f11 100644 --- a/x/upgrade/abci.go +++ b/x/upgrade/abci.go @@ -2,11 +2,9 @@ package upgrade import ( "fmt" - "time" abci "github.com/cometbft/cometbft/abci/types" - "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/upgrade/keeper" "github.com/cosmos/cosmos-sdk/x/upgrade/types" @@ -20,9 +18,7 @@ import ( // The purpose is to ensure the binary is switched EXACTLY at the desired block, and to allow // 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 BeginBlocker(k *keeper.Keeper, ctx sdk.Context, _ abci.RequestBeginBlock) { - defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) - +func BeginBlocker(k keeper.Keeper, ctx sdk.Context, _ abci.RequestBeginBlock) { plan, found := k.GetUpgradePlan(ctx) if !k.DowngradeVerified() {