From 043fc7cefcfab95311ebc6d6096bb36907e56755 Mon Sep 17 00:00:00 2001 From: Mark Phelps <209477+markphelps@users.noreply.github.com> Date: Wed, 24 Apr 2024 12:20:44 -0400 Subject: [PATCH] chore: respond to PR feedback --- internal/cmd/grpc.go | 13 +++---------- internal/config/metrics.go | 2 +- internal/metrics/metrics.go | 28 +++++++++++++--------------- 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/internal/cmd/grpc.go b/internal/cmd/grpc.go index 0b5aba91bc..40624b1e7b 100644 --- a/internal/cmd/grpc.go +++ b/internal/cmd/grpc.go @@ -42,8 +42,6 @@ import ( "go.flipt.io/flipt/internal/tracing" "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "go.opentelemetry.io/otel" - metric "go.opentelemetry.io/otel/metric" - metricnoop "go.opentelemetry.io/otel/metric/noop" metricsdk "go.opentelemetry.io/otel/sdk/metric" tracesdk "go.opentelemetry.io/otel/sdk/trace" "go.uber.org/zap" @@ -154,9 +152,6 @@ func NewGRPCServer( logger.Debug("store enabled", zap.Stringer("store", store)) - // Default to a no-op OTEL meter provider - var meterProvider metric.MeterProvider = metricnoop.NewMeterProvider() - // Initialize metrics exporter if enabled if cfg.Metrics.Enabled { metricExp, metricExpShutdown, err := metrics.GetExporter(ctx, &cfg.Metrics) @@ -166,14 +161,12 @@ func NewGRPCServer( server.onShutdown(metricExpShutdown) - meterProvider = metricsdk.NewMeterProvider(metricsdk.WithReader(metricExp)) + meterProvider := metricsdk.NewMeterProvider(metricsdk.WithReader(metricExp)) + otel.SetMeterProvider(meterProvider) + logger.Debug("otel metrics enabled", zap.String("exporter", string(cfg.Metrics.Exporter))) } - otel.SetMeterProvider(meterProvider) - - metrics.Meter = meterProvider.Meter("github.com/flipt-io/flipt") - // Initialize tracingProvider regardless of configuration. No extraordinary resources // are consumed, or goroutines initialized until a SpanProcessor is registered. tracingProvider, err := tracing.NewProvider(ctx, info.Version, cfg.Tracing) diff --git a/internal/config/metrics.go b/internal/config/metrics.go index ba7dce92ff..fb085feab7 100644 --- a/internal/config/metrics.go +++ b/internal/config/metrics.go @@ -18,7 +18,7 @@ const ( type MetricsConfig struct { Enabled bool `json:"enabled" mapstructure:"enabled" yaml:"enabled"` Exporter MetricsExporter `json:"exporter,omitempty" mapstructure:"exporter" yaml:"exporter,omitempty"` - OTLP *OTLPMetricsConfig `json:"otlp,omitempty" mapstructure:"otlp" yaml:"otlp,omitempty"` + OTLP *OTLPMetricsConfig `json:"otlp,omitempty" mapstructure:"otlp,omitempty" yaml:"otlp,omitempty"` } type OTLPMetricsConfig struct { diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index e4e4263b82..56ffe208d6 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -7,6 +7,7 @@ import ( "sync" "go.flipt.io/flipt/internal/config" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp" "go.opentelemetry.io/otel/exporters/prometheus" @@ -15,16 +16,13 @@ import ( sdkmetric "go.opentelemetry.io/otel/sdk/metric" ) -// Meter is the default Flipt-wide otel metric Meter. -var Meter metric.Meter - func init() { - // If the global Meter is nil, create a new noop Meter. - // Useful for testing. - if Meter == nil { - meterProvider := metricnoop.NewMeterProvider() - Meter = meterProvider.Meter("github.com/flipt-io/flipt") - } + otel.SetMeterProvider(metricnoop.NewMeterProvider()) +} + +// This is memoized in the OTEL library to avoid creating multiple instances of the same exporter. +func meter() metric.Meter { + return otel.Meter("github.com/flipt-io/flipt") } // MustInt64 returns an instrument provider based on the global Meter. @@ -55,7 +53,7 @@ type mustInt64Meter struct{} // Counter creates an instrument for recording increasing values. func (m mustInt64Meter) Counter(name string, opts ...metric.Int64CounterOption) metric.Int64Counter { - counter, err := Meter.Int64Counter(name, opts...) + counter, err := meter().Int64Counter(name, opts...) if err != nil { panic(err) } @@ -65,7 +63,7 @@ func (m mustInt64Meter) Counter(name string, opts ...metric.Int64CounterOption) // UpDownCounter creates an instrument for recording changes of a value. func (m mustInt64Meter) UpDownCounter(name string, opts ...metric.Int64UpDownCounterOption) metric.Int64UpDownCounter { - counter, err := Meter.Int64UpDownCounter(name, opts...) + counter, err := meter().Int64UpDownCounter(name, opts...) if err != nil { panic(err) } @@ -75,7 +73,7 @@ func (m mustInt64Meter) UpDownCounter(name string, opts ...metric.Int64UpDownCou // Histogram creates an instrument for recording a distribution of values. func (m mustInt64Meter) Histogram(name string, opts ...metric.Int64HistogramOption) metric.Int64Histogram { - hist, err := Meter.Int64Histogram(name, opts...) + hist, err := meter().Int64Histogram(name, opts...) if err != nil { panic(err) } @@ -111,7 +109,7 @@ type mustFloat64Meter struct{} // Counter creates an instrument for recording increasing values. func (m mustFloat64Meter) Counter(name string, opts ...metric.Float64CounterOption) metric.Float64Counter { - counter, err := Meter.Float64Counter(name, opts...) + counter, err := meter().Float64Counter(name, opts...) if err != nil { panic(err) } @@ -121,7 +119,7 @@ func (m mustFloat64Meter) Counter(name string, opts ...metric.Float64CounterOpti // UpDownCounter creates an instrument for recording changes of a value. func (m mustFloat64Meter) UpDownCounter(name string, opts ...metric.Float64UpDownCounterOption) metric.Float64UpDownCounter { - counter, err := Meter.Float64UpDownCounter(name, opts...) + counter, err := meter().Float64UpDownCounter(name, opts...) if err != nil { panic(err) } @@ -131,7 +129,7 @@ func (m mustFloat64Meter) UpDownCounter(name string, opts ...metric.Float64UpDow // Histogram creates an instrument for recording a distribution of values. func (m mustFloat64Meter) Histogram(name string, opts ...metric.Float64HistogramOption) metric.Float64Histogram { - hist, err := Meter.Float64Histogram(name, opts...) + hist, err := meter().Float64Histogram(name, opts...) if err != nil { panic(err) }