Skip to content

Commit

Permalink
Add Deregister to metrics.MultiGatherer interface (#3498)
Browse files Browse the repository at this point in the history
  • Loading branch information
StephenButtolph authored Oct 26, 2024
1 parent f212c3e commit acb3d7d
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 29 deletions.
14 changes: 8 additions & 6 deletions api/metrics/label_gatherer.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ func (g *labelGatherer) Register(labelValue string, gatherer prometheus.Gatherer
)
}

g.names = append(g.names, labelValue)
g.gatherers = append(g.gatherers, &labeledGatherer{
labelName: g.labelName,
labelValue: labelValue,
gatherer: gatherer,
})
g.register(
labelValue,
&labeledGatherer{
labelName: g.labelName,
labelValue: labelValue,
gatherer: gatherer,
},
)
return nil
}

Expand Down
100 changes: 88 additions & 12 deletions api/metrics/label_gatherer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,13 @@ func TestLabelGatherer_Gather(t *testing.T) {
}
}

func TestLabelGatherer_Register(t *testing.T) {
func TestLabelGatherer_Registration(t *testing.T) {
const (
firstName = "first"
secondName = "second"
)
firstLabeledGatherer := &labeledGatherer{
labelValue: "first",
labelValue: firstName,
gatherer: &testGatherer{},
}
firstLabelGatherer := func() *labelGatherer {
Expand All @@ -154,25 +158,37 @@ func TestLabelGatherer_Register(t *testing.T) {
}
}
secondLabeledGatherer := &labeledGatherer{
labelValue: "second",
labelValue: secondName,
gatherer: &testGatherer{
mfs: []*dto.MetricFamily{{}},
},
}
secondLabelGatherer := &labelGatherer{
secondLabelGatherer := func() *labelGatherer {
return &labelGatherer{
multiGatherer: multiGatherer{
names: []string{
firstLabeledGatherer.labelValue,
secondLabeledGatherer.labelValue,
},
gatherers: prometheus.Gatherers{
firstLabeledGatherer,
secondLabeledGatherer,
},
},
}
}
onlySecondLabeledGatherer := &labelGatherer{
multiGatherer: multiGatherer{
names: []string{
firstLabeledGatherer.labelValue,
secondLabeledGatherer.labelValue,
},
gatherers: prometheus.Gatherers{
firstLabeledGatherer,
secondLabeledGatherer,
},
},
}

tests := []struct {
registerTests := []struct {
name string
labelGatherer *labelGatherer
labelValue string
Expand All @@ -183,29 +199,29 @@ func TestLabelGatherer_Register(t *testing.T) {
{
name: "first registration",
labelGatherer: &labelGatherer{},
labelValue: "first",
labelValue: firstName,
gatherer: firstLabeledGatherer.gatherer,
expectedErr: nil,
expectedLabelGatherer: firstLabelGatherer(),
},
{
name: "second registration",
labelGatherer: firstLabelGatherer(),
labelValue: "second",
labelValue: secondName,
gatherer: secondLabeledGatherer.gatherer,
expectedErr: nil,
expectedLabelGatherer: secondLabelGatherer,
expectedLabelGatherer: secondLabelGatherer(),
},
{
name: "conflicts with previous registration",
labelGatherer: firstLabelGatherer(),
labelValue: "first",
labelValue: firstName,
gatherer: secondLabeledGatherer.gatherer,
expectedErr: errDuplicateGatherer,
expectedLabelGatherer: firstLabelGatherer(),
},
}
for _, test := range tests {
for _, test := range registerTests {
t.Run(test.name, func(t *testing.T) {
require := require.New(t)

Expand All @@ -214,4 +230,64 @@ func TestLabelGatherer_Register(t *testing.T) {
require.Equal(test.expectedLabelGatherer, test.labelGatherer)
})
}

deregisterTests := []struct {
name string
labelGatherer *labelGatherer
labelValue string
expectedRemoved bool
expectedLabelGatherer *labelGatherer
}{
{
name: "remove from nothing",
labelGatherer: &labelGatherer{},
labelValue: firstName,
expectedRemoved: false,
expectedLabelGatherer: &labelGatherer{},
},
{
name: "remove unknown name",
labelGatherer: firstLabelGatherer(),
labelValue: secondName,
expectedRemoved: false,
expectedLabelGatherer: firstLabelGatherer(),
},
{
name: "remove first name",
labelGatherer: firstLabelGatherer(),
labelValue: firstName,
expectedRemoved: true,
expectedLabelGatherer: &labelGatherer{
multiGatherer: multiGatherer{
// We must populate with empty slices rather than nil slices
// to pass the equality check.
names: []string{},
gatherers: prometheus.Gatherers{},
},
},
},
{
name: "remove second name",
labelGatherer: secondLabelGatherer(),
labelValue: secondName,
expectedRemoved: true,
expectedLabelGatherer: firstLabelGatherer(),
},
{
name: "remove only first name",
labelGatherer: secondLabelGatherer(),
labelValue: firstName,
expectedRemoved: true,
expectedLabelGatherer: onlySecondLabeledGatherer,
},
}
for _, test := range deregisterTests {
t.Run(test.name, func(t *testing.T) {
require := require.New(t)

removed := test.labelGatherer.Deregister(test.labelValue)
require.Equal(test.expectedRemoved, removed)
require.Equal(test.expectedLabelGatherer, test.labelGatherer)
})
}
}
32 changes: 26 additions & 6 deletions api/metrics/multi_gatherer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ package metrics

import (
"fmt"
"slices"
"sync"

"github.com/prometheus/client_golang/prometheus"

"github.com/ava-labs/avalanchego/utils"

dto "github.com/prometheus/client_model/go"
)

Expand All @@ -20,13 +23,11 @@ type MultiGatherer interface {
// Register adds the outputs of [gatherer] to the results of future calls to
// Gather with the provided [name] added to the metrics.
Register(name string, gatherer prometheus.Gatherer) error
}

// Deprecated: Use NewPrefixGatherer instead.
//
// TODO: Remove once coreth is updated.
func NewMultiGatherer() MultiGatherer {
return NewPrefixGatherer()
// Deregister removes the outputs of a gatherer with [name] from the results
// of future calls to Gather. Returns true if a gatherer with [name] was
// found.
Deregister(name string) bool
}

type multiGatherer struct {
Expand All @@ -42,6 +43,25 @@ func (g *multiGatherer) Gather() ([]*dto.MetricFamily, error) {
return g.gatherers.Gather()
}

func (g *multiGatherer) register(name string, gatherer prometheus.Gatherer) {
g.names = append(g.names, name)
g.gatherers = append(g.gatherers, gatherer)
}

func (g *multiGatherer) Deregister(name string) bool {
g.lock.Lock()
defer g.lock.Unlock()

index := slices.Index(g.names, name)
if index == -1 {
return false
}

g.names = utils.DeleteIndex(g.names, index)
g.gatherers = utils.DeleteIndex(g.gatherers, index)
return true
}

func MakeAndRegister(gatherer MultiGatherer, name string) (*prometheus.Registry, error) {
reg := prometheus.NewRegistry()
if err := gatherer.Register(name, reg); err != nil {
Expand Down
12 changes: 7 additions & 5 deletions api/metrics/prefix_gatherer.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,13 @@ func (g *prefixGatherer) Register(prefix string, gatherer prometheus.Gatherer) e
}
}

g.names = append(g.names, prefix)
g.gatherers = append(g.gatherers, &prefixedGatherer{
prefix: prefix,
gatherer: gatherer,
})
g.register(
prefix,
&prefixedGatherer{
prefix: prefix,
gatherer: gatherer,
},
)
return nil
}

Expand Down
18 changes: 18 additions & 0 deletions utils/slice.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package utils

// DeleteIndex moves the last element in the slice to index [i] and shrinks the
// size of the slice by 1.
//
// This is an O(1) operation that allows the removal of an element from a slice
// when the order of the slice is not important.
//
// If [i] is out of bounds, this function will panic.
func DeleteIndex[S ~[]E, E any](s S, i int) S {
newSize := len(s) - 1
s[i] = s[newSize]
s[newSize] = Zero[E]()
return s[:newSize]
}
50 changes: 50 additions & 0 deletions utils/slice_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package utils

import (
"testing"

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

func TestDeleteIndex(t *testing.T) {
tests := []struct {
name string
s []int
i int
expected []int
}{
{
name: "delete only element",
s: []int{0},
i: 0,
expected: []int{},
},
{
name: "delete first element",
s: []int{0, 1},
i: 0,
expected: []int{1},
},
{
name: "delete middle element",
s: []int{0, 1, 2, 3},
i: 1,
expected: []int{0, 3, 2},
},
{
name: "delete last element",
s: []int{0, 1, 2, 3},
i: 3,
expected: []int{0, 1, 2},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
s := DeleteIndex(test.s, test.i)
require.Equal(t, test.expected, s)
})
}
}

0 comments on commit acb3d7d

Please sign in to comment.