diff --git a/pkg/infra/metrics/service.go b/pkg/infra/metrics/service.go index 5a39501f82f..6b7f1d2354f 100644 --- a/pkg/infra/metrics/service.go +++ b/pkg/infra/metrics/service.go @@ -2,9 +2,10 @@ package metrics import ( "context" - "errors" + "fmt" "regexp" "sort" + "strings" "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" @@ -108,7 +109,7 @@ func (g *addPrefixWrapper) Gather() ([]*dto.MetricFamily, error) { *m.Name = "grafana_" + *m.Name // since we are modifying the name, we need to check for duplicates in the gatherer if _, exists := names[*m.Name]; exists { - return nil, errors.New("duplicate metric name: " + *m.Name) + return nil, fmt.Errorf("duplicate metric name: %s", *m.Name) } } // keep track of names to detect duplicates @@ -140,11 +141,20 @@ func (r *multiRegistry) Gather() (mfs []*dto.MetricFamily, err error) { for i := 0; i < len(mf); i++ { m := mf[i] + _, exists := names[*m.Name] // prevent duplicate metric names - if _, exists := names[*m.Name]; !exists { - names[*m.Name] = struct{}{} - mfs = append(mfs, m) + if exists { + // we can skip go_ metrics without returning an error + // because they are known to be duplicates in both + // the k8s and prometheus gatherers. + if strings.HasPrefix(*m.Name, "go_") { + continue + } + errs = append(errs, fmt.Errorf("duplicate metric name: %s", *m.Name)) + continue } + names[*m.Name] = struct{}{} + mfs = append(mfs, m) } } diff --git a/pkg/infra/metrics/service_test.go b/pkg/infra/metrics/service_test.go index b5c9ea92f50..af489a23965 100644 --- a/pkg/infra/metrics/service_test.go +++ b/pkg/infra/metrics/service_test.go @@ -7,7 +7,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestK8sGathererWrapper_Gather(t *testing.T) { +func TestGathererPrefixWrapper_Gather(t *testing.T) { orig := &mockGatherer{} g := newAddPrefixWrapper(orig) @@ -48,6 +48,88 @@ func TestK8sGathererWrapper_Gather(t *testing.T) { }) } +func TestMultiRegistry_Gather(t *testing.T) { + one := &mockGatherer{} + two := &mockGatherer{} + g := newMultiRegistry(one, two) + + t.Run("should merge and sort metrics", func(t *testing.T) { + one.GatherFunc = func() ([]*dto.MetricFamily, error) { + return []*dto.MetricFamily{ + {Name: strptr("b")}, + {Name: strptr("a")}, + }, nil + } + + two.GatherFunc = func() ([]*dto.MetricFamily, error) { + return []*dto.MetricFamily{ + {Name: strptr("d")}, + {Name: strptr("c")}, + }, nil + } + + expectedMF := []*dto.MetricFamily{ + {Name: strptr("a")}, + {Name: strptr("b")}, + {Name: strptr("c")}, + {Name: strptr("d")}, + } + + mf, err := g.Gather() + require.NoError(t, err) + require.Equal(t, expectedMF, mf) + }) + + t.Run("duplicate metrics result in an error", func(t *testing.T) { + one.GatherFunc = func() ([]*dto.MetricFamily, error) { + return []*dto.MetricFamily{ + {Name: strptr("b")}, + {Name: strptr("a")}, + }, nil + } + + two.GatherFunc = func() ([]*dto.MetricFamily, error) { + return []*dto.MetricFamily{ + {Name: strptr("d")}, + {Name: strptr("c")}, + {Name: strptr("a")}, + }, nil + } + _, err := g.Gather() + require.Error(t, err) + }) + + t.Run("duplicate go_ prefixed metrics do not result in an error", func(t *testing.T) { + one.GatherFunc = func() ([]*dto.MetricFamily, error) { + return []*dto.MetricFamily{ + {Name: strptr("b")}, + {Name: strptr("a")}, + {Name: strptr("go_a")}, + }, nil + } + + two.GatherFunc = func() ([]*dto.MetricFamily, error) { + return []*dto.MetricFamily{ + {Name: strptr("d")}, + {Name: strptr("c")}, + {Name: strptr("go_a")}, + }, nil + } + + expectedMF := []*dto.MetricFamily{ + {Name: strptr("a")}, + {Name: strptr("b")}, + {Name: strptr("c")}, + {Name: strptr("d")}, + {Name: strptr("go_a")}, + } + + mf, err := g.Gather() + require.NoError(t, err) + require.Equal(t, expectedMF, mf) + }) +} + type mockGatherer struct { GatherFunc func() ([]*dto.MetricFamily, error) }