Add a metrics.registry for building a metrics catalog

common/metrics/config.go

@@ -448,9 +448,9 @@ func newPrometheusScope(
 }
 
 // MetricsHandlerFromConfig is used at startup to construct a MetricsHandler
-func MetricsHandlerFromConfig(logger log.Logger, c *Config) Handler {
+func MetricsHandlerFromConfig(logger log.Logger, c *Config) (Handler, error) {
 	if c == nil {
-		return NoopMetricsHandler
+		return NoopMetricsHandler, nil
 	}
 
 	setDefaultPerUnitHistogramBoundaries(&c.ClientConfig)
@@ -467,7 +467,7 @@ func MetricsHandlerFromConfig(logger log.Logger, c *Config) Handler {
 	return NewTallyMetricsHandler(
 		c.ClientConfig,
 		NewScope(logger, c),
-	)
+	), nil
 }
 
 func configExcludeTags(cfg ClientConfig) map[string]map[string]struct{} {

common/metrics/config_test.go

@@ -28,6 +28,7 @@ import (
 	"testing"
 
 	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	"github.com/uber-go/tally/v4"
@@ -160,3 +161,54 @@ func (s *MetricsSuite) TestSetDefaultPerUnitHistogramBoundaries() {
 		s.Equal(test.expectResult, config.PerUnitHistogramBoundaries)
 	}
 }
+
+func TestMetricsHandlerFromConfig(t *testing.T) {
+	t.Parallel()
+
+	logger := log.NewTestLogger()
+
+	for _, c := range []struct {
+		name         string
+		cfg          *Config
+		expectedType interface{}
+	}{
+		{
+			name:         "nil config",
+			cfg:          nil,
+			expectedType: &noopMetricsHandler{},
+		},
+		{
+			name: "tally",
+			cfg: &Config{
+				Prometheus: &PrometheusConfig{
+					Framework:     FrameworkTally,
+					ListenAddress: "localhost:0",
+				},
+			},
+			expectedType: &tallyMetricsHandler{},
+		},
+		{
+			name: "opentelemetry",
+			cfg: &Config{
+				Prometheus: &PrometheusConfig{
+					Framework:     FrameworkOpentelemetry,
+					ListenAddress: "localhost:0",
+				},
+			},
+			expectedType: &otelMetricsHandler{},
+		},
+	} {
+		c := c
+		t.Run(c.name, func(t *testing.T) {
+			t.Parallel()
+
+			handler, err := MetricsHandlerFromConfig(logger, c.cfg)
+			require.NoError(t, err)
+			defer func() {
+				handler.Stop(logger)
+			}()
+			assert.IsType(t, c.expectedType, handler)
+		})
+	}
+
+}

common/metrics/defs.go

@@ -31,8 +31,9 @@ type (
 
 	// metricDefinition contains the definition for a metric
 	metricDefinition struct {
-		name string
-		unit MetricUnit
+		name        string
+		description string
+		unit        MetricUnit
 	}
 )
 
@@ -52,26 +53,26 @@ func (md metricDefinition) GetMetricUnit() MetricUnit {
 	return md.unit
 }
 
-func NewTimerDef(name string) metricDefinition {
-	return metricDefinition{name: name, unit: Milliseconds}
+func NewTimerDef(name string, opts ...Option) metricDefinition {
+	return globalRegistry.register(name, append(opts, WithUnit(Milliseconds))...)
 }
 
-func NewBytesHistogramDef(name string) metricDefinition {
-	return metricDefinition{name: name, unit: Bytes}
+func NewBytesHistogramDef(name string, opts ...Option) metricDefinition {
+	return globalRegistry.register(name, append(opts, WithUnit(Bytes))...)
 }
 
-func NewDimensionlessHistogramDef(name string) metricDefinition {
-	return metricDefinition{name: name, unit: Dimensionless}
+func NewDimensionlessHistogramDef(name string, opts ...Option) metricDefinition {
+	return globalRegistry.register(name, append(opts, WithUnit(Dimensionless))...)
 }
 
-func NewTimeHistogramDef(name string) metricDefinition {
-	return metricDefinition{name: name, unit: Milliseconds}
+func NewTimeHistogramDef(name string, opts ...Option) metricDefinition {
+	return globalRegistry.register(name, append(opts, WithUnit(Milliseconds))...)
 }
 
-func NewCounterDef(name string) metricDefinition {
-	return metricDefinition{name: name}
+func NewCounterDef(name string, opts ...Option) metricDefinition {
+	return globalRegistry.register(name, opts...)
 }
 
-func NewGaugeDef(name string) metricDefinition {
-	return metricDefinition{name: name}
+func NewGaugeDef(name string, opts ...Option) metricDefinition {
+	return globalRegistry.register(name, opts...)
 }

common/metrics/metric_defs.go

@@ -1217,7 +1217,10 @@ const (
 )
 
 var (
-	ServiceRequests                               = NewCounterDef("service_requests")
+	ServiceRequests = NewCounterDef(
+		"service_requests",
+		WithDescription("The number of gRPC requests received by the service."),
+	)
 	ServicePendingRequests                        = NewGaugeDef("service_pending_requests")
 	ServiceFailures                               = NewCounterDef("service_errors")
 	ServiceErrorWithType                          = NewCounterDef("service_error_with_type")

common/metrics/metricstest/metricstest.go

@@ -109,7 +109,10 @@ func NewHandler(logger log.Logger, clientConfig metrics.ClientConfig) (*Handler,
 	)
 	meter := provider.Meter("temporal")
 
-	otelHandler := metrics.NewOtelMetricsHandler(logger, &otelProvider{meter: meter}, clientConfig)
+	otelHandler, err := metrics.NewOtelMetricsHandler(logger, &otelProvider{meter: meter}, clientConfig)
+	if err != nil {
+		return nil, err
+	}
 	metricsHandler := &Handler{
 		Handler: otelHandler,
 		reg:     registry,

common/metrics/option.go

@@ -0,0 +1,45 @@
+// The MIT License
+//
+// Copyright (c) 2020 Temporal Technologies Inc.  All rights reserved.
+//
+// Copyright (c) 2020 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package metrics
+
+// Option is used to configure a metric definition. Note that options are currently only supported when using the
+// Prometheus reporter with the OpenTelemetry framework.
+type Option interface {
+	apply(m *metricDefinition)
+}
+
+// WithDescription sets the description, or "help text", of a metric. See [ServiceRequests] for an example.
+type WithDescription string
+
+func (h WithDescription) apply(m *metricDefinition) {
+	m.description = string(h)
+}
+
+// WithUnit sets the unit of a metric. See NewBytesHistogramDef for an example.
+type WithUnit MetricUnit
+
+func (h WithUnit) apply(m *metricDefinition) {
+	m.unit = MetricUnit(h)
+}

common/metrics/otel_metrics_handler.go

@@ -26,6 +26,7 @@ package metrics
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	"go.opentelemetry.io/otel/attribute"
@@ -41,31 +42,45 @@ type otelMetricsHandler struct {
 	tags        []Tag
 	provider    OpenTelemetryProvider
 	excludeTags excludeTags
+	catalog     catalog
 }
 
 var _ Handler = (*otelMetricsHandler)(nil)
 
-func NewOtelMetricsHandler(l log.Logger, o OpenTelemetryProvider, cfg ClientConfig) *otelMetricsHandler {
+// NewOtelMetricsHandler returns a new Handler that uses the provided OpenTelemetry [metric.Meter] to record metrics.
+// This OTel handler supports metric descriptions for metrics registered with the New*Def functions. However, those
+// functions must be called before this constructor. Otherwise, the descriptions will be empty. This is because the
+// OTel metric descriptions are generated from the globalRegistry. You may also record metrics that are not registered
+// via the New*Def functions. In that case, the metric description will be the OTel default (the metric name itself).
+func NewOtelMetricsHandler(
+	l log.Logger,
+	o OpenTelemetryProvider,
+	cfg ClientConfig,
+) (*otelMetricsHandler, error) {
+	c, err := globalRegistry.buildCatalog()
+	if err != nil {
+		return nil, fmt.Errorf("failed to build metrics catalog: %w", err)
+	}
 	return &otelMetricsHandler{
 		l:           l,
 		provider:    o,
 		excludeTags: configExcludeTags(cfg),
-	}
+		catalog:     c,
+	}, nil
 }
 
 // WithTags creates a new Handler with the provided Tag list.
 // Tags are merged with the existing tags.
 func (omp *otelMetricsHandler) WithTags(tags ...Tag) Handler {
-	return &otelMetricsHandler{
-		provider:    omp.provider,
-		excludeTags: omp.excludeTags,
-		tags:        append(omp.tags, tags...),
-	}
+	newHandler := *omp
+	newHandler.tags = append(newHandler.tags, tags...)
+	return &newHandler
 }
 
 // Counter obtains a counter for the given name.
 func (omp *otelMetricsHandler) Counter(counter string) CounterIface {
-	c, err := omp.provider.GetMeter().Int64Counter(counter)
+	opts := addOptions(omp, counterOptions{}, counter)
+	c, err := omp.provider.GetMeter().Int64Counter(counter, opts...)
 	if err != nil {
 		omp.l.Error("error getting metric", tag.NewStringTag("MetricName", counter), tag.Error(err))
 		return CounterFunc(func(i int64, t ...Tag) {})
@@ -79,7 +94,8 @@ func (omp *otelMetricsHandler) Counter(counter string) CounterIface {
 
 // Gauge obtains a gauge for the given name.
 func (omp *otelMetricsHandler) Gauge(gauge string) GaugeIface {
-	c, err := omp.provider.GetMeter().Float64ObservableGauge(gauge)
+	opts := addOptions(omp, gaugeOptions{}, gauge)
+	c, err := omp.provider.GetMeter().Float64ObservableGauge(gauge, opts...)
 	if err != nil {
 		omp.l.Error("error getting metric", tag.NewStringTag("MetricName", gauge), tag.Error(err))
 		return GaugeFunc(func(i float64, t ...Tag) {})
@@ -99,7 +115,8 @@ func (omp *otelMetricsHandler) Gauge(gauge string) GaugeIface {
 
 // Timer obtains a timer for the given name.
 func (omp *otelMetricsHandler) Timer(timer string) TimerIface {
-	c, err := omp.provider.GetMeter().Int64Histogram(timer, metric.WithUnit(Milliseconds))
+	opts := addOptions(omp, histogramOptions{metric.WithUnit(Milliseconds)}, timer)
+	c, err := omp.provider.GetMeter().Int64Histogram(timer, opts...)
 	if err != nil {
 		omp.l.Error("error getting metric", tag.NewStringTag("MetricName", timer), tag.Error(err))
 		return TimerFunc(func(i time.Duration, t ...Tag) {})
@@ -113,7 +130,8 @@ func (omp *otelMetricsHandler) Timer(timer string) TimerIface {
 
 // Histogram obtains a histogram for the given name.
 func (omp *otelMetricsHandler) Histogram(histogram string, unit MetricUnit) HistogramIface {
-	c, err := omp.provider.GetMeter().Int64Histogram(histogram, metric.WithUnit(string(unit)))
+	opts := addOptions(omp, histogramOptions{metric.WithUnit(string(unit))}, histogram)
+	c, err := omp.provider.GetMeter().Int64Histogram(histogram, opts...)
 	if err != nil {
 		omp.l.Error("error getting metric", tag.NewStringTag("MetricName", histogram), tag.Error(err))
 		return HistogramFunc(func(i int64, t ...Tag) {})

common/metrics/otel_metrics_handler_test.go

@@ -34,6 +34,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/metric"
 	sdkmetrics "go.opentelemetry.io/otel/sdk/metric"
@@ -101,11 +102,17 @@ func TestMeter(t *testing.T) {
 			),
 		),
 	)
-	p := NewOtelMetricsHandler(log.NewTestLogger(), &testProvider{meter: provider.Meter("test")}, defaultConfig)
+
+	p, err := NewOtelMetricsHandler(
+		log.NewTestLogger(),
+		&testProvider{meter: provider.Meter("test")},
+		defaultConfig,
+	)
+	require.NoError(t, err)
 	recordMetrics(p)
 
 	var got metricdata.ResourceMetrics
-	err := rdr.Collect(ctx, &got)
+	err = rdr.Collect(ctx, &got)
 	assert.Nil(t, err)
 
 	want := []metricdata.Metrics{
@@ -257,7 +264,8 @@ func TestOtelMetricsHandler_Error(t *testing.T) {
 	meter := erroneousMeter{err: testErr}
 	provider := &testProvider{meter: meter}
 	cfg := ClientConfig{}
-	handler := NewOtelMetricsHandler(logger, provider, cfg)
+	handler, err := NewOtelMetricsHandler(logger, provider, cfg)
+	require.NoError(t, err)
 	msg := "error getting metric"
 	errTag := tag.Error(testErr)