From 2cf7bd6a053029a808c6f0a6fd244dd50330f166 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 4 Jun 2024 12:39:04 -0400 Subject: [PATCH] Implement label gatherer (#3074) --- api/metrics/gatherer_test.go | 11 +- api/metrics/label_gatherer.go | 76 ++++++++++ api/metrics/label_gatherer_test.go | 217 ++++++++++++++++++++++++++++ api/metrics/multi_gatherer.go | 76 ++-------- api/metrics/multi_gatherer_test.go | 137 ------------------ api/metrics/prefix_gatherer.go | 66 +++++++++ api/metrics/prefix_gatherer_test.go | 150 +++++++++++++++++++ 7 files changed, 527 insertions(+), 206 deletions(-) create mode 100644 api/metrics/label_gatherer.go create mode 100644 api/metrics/label_gatherer_test.go delete mode 100644 api/metrics/multi_gatherer_test.go create mode 100644 api/metrics/prefix_gatherer.go create mode 100644 api/metrics/prefix_gatherer_test.go diff --git a/api/metrics/gatherer_test.go b/api/metrics/gatherer_test.go index 334c361ebcc..83a438867fb 100644 --- a/api/metrics/gatherer_test.go +++ b/api/metrics/gatherer_test.go @@ -4,14 +4,15 @@ package metrics import ( + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" ) -var ( - hello = "hello" - world = "world" - helloWorld = "hello_world" -) +var counterOpts = prometheus.CounterOpts{ + Name: "counter", + Help: "help", +} type testGatherer struct { mfs []*dto.MetricFamily diff --git a/api/metrics/label_gatherer.go b/api/metrics/label_gatherer.go new file mode 100644 index 00000000000..3b8951a75b7 --- /dev/null +++ b/api/metrics/label_gatherer.go @@ -0,0 +1,76 @@ +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package metrics + +import ( + "errors" + "fmt" + "slices" + + "github.com/prometheus/client_golang/prometheus" + + dto "github.com/prometheus/client_model/go" +) + +var ( + _ MultiGatherer = (*prefixGatherer)(nil) + + errDuplicateGatherer = errors.New("attempt to register duplicate gatherer") +) + +// NewLabelGatherer returns a new MultiGatherer that merges metrics by adding a +// new label. +func NewLabelGatherer(labelName string) MultiGatherer { + return &labelGatherer{ + labelName: labelName, + } +} + +type labelGatherer struct { + multiGatherer + + labelName string +} + +func (g *labelGatherer) Register(labelValue string, gatherer prometheus.Gatherer) error { + g.lock.Lock() + defer g.lock.Unlock() + + if slices.Contains(g.names, labelValue) { + return fmt.Errorf("%w: for %q with label %q", + errDuplicateGatherer, + g.labelName, + labelValue, + ) + } + + g.names = append(g.names, labelValue) + g.gatherers = append(g.gatherers, &labeledGatherer{ + labelName: g.labelName, + labelValue: labelValue, + gatherer: gatherer, + }) + return nil +} + +type labeledGatherer struct { + labelName string + labelValue string + gatherer prometheus.Gatherer +} + +func (g *labeledGatherer) Gather() ([]*dto.MetricFamily, error) { + // Gather returns partially filled metrics in the case of an error. So, it + // is expected to still return the metrics in the case an error is returned. + metricFamilies, err := g.gatherer.Gather() + for _, metricFamily := range metricFamilies { + for _, metric := range metricFamily.Metric { + metric.Label = append(metric.Label, &dto.LabelPair{ + Name: &g.labelName, + Value: &g.labelValue, + }) + } + } + return metricFamilies, err +} diff --git a/api/metrics/label_gatherer_test.go b/api/metrics/label_gatherer_test.go new file mode 100644 index 00000000000..d5f30fd6529 --- /dev/null +++ b/api/metrics/label_gatherer_test.go @@ -0,0 +1,217 @@ +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package metrics + +import ( + "testing" + + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" + + dto "github.com/prometheus/client_model/go" +) + +func TestLabelGatherer_Gather(t *testing.T) { + const ( + labelName = "smith" + labelValueA = "rick" + labelValueB = "morty" + customLabelName = "tag" + customLabelValueA = "a" + customLabelValueB = "b" + ) + tests := []struct { + name string + labelName string + expectedMetrics []*dto.Metric + expectErr bool + }{ + { + name: "no overlap", + labelName: customLabelName, + expectedMetrics: []*dto.Metric{ + { + Label: []*dto.LabelPair{ + { + Name: proto.String(labelName), + Value: proto.String(labelValueB), + }, + { + Name: proto.String(customLabelName), + Value: proto.String(customLabelValueB), + }, + }, + Counter: &dto.Counter{ + Value: proto.Float64(1), + }, + }, + { + Label: []*dto.LabelPair{ + { + Name: proto.String(labelName), + Value: proto.String(labelValueA), + }, + { + Name: proto.String(customLabelName), + Value: proto.String(customLabelValueA), + }, + }, + Counter: &dto.Counter{ + Value: proto.Float64(0), + }, + }, + }, + expectErr: false, + }, + { + name: "has overlap", + labelName: labelName, + expectedMetrics: []*dto.Metric{ + { + Label: []*dto.LabelPair{ + { + Name: proto.String(labelName), + Value: proto.String(labelValueB), + }, + { + Name: proto.String(customLabelName), + Value: proto.String(customLabelValueB), + }, + }, + Counter: &dto.Counter{ + Value: proto.Float64(1), + }, + }, + }, + expectErr: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require := require.New(t) + + gatherer := NewLabelGatherer(labelName) + require.NotNil(gatherer) + + registerA := prometheus.NewRegistry() + require.NoError(gatherer.Register(labelValueA, registerA)) + { + counterA := prometheus.NewCounterVec( + counterOpts, + []string{test.labelName}, + ) + counterA.With(prometheus.Labels{test.labelName: customLabelValueA}) + require.NoError(registerA.Register(counterA)) + } + + registerB := prometheus.NewRegistry() + require.NoError(gatherer.Register(labelValueB, registerB)) + { + counterB := prometheus.NewCounterVec( + counterOpts, + []string{customLabelName}, + ) + counterB.With(prometheus.Labels{customLabelName: customLabelValueB}).Inc() + require.NoError(registerB.Register(counterB)) + } + + metrics, err := gatherer.Gather() + if test.expectErr { + require.Error(err) //nolint:forbidigo // the error is not exported + } else { + require.NoError(err) + } + require.Equal( + []*dto.MetricFamily{ + { + Name: proto.String(counterOpts.Name), + Help: proto.String(counterOpts.Help), + Type: dto.MetricType_COUNTER.Enum(), + Metric: test.expectedMetrics, + }, + }, + metrics, + ) + }) + } +} + +func TestLabelGatherer_Register(t *testing.T) { + firstLabeledGatherer := &labeledGatherer{ + labelValue: "first", + gatherer: &testGatherer{}, + } + firstLabelGatherer := func() *labelGatherer { + return &labelGatherer{ + multiGatherer: multiGatherer{ + names: []string{firstLabeledGatherer.labelValue}, + gatherers: prometheus.Gatherers{ + firstLabeledGatherer, + }, + }, + } + } + secondLabeledGatherer := &labeledGatherer{ + labelValue: "second", + gatherer: &testGatherer{ + mfs: []*dto.MetricFamily{{}}, + }, + } + secondLabelGatherer := &labelGatherer{ + multiGatherer: multiGatherer{ + names: []string{ + firstLabeledGatherer.labelValue, + secondLabeledGatherer.labelValue, + }, + gatherers: prometheus.Gatherers{ + firstLabeledGatherer, + secondLabeledGatherer, + }, + }, + } + + tests := []struct { + name string + labelGatherer *labelGatherer + labelValue string + gatherer prometheus.Gatherer + expectedErr error + expectedLabelGatherer *labelGatherer + }{ + { + name: "first registration", + labelGatherer: &labelGatherer{}, + labelValue: "first", + gatherer: firstLabeledGatherer.gatherer, + expectedErr: nil, + expectedLabelGatherer: firstLabelGatherer(), + }, + { + name: "second registration", + labelGatherer: firstLabelGatherer(), + labelValue: "second", + gatherer: secondLabeledGatherer.gatherer, + expectedErr: nil, + expectedLabelGatherer: secondLabelGatherer, + }, + { + name: "conflicts with previous registration", + labelGatherer: firstLabelGatherer(), + labelValue: "first", + gatherer: secondLabeledGatherer.gatherer, + expectedErr: errDuplicateGatherer, + expectedLabelGatherer: firstLabelGatherer(), + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require := require.New(t) + + err := test.labelGatherer.Register(test.labelValue, test.gatherer) + require.ErrorIs(err, test.expectedErr) + require.Equal(test.expectedLabelGatherer, test.labelGatherer) + }) + } +} diff --git a/api/metrics/multi_gatherer.go b/api/metrics/multi_gatherer.go index d8d4d93d2d7..b2fede55643 100644 --- a/api/metrics/multi_gatherer.go +++ b/api/metrics/multi_gatherer.go @@ -4,94 +4,42 @@ package metrics import ( - "cmp" - "errors" "fmt" - "slices" "sync" "github.com/prometheus/client_golang/prometheus" - "github.com/ava-labs/avalanchego/utils/metric" - dto "github.com/prometheus/client_model/go" ) -var ( - _ MultiGatherer = (*multiGatherer)(nil) - - errReregisterGatherer = errors.New("attempt to register existing gatherer") -) - // MultiGatherer extends the Gatherer interface by allowing additional gatherers // to be registered. type MultiGatherer interface { prometheus.Gatherer // Register adds the outputs of [gatherer] to the results of future calls to - // Gather with the provided [namespace] added to the metrics. - Register(namespace string, gatherer prometheus.Gatherer) error + // Gather with the provided [name] added to the metrics. + Register(name string, gatherer prometheus.Gatherer) error } -type multiGatherer struct { - lock sync.RWMutex - gatherers map[string]prometheus.Gatherer +// Deprecated: Use NewPrefixGatherer instead. +// +// TODO: Remove once coreth is updated. +func NewMultiGatherer() MultiGatherer { + return NewPrefixGatherer() } -func NewMultiGatherer() MultiGatherer { - return &multiGatherer{ - gatherers: make(map[string]prometheus.Gatherer), - } +type multiGatherer struct { + lock sync.RWMutex + names []string + gatherers prometheus.Gatherers } func (g *multiGatherer) Gather() ([]*dto.MetricFamily, error) { g.lock.RLock() defer g.lock.RUnlock() - var results []*dto.MetricFamily - for namespace, gatherer := range g.gatherers { - gatheredMetrics, err := gatherer.Gather() - if err != nil { - return nil, err - } - for _, gatheredMetric := range gatheredMetrics { - var name string - if gatheredMetric.Name != nil { - name = metric.AppendNamespace(namespace, *gatheredMetric.Name) - } else { - name = namespace - } - gatheredMetric.Name = &name - results = append(results, gatheredMetric) - } - } - // Because we overwrite every metric's name, we are guaranteed that there - // are no metrics with nil names. - sortMetrics(results) - return results, nil -} - -func (g *multiGatherer) Register(namespace string, gatherer prometheus.Gatherer) error { - g.lock.Lock() - defer g.lock.Unlock() - - if existingGatherer, exists := g.gatherers[namespace]; exists { - return fmt.Errorf("%w for namespace %q; existing: %#v; new: %#v", - errReregisterGatherer, - namespace, - existingGatherer, - gatherer, - ) - } - - g.gatherers[namespace] = gatherer - return nil -} - -func sortMetrics(m []*dto.MetricFamily) { - slices.SortFunc(m, func(i, j *dto.MetricFamily) int { - return cmp.Compare(*i.Name, *j.Name) - }) + return g.gatherers.Gather() } func MakeAndRegister(gatherer MultiGatherer, name string) (*prometheus.Registry, error) { diff --git a/api/metrics/multi_gatherer_test.go b/api/metrics/multi_gatherer_test.go deleted file mode 100644 index 51b548d18a6..00000000000 --- a/api/metrics/multi_gatherer_test.go +++ /dev/null @@ -1,137 +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/prometheus/client_golang/prometheus" - "github.com/stretchr/testify/require" - - dto "github.com/prometheus/client_model/go" -) - -func TestMultiGathererEmptyGather(t *testing.T) { - require := require.New(t) - - g := NewMultiGatherer() - - mfs, err := g.Gather() - require.NoError(err) - require.Empty(mfs) -} - -func TestMultiGathererDuplicatedPrefix(t *testing.T) { - require := require.New(t) - - g := NewMultiGatherer() - og := prometheus.NewRegistry() - - require.NoError(g.Register("", og)) - - err := g.Register("", og) - require.ErrorIs(err, errReregisterGatherer) - - require.NoError(g.Register("lol", og)) -} - -func TestMultiGathererAddedError(t *testing.T) { - require := require.New(t) - - g := NewMultiGatherer() - - errTest := errors.New("non-nil error") - tg := &testGatherer{ - err: errTest, - } - - require.NoError(g.Register("", tg)) - - mfs, err := g.Gather() - require.ErrorIs(err, errTest) - require.Empty(mfs) -} - -func TestMultiGathererNoAddedPrefix(t *testing.T) { - require := require.New(t) - - g := NewMultiGatherer() - - 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) -} - -func TestMultiGathererAddedPrefix(t *testing.T) { - require := require.New(t) - - g := NewMultiGatherer() - - tg := &testGatherer{ - mfs: []*dto.MetricFamily{{ - Name: &world, - }}, - } - - require.NoError(g.Register(hello, tg)) - - mfs, err := g.Gather() - require.NoError(err) - require.Len(mfs, 1) - require.Equal(&helloWorld, mfs[0].Name) -} - -func TestMultiGathererJustPrefix(t *testing.T) { - require := require.New(t) - - g := NewMultiGatherer() - - tg := &testGatherer{ - mfs: []*dto.MetricFamily{{}}, - } - - require.NoError(g.Register(hello, tg)) - - mfs, err := g.Gather() - require.NoError(err) - require.Len(mfs, 1) - require.Equal(&hello, mfs[0].Name) -} - -func TestMultiGathererSorted(t *testing.T) { - require := require.New(t) - - g := NewMultiGatherer() - - name0 := "a" - name1 := "z" - tg := &testGatherer{ - mfs: []*dto.MetricFamily{ - { - Name: &name1, - }, - { - Name: &name0, - }, - }, - } - - require.NoError(g.Register("", tg)) - - mfs, err := g.Gather() - require.NoError(err) - require.Len(mfs, 2) - require.Equal(&name0, mfs[0].Name) - require.Equal(&name1, mfs[1].Name) -} diff --git a/api/metrics/prefix_gatherer.go b/api/metrics/prefix_gatherer.go new file mode 100644 index 00000000000..1f0b78a2438 --- /dev/null +++ b/api/metrics/prefix_gatherer.go @@ -0,0 +1,66 @@ +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package metrics + +import ( + "fmt" + "slices" + + "github.com/prometheus/client_golang/prometheus" + "google.golang.org/protobuf/proto" + + "github.com/ava-labs/avalanchego/utils/metric" + + dto "github.com/prometheus/client_model/go" +) + +var _ MultiGatherer = (*prefixGatherer)(nil) + +// NewPrefixGatherer returns a new MultiGatherer that merges metrics by adding a +// prefix to their names. +func NewPrefixGatherer() MultiGatherer { + return &prefixGatherer{} +} + +type prefixGatherer struct { + multiGatherer +} + +func (g *prefixGatherer) Register(prefix string, gatherer prometheus.Gatherer) error { + g.lock.Lock() + defer g.lock.Unlock() + + // TODO: Restrict prefixes to avoid potential conflicts + if slices.Contains(g.names, prefix) { + return fmt.Errorf("%w: %q", + errDuplicateGatherer, + prefix, + ) + } + + g.names = append(g.names, prefix) + g.gatherers = append(g.gatherers, &prefixedGatherer{ + prefix: prefix, + gatherer: gatherer, + }) + return nil +} + +type prefixedGatherer struct { + prefix string + gatherer prometheus.Gatherer +} + +func (g *prefixedGatherer) Gather() ([]*dto.MetricFamily, error) { + // Gather returns partially filled metrics in the case of an error. So, it + // is expected to still return the metrics in the case an error is returned. + metricFamilies, err := g.gatherer.Gather() + for _, metricFamily := range metricFamilies { + metricFamily.Name = proto.String(metric.AppendNamespace( + g.prefix, + metricFamily.GetName(), + )) + } + return metricFamilies, err +} diff --git a/api/metrics/prefix_gatherer_test.go b/api/metrics/prefix_gatherer_test.go new file mode 100644 index 00000000000..ba37540b01e --- /dev/null +++ b/api/metrics/prefix_gatherer_test.go @@ -0,0 +1,150 @@ +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package metrics + +import ( + "testing" + + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" + + dto "github.com/prometheus/client_model/go" +) + +func TestPrefixGatherer_Gather(t *testing.T) { + require := require.New(t) + + gatherer := NewPrefixGatherer() + require.NotNil(gatherer) + + registerA := prometheus.NewRegistry() + require.NoError(gatherer.Register("a", registerA)) + { + counterA := prometheus.NewCounter(counterOpts) + require.NoError(registerA.Register(counterA)) + } + + registerB := prometheus.NewRegistry() + require.NoError(gatherer.Register("b", registerB)) + { + counterB := prometheus.NewCounter(counterOpts) + counterB.Inc() + require.NoError(registerB.Register(counterB)) + } + + metrics, err := gatherer.Gather() + require.NoError(err) + require.Equal( + []*dto.MetricFamily{ + { + Name: proto.String("a_counter"), + Help: proto.String(counterOpts.Help), + Type: dto.MetricType_COUNTER.Enum(), + Metric: []*dto.Metric{ + { + Label: []*dto.LabelPair{}, + Counter: &dto.Counter{ + Value: proto.Float64(0), + }, + }, + }, + }, + { + Name: proto.String("b_counter"), + Help: proto.String(counterOpts.Help), + Type: dto.MetricType_COUNTER.Enum(), + Metric: []*dto.Metric{ + { + Label: []*dto.LabelPair{}, + Counter: &dto.Counter{ + Value: proto.Float64(1), + }, + }, + }, + }, + }, + metrics, + ) +} + +func TestPrefixGatherer_Register(t *testing.T) { + firstPrefixedGatherer := &prefixedGatherer{ + prefix: "first", + gatherer: &testGatherer{}, + } + firstPrefixGatherer := func() *prefixGatherer { + return &prefixGatherer{ + multiGatherer: multiGatherer{ + names: []string{ + firstPrefixedGatherer.prefix, + }, + gatherers: prometheus.Gatherers{ + firstPrefixedGatherer, + }, + }, + } + } + secondPrefixedGatherer := &prefixedGatherer{ + prefix: "second", + gatherer: &testGatherer{ + mfs: []*dto.MetricFamily{{}}, + }, + } + secondPrefixGatherer := &prefixGatherer{ + multiGatherer: multiGatherer{ + names: []string{ + firstPrefixedGatherer.prefix, + secondPrefixedGatherer.prefix, + }, + gatherers: prometheus.Gatherers{ + firstPrefixedGatherer, + secondPrefixedGatherer, + }, + }, + } + + tests := []struct { + name string + prefixGatherer *prefixGatherer + prefix string + gatherer prometheus.Gatherer + expectedErr error + expectedPrefixGatherer *prefixGatherer + }{ + { + name: "first registration", + prefixGatherer: &prefixGatherer{}, + prefix: firstPrefixedGatherer.prefix, + gatherer: firstPrefixedGatherer.gatherer, + expectedErr: nil, + expectedPrefixGatherer: firstPrefixGatherer(), + }, + { + name: "second registration", + prefixGatherer: firstPrefixGatherer(), + prefix: secondPrefixedGatherer.prefix, + gatherer: secondPrefixedGatherer.gatherer, + expectedErr: nil, + expectedPrefixGatherer: secondPrefixGatherer, + }, + { + name: "conflicts with previous registration", + prefixGatherer: firstPrefixGatherer(), + prefix: firstPrefixedGatherer.prefix, + gatherer: secondPrefixedGatherer.gatherer, + expectedErr: errDuplicateGatherer, + expectedPrefixGatherer: firstPrefixGatherer(), + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require := require.New(t) + + err := test.prefixGatherer.Register(test.prefix, test.gatherer) + require.ErrorIs(err, test.expectedErr) + require.Equal(test.expectedPrefixGatherer, test.prefixGatherer) + }) + } +}