From 6d738bc66b80b9b14b59809ce979c26c0c6b8e54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20O=27NEILL?= Date: Wed, 5 Feb 2025 17:53:23 +0100 Subject: [PATCH] feat(otel): address first review --- otel/README.md | 68 +++++++++-------------------- otel/env.go | 46 -------------------- otel/go.mod | 6 ++- otel/go.sum | 20 ++++++++- otel/otel.go | 115 ++++++++++++++++++------------------------------- 5 files changed, 85 insertions(+), 170 deletions(-) delete mode 100644 otel/env.go diff --git a/otel/README.md b/otel/README.md index 335cc51b..68915d24 100644 --- a/otel/README.md +++ b/otel/README.md @@ -5,57 +5,29 @@ package main import ( - "context" - "errors" - "fmt" - "log" - "time" + "context" - "github.com/Scalingo/go-utils/otel" + "github.com/Scalingo/go-utils/otel" + "github.com/kelseyhightower/envconfig" ) func main() { - ctx := context.Background() - // Set up OpenTelemetry SDK - // Method 1: Using the env - otelShutdown, err := otel.InitFromEnv(ctx) - if err != nil { - return - } - // Method 2: configuring it ourselves - // otelConfig := otel.ConfigFromEnv() - // otelConfig.Exporter = ... - // otelShutdown, err := otel.InitGlobalWrapper(ctx, otelConfig) - - // Handle shutdown properly so nothing leaks. - defer func() { - err = errors.Join(err, otelShutdown(context.Background())) - }() - - // 4. Get Global Meter and Send Metrics - if err := globalWrapperSendMetrics(ctx); err != nil { - log.Fatalf("failed to send metrics: %v", err) - } - - fmt.Println("Metrics successfully sent!") -} - -// globalWrapperSendMetrics sends an example metric -func globalWrapperSendMetrics(ctx context.Context) error { - // Example of sending a counter metric - err := otel.SendMetric(ctx, "example.counter", 1.0, "An example counter metric") - if err != nil { - return err - } - - // Simulating some load - time.Sleep(2 * time.Second) - - err = otel.SendMetric(ctx, "example.counter", 2.5, "Another counter increment") - if err != nil { - return err - } - - return nil + ctx := context.Background() + + // Get Otel configuration from environment + var otelConfig otel.Config + err := envconfig.Process("OTEL", &otelConfig) + if err != nil { + return + } + + // Initialize Otel + err := otel.New(ctx, otelConfig) + if err != nil { + return + } + + // Handle shutdown properly so nothing leaks. + defer otel.Shutdown(ctx) } ``` diff --git a/otel/env.go b/otel/env.go deleted file mode 100644 index efc0181e..00000000 --- a/otel/env.go +++ /dev/null @@ -1,46 +0,0 @@ -package otel - -import ( - "context" - "strconv" - "time" - - "github.com/Scalingo/go-utils/env" -) - -func ConfigFromEnv() Config { - E := env.InitMapFromEnv(map[string]string{ - "OTEL_SERVICE_NAME": "", - "OTEL_DEBUG": "false", - "OTEL_EXPORTER_TYPE": "http", - "OTEL_EXPORTER_OTLP_ENDPOINT": "", - "OTEL_EXPORTER_OTLP_TIMEOUT": "", - "OTEL_EXPORTER_OTLP_CERTIFICATE": "", - "OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE": "", - "OTEL_EXPORTER_OTLP_CLIENT_KEY": "", - "OTEL_COLLECTION_INTERVAL": "", - }) - - interval := 10 * time.Second - if E["OTEL_COLLECTION_INTERVAL"] != "" { - if secs, err := strconv.Atoi(E["OTEL_COLLECTION_INTERVAL"]); err == nil { - interval = time.Duration(secs) * time.Second - } - } - - return Config{ - ServiceName: E["OTEL_SERVICE_NAME"], - Debug: E["OTEL_DEBUG"] == "true", - ExporterType: E["OTEL_EXPORTER_TYPE"], - ExporterEndpoint: E["OTEL_EXPORTER_OTLP_ENDPOINT"], - ExporterTimeout: E["OTEL_EXPORTER_OTLP_TIMEOUT"], - ExporterCertificate: E["OTEL_EXPORTER_OTLP_CERTIFICATE"], - ExporterClientCertificate: E["OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE"], - ExporterClientKey: E["OTEL_EXPORTER_OTLP_CLIENT_KEY"], - CollectionInterval: interval, - } -} - -func InitFromEnv(ctx context.Context) (func(context.Context) error, error) { - return InitGlobalWrapper(ctx, ConfigFromEnv()) -} diff --git a/otel/go.mod b/otel/go.mod index 2ca54270..24f31682 100644 --- a/otel/go.mod +++ b/otel/go.mod @@ -3,12 +3,11 @@ module github.com/Scalingo/go-utils/otel go 1.23.4 require ( - github.com/Scalingo/go-utils/env v1.1.1 + github.com/Scalingo/go-utils/errors/v2 v2.4.0 go.opentelemetry.io/otel v1.34.0 go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.34.0 go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v1.34.0 go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v1.34.0 - go.opentelemetry.io/otel/metric v1.34.0 go.opentelemetry.io/otel/sdk v1.34.0 go.opentelemetry.io/otel/sdk/metric v1.34.0 ) @@ -19,7 +18,9 @@ require ( github.com/go-logr/stdr v1.2.2 // indirect github.com/google/uuid v1.6.0 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.25.1 // indirect + github.com/pkg/errors v0.9.1 // indirect go.opentelemetry.io/auto/sdk v1.1.0 // indirect + go.opentelemetry.io/otel/metric v1.34.0 // indirect go.opentelemetry.io/otel/trace v1.34.0 // indirect go.opentelemetry.io/proto/otlp v1.5.0 // indirect golang.org/x/net v0.34.0 // indirect @@ -29,4 +30,5 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20250115164207-1a7da9e5054f // indirect google.golang.org/grpc v1.69.4 // indirect google.golang.org/protobuf v1.36.3 // indirect + gopkg.in/errgo.v1 v1.0.1 // indirect ) diff --git a/otel/go.sum b/otel/go.sum index ae11ab0c..602adce9 100644 --- a/otel/go.sum +++ b/otel/go.sum @@ -1,9 +1,11 @@ -github.com/Scalingo/go-utils/env v1.1.1 h1:N0lqjTLVTPyyTr0W2PcJUrd7EBXcBnlAweTB38vrWHY= -github.com/Scalingo/go-utils/env v1.1.1/go.mod h1:/vSTf+LpZnO2u3IscTY5xE/yxjf+6Rv5OI8SdVQ8VLM= +github.com/Scalingo/go-utils/errors/v2 v2.4.0 h1:vKG0Js3kzWG7+03LEvH7j8fw+picEcRhbjMm3i9Xbb8= +github.com/Scalingo/go-utils/errors/v2 v2.4.0/go.mod h1:WU6Kzi19AlZyUfoxFkdvEeYkIa0W0f172hKPqkOeIpU= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/frankban/quicktest v1.2.2 h1:xfmOhhoH5fGPgbEAlhLpJH9p0z/0Qizio9osmvn9IUY= +github.com/frankban/quicktest v1.2.2/go.mod h1:Qh/WofXFeiAFII1aEBu529AtJo6Zg2VHscnEsbBnJ20= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY= github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= @@ -11,14 +13,26 @@ github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= +github.com/google/go-cmp v0.2.1-0.20190312032427-6f77996f0c42/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/grpc-ecosystem/grpc-gateway/v2 v2.25.1 h1:VNqngBF40hVlDloBruUehVYC3ArSgIyScOAyMRqBxRg= github.com/grpc-ecosystem/grpc-gateway/v2 v2.25.1/go.mod h1:RBRO7fro65R6tjKzYgLAFo0t1QEXY1Dp+i/bvpRiqiQ= +github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= +github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= +github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o= github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= go.opentelemetry.io/auto/sdk v1.1.0 h1:cH53jehLUN6UFLY71z+NDOiNJqDdPRaXzTel0sJySYA= @@ -55,5 +69,7 @@ google.golang.org/grpc v1.69.4 h1:MF5TftSMkd8GLw/m0KM6V8CMOCY6NZ1NQDPGFgbTt4A= google.golang.org/grpc v1.69.4/go.mod h1:vyjdE6jLBI76dgpDojsFGNaHlxdjXN9ghpnd2o7JGZ4= google.golang.org/protobuf v1.36.3 h1:82DV7MYdb8anAVi3qge1wSnMDrnKK7ebr+I0hHRN1BU= google.golang.org/protobuf v1.36.3/go.mod h1:9fA7Ob0pmnwhb644+1+CVWFRbNajQ6iRojtC/QF5bRE= +gopkg.in/errgo.v1 v1.0.1 h1:oQFRXzZ7CkBGdm1XZm/EbQYaYNNEElNBOd09M6cqNso= +gopkg.in/errgo.v1 v1.0.1/go.mod h1:3NjfXwocQRYAPTq4/fzX+CwUhPRcR/azYRhj8G+LqMo= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/otel/otel.go b/otel/otel.go index 42606380..8cc661f9 100644 --- a/otel/otel.go +++ b/otel/otel.go @@ -2,8 +2,6 @@ package otel import ( "context" - "errors" - "fmt" "sync" "time" @@ -11,109 +9,95 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp" "go.opentelemetry.io/otel/exporters/stdout/stdoutmetric" - "go.opentelemetry.io/otel/metric" sdkmetric "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/resource" semconv "go.opentelemetry.io/otel/semconv/v1.26.0" + + "github.com/Scalingo/go-utils/errors/v2" ) -// Config defines the Otel Wrapper configuration for metrics type Config struct { - ServiceName string - Debug bool - ExporterType string - ExporterEndpoint string - ExporterTimeout string - ExporterCertificate string - ExporterClientCertificate string - ExporterClientKey string - CollectionInterval time.Duration - MetricsReader sdkmetric.Reader - MetricsExporter sdkmetric.Exporter + ServiceName string `required:"true" split_words:"true"` + Debug bool `default:"false"` + ExporterType string `default:"http" split_words:"true"` + CollectionInterval time.Duration `default:"10s" split_words:"true"` } -// OtelWrapper encapsulates OpenTelemetry MetricProvider and utilities -type OtelWrapper struct { +// OtelProviders encapsulates OpenTelemetry providers and utilities +type OtelProviders struct { meterProvider *sdkmetric.MeterProvider - globalMeter metric.Meter + shutdownFunc func(context.Context) error config Config } var ( - globalWrapper *OtelWrapper - - // singletonShutdown holds the shutdown function once the meter provider is initialized. - singletonShutdown func(context.Context) error + globalProvider *OtelProviders // once ensures that initialization happens only once. globalOnce sync.Once - - // initErr captures any error encountered during initialization. - initErr error ) -// InitSingleton initializes the MeterProvider as a singleton. -// Subsequent calls to this function will return the same shutdown function and error. +// Initializes the OtelWrapper as a singleton. // The configuration is used only on the first call. -func InitGlobalWrapper(ctx context.Context, cfg Config) (func(context.Context) error, error) { +func New(ctx context.Context, cfg Config) error { + var err error globalOnce.Do(func() { - singletonShutdown, initErr = New(ctx, cfg) + globalProvider, err = setupProviders(ctx, cfg) }) - return singletonShutdown, initErr + return err } -func New(ctx context.Context, cfg Config) (shutdown func(context.Context) error, err error) { +func setupProviders(ctx context.Context, cfg Config) (*OtelProviders, error) { var shutdownFuncs []func(context.Context) error // shutdown calls cleanup functions registered via shutdownFuncs. // The errors from the calls are joined. // Each registered cleanup will be invoked once. - shutdown = func(ctx context.Context) error { + shutdown := func(ctx context.Context) error { var err error for _, fn := range shutdownFuncs { - err = errors.Join(err, fn(ctx)) + shutdownErr := fn(ctx) + if shutdownErr != nil { + err = errors.Wrapf(ctx, err, "failed to shutdown provider: %v", shutdownErr) + } } shutdownFuncs = nil return err } // handleErr calls shutdown for cleanup and makes sure that all errors are returned. - handleErr := func(inErr error) { - err = errors.Join(inErr, shutdown(ctx)) + handleErr := func(inErr error) error { + err := shutdown(ctx) + if err != nil { + return errors.Wrapf(ctx, inErr, "failed to shutdown otel providers: %v", err) + } + return inErr } // Set up meter provider. meterProvider, err := newMeterProvider(ctx, cfg) if err != nil { - handleErr(err) - return + return nil, handleErr(err) } shutdownFuncs = append(shutdownFuncs, meterProvider.Shutdown) otelsdk.SetMeterProvider(meterProvider) - globalWrapper = &OtelWrapper{ + return &OtelProviders{ meterProvider: meterProvider, - globalMeter: meterProvider.Meter(cfg.ServiceName), + shutdownFunc: shutdown, config: cfg, - } - - return + }, nil } func newMeterProvider(ctx context.Context, cfg Config) (*sdkmetric.MeterProvider, error) { if cfg.ServiceName == "" { - return nil, errors.New("ServiceName is required") - } - if cfg.MetricsExporter == nil { - exporter, err := newMetricsExporter(ctx, cfg) - if err != nil { - return nil, fmt.Errorf("failed to load exporter", err) - } - cfg.MetricsExporter = exporter + return nil, errors.New(ctx, "ServiceName is required") } - if cfg.MetricsReader == nil { - cfg.MetricsReader = sdkmetric.NewPeriodicReader(cfg.MetricsExporter, sdkmetric.WithInterval(cfg.CollectionInterval)) + metricsExporter, err := newMetricsExporter(ctx, cfg) + if err != nil { + return nil, errors.Wrap(ctx, err, "failed to load exporter") } + metricsReader := sdkmetric.NewPeriodicReader(metricsExporter, sdkmetric.WithInterval(cfg.CollectionInterval)) res, err := resource.Merge( resource.Default(), @@ -123,12 +107,12 @@ func newMeterProvider(ctx context.Context, cfg Config) (*sdkmetric.MeterProvider ), ) if err != nil { - return nil, fmt.Errorf("failed to create resource: %w", err) + return nil, errors.Wrap(ctx, err, "failed to create resource") } // Initialize MeterProvider provider := sdkmetric.NewMeterProvider( - sdkmetric.WithReader(cfg.MetricsReader), + sdkmetric.WithReader(metricsReader), sdkmetric.WithResource(res), ) @@ -145,27 +129,14 @@ func newMetricsExporter(ctx context.Context, cfg Config) (sdkmetric.Exporter, er case "grpc": return otlpmetricgrpc.New(ctx) default: - return nil, errors.New("invalid exporter type") - } -} - -// GetGlobalMeter returns the global Meter -func GetGlobalMeter() metric.Meter { - if globalWrapper == nil { - panic("Global OtelWrapper is not initialized") + return nil, errors.New(ctx, "invalid exporter type") } - return globalWrapper.globalMeter } -// SendMetric wraps the meter to send a metric -func SendMetric(ctx context.Context, name string, value float64, description string) error { - counter, err := GetGlobalMeter().Float64Counter( - name, - metric.WithDescription(description), - ) - if err != nil { - return fmt.Errorf("failed to create counter: %w", err) +// Gracefully shuts down the providers +func Shutdown(ctx context.Context) error { + if err := globalProvider.shutdownFunc(ctx); err != nil { + return errors.Wrap(ctx, err, "failed to shutdown otel providers") } - counter.Add(ctx, value) return nil }