Skip to content

Commit

Permalink
Return an error from Register and disallow dupes
Browse files Browse the repository at this point in the history
This is a change to the interface that will appear as a breaking change
for authors of Registry implementations.  The change was made in support
of the change described below.

rcrowley/go-tigertonic#60 points out that metrics can be reregistered
causing the original metric to be lost.  This patch change the Registry
interface's Register method to return an error and implements this
change in StandardRegistry.Register and Register.
StandardRegistry.Register (and therefore Register) returns a
DuplicateMetric error if a name is already registered.
  • Loading branch information
rcrowley committed Apr 24, 2014
1 parent d177436 commit 426a433
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 8 deletions.
32 changes: 24 additions & 8 deletions registry.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
package metrics

import (
"fmt"
"reflect"
"sync"
)

// DuplicateMetric is the error returned by Registry.Register when a metric
// already exists. If you mean to Register that metric you must first
// Unregister the existing metric.
type DuplicateMetric string

func (err DuplicateMetric) Error() string {
return fmt.Sprintf("duplicate metric: %s", err)
}

// A Registry holds references to a set of metrics by name and can iterate
// over them, calling callback functions provided by the user.
//
Expand All @@ -24,7 +34,7 @@ type Registry interface {
GetOrRegister(string, interface{}) interface{}

// Register the given metric under the given name.
Register(string, interface{})
Register(string, interface{}) error

// Run all registered healthchecks.
RunHealthchecks()
Expand Down Expand Up @@ -76,11 +86,12 @@ func (r *StandardRegistry) GetOrRegister(name string, i interface{}) interface{}
return i
}

// Register the given metric under the given name.
func (r *StandardRegistry) Register(name string, i interface{}) {
// Register the given metric under the given name. Returns a DuplicateMetric
// if a metric by the given name is already registered.
func (r *StandardRegistry) Register(name string, i interface{}) error {
r.mutex.Lock()
defer r.mutex.Unlock()
r.register(name, i)
return r.register(name, i)
}

// Run all registered healthchecks.
Expand All @@ -101,11 +112,15 @@ func (r *StandardRegistry) Unregister(name string) {
delete(r.metrics, name)
}

func (r *StandardRegistry) register(name string, i interface{}) {
func (r *StandardRegistry) register(name string, i interface{}) error {
if _, ok := r.metrics[name]; ok {
return DuplicateMetric(name)
}
switch i.(type) {
case Counter, Gauge, GaugeFloat64, Healthcheck, Histogram, Meter, Timer:
r.metrics[name] = i
}
return nil
}

func (r *StandardRegistry) registered() map[string]interface{} {
Expand Down Expand Up @@ -136,9 +151,10 @@ func GetOrRegister(name string, i interface{}) interface{} {
return DefaultRegistry.GetOrRegister(name, i)
}

// Register the given metric under the given name.
func Register(name string, i interface{}) {
DefaultRegistry.Register(name, i)
// Register the given metric under the given name. Returns a DuplicateMetric
// if a metric by the given name is already registered.
func Register(name string, i interface{}) error {
return DefaultRegistry.Register(name, i)
}

// Run all registered healthchecks.
Expand Down
20 changes: 20 additions & 0 deletions registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,26 @@ func TestRegistry(t *testing.T) {
}
}

func TestRegistryDuplicate(t *testing.T) {
r := NewRegistry()
if err := r.Register("foo", NewCounter()); nil != err {
t.Fatal(err)
}
if err := r.Register("foo", NewGauge()); nil == err {
t.Fatal(err)
}
i := 0
r.Each(func(name string, iface interface{}) {
i++
if _, ok := iface.(Counter); !ok {
t.Fatal(iface)
}
})
if 1 != i {
t.Fatal(i)
}
}

func TestRegistryGet(t *testing.T) {
r := NewRegistry()
r.Register("foo", NewCounter())
Expand Down

0 comments on commit 426a433

Please sign in to comment.