zalando/skipper

Deprecate metrics.Default

AlexanderYastrebov opened this issue · 0 comments

Is your feature request related to a problem? Please describe.

There are quite a few places that rely on global metrics.Default variable

skipper/metrics/metrics.go

Lines 199 to 202 in 0ce11b9

var (
Default Metrics
Void Metrics
)

This is a bad design which is not thread-safe and complicates tests, e.g.:

func TestMetricsUncompressed(t *testing.T) {
dm := metrics.Default
t.Cleanup(func() { metrics.Default = dm })
m := &metricstest.MockMetrics{}
metrics.Default = m

Describe the solution you would like

Components that update metrics receive metrics instance via configuration.
First we may refactor components to make metrics configurable and fallback to metrics.Default for backwards compatibility.
Then deprecate and later remove metrics.Default completely.

Would you like to work on it?

Yes