Make the AlreadyRegisteredError useful for wrapped registries

Signed-off-by: beorn7 <beorn@grafana.com>
This commit is contained in:
beorn7 2019-06-14 16:16:15 +02:00
parent 92d8f4a558
commit 2f3a0f8f2e
3 changed files with 133 additions and 21 deletions

View File

@ -325,9 +325,17 @@ func (r *Registry) Register(c Collector) error {
return nil
}
if existing, exists := r.collectorsByID[collectorID]; exists {
return AlreadyRegisteredError{
ExistingCollector: existing,
NewCollector: c,
switch e := existing.(type) {
case *wrappingCollector:
return AlreadyRegisteredError{
ExistingCollector: e.unwrapRecursively(),
NewCollector: c,
}
default:
return AlreadyRegisteredError{
ExistingCollector: e,
NewCollector: c,
}
}
}
// If the collectorID is new, but at least one of the descs existed

View File

@ -746,37 +746,120 @@ func BenchmarkHandler(b *testing.B) {
}
func TestAlreadyRegistered(t *testing.T) {
reg := prometheus.NewRegistry()
original := prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "test",
Help: "help",
Name: "test",
Help: "help",
ConstLabels: prometheus.Labels{"const": "label"},
},
[]string{"foo", "bar"},
)
equalButNotSame := prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "test",
Help: "help",
ConstLabels: prometheus.Labels{"const": "label"},
},
[]string{"foo", "bar"},
)
originalWithoutConstLabel := prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "test",
Help: "help",
},
[]string{"foo", "bar"},
)
var err error
if err = reg.Register(original); err != nil {
t.Fatal(err)
equalButNotSameWithoutConstLabel := prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "test",
Help: "help",
},
[]string{"foo", "bar"},
)
scenarios := []struct {
name string
originalCollector prometheus.Collector
registerWith func(prometheus.Registerer) prometheus.Registerer
newCollector prometheus.Collector
reRegisterWith func(prometheus.Registerer) prometheus.Registerer
}{
{
"RegisterNormallyReregisterNormally",
original,
func(r prometheus.Registerer) prometheus.Registerer { return r },
equalButNotSame,
func(r prometheus.Registerer) prometheus.Registerer { return r },
},
{
"RegisterNormallyReregisterWrapped",
original,
func(r prometheus.Registerer) prometheus.Registerer { return r },
equalButNotSameWithoutConstLabel,
func(r prometheus.Registerer) prometheus.Registerer {
return prometheus.WrapRegistererWith(prometheus.Labels{"const": "label"}, r)
},
},
{
"RegisterWrappedReregisterWrapped",
originalWithoutConstLabel,
func(r prometheus.Registerer) prometheus.Registerer {
return prometheus.WrapRegistererWith(prometheus.Labels{"const": "label"}, r)
},
equalButNotSameWithoutConstLabel,
func(r prometheus.Registerer) prometheus.Registerer {
return prometheus.WrapRegistererWith(prometheus.Labels{"const": "label"}, r)
},
},
{
"RegisterWrappedReregisterNormally",
originalWithoutConstLabel,
func(r prometheus.Registerer) prometheus.Registerer {
return prometheus.WrapRegistererWith(prometheus.Labels{"const": "label"}, r)
},
equalButNotSame,
func(r prometheus.Registerer) prometheus.Registerer { return r },
},
{
"RegisterDoublyWrappedReregisterDoublyWrapped",
originalWithoutConstLabel,
func(r prometheus.Registerer) prometheus.Registerer {
return prometheus.WrapRegistererWithPrefix(
"wrap_",
prometheus.WrapRegistererWith(prometheus.Labels{"const": "label"}, r),
)
},
equalButNotSameWithoutConstLabel,
func(r prometheus.Registerer) prometheus.Registerer {
return prometheus.WrapRegistererWithPrefix(
"wrap_",
prometheus.WrapRegistererWith(prometheus.Labels{"const": "label"}, r),
)
},
},
}
if err = reg.Register(equalButNotSame); err == nil {
t.Fatal("expected error when registering equal collector")
}
if are, ok := err.(prometheus.AlreadyRegisteredError); ok {
if are.ExistingCollector != original {
t.Error("expected original collector but got something else")
}
if are.ExistingCollector == equalButNotSame {
t.Error("expected original callector but got new one")
}
} else {
t.Error("unexpected error:", err)
for _, s := range scenarios {
t.Run(s.name, func(t *testing.T) {
var err error
reg := prometheus.NewRegistry()
if err = s.registerWith(reg).Register(s.originalCollector); err != nil {
t.Fatal(err)
}
if err = s.reRegisterWith(reg).Register(s.newCollector); err == nil {
t.Fatal("expected error when registering new collector")
}
if are, ok := err.(prometheus.AlreadyRegisteredError); ok {
if are.ExistingCollector != s.originalCollector {
t.Error("expected original collector but got something else")
}
if are.ExistingCollector == s.newCollector {
t.Error("expected original collector but got new one")
}
} else {
t.Error("unexpected error:", err)
}
})
}
}

View File

@ -32,6 +32,12 @@ import (
// WrapRegistererWith provides a way to add fixed labels to a subset of
// Collectors. It should not be used to add fixed labels to all metrics exposed.
//
// Conflicts between Collectors registered through the original Registerer with
// Collectors registered through the wrapping Registerer will still be
// detected. Any AlreadyRegisteredError returned by the Register method of
// either Registerer will contain the ExistingCollector in the form it was
// provided to the respective registry.
//
// The Collector example demonstrates a use of WrapRegistererWith.
func WrapRegistererWith(labels Labels, reg Registerer) Registerer {
return &wrappingRegisterer{
@ -54,6 +60,12 @@ func WrapRegistererWith(labels Labels, reg Registerer) Registerer {
// (see NewGoCollector) and the process collector (see NewProcessCollector). (In
// fact, those metrics are already prefixed with “go_” or “process_”,
// respectively.)
//
// Conflicts between Collectors registered through the original Registerer with
// Collectors registered through the wrapping Registerer will still be
// detected. Any AlreadyRegisteredError returned by the Register method of
// either Registerer will contain the ExistingCollector in the form it was
// provided to the respective registry.
func WrapRegistererWithPrefix(prefix string, reg Registerer) Registerer {
return &wrappingRegisterer{
wrappedRegisterer: reg,
@ -123,6 +135,15 @@ func (c *wrappingCollector) Describe(ch chan<- *Desc) {
}
}
func (c *wrappingCollector) unwrapRecursively() Collector {
switch wc := c.wrappedCollector.(type) {
case *wrappingCollector:
return wc.unwrapRecursively()
default:
return wc
}
}
type wrappingMetric struct {
wrappedMetric Metric
prefix string