From 9180854cf5a015b52b88b8c4d3aebeef411fbe78 Mon Sep 17 00:00:00 2001 From: Sebastien Launay Date: Sat, 2 Sep 2017 15:25:18 -0700 Subject: [PATCH] Use set of meters and update documentation - use map[*StandardMeter]struct{} instead of map[int64]*StandardMeter to avoid id management pitfalls - update documentation and examples --- README.md | 12 +++++++++++- meter.go | 28 +++++++++++++--------------- meter_test.go | 5 ++--- timer.go | 6 ++++++ 4 files changed, 32 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 2d1a6dc..0405fbd 100644 --- a/README.md +++ b/README.md @@ -42,12 +42,22 @@ t.Update(47) Register() is not threadsafe. For threadsafe metric registration use GetOrRegister: -``` +```go t := metrics.GetOrRegisterTimer("account.create.latency", nil) t.Time(func() {}) t.Update(47) ``` +**NOTE:** Be sure to either unregister meters and timers otherwise and they will +leak memory: + +```go +// Will call Stop() on the Meter to allow for garbage collection +metrics.Unregister("quux") +// Or similarly for a Timer that embeds a Meter +metrics.Unregister("bang") +``` + Periodically log every metric in human-readable form to standard error: ```go diff --git a/meter.go b/meter.go index 882029e..53ff329 100644 --- a/meter.go +++ b/meter.go @@ -20,6 +20,8 @@ type Meter interface { // GetOrRegisterMeter returns an existing Meter or constructs and registers a // new StandardMeter. +// Be sure to unregister the meter from the registry once it is of no use to +// allow for garbage collection. func GetOrRegisterMeter(name string, r Registry) Meter { if nil == r { r = DefaultRegistry @@ -28,6 +30,7 @@ func GetOrRegisterMeter(name string, r Registry) Meter { } // NewMeter constructs a new StandardMeter and launches a goroutine. +// Be sure to call Stop() once the meter is of no use to allow for garbage collection. func NewMeter() Meter { if UseNilMetrics { return NilMeter{} @@ -35,8 +38,7 @@ func NewMeter() Meter { m := newStandardMeter() arbiter.Lock() defer arbiter.Unlock() - m.id = arbiter.newID() - arbiter.meters[m.id] = m + arbiter.meters[m] = struct{}{} if !arbiter.started { arbiter.started = true go arbiter.tick() @@ -46,6 +48,8 @@ func NewMeter() Meter { // NewMeter constructs and registers a new StandardMeter and launches a // goroutine. +// Be sure to unregister the meter from the registry once it is of no use to +// allow for garbage collection. func NewRegisteredMeter(name string, r Registry) Meter { c := NewMeter() if nil == r { @@ -125,7 +129,6 @@ type StandardMeter struct { a1, a5, a15 EWMA startTime time.Time stopped bool - id int64 } func newStandardMeter() *StandardMeter { @@ -138,7 +141,7 @@ func newStandardMeter() *StandardMeter { } } -// Stop stops the meter, Mark() will panic if you use it after being stopped. +// Stop stops the meter, Mark() will be a no-op if you use it after being stopped. func (m *StandardMeter) Stop() { m.lock.Lock() stopped := m.stopped @@ -146,7 +149,7 @@ func (m *StandardMeter) Stop() { m.lock.Unlock() if !stopped { arbiter.Lock() - delete(arbiter.meters, m.id) + delete(arbiter.meters, m) arbiter.Unlock() } } @@ -231,15 +234,16 @@ func (m *StandardMeter) tick() { m.updateSnapshot() } +// meterArbiter ticks meters every 5s from a single goroutine. +// meters are references in a set for future stopping. type meterArbiter struct { sync.RWMutex started bool - meters map[int64]*StandardMeter + meters map[*StandardMeter]struct{} ticker *time.Ticker - id int64 } -var arbiter = meterArbiter{ticker: time.NewTicker(5e9), meters: make(map[int64]*StandardMeter)} +var arbiter = meterArbiter{ticker: time.NewTicker(5e9), meters: make(map[*StandardMeter]struct{})} // Ticks meters on the scheduled interval func (ma *meterArbiter) tick() { @@ -251,16 +255,10 @@ func (ma *meterArbiter) tick() { } } -// should only be called with Lock() held -func (ma *meterArbiter) newID() int64 { - ma.id++ - return ma.id -} - func (ma *meterArbiter) tickMeters() { ma.RLock() defer ma.RUnlock() - for _, meter := range ma.meters { + for meter := range ma.meters { meter.tick() } } diff --git a/meter_test.go b/meter_test.go index a9f66d7..e889222 100644 --- a/meter_test.go +++ b/meter_test.go @@ -24,11 +24,10 @@ func TestGetOrRegisterMeter(t *testing.T) { func TestMeterDecay(t *testing.T) { ma := meterArbiter{ ticker: time.NewTicker(time.Millisecond), - meters: make(map[int64]*StandardMeter), + meters: make(map[*StandardMeter]struct{}), } m := newStandardMeter() - m.id = ma.newID() - ma.meters[m.id] = m + ma.meters[m] = struct{}{} go ma.tick() m.Mark(1) rateMean := m.RateMean() diff --git a/timer.go b/timer.go index d817e78..d6ec4c6 100644 --- a/timer.go +++ b/timer.go @@ -29,6 +29,8 @@ type Timer interface { // GetOrRegisterTimer returns an existing Timer or constructs and registers a // new StandardTimer. +// Be sure to unregister the meter from the registry once it is of no use to +// allow for garbage collection. func GetOrRegisterTimer(name string, r Registry) Timer { if nil == r { r = DefaultRegistry @@ -37,6 +39,7 @@ func GetOrRegisterTimer(name string, r Registry) Timer { } // NewCustomTimer constructs a new StandardTimer from a Histogram and a Meter. +// Be sure to call Stop() once the timer is of no use to allow for garbage collection. func NewCustomTimer(h Histogram, m Meter) Timer { if UseNilMetrics { return NilTimer{} @@ -48,6 +51,8 @@ func NewCustomTimer(h Histogram, m Meter) Timer { } // NewRegisteredTimer constructs and registers a new StandardTimer. +// Be sure to unregister the meter from the registry once it is of no use to +// allow for garbage collection. func NewRegisteredTimer(name string, r Registry) Timer { c := NewTimer() if nil == r { @@ -59,6 +64,7 @@ func NewRegisteredTimer(name string, r Registry) Timer { // NewTimer constructs a new StandardTimer using an exponentially-decaying // sample with the same reservoir size and alpha as UNIX load averages. +// Be sure to call Stop() once the timer is of no use to allow for garbage collection. func NewTimer() Timer { if UseNilMetrics { return NilTimer{}