From 4fbf9d3c39e6e39664f975b7d9bb4f0cfb3628a6 Mon Sep 17 00:00:00 2001 From: Gianni Gambetti <99784476+ggambetti@users.noreply.github.com> Date: Tue, 19 Apr 2022 18:22:29 -0400 Subject: [PATCH] Updated documentation and adds tests --- circonus/circonus.go | 6 ++++-- const_unix.go | 1 - const_windows.go | 1 - datadog/dogstatsd.go | 1 + prometheus/prometheus.go | 4 ++++ sink.go | 4 +++- sink_test.go | 14 ++++++++++---- start.go | 8 +++++--- start_test.go | 20 ++++++++++++++++++++ 9 files changed, 47 insertions(+), 12 deletions(-) diff --git a/circonus/circonus.go b/circonus/circonus.go index 2892d24..9f1ea18 100644 --- a/circonus/circonus.go +++ b/circonus/circonus.go @@ -97,9 +97,11 @@ func (s *CirconusSink) AddSampleWithLabels(key []string, val float32, labels []m s.metrics.RecordValue(flatKey, float64(val)) } +// Shutdown blocks while flushing metrics to the backend. func (s *CirconusSink) Shutdown() { - // The used version of the circonus metrics library does not support a shutdown operation. - // Instead we call Flush which blocks until metrics are submitted to storage, and then exit + // The version of circonus metrics in go.mod (v2.3.1), and the current + // version (v3.4.6) do not support a shutdown operation. Instead we call + // Flush which blocks until metrics are submitted to storage, and then exit // as the README examples do. s.metrics.Flush() } diff --git a/const_unix.go b/const_unix.go index 511202d..31098dd 100644 --- a/const_unix.go +++ b/const_unix.go @@ -1,4 +1,3 @@ -//go:build !windows // +build !windows package metrics diff --git a/const_windows.go b/const_windows.go index 6bb1897..38136af 100644 --- a/const_windows.go +++ b/const_windows.go @@ -1,4 +1,3 @@ -//go:build windows // +build windows package metrics diff --git a/datadog/dogstatsd.go b/datadog/dogstatsd.go index 637ddf8..c980004 100644 --- a/datadog/dogstatsd.go +++ b/datadog/dogstatsd.go @@ -120,6 +120,7 @@ func (s *DogStatsdSink) AddSampleWithLabels(key []string, val float32, labels [] s.client.TimeInMilliseconds(flatKey, float64(val), tags, rate) } +// Shutdown disables further metric collection, blocks to flush data, and tears down the sink. func (s *DogStatsdSink) Shutdown() { s.client.Close() } diff --git a/prometheus/prometheus.go b/prometheus/prometheus.go index 9789e20..f89cfd9 100644 --- a/prometheus/prometheus.go +++ b/prometheus/prometheus.go @@ -451,6 +451,10 @@ func (s *PrometheusPushSink) flushMetrics() { }() } +// Shutdown tears down the PrometheusPushSink, and blocks while flushing metrics to the backend. func (s *PrometheusPushSink) Shutdown() { close(s.stopChan) + // Closing the channel only stops the running goroutine that pushes metrics. + // To minimize the chance of data loss pusher.Push is called one last time. + s.pusher.Push() } diff --git a/sink.go b/sink.go index 4e6a8df..b839844 100644 --- a/sink.go +++ b/sink.go @@ -23,7 +23,9 @@ type MetricSink interface { AddSample(key []string, val float32) AddSampleWithLabels(key []string, val float32, labels []Label) - // Shutdown the sink, flushing data, and performing cleanup as necessary. + // Shutdown the metric sink, flush metrics to storage, and cleanup resources. + // Called immediately prior to application exit. Implementations must block + // until metrics are flushed to storage. Shutdown() } diff --git a/sink_test.go b/sink_test.go index e10f678..dbaab7d 100644 --- a/sink_test.go +++ b/sink_test.go @@ -10,9 +10,10 @@ import ( type MockSink struct { lock sync.Mutex - keys [][]string - vals []float32 - labels [][]Label + shutdown bool + keys [][]string + vals []float32 + labels [][]Label } func (m *MockSink) getKeys() [][]string { @@ -63,7 +64,12 @@ func (m *MockSink) AddSampleWithLabels(key []string, val float32, labels []Label m.vals = append(m.vals, val) m.labels = append(m.labels, labels) } -func (m *MockSink) Shutdown() {} +func (m *MockSink) Shutdown() { + m.lock.Lock() + defer m.lock.Unlock() + + m.shutdown = true +} func TestFanoutSink_Gauge(t *testing.T) { m1 := &MockSink{} diff --git a/start.go b/start.go index 4576f5a..38976f8 100644 --- a/start.go +++ b/start.go @@ -145,12 +145,14 @@ func UpdateFilterAndLabels(allow, block, allowedLabels, blockedLabels []string) globalMetrics.Load().(*Metrics).UpdateFilterAndLabels(allow, block, allowedLabels, blockedLabels) } -// Shutdown flushes and disables metric collection, blocking while waiting for this to complete. -// WARNING: Not all MetricSink backends support this functionality, and calling this will cause resource leaks. +// Shutdown disables metric collection, then blocks while attempting to flush metrics to storage. +// WARNING: Not all MetricSink backends support this functionality, and calling this will cause them to leak resources. // This is intended for use immediately prior to application exit. func Shutdown() { m := globalMetrics.Load().(*Metrics) - // Replace global metrics with the BlackholeSink like how init setup the library. + // Swap whatever MetricSink is currently active with a BlackholeSink. Callers must not have a + // reason to expect that calls to the library will successfully collect metrics after Shutdown + // has been called. globalMetrics.Store(&Metrics{sink: &BlackholeSink{}}) m.Shutdown() } diff --git a/start_test.go b/start_test.go index 8ff5ca0..38a7b26 100644 --- a/start_test.go +++ b/start_test.go @@ -192,6 +192,26 @@ func Test_GlobalMetrics_UpdateFilter(t *testing.T) { } } +func Test_GlobalMetrics_Shutdown(t *testing.T) { + s := &MockSink{} + m := &Metrics{sink: s} + globalMetrics.Store(m) + + Shutdown() + + loaded := globalMetrics.Load() + metrics, ok := loaded.(*Metrics) + if !ok { + t.Fatalf("Expected globalMetrics to contain a Metrics pointer, but found: %v", loaded) + } + if metrics == m { + t.Errorf("Calling shutdown should have replaced the Metrics struct stored in globalMetrics") + } + if !s.shutdown { + t.Errorf("Expected Shutdown to have been called on MockSink") + } +} + // Benchmark_GlobalMetrics_Direct/direct-8 5000000 278 ns/op // Benchmark_GlobalMetrics_Direct/atomic.Value-8 5000000 235 ns/op func Benchmark_GlobalMetrics_Direct(b *testing.B) {