[metrics] move prometheus specific configs under metrics.prometheus config

Author: gow

common/metrics/config.go

@@ -50,22 +50,6 @@ type (
 		// - "milliseconds"
 		// - "bytes"
 		PerUnitHistogramBoundaries map[string][]float64 `yaml:"perUnitHistogramBoundaries"`
-
-		// Following configs are added for backwards compatibility when switching from tally to opentelemetry
-		// All configs should be set to true when using opentelemetry framework to have the same behavior as tally.
-
-		// WithoutUnitSuffix controls the additional of unit suffixes to metric names.
-		// This config only takes effect when using opentelemetry framework.
-		// Note: this config only takes effect when using prometheus via opentelemetry framework
-		WithoutUnitSuffix bool `yaml:"withoutUnitSuffix"`
-		// WithoutCounterSuffix controls the additional of _total suffixes to counter metric names.
-		// This config only takes effect when using opentelemetry framework.
-		// Note: this config only takes effect when using prometheus via opentelemetry framework
-		WithoutCounterSuffix bool `yaml:"withoutCounterSuffix"`
-		// RecordTimerInSeconds controls if Timer metric should be emitted as number of seconds
-		// (instead of milliseconds).
-		// This config only takes effect when using opentelemetry framework for both statsd and prometheus.
-		RecordTimerInSeconds bool `yaml:"recordTimerInSeconds"`
 	}
 
 	// StatsdConfig contains the config items for statsd metrics reporter
@@ -143,6 +127,20 @@ type (
 		// specify which characters are valid and/or should be replaced before metrics
 		// are emitted.
 		SanitizeOptions *SanitizeOptions `yaml:"sanitizeOptions"`
+
+		// Following configs are added for backwards compatibility when switching from tally to opentelemetry
+		// All configs should be set to true when using opentelemetry framework to have the same behavior as tally.
+
+		// WithoutUnitSuffix controls the addition of unit suffixes to metric names.
+		// This config only takes effect when using prometheus via opentelemetry framework.
+		WithoutUnitSuffix bool `yaml:"withoutUnitSuffix"`
+		// WithoutCounterSuffix controls the additional of _total suffixes to counter metric names.
+		// This config only takes effect when using prometheus via opentelemetry framework.
+		WithoutCounterSuffix bool `yaml:"withoutCounterSuffix"`
+		// RecordTimerInSeconds controls if Timer metric should be emitted as number of seconds
+		// (instead of milliseconds).
+		// This config only takes effect when using prometheus via opentelemetry framework.
+		RecordTimerInSeconds bool `yaml:"recordTimerInSeconds"`
 	}
 )
 
@@ -463,23 +461,23 @@ func MetricsHandlerFromConfig(logger log.Logger, c *Config) (Handler, error) {
 
 	setDefaultPerUnitHistogramBoundaries(&c.ClientConfig)
 
-	fatalOnListenerError := true
 	if c.Statsd != nil && c.Statsd.Framework == FrameworkOpentelemetry {
 		// create opentelemetry provider with just statsd
 		otelProvider, err := NewOpenTelemetryProviderWithStatsd(logger, c.Statsd, &c.ClientConfig)
 		if err != nil {
 			logger.Fatal(err.Error())
 		}
-		return NewOtelMetricsHandler(logger, otelProvider, c.ClientConfig)
+		return NewOtelMetricsHandler(logger, otelProvider, c.ClientConfig, false)
 	}
 
 	if c.Prometheus != nil && c.Prometheus.Framework == FrameworkOpentelemetry {
 		// create opentelemetry provider with just prometheus
+		fatalOnListenerError := true
 		otelProvider, err := NewOpenTelemetryProviderWithPrometheus(logger, c.Prometheus, &c.ClientConfig, fatalOnListenerError)
 		if err != nil {
 			logger.Fatal(err.Error())
 		}
-		return NewOtelMetricsHandler(logger, otelProvider, c.ClientConfig)
+		return NewOtelMetricsHandler(logger, otelProvider, c.ClientConfig, c.Prometheus.RecordTimerInSeconds)
 	}
 
 	// fallback to tally if no framework is specified

common/metrics/metricstest/metricstest.go

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

common/metrics/opentelemetry_provider.go

@@ -65,10 +65,10 @@ func NewOpenTelemetryProviderWithPrometheus(
 	}
 	reg := prometheus.NewRegistry()
 	exporterOpts := []exporters.Option{exporters.WithRegisterer(reg)}
-	if clientConfig.WithoutUnitSuffix {
+	if prometheusConfig.WithoutUnitSuffix {
 		exporterOpts = append(exporterOpts, exporters.WithoutUnits())
 	}
-	if clientConfig.WithoutCounterSuffix {
+	if prometheusConfig.WithoutCounterSuffix {
 		exporterOpts = append(exporterOpts, exporters.WithoutCounterSuffixes())
 	}
 	if clientConfig.Prefix != "" {

common/metrics/otel_metrics_handler.go

@@ -56,6 +56,7 @@ func NewOtelMetricsHandler(
 	l log.Logger,
 	o OpenTelemetryProvider,
 	cfg ClientConfig,
+	shouldRecordTimerInSeconds bool,
 ) (*otelMetricsHandler, error) {
 	c, err := globalRegistry.buildCatalog()
 	if err != nil {
@@ -69,7 +70,7 @@ func NewOtelMetricsHandler(
 		excludeTags:          configExcludeTags(cfg),
 		catalog:              c,
 		gauges:               new(sync.Map),
-		recordTimerInSeconds: cfg.RecordTimerInSeconds,
+		recordTimerInSeconds: shouldRecordTimerInSeconds,
 	}, nil
 }
 

common/metrics/otel_metrics_handler_test.go

@@ -81,6 +81,7 @@ func TestMeter(t *testing.T) {
 		log.NewTestLogger(),
 		&testProvider{meter: provider.Meter("test")},
 		defaultConfig,
+		false,
 	)
 	require.NoError(t, err)
 	recordMetrics(p)
@@ -219,12 +220,12 @@ func TestMeter_TimerInSeconds(t *testing.T) {
 		),
 	)
 
-	timerInSecondsConfig := defaultConfig
-	timerInSecondsConfig.RecordTimerInSeconds = true
+	shouldRecordTimerInSeconds := true
 	p, err := NewOtelMetricsHandler(
 		log.NewTestLogger(),
 		&testProvider{meter: provider.Meter("test")},
-		timerInSecondsConfig,
+		defaultConfig,
+		shouldRecordTimerInSeconds,
 	)
 	require.NoError(t, err)
 	recordTimer(p)
@@ -316,7 +317,7 @@ func TestOtelMetricsHandler_Error(t *testing.T) {
 	meter := erroneousMeter{err: testErr}
 	provider := &testProvider{meter: meter}
 	cfg := ClientConfig{}
-	handler, err := NewOtelMetricsHandler(logger, provider, cfg)
+	handler, err := NewOtelMetricsHandler(logger, provider, cfg, false)
 	require.NoError(t, err)
 	msg := "error getting metric"
 	errTag := tag.Error(testErr)