add default metricoptions where nil (#2993)

* add default metricoptions where nil * revert changes to MetricProvider interface
temporalio · Jun 14, 2022 · 2453946 · 2453946
1 parent b3c2810
commit 2453946
Show file tree

Hide file tree

Showing 10 changed files with 100 additions and 169 deletions.
diff --git a/common/metrics/events.go b/common/metrics/events.go
@@ -48,8 +48,19 @@ type (
 )
 
 var (
-	_                      MetricProvider = (*eventsMetricProvider)(nil)
-	defaultMetricNamespace                = "go.temporal.io/server"
+	_ MetricProvider = (*eventsMetricProvider)(nil)
+
+	defaultOptions = &event.MetricOptions{
+		Namespace: defaultMetricNamespace,
+		Unit:      Dimensionless,
+	}
+
+	defaultTimerOptions = &event.MetricOptions{
+		Namespace: defaultMetricNamespace,
+		Unit:      Milliseconds,
+	}
+
+	defaultMetricNamespace = "go.temporal.io/server"
 )
 
 // MetricHandlerFromConfig is used at startup to construct
@@ -118,32 +129,35 @@ func (emp *eventsMetricProvider) WithTags(tags ...Tag) MetricProvider {
 }
 
 // Counter obtains a counter for the given name.
-func (emp *eventsMetricProvider) Counter(n string, m *MetricOptions) CounterMetric {
-	e := event.NewCounter(n, m)
+func (emp *eventsMetricProvider) Counter(n string) CounterMetric {
+	e := event.NewCounter(n, defaultOptions)
 	return CounterMetricFunc(func(i int64, t ...Tag) {
 		e.Record(emp.ctx, i, tagsToLabels(emp.tags, t)...)
 	})
 }
 
 // Gauge obtains a gauge for the given name.
-func (emp *eventsMetricProvider) Gauge(n string, m *MetricOptions) GaugeMetric {
-	e := event.NewFloatGauge(n, m)
+func (emp *eventsMetricProvider) Gauge(n string) GaugeMetric {
+	e := event.NewFloatGauge(n, defaultOptions)
 	return GaugeMetricFunc(func(f float64, t ...Tag) {
 		e.Record(emp.ctx, f, tagsToLabels(emp.tags, t)...)
 	})
 }
 
 // Timer obtains a timer for the given name.
-func (emp *eventsMetricProvider) Timer(n string, m *MetricOptions) TimerMetric {
-	e := event.NewDuration(n, m)
+func (emp *eventsMetricProvider) Timer(n string) TimerMetric {
+	e := event.NewDuration(n, defaultTimerOptions)
 	return TimerMetricFunc(func(d time.Duration, t ...Tag) {
 		e.Record(emp.ctx, d, tagsToLabels(emp.tags, t)...)
 	})
 }
 
 // Histogram obtains a histogram for the given name.
-func (emp *eventsMetricProvider) Histogram(n string, m *MetricOptions) HistogramMetric {
-	e := event.NewIntDistribution(n, m)
+func (emp *eventsMetricProvider) Histogram(n string, m MetricUnit) HistogramMetric {
+	e := event.NewIntDistribution(n, &event.MetricOptions{
+		Namespace: defaultMetricNamespace,
+		Unit:      event.Unit(m),
+	})
 	return HistogramMetricFunc(func(i int64, t ...Tag) {
 		e.Record(emp.ctx, i, tagsToLabels(emp.tags, t)...)
 	})

diff --git a/common/metrics/events_scope.go b/common/metrics/events_scope.go
@@ -28,7 +28,6 @@ import (
 	"fmt"
 	"time"
 
-	"golang.org/x/exp/event"
 	"golang.org/x/exp/maps"
 )
 
@@ -81,16 +80,10 @@ func (e *eventsScope) IncCounter(counter int) {
 func (e *eventsScope) AddCounter(counter int, delta int64) {
 	m := getDefinition(e.serviceIdx, counter)
 
-	e.provider.Counter(m.metricName.String(), &MetricOptions{
-		Namespace: defaultMetricNamespace,
-		Unit:      event.Unit(m.unit),
-	}).Record(int64(delta), e.scopeTags...)
+	e.provider.Counter(m.metricName.String()).Record(int64(delta), e.scopeTags...)
 
 	if !m.metricRollupName.Empty() {
-		e.provider.Counter(m.metricRollupName.String(), &event.MetricOptions{
-			Namespace: defaultMetricNamespace,
-			Unit:      event.Unit(m.unit),
-		}).Record(int64(delta), e.scopeTags...)
+		e.provider.Counter(m.metricRollupName.String()).Record(int64(delta), e.scopeTags...)
 	}
 }
 
@@ -101,16 +94,10 @@ func (e *eventsScope) StartTimer(timer int) Stopwatch {
 
 	return &eventsStopwatch{
 		recordFunc: func(d time.Duration) {
-			e.provider.Timer(m.metricName.String(), &MetricOptions{
-				Namespace: defaultMetricNamespace,
-				Unit:      event.Unit(m.unit),
-			}).Record(d, e.scopeTags...)
+			e.provider.Timer(m.metricName.String()).Record(d, e.scopeTags...)
 
 			if !m.metricRollupName.Empty() {
-				e.provider.Timer(m.metricRollupName.String(), &MetricOptions{
-					Namespace: defaultMetricNamespace,
-					Unit:      event.Unit(m.unit),
-				}).Record(d, e.scopeTags...)
+				e.provider.Timer(m.metricRollupName.String()).Record(d, e.scopeTags...)
 			}
 		},
 		start: time.Now(),
@@ -121,16 +108,10 @@ func (e *eventsScope) StartTimer(timer int) Stopwatch {
 func (e *eventsScope) RecordTimer(timer int, d time.Duration) {
 	m := getDefinition(e.serviceIdx, timer)
 
-	e.provider.Timer(m.metricName.String(), &MetricOptions{
-		Namespace: defaultMetricNamespace,
-		Unit:      event.Unit(m.unit),
-	}).Record(d, e.scopeTags...)
+	e.provider.Timer(m.metricName.String()).Record(d, e.scopeTags...)
 
 	if !m.metricRollupName.Empty() {
-		e.provider.Timer(m.metricRollupName.String(), &event.MetricOptions{
-			Namespace: defaultMetricNamespace,
-			Unit:      event.Unit(m.unit),
-		}).Record(d, e.scopeTags...)
+		e.provider.Timer(m.metricRollupName.String()).Record(d, e.scopeTags...)
 	}
 }
 
@@ -139,33 +120,21 @@ func (e *eventsScope) RecordTimer(timer int, d time.Duration) {
 func (e *eventsScope) RecordDistribution(id int, d int) {
 	m := getDefinition(e.serviceIdx, id)
 
-	e.provider.Histogram(m.metricName.String(), &MetricOptions{
-		Namespace: defaultMetricNamespace,
-		Unit:      event.Unit(m.unit),
-	}).Record(int64(d), e.scopeTags...)
+	e.provider.Histogram(m.metricName.String(), m.unit).Record(int64(d), e.scopeTags...)
 
 	if !m.metricRollupName.Empty() {
-		e.provider.Histogram(m.metricRollupName.String(), &event.MetricOptions{
-			Namespace: defaultMetricNamespace,
-			Unit:      event.Unit(m.unit),
-		}).Record(int64(d), e.scopeTags...)
+		e.provider.Histogram(m.metricRollupName.String(), m.unit).Record(int64(d), e.scopeTags...)
 	}
 }
 
 // UpdateGauge reports Gauge type absolute value metric
 func (e *eventsScope) UpdateGauge(gauge int, value float64) {
 	m := getDefinition(e.serviceIdx, gauge)
 
-	e.provider.Gauge(m.metricName.String(), &MetricOptions{
-		Namespace: defaultMetricNamespace,
-		Unit:      event.Unit(m.unit),
-	}).Record(value, e.scopeTags...)
+	e.provider.Gauge(m.metricName.String()).Record(value, e.scopeTags...)
 
 	if !m.metricRollupName.Empty() {
-		e.provider.Gauge(m.metricRollupName.String(), &event.MetricOptions{
-			Namespace: defaultMetricNamespace,
-			Unit:      event.Unit(m.unit),
-		}).Record(value, e.scopeTags...)
+		e.provider.Gauge(m.metricRollupName.String()).Record(value, e.scopeTags...)
 	}
 }
 

diff --git a/common/metrics/events_test.go b/common/metrics/events_test.go
@@ -154,9 +154,7 @@ func (s *eventsSuite) TestCounterMetricFunc_Record() {
 	for _, tt := range tests {
 		s.T().Run(tt.name, func(t *testing.T) {
 			emp := NewEventsMetricProvider(NewOtelMetricHandler(log.NewTestLogger(), &testProvider{meter: meterProvider.Meter("test")}, ClientConfig{}))
-			emp.Counter(tt.name, &MetricOptions{
-				Description: "what you see is not a test",
-			}).Record(tt.v, tt.tags...)
+			emp.Counter(tt.name).Record(tt.v, tt.tags...)
 			testexporter.Collect(ctx)
 
 			s.NotEmpty(testexporter.Records)
@@ -212,9 +210,7 @@ func (s *eventsSuite) TestGaugeMetricFunc_Record() {
 	for _, tt := range tests {
 		s.T().Run(tt.name, func(t *testing.T) {
 			emp := NewEventsMetricProvider(NewOtelMetricHandler(log.NewTestLogger(), &testProvider{meter: meterProvider.Meter("test")}, ClientConfig{}))
-			emp.Gauge(tt.name, &MetricOptions{
-				Description: "what you see is not a test",
-			}).Record(tt.v, tt.tags...)
+			emp.Gauge(tt.name).Record(tt.v, tt.tags...)
 			testexporter.Collect(ctx)
 
 			s.NotEmpty(testexporter.Records)
@@ -276,10 +272,7 @@ func (s *eventsSuite) TestTimerMetricFunc_Record() {
 	for _, tt := range tests {
 		s.T().Run(tt.name, func(t *testing.T) {
 			emp := NewEventsMetricProvider(NewOtelMetricHandler(log.NewTestLogger(), &testProvider{meter: meterProvider.Meter("test")}, ClientConfig{}))
-			emp.Timer(tt.name, &MetricOptions{
-				Description: "what you see is not a test",
-				Unit:        Milliseconds,
-			}).Record(tt.v, tt.tags...)
+			emp.Timer(tt.name).Record(tt.v, tt.tags...)
 			testexporter.Collect(ctx)
 
 			s.NotEmpty(testexporter.Records)
@@ -341,10 +334,7 @@ func (s *eventsSuite) TestHistogramMetricFunc_Record() {
 	for _, tt := range tests {
 		s.T().Run(tt.name, func(t *testing.T) {
 			emp := NewEventsMetricProvider(NewOtelMetricHandler(log.NewTestLogger(), &testProvider{meter: meterProvider.Meter("test")}, ClientConfig{}))
-			emp.Histogram(tt.name, &MetricOptions{
-				Description: "what you see is not a test",
-				Unit:        Bytes,
-			}).Record(tt.v, tt.tags...)
+			emp.Histogram(tt.name, Bytes).Record(tt.v, tt.tags...)
 			testexporter.Collect(ctx)
 
 			s.NotEmpty(testexporter.Records)
@@ -405,9 +395,7 @@ func (s *eventsSuite) TestCounterMetricWithTagsMergeFunc_Record() {
 	for _, tt := range tests {
 		s.T().Run(tt.name, func(t *testing.T) {
 			emp := NewEventsMetricProvider(NewOtelMetricHandler(log.NewTestLogger(), &testProvider{meter: meterProvider.Meter("test")}, ClientConfig{})).WithTags(tt.rootTags...).(*eventsMetricProvider)
-			emp.Counter(tt.name, &MetricOptions{
-				Description: "what you see is not a test",
-			}).Record(tt.v, tt.tags...)
+			emp.Counter(tt.name).Record(tt.v, tt.tags...)
 			testexporter.Collect(ctx)
 
 			s.NotEmpty(testexporter.Records)
@@ -429,10 +417,7 @@ func BenchmarkParallelHistogram(b *testing.B) {
 	b.RunParallel(
 		func(p *testing.PB) {
 			for p.Next() {
-				emp.Histogram("test-bench-histogram", &MetricOptions{
-					Description: "what you see is not a test",
-					Unit:        Bytes,
-				}).Record(1024)
+				emp.Histogram("test-bench-histogram", Bytes).Record(1024)
 			}
 		},
 	)
@@ -446,7 +431,7 @@ func BenchmarkParallelCounter(b *testing.B) {
 	b.RunParallel(
 		func(p *testing.PB) {
 			for p.Next() {
-				emp.Counter("test-bench-counter", nil).Record(1024)
+				emp.Counter("test-bench-counter").Record(1024)
 			}
 		},
 	)
@@ -460,7 +445,7 @@ func BenchmarkParallelGauge(b *testing.B) {
 	b.RunParallel(
 		func(p *testing.PB) {
 			for p.Next() {
-				emp.Gauge("test-bench-gauge", nil).Record(1024)
+				emp.Gauge("test-bench-gauge").Record(1024)
 			}
 		},
 	)
@@ -474,7 +459,7 @@ func BenchmarkParallelTimer(b *testing.B) {
 	b.RunParallel(
 		func(p *testing.PB) {
 			for p.Next() {
-				emp.Timer("test-bench-timer", nil).Record(time.Hour)
+				emp.Timer("test-bench-timer").Record(time.Hour)
 			}
 		},
 	)
@@ -488,14 +473,10 @@ func BenchmarkAllTheMetrics(b *testing.B) {
 	b.RunParallel(
 		func(p *testing.PB) {
 			for p.Next() {
-				emp.Histogram("test-bench-histogram", &MetricOptions{
-					Unit: Bytes,
-				}).Record(1024, ServiceTypeTag("test-service"))
-				emp.Counter("test-bench-counter", nil).Record(1024, ServiceTypeTag("test-service"))
-				emp.Gauge("test-bench-gauge", nil).Record(1024, ServiceTypeTag("test-service"))
-				emp.Timer("test-bench-timer", &MetricOptions{
-					Unit: Milliseconds,
-				}).Record(time.Hour, ServiceTypeTag("test-service"))
+				emp.Histogram("test-bench-histogram", Bytes).Record(1024, ServiceTypeTag("test-service"))
+				emp.Counter("test-bench-counter").Record(1024, ServiceTypeTag("test-service"))
+				emp.Gauge("test-bench-gauge").Record(1024, ServiceTypeTag("test-service"))
+				emp.Timer("test-bench-timer").Record(time.Hour, ServiceTypeTag("test-service"))
 			}
 		},
 	)
@@ -510,30 +491,18 @@ func BenchmarkAllTheMetricsAgain(b *testing.B) {
 	b.RunParallel(
 		func(p *testing.PB) {
 			for p.Next() {
-				emp.Histogram("test-bench-histogram", &MetricOptions{
-					Unit: Bytes,
-				}).Record(1024, ServiceTypeTag("test-service"))
-				emp.Counter("test-bench-counter", nil).Record(1024, ServiceTypeTag("test-service"))
-				emp.Gauge("test-bench-gauge", nil).Record(1024, ServiceTypeTag("test-service"))
-				emp.Timer("test-bench-timer", &MetricOptions{
-					Unit: Milliseconds,
-				}).Record(time.Hour, ServiceTypeTag("test-service"))
-				emp.Histogram("test-bench-histogram", &MetricOptions{
-					Unit: Bytes,
-				}).Record(1024, ServiceTypeTag("test-service"))
-				emp.Counter("test-bench-counter", nil).Record(1024, ServiceTypeTag("test-service"))
-				emp.Gauge("test-bench-gauge", nil).Record(1024, ServiceTypeTag("test-service"))
-				emp.Timer("test-bench-timer", &MetricOptions{
-					Unit: Milliseconds,
-				}).Record(time.Hour, ServiceTypeTag("test-service"))
-				emp.Histogram("test-bench-histogram", &MetricOptions{
-					Unit: Bytes,
-				}).Record(1024, ServiceTypeTag("test-service"))
-				emp.Counter("test-bench-counter", nil).Record(1024, ServiceTypeTag("test-service"))
-				emp.Gauge("test-bench-gauge", nil).Record(1024, ServiceTypeTag("test-service"))
-				emp.Timer("test-bench-timer", &MetricOptions{
-					Unit: Milliseconds,
-				}).Record(time.Hour, ServiceTypeTag("test-service"))
+				emp.Histogram("test-bench-histogram", Bytes).Record(1024, ServiceTypeTag("test-service"))
+				emp.Counter("test-bench-counter").Record(1024, ServiceTypeTag("test-service"))
+				emp.Gauge("test-bench-gauge").Record(1024, ServiceTypeTag("test-service"))
+				emp.Timer("test-bench-timer").Record(time.Hour, ServiceTypeTag("test-service"))
+				emp.Histogram("test-bench-histogram", Bytes).Record(1024, ServiceTypeTag("test-service"))
+				emp.Counter("test-bench-counter").Record(1024, ServiceTypeTag("test-service"))
+				emp.Gauge("test-bench-gauge").Record(1024, ServiceTypeTag("test-service"))
+				emp.Timer("test-bench-timer").Record(time.Hour, ServiceTypeTag("test-service"))
+				emp.Histogram("test-bench-histogram", Bytes).Record(1024, ServiceTypeTag("test-service"))
+				emp.Counter("test-bench-counter").Record(1024, ServiceTypeTag("test-service"))
+				emp.Gauge("test-bench-gauge").Record(1024, ServiceTypeTag("test-service"))
+				emp.Timer("test-bench-timer").Record(time.Hour, ServiceTypeTag("test-service"))
 			}
 		},
 	)

diff --git a/common/metrics/events_userscope.go b/common/metrics/events_userscope.go
@@ -27,7 +27,6 @@ package metrics
 import (
 	"time"
 
-	"golang.org/x/exp/event"
 	"golang.org/x/exp/maps"
 )
 
@@ -38,19 +37,7 @@ type (
 	}
 )
 
-var (
-	defaultOptions *MetricOptions = &MetricOptions{
-		Namespace: defaultMetricNamespace,
-		Unit:      Dimensionless,
-	}
-
-	defaultTimerOptions *MetricOptions = &MetricOptions{
-		Namespace: defaultMetricNamespace,
-		Unit:      Milliseconds,
-	}
-
-	_ UserScope = (*eventsUserScope)(nil)
-)
+var _ UserScope = (*eventsUserScope)(nil)
 
 func newEventsUserScope(provider MetricProvider, tags map[string]string) *eventsUserScope {
 	return &eventsUserScope{
@@ -66,37 +53,34 @@ func (e *eventsUserScope) IncCounter(counter string) {
 
 // AddCounter adds delta to the counter metric
 func (e *eventsUserScope) AddCounter(counter string, delta int64) {
-	e.provider.Counter(counter, defaultOptions).Record(delta, mapToTags(e.tags)...)
+	e.provider.Counter(counter).Record(delta, mapToTags(e.tags)...)
 }
 
 // StartTimer starts a timer for the given metric name.
 // Time will be recorded when stopwatch is stopped.
 func (e *eventsUserScope) StartTimer(timer string) Stopwatch {
 	return &eventsStopwatch{
 		recordFunc: func(d time.Duration) {
-			e.provider.Timer(timer, defaultTimerOptions).Record(d, mapToTags(e.tags)...)
+			e.provider.Timer(timer).Record(d, mapToTags(e.tags)...)
 		},
 		start: time.Now(),
 	}
 }
 
 // RecordTimer records a timer for the given metric name
 func (e *eventsUserScope) RecordTimer(timer string, d time.Duration) {
-	e.provider.Timer(timer, defaultTimerOptions).Record(d, mapToTags(e.tags)...)
+	e.provider.Timer(timer).Record(d, mapToTags(e.tags)...)
 }
 
 // RecordDistribution records a distribution (wrapper on top of timer) for the given
 // metric name
 func (e *eventsUserScope) RecordDistribution(id string, unit MetricUnit, d int) {
-	e.provider.Histogram(id, &MetricOptions{
-		Namespace: defaultMetricNamespace,
-		Unit:      event.Unit(unit),
-	}).Record(int64(d), mapToTags(e.tags)...)
+	e.provider.Histogram(id, unit).Record(int64(d), mapToTags(e.tags)...)
 }
 
 // UpdateGauge reports Gauge type absolute value metric
 func (e *eventsUserScope) UpdateGauge(gauge string, value float64) {
-	e.provider.Gauge(gauge, defaultOptions).Record(value, mapToTags(e.tags)...)
+	e.provider.Gauge(gauge).Record(value, mapToTags(e.tags)...)
 }
 
 // Tagged returns a new scope with added and/or overriden tags values that can be used