From d431eb98456e336b20698b9b79d3d0429c895b4f Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Fri, 24 May 2024 19:00:57 -0400 Subject: [PATCH] Remove optional gatherer --- api/metrics/multi_gatherer_test.go | 6 ++- api/metrics/optional_gatherer.go | 61 ----------------------- api/metrics/optional_gatherer_test.go | 71 --------------------------- chains/linearizable_vm.go | 2 +- chains/manager.go | 15 ++---- snow/context.go | 2 +- snow/snowtest/snowtest.go | 2 +- vms/avm/vm.go | 2 +- vms/metervm/block_vm.go | 10 ++-- vms/metervm/vertex_vm.go | 10 ++-- vms/platformvm/vm.go | 2 +- vms/proposervm/vm.go | 13 ++--- vms/rpcchainvm/vm_client.go | 15 ++---- vms/rpcchainvm/vm_server.go | 2 +- 14 files changed, 30 insertions(+), 183 deletions(-) delete mode 100644 api/metrics/optional_gatherer.go delete mode 100644 api/metrics/optional_gatherer_test.go diff --git a/api/metrics/multi_gatherer_test.go b/api/metrics/multi_gatherer_test.go index 033e3e88b1e..8e63ef6164b 100644 --- a/api/metrics/multi_gatherer_test.go +++ b/api/metrics/multi_gatherer_test.go @@ -4,13 +4,17 @@ package metrics import ( + "errors" "testing" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" dto "github.com/prometheus/client_model/go" ) +var errTest = errors.New("non-nil error") + func TestMultiGathererEmptyGather(t *testing.T) { require := require.New(t) @@ -25,7 +29,7 @@ func TestMultiGathererDuplicatedPrefix(t *testing.T) { require := require.New(t) g := NewMultiGatherer() - og := NewOptionalGatherer() + og := prometheus.NewRegistry() require.NoError(g.Register("", og)) diff --git a/api/metrics/optional_gatherer.go b/api/metrics/optional_gatherer.go deleted file mode 100644 index 686856efcc8..00000000000 --- a/api/metrics/optional_gatherer.go +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package metrics - -import ( - "fmt" - "sync" - - "github.com/prometheus/client_golang/prometheus" - - dto "github.com/prometheus/client_model/go" -) - -var _ OptionalGatherer = (*optionalGatherer)(nil) - -// OptionalGatherer extends the Gatherer interface by allowing the optional -// registration of a single gatherer. If no gatherer is registered, Gather will -// return no metrics and no error. If a gatherer is registered, Gather will -// return the results of calling Gather on the provided gatherer. -type OptionalGatherer interface { - prometheus.Gatherer - - // Register the provided gatherer. If a gatherer was previously registered, - // an error will be returned. - Register(gatherer prometheus.Gatherer) error -} - -type optionalGatherer struct { - lock sync.RWMutex - gatherer prometheus.Gatherer -} - -func NewOptionalGatherer() OptionalGatherer { - return &optionalGatherer{} -} - -func (g *optionalGatherer) Gather() ([]*dto.MetricFamily, error) { - g.lock.RLock() - defer g.lock.RUnlock() - - if g.gatherer == nil { - return nil, nil - } - return g.gatherer.Gather() -} - -func (g *optionalGatherer) Register(gatherer prometheus.Gatherer) error { - g.lock.Lock() - defer g.lock.Unlock() - - if g.gatherer != nil { - return fmt.Errorf("%w; existing: %#v; new: %#v", - errReregisterGatherer, - g.gatherer, - gatherer, - ) - } - g.gatherer = gatherer - return nil -} diff --git a/api/metrics/optional_gatherer_test.go b/api/metrics/optional_gatherer_test.go deleted file mode 100644 index 20175070131..00000000000 --- a/api/metrics/optional_gatherer_test.go +++ /dev/null @@ -1,71 +0,0 @@ -// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package metrics - -import ( - "errors" - "testing" - - "github.com/stretchr/testify/require" - - dto "github.com/prometheus/client_model/go" -) - -var errTest = errors.New("non-nil error") - -func TestOptionalGathererEmptyGather(t *testing.T) { - require := require.New(t) - - g := NewOptionalGatherer() - - mfs, err := g.Gather() - require.NoError(err) - require.Empty(mfs) -} - -func TestOptionalGathererDuplicated(t *testing.T) { - require := require.New(t) - - g := NewOptionalGatherer() - og := NewOptionalGatherer() - - require.NoError(g.Register(og)) - err := g.Register(og) - require.ErrorIs(err, errReregisterGatherer) -} - -func TestOptionalGathererAddedError(t *testing.T) { - require := require.New(t) - - g := NewOptionalGatherer() - - tg := &testGatherer{ - err: errTest, - } - - require.NoError(g.Register(tg)) - - mfs, err := g.Gather() - require.ErrorIs(err, errTest) - require.Empty(mfs) -} - -func TestMultiGathererAdded(t *testing.T) { - require := require.New(t) - - g := NewOptionalGatherer() - - tg := &testGatherer{ - mfs: []*dto.MetricFamily{{ - Name: &hello, - }}, - } - - require.NoError(g.Register(tg)) - - mfs, err := g.Gather() - require.NoError(err) - require.Len(mfs, 1) - require.Equal(&hello, mfs[0].Name) -} diff --git a/chains/linearizable_vm.go b/chains/linearizable_vm.go index 97fe9eb4d1f..0521e418667 100644 --- a/chains/linearizable_vm.go +++ b/chains/linearizable_vm.go @@ -29,7 +29,7 @@ type initializeOnLinearizeVM struct { vmToInitialize common.VM vmToLinearize *linearizeOnInitializeVM - registerer metrics.OptionalGatherer + registerer metrics.MultiGatherer ctx *snow.Context db database.Database genesisBytes []byte diff --git a/chains/manager.go b/chains/manager.go index 5c8d21d2038..c5b79dd470e 100644 --- a/chains/manager.go +++ b/chains/manager.go @@ -434,7 +434,7 @@ func (m *manager) buildChain(chainParams ChainParameters, sb subnets.Subnet) (*c return nil, fmt.Errorf("error while registering DAG metrics %w", err) } - vmMetrics := metrics.NewOptionalGatherer() + vmMetrics := metrics.NewMultiGatherer() vmNamespace := metric.AppendNamespace(chainNamespace, "vm") if err := m.Metrics.Register(vmNamespace, vmMetrics); err != nil { return nil, fmt.Errorf("error while registering vm's metrics %w", err) @@ -642,17 +642,12 @@ func (m *manager) createAvalancheChain( }, ) - avalancheRegisterer := metrics.NewOptionalGatherer() - snowmanRegisterer := metrics.NewOptionalGatherer() - - registerer := metrics.NewMultiGatherer() - if err := registerer.Register("avalanche", avalancheRegisterer); err != nil { - return nil, err - } - if err := registerer.Register("", snowmanRegisterer); err != nil { + avalancheRegisterer := metrics.NewMultiGatherer() + snowmanRegisterer := metrics.NewMultiGatherer() + if err := ctx.Context.Metrics.Register("avalanche", avalancheRegisterer); err != nil { return nil, err } - if err := ctx.Context.Metrics.Register(registerer); err != nil { + if err := ctx.Context.Metrics.Register("", snowmanRegisterer); err != nil { return nil, err } diff --git a/snow/context.go b/snow/context.go index 2cbbedb38b4..f610adca999 100644 --- a/snow/context.go +++ b/snow/context.go @@ -46,7 +46,7 @@ type Context struct { Keystore keystore.BlockchainKeystore SharedMemory atomic.SharedMemory BCLookup ids.AliaserReader - Metrics metrics.OptionalGatherer + Metrics metrics.MultiGatherer WarpSigner warp.Signer diff --git a/snow/snowtest/snowtest.go b/snow/snowtest/snowtest.go index 9879b726955..0ddee75707a 100644 --- a/snow/snowtest/snowtest.go +++ b/snow/snowtest/snowtest.go @@ -90,7 +90,7 @@ func Context(tb testing.TB, chainID ids.ID) *snow.Context { Log: logging.NoLog{}, BCLookup: aliaser, - Metrics: metrics.NewOptionalGatherer(), + Metrics: metrics.NewMultiGatherer(), ValidatorState: validatorState, ChainDataDir: "", diff --git a/vms/avm/vm.go b/vms/avm/vm.go index c2dce8d2705..b8fe322ef95 100644 --- a/vms/avm/vm.go +++ b/vms/avm/vm.go @@ -174,7 +174,7 @@ func (vm *VM) Initialize( ) registerer := prometheus.NewRegistry() - if err := ctx.Metrics.Register(registerer); err != nil { + if err := ctx.Metrics.Register("", registerer); err != nil { return err } vm.registerer = registerer diff --git a/vms/metervm/block_vm.go b/vms/metervm/block_vm.go index 7c93b907857..0ecb982c474 100644 --- a/vms/metervm/block_vm.go +++ b/vms/metervm/block_vm.go @@ -70,18 +70,14 @@ func (vm *blockVM) Initialize( return err } - optionalGatherer := metrics.NewOptionalGatherer() multiGatherer := metrics.NewMultiGatherer() - if err := multiGatherer.Register("metervm", registerer); err != nil { + if err := chainCtx.Metrics.Register("metervm", registerer); err != nil { return err } - if err := multiGatherer.Register("", optionalGatherer); err != nil { + if err := chainCtx.Metrics.Register("", multiGatherer); err != nil { return err } - if err := chainCtx.Metrics.Register(multiGatherer); err != nil { - return err - } - chainCtx.Metrics = optionalGatherer + chainCtx.Metrics = multiGatherer return vm.ChainVM.Initialize(ctx, chainCtx, db, genesisBytes, upgradeBytes, configBytes, toEngine, fxs, appSender) } diff --git a/vms/metervm/vertex_vm.go b/vms/metervm/vertex_vm.go index 8992b486328..7cd112ffde2 100644 --- a/vms/metervm/vertex_vm.go +++ b/vms/metervm/vertex_vm.go @@ -50,18 +50,14 @@ func (vm *vertexVM) Initialize( return err } - optionalGatherer := metrics.NewOptionalGatherer() multiGatherer := metrics.NewMultiGatherer() - if err := multiGatherer.Register("metervm", registerer); err != nil { + if err := chainCtx.Metrics.Register("metervm", registerer); err != nil { return err } - if err := multiGatherer.Register("", optionalGatherer); err != nil { + if err := chainCtx.Metrics.Register("", multiGatherer); err != nil { return err } - if err := chainCtx.Metrics.Register(multiGatherer); err != nil { - return err - } - chainCtx.Metrics = optionalGatherer + chainCtx.Metrics = multiGatherer return vm.LinearizableVMWithEngine.Initialize( ctx, diff --git a/vms/platformvm/vm.go b/vms/platformvm/vm.go index 3358bed5197..f33451b18d4 100644 --- a/vms/platformvm/vm.go +++ b/vms/platformvm/vm.go @@ -114,7 +114,7 @@ func (vm *VM) Initialize( chainCtx.Log.Info("using VM execution config", zap.Reflect("config", execConfig)) registerer := prometheus.NewRegistry() - if err := chainCtx.Metrics.Register(registerer); err != nil { + if err := chainCtx.Metrics.Register("", registerer); err != nil { return err } diff --git a/vms/proposervm/vm.go b/vms/proposervm/vm.go index 02af69b11dd..dfff407a03d 100644 --- a/vms/proposervm/vm.go +++ b/vms/proposervm/vm.go @@ -132,20 +132,15 @@ func (vm *VM) Initialize( ) error { // TODO: Add a helper for this metrics override, it is performed in multiple // places. - multiGatherer := metrics.NewMultiGatherer() registerer := prometheus.NewRegistry() - if err := multiGatherer.Register("proposervm", registerer); err != nil { + if err := chainCtx.Metrics.Register("proposervm", registerer); err != nil { return err } - - optionalGatherer := metrics.NewOptionalGatherer() - if err := multiGatherer.Register("", optionalGatherer); err != nil { - return err - } - if err := chainCtx.Metrics.Register(multiGatherer); err != nil { + multiGatherer := metrics.NewMultiGatherer() + if err := chainCtx.Metrics.Register("", multiGatherer); err != nil { return err } - chainCtx.Metrics = optionalGatherer + chainCtx.Metrics = multiGatherer vm.ctx = chainCtx vm.db = versiondb.New(prefixdb.New(dbPrefix, db)) diff --git a/vms/rpcchainvm/vm_client.go b/vms/rpcchainvm/vm_client.go index 04a3c8fd308..038a728c0ff 100644 --- a/vms/rpcchainvm/vm_client.go +++ b/vms/rpcchainvm/vm_client.go @@ -18,7 +18,6 @@ import ( "google.golang.org/protobuf/types/known/emptypb" "github.com/ava-labs/avalanchego/api/keystore/gkeystore" - "github.com/ava-labs/avalanchego/api/metrics" "github.com/ava-labs/avalanchego/chains/atomic/gsharedmemory" "github.com/ava-labs/avalanchego/database" "github.com/ava-labs/avalanchego/database/rpcdb" @@ -137,15 +136,14 @@ func (vm *VMClient) Initialize( // Register metrics registerer := prometheus.NewRegistry() - multiGatherer := metrics.NewMultiGatherer() vm.grpcServerMetrics = grpc_prometheus.NewServerMetrics() if err := registerer.Register(vm.grpcServerMetrics); err != nil { return err } - if err := multiGatherer.Register("rpcchainvm", registerer); err != nil { + if err := chainCtx.Metrics.Register("rpcchainvm", registerer); err != nil { return err } - if err := multiGatherer.Register("", vm); err != nil { + if err := chainCtx.Metrics.Register("", vm); err != nil { return err } @@ -226,7 +224,7 @@ func (vm *VMClient) Initialize( time: time, } - chainState, err := chain.NewMeteredState( + vm.State, err = chain.NewMeteredState( registerer, &chain.Config{ DecidedCacheSize: decidedCacheSize, @@ -241,12 +239,7 @@ func (vm *VMClient) Initialize( BuildBlockWithContext: vm.buildBlockWithContext, }, ) - if err != nil { - return err - } - vm.State = chainState - - return chainCtx.Metrics.Register(multiGatherer) + return err } func (vm *VMClient) newDBServer(db database.Database) *grpc.Server { diff --git a/vms/rpcchainvm/vm_server.go b/vms/rpcchainvm/vm_server.go index 96132792725..67a55187426 100644 --- a/vms/rpcchainvm/vm_server.go +++ b/vms/rpcchainvm/vm_server.go @@ -225,7 +225,7 @@ func (vm *VMServer) Initialize(ctx context.Context, req *vmpb.InitializeRequest) Keystore: keystoreClient, SharedMemory: sharedMemoryClient, BCLookup: bcLookupClient, - Metrics: metrics.NewOptionalGatherer(), + Metrics: metrics.NewMultiGatherer(), // Signs warp messages WarpSigner: warpSignerClient,