From 80d77f8cd912cc4d002b402154bec3f803471bae Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 22 Aug 2023 08:12:14 -0400 Subject: [PATCH 01/26] autoexport: Add support for metrics Fixes #4131 --- exporters/autoexport/exporter.go | 85 ++++++++++++++++--- exporters/autoexport/go.mod | 10 +++ exporters/autoexport/go.sum | 26 ++++++ exporters/autoexport/registry.go | 115 +++++++++++++++++++++----- exporters/autoexport/registry_test.go | 12 +-- 5 files changed, 211 insertions(+), 37 deletions(-) diff --git a/exporters/autoexport/exporter.go b/exporters/autoexport/exporter.go index c7fbcfcac62..1abfcde4f20 100644 --- a/exporters/autoexport/exporter.go +++ b/exporters/autoexport/exporter.go @@ -18,15 +18,18 @@ import ( "context" "os" + "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/trace" ) const ( - otelTracesExportersEnvKey = "OTEL_TRACES_EXPORTER" + otelTracesExportersEnvKey = "OTEL_TRACES_EXPORTER" + otelMetricsExportersEnvKey = "OTEL_METRICS_EXPORTER" ) type config struct { - fallbackExporter trace.SpanExporter + fallbackSpanExporter trace.SpanExporter + fallbackMetricReader metric.Reader } func newConfig(ctx context.Context, opts ...Option) (config, error) { @@ -35,14 +38,24 @@ func newConfig(ctx context.Context, opts ...Option) (config, error) { cfg = opt.apply(cfg) } - // if no fallback exporter is configured, use otlp exporter - if cfg.fallbackExporter == nil { - exp, err := spanExporter(context.Background(), "otlp") + // if no fallback span exporter is configured, use otlp exporter + if cfg.fallbackSpanExporter == nil { + exp, err := spanExporter(ctx, "otlp") if err != nil { return cfg, err } - cfg.fallbackExporter = exp + cfg.fallbackSpanExporter = exp } + + // if no fallback metric reader is configured, use otlp exporter + if cfg.fallbackMetricReader == nil { + r, err := metricReader(ctx, "otlp") + if err != nil { + return cfg, err + } + cfg.fallbackMetricReader = r + } + return cfg, nil } @@ -61,7 +74,7 @@ func (fn optionFunc) apply(cfg config) config { // is configured through the OTEL_TRACES_EXPORTER environment variable. func WithFallbackSpanExporter(exporter trace.SpanExporter) Option { return optionFunc(func(cfg config) config { - cfg.fallbackExporter = exporter + cfg.fallbackSpanExporter = exporter return cfg }) } @@ -91,7 +104,7 @@ func WithFallbackSpanExporter(exporter trace.SpanExporter) Option { func NewSpanExporter(ctx context.Context, opts ...Option) (trace.SpanExporter, error) { // prefer exporter configured via environment variables over exporter // passed in via exporter parameter - envExporter, err := makeExporterFromEnv(ctx) + envExporter, err := makeSpanExporterFromEnv(ctx) if err != nil { return nil, err } @@ -102,16 +115,66 @@ func NewSpanExporter(ctx context.Context, opts ...Option) (trace.SpanExporter, e if err != nil { return nil, err } - return config.fallbackExporter, nil + return config.fallbackSpanExporter, nil } -// makeExporterFromEnv returns a configured SpanExporter defined by the OTEL_TRACES_EXPORTER +// NewMetricReader returns a configured [go.opentelemetry.io/otel/sdk/metric.Reader] +// defined using the environment variables described below. +// +// OTEL_METRICS_EXPORTER defines the metrics exporter; supported values: +// - "none" - "no operation" exporter +// - "otlp" (default) - OTLP exporter; see [go.opentelemetry.io/otel/exporters/otlp/otlptrace] +// +// OTEL_EXPORTER_OTLP_PROTOCOL defines OTLP exporter's transport protocol; +// supported values: +// - "grpc" - protobuf-encoded data using gRPC wire format over HTTP/2 connection; +// see: [go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc] +// - "http/protobuf" (default) - protobuf-encoded data over HTTP connection; +// see: [go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp] +// +// An error is returned if an environment value is set to an unhandled value. +// +// Use [RegisterMetricReader] to handle more values of OTEL_METRICS_EXPORTER. +// +// Use [WithFallbackMetricReader] option to change the returned exporter +// when OTEL_TRACES_EXPORTER is unset or empty. +// +// Use [IsNoneSpanExporter] to check if the retured exporter is a "no operation" exporter. +func NewMetricReader(ctx context.Context, opts ...Option) (metric.Reader, error) { + // prefer exporter configured via environment variables over exporter + // passed in via exporter parameter + envReader, err := makeMetricReaderFromEnv(ctx) + if err != nil { + return nil, err + } + if envReader != nil { + return envReader, nil + } + config, err := newConfig(ctx, opts...) + if err != nil { + return nil, err + } + return config.fallbackMetricReader, nil +} + +// makeSpanExporterFromEnv returns a configured SpanExporter defined by the OTEL_TRACES_EXPORTER // environment variable. // nil is returned if no exporter is defined for the environment variable. -func makeExporterFromEnv(ctx context.Context) (trace.SpanExporter, error) { +func makeSpanExporterFromEnv(ctx context.Context) (trace.SpanExporter, error) { expType := os.Getenv(otelTracesExportersEnvKey) if expType == "" { return nil, nil } return spanExporter(ctx, expType) } + +// makeMetricReaderFromEnv returns a configured metric.Reader defined by the OTEL_METRICS_EXPORTER +// environment variable. +// nil is returned if no exporter is defined for the environment variable. +func makeMetricReaderFromEnv(ctx context.Context) (metric.Reader, error) { + expType := os.Getenv(otelMetricsExportersEnvKey) + if expType == "" { + return nil, nil + } + return metricReader(ctx, expType) +} diff --git a/exporters/autoexport/go.mod b/exporters/autoexport/go.mod index b4b96b4c82d..4d2e21f418a 100644 --- a/exporters/autoexport/go.mod +++ b/exporters/autoexport/go.mod @@ -11,6 +11,8 @@ require ( go.opentelemetry.io/otel/sdk v1.19.0 ) +require go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.39.0 // indirect + require ( github.com/cenkalti/backoff/v4 v4.2.1 // indirect github.com/davecgh/go-spew v1.1.1 // indirect @@ -22,6 +24,14 @@ require ( go.opentelemetry.io/otel v1.19.0 // indirect go.opentelemetry.io/otel/metric v1.19.0 // indirect go.opentelemetry.io/otel/trace v1.19.0 // indirect + github.com/rogpeppe/go-internal v1.10.0 // indirect + go.opentelemetry.io/otel v1.16.0 // indirect + go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.16.0 // indirect + go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.39.0 + go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.39.0 + go.opentelemetry.io/otel/metric v1.16.0 // indirect + go.opentelemetry.io/otel/sdk/metric v0.39.0 + go.opentelemetry.io/otel/trace v1.16.0 // indirect go.opentelemetry.io/proto/otlp v1.0.0 // indirect golang.org/x/net v0.17.0 // indirect golang.org/x/sys v0.13.0 // indirect diff --git a/exporters/autoexport/go.sum b/exporters/autoexport/go.sum index 98287534240..71dca04a33c 100644 --- a/exporters/autoexport/go.sum +++ b/exporters/autoexport/go.sum @@ -38,6 +38,32 @@ go.opentelemetry.io/otel/sdk v1.19.0 h1:6USY6zH+L8uMH8L3t1enZPR3WFEmSTADlqldyHtJ go.opentelemetry.io/otel/sdk v1.19.0/go.mod h1:NedEbbS4w3C6zElbLdPJKOpJQOrGUJ+GfzpjUvI0v1A= go.opentelemetry.io/otel/trace v1.19.0 h1:DFVQmlVbfVeOuBRrwdtaehRrWiL1JoVs9CPIQ1Dzxpg= go.opentelemetry.io/otel/trace v1.19.0/go.mod h1:mfaSyvGyEJEI0nyV2I4qhNQnbBOUUmYZpYojqMnX2vo= +go.opentelemetry.io/otel v1.16.0 h1:Z7GVAX/UkAXPKsy94IU+i6thsQS4nb7LviLpnaNeW8s= +go.opentelemetry.io/otel v1.16.0/go.mod h1:vl0h9NUa1D5s1nv3A5vZOYWn8av4K8Ml6JDeHrT/bx4= +go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.16.0 h1:t4ZwRPU+emrcvM2e9DHd0Fsf0JTPVcbfa/BhTDF03d0= +go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.16.0/go.mod h1:vLarbg68dH2Wa77g71zmKQqlQ8+8Rq3GRG31uc0WcWI= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.39.0 h1:f6BwB2OACc3FCbYVznctQ9V6KK7Vq6CjmYXJ7DeSs4E= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.39.0/go.mod h1:UqL5mZ3qs6XYhDnZaW1Ps4upD+PX6LipH40AoeuIlwU= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.39.0 h1:rm+Fizi7lTM2UefJ1TO347fSRcwmIsUAaZmYmIGBRAo= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.39.0/go.mod h1:sWFbI3jJ+6JdjOVepA5blpv/TJ20Hw+26561iMbWcwU= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.39.0 h1:IZXpCEtI7BbX01DRQEWTGDkvjMB6hEhiEZXS+eg2YqY= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.39.0/go.mod h1:xY111jIZtWb+pUUgT4UiiSonAaY2cD2Ts5zvuKLki3o= +go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.16.0 h1:cbsD4cUcviQGXdw8+bo5x2wazq10SKz8hEbtCRPcU78= +go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.16.0/go.mod h1:JgXSGah17croqhJfhByOLVY719k1emAXC8MVhCIJlRs= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.16.0 h1:TVQp/bboR4mhZSav+MdgXB8FaRho1RC8UwVn3T0vjVc= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.16.0/go.mod h1:I33vtIe0sR96wfrUcilIzLoA3mLHhRmz9S9Te0S3gDo= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.16.0 h1:iqjq9LAB8aK++sKVcELezzn655JnBNdsDhghU4G/So8= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.16.0/go.mod h1:hGXzO5bhhSHZnKvrDaXB82Y9DRFour0Nz/KrBh7reWw= +go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.16.0 h1:+XWJd3jf75RXJq29mxbuXhCXFDG3S3R4vBUeSI2P7tE= +go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.16.0/go.mod h1:hqgzBPTf4yONMFgdZvL/bK42R/iinTyVQtiWihs3SZc= +go.opentelemetry.io/otel/metric v1.16.0 h1:RbrpwVG1Hfv85LgnZ7+txXioPDoh6EdbZHo26Q3hqOo= +go.opentelemetry.io/otel/metric v1.16.0/go.mod h1:QE47cpOmkwipPiefDwo2wDzwJrlfxxNYodqc4xnGCo4= +go.opentelemetry.io/otel/sdk v1.16.0 h1:Z1Ok1YsijYL0CSJpHt4cS3wDDh7p572grzNrBMiMWgE= +go.opentelemetry.io/otel/sdk v1.16.0/go.mod h1:tMsIuKXuuIWPBAOrH+eHtvhTL+SntFtXF9QD68aP6p4= +go.opentelemetry.io/otel/sdk/metric v0.39.0 h1:Kun8i1eYf48kHH83RucG93ffz0zGV1sh46FAScOTuDI= +go.opentelemetry.io/otel/sdk/metric v0.39.0/go.mod h1:piDIRgjcK7u0HCL5pCA4e74qpK/jk3NiUoAHATVAmiI= +go.opentelemetry.io/otel/trace v1.16.0 h1:8JRpaObFoW0pxuVPapkgH8UhHQj+bJW8jJsCZEu5MQs= +go.opentelemetry.io/otel/trace v1.16.0/go.mod h1:Yt9vYq1SdNz3xdjZZK7wcXv1qv2pwLkqr2QVwea0ef0= go.opentelemetry.io/proto/otlp v1.0.0 h1:T0TX0tmXU8a3CbNXzEKGeU5mIVOdf0oykP+u2lIVU/I= go.opentelemetry.io/proto/otlp v1.0.0/go.mod h1:Sy6pihPLfYHkr3NkUbEhGHFhINUSI/v80hjKIs5JXpM= go.uber.org/goleak v1.2.1 h1:NBol2c7O1ZokfZ0LEU9K6Whx/KnwvepVetCUhtKja4A= diff --git a/exporters/autoexport/registry.go b/exporters/autoexport/registry.go index 9eae81e76f8..60cfe820c3b 100644 --- a/exporters/autoexport/registry.go +++ b/exporters/autoexport/registry.go @@ -21,8 +21,10 @@ import ( "os" "sync" + "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" + "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/trace" ) @@ -30,28 +32,42 @@ const ( otelExporterOTLPProtoEnvKey = "OTEL_EXPORTER_OTLP_PROTOCOL" ) -// registry maintains a map of exporter names to SpanExporter factories -// func(context.Context) (trace.SpanExporter, error) that is safe for concurrent use by multiple +// registry maintains a map of exporter names to exporter factories +// func(context.Context) (T, error) that is safe for concurrent use by multiple // goroutines without additional locking or coordination. -type registry struct { +type registry[T any] struct { mu sync.Mutex - names map[string]func(context.Context) (trace.SpanExporter, error) + names map[string]func(context.Context) (T, error) } -func newRegistry() registry { - return registry{ +func newSpanExporterRegistry() registry[trace.SpanExporter] { + return registry[trace.SpanExporter]{ names: map[string]func(context.Context) (trace.SpanExporter, error){ - "": buildOTLPExporter, - "otlp": buildOTLPExporter, + "": buildOTLPSpanExporter, + "otlp": buildOTLPSpanExporter, "none": func(ctx context.Context) (trace.SpanExporter, error) { return noop{}, nil }, }, } } +func newMetricReaderRegistry() registry[metric.Reader] { + return registry[metric.Reader]{ + names: map[string]func(context.Context) (metric.Reader, error){ + "": buildOTLPMetricReader, + "otlp": buildOTLPMetricReader, + "none": func(ctx context.Context) (metric.Reader, error) { return metric.NewManualReader(), nil }, + }, + } +} + var ( - // envRegistry is the package level registry of exporter registrations + // spanExporterRegistry is the package level registry of exporter registrations // and their mapping to a SpanExporter factory func(context.Context) (trace.SpanExporter, error). - envRegistry = newRegistry() + spanExporterRegistry = newSpanExporterRegistry() + + // metricReaderRegistry is the package level registry of exporter registrations + // and their mapping to a MetricReader factory func(context.Context) (metric.MetricReader, error). + metricReaderRegistry = newMetricReaderRegistry() // errUnknownExporter is returned when an unknown exporter name is used in // the OTEL_*_EXPORTER environment variables. @@ -65,23 +81,24 @@ var ( errDuplicateRegistration = errors.New("duplicate registration") ) -// load returns tries to find the SpanExporter factory with the key and +// load returns tries to find the exporter factory with the key and // then execute the factory, returning the created SpanExporter. // errUnknownExporter is returned if the registration is missing and the error from // executing the factory if not nil. -func (r *registry) load(ctx context.Context, key string) (trace.SpanExporter, error) { +func (r *registry[T]) load(ctx context.Context, key string) (T, error) { r.mu.Lock() defer r.mu.Unlock() factory, ok := r.names[key] if !ok { - return nil, errUnknownExporter + var zero T + return zero, errUnknownExporter } return factory(ctx) } // store sets the factory for a key if is not already in the registry. errDuplicateRegistration // is returned if the registry already contains key. -func (r *registry) store(key string, factory func(context.Context) (trace.SpanExporter, error)) error { +func (r *registry[T]) store(key string, factory func(context.Context) (T, error)) error { r.mu.Lock() defer r.mu.Unlock() if _, ok := r.names[key]; ok { @@ -92,7 +109,7 @@ func (r *registry) store(key string, factory func(context.Context) (trace.SpanEx } // drop removes key from the registry if it exists, otherwise nothing. -func (r *registry) drop(key string) { +func (r *registry[T]) drop(key string) { r.mu.Lock() defer r.mu.Unlock() delete(r.names, key) @@ -102,8 +119,26 @@ func (r *registry) drop(key string) { // OTEL_TRACES_EXPORTERS environment variable contains the exporter name. This // will panic if name has already been registered. func RegisterSpanExporter(name string, factory func(context.Context) (trace.SpanExporter, error)) { - if err := envRegistry.store(name, factory); err != nil { - // envRegistry.store will return errDuplicateRegistration if name is already + if err := spanExporterRegistry.store(name, factory); err != nil { + // registry.store will return errDuplicateRegistration if name is already + // registered. Panic here so the user is made aware of the duplicate + // registration, which could be done by malicious code trying to + // intercept cross-cutting concerns. + // + // Panic for all other errors as well. At this point there should not + // be any other errors returned from the store operation. If there + // are, alert the developer that adding them as soon as possible that + // they need to be handled here. + panic(err) + } +} + +// RegisterMetricReader sets the MetricReader factory to be used when the +// OTEL_METRICS_EXPORTERS environment variable contains the exporter name. This +// will panic if name has already been registered. +func RegisterMetricReader(name string, factory func(context.Context) (metric.Reader, error)) { + if err := metricReaderRegistry.store(name, factory); err != nil { + // registry.store will return errDuplicateRegistration if name is already // registered. Panic here so the user is made aware of the duplicate // registration, which could be done by malicious code trying to // intercept cross-cutting concerns. @@ -122,17 +157,30 @@ func RegisterSpanExporter(name string, factory func(context.Context) (trace.Span // under both an empty string "" and "otlp". // An error is returned for any unknown exporters. func spanExporter(ctx context.Context, name string) (trace.SpanExporter, error) { - exp, err := envRegistry.load(ctx, name) + exp, err := spanExporterRegistry.load(ctx, name) if err != nil { return nil, err } return exp, nil } -// buildOTLPExporter creates an OTLP exporter using the environment variable +// metricReader returns a metric reader using the passed in name +// from the list of registered metric.Readers. Each name must match an +// already registered metric.Reader. A default OTLP exporter is registered +// under both an empty string "" and "otlp". +// An error is returned for any unknown exporters. +func metricReader(ctx context.Context, name string) (metric.Reader, error) { + exp, err := metricReaderRegistry.load(ctx, name) + if err != nil { + return nil, err + } + return exp, nil +} + +// buildOTLPSpanExporter creates an OTLP span exporter using the environment variable // OTEL_EXPORTER_OTLP_PROTOCOL to determine the exporter protocol. // Defaults to http/protobuf protocol. -func buildOTLPExporter(ctx context.Context) (trace.SpanExporter, error) { +func buildOTLPSpanExporter(ctx context.Context) (trace.SpanExporter, error) { proto := os.Getenv(otelExporterOTLPProtoEnvKey) if proto == "" { proto = "http/protobuf" @@ -147,3 +195,30 @@ func buildOTLPExporter(ctx context.Context) (trace.SpanExporter, error) { return nil, errInvalidOTLPProtocol } } + +// buildOTLPMetricReader creates an OTLP metric reader using the environment variable +// OTEL_EXPORTER_OTLP_PROTOCOL to determine the exporter protocol. +// Defaults to http/protobuf protocol. +func buildOTLPMetricReader(ctx context.Context) (metric.Reader, error) { + proto := os.Getenv(otelExporterOTLPProtoEnvKey) + if proto == "" { + proto = "http/protobuf" + } + + switch proto { + case "grpc": + r, err := otlpmetricgrpc.New(ctx) + if err != nil { + return nil, err + } + return metric.NewPeriodicReader(r), nil + case "http/protobuf": + r, err := otlpmetricgrpc.New(ctx) + if err != nil { + return nil, err + } + return metric.NewPeriodicReader(r), nil + default: + return nil, errInvalidOTLPProtocol + } +} diff --git a/exporters/autoexport/registry_test.go b/exporters/autoexport/registry_test.go index 72808cee65f..f1faa10bd7b 100644 --- a/exporters/autoexport/registry_test.go +++ b/exporters/autoexport/registry_test.go @@ -36,14 +36,14 @@ var stdoutFactory = func(ctx context.Context) (trace.SpanExporter, error) { } func TestCanStoreExporterFactory(t *testing.T) { - r := newRegistry() + r := newSpanExporterRegistry() assert.NotPanics(t, func() { require.NoError(t, r.store("first", stdoutFactory)) }) } func TestLoadOfUnknownExporterReturnsError(t *testing.T) { - r := newRegistry() + r := newSpanExporterRegistry() assert.NotPanics(t, func() { exp, err := r.load(context.Background(), "non-existent") assert.Equal(t, err, errUnknownExporter, "empty registry should hold nothing") @@ -54,7 +54,7 @@ func TestLoadOfUnknownExporterReturnsError(t *testing.T) { func TestRegistryIsConcurrentSafe(t *testing.T) { const exporterName = "stdout" - r := newRegistry() + r := newSpanExporterRegistry() assert.NotPanics(t, func() { require.NoError(t, r.store(exporterName, stdoutFactory)) }) @@ -108,9 +108,9 @@ func TestDefaultOTLPExporterFactoriesAreAutomaticallyRegistered(t *testing.T) { func TestEnvRegistryCanRegisterExporterFactory(t *testing.T) { const exporterName = "custom" RegisterSpanExporter(exporterName, stdoutFactory) - t.Cleanup(func() { envRegistry.drop(exporterName) }) + t.Cleanup(func() { spanExporterRegistry.drop(exporterName) }) - exp, err := envRegistry.load(context.Background(), exporterName) + exp, err := spanExporterRegistry.load(context.Background(), exporterName) assert.Nil(t, err, "missing exporter in envRegistry") assert.IsType(t, &stdouttrace.Exporter{}, exp) } @@ -118,7 +118,7 @@ func TestEnvRegistryCanRegisterExporterFactory(t *testing.T) { func TestEnvRegistryPanicsOnDuplicateRegisterCalls(t *testing.T) { const exporterName = "custom" RegisterSpanExporter(exporterName, stdoutFactory) - t.Cleanup(func() { envRegistry.drop(exporterName) }) + t.Cleanup(func() { spanExporterRegistry.drop(exporterName) }) errString := fmt.Sprintf("%s: %q", errDuplicateRegistration, exporterName) assert.PanicsWithError(t, errString, func() { From 78ef081af353132cca23989fcc211e91655b5571 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 22 Aug 2023 10:02:05 -0400 Subject: [PATCH 02/26] Format and fix typo --- exporters/autoexport/go.mod | 12 ++++++++---- exporters/autoexport/registry.go | 3 ++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/exporters/autoexport/go.mod b/exporters/autoexport/go.mod index 4d2e21f418a..9535cf970bc 100644 --- a/exporters/autoexport/go.mod +++ b/exporters/autoexport/go.mod @@ -9,10 +9,15 @@ require ( go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0 go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.19.0 go.opentelemetry.io/otel/sdk v1.19.0 + go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.39.0 + go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.39.0 + go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.16.0 + go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.16.0 + go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.16.0 + go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.16.0 + go.opentelemetry.io/otel/sdk v1.16.0 ) -require go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.39.0 // indirect - require ( github.com/cenkalti/backoff/v4 v4.2.1 // indirect github.com/davecgh/go-spew v1.1.1 // indirect @@ -27,8 +32,7 @@ require ( github.com/rogpeppe/go-internal v1.10.0 // indirect go.opentelemetry.io/otel v1.16.0 // indirect go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.16.0 // indirect - go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.39.0 - go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.39.0 + go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.39.0 // indirect go.opentelemetry.io/otel/metric v1.16.0 // indirect go.opentelemetry.io/otel/sdk/metric v0.39.0 go.opentelemetry.io/otel/trace v1.16.0 // indirect diff --git a/exporters/autoexport/registry.go b/exporters/autoexport/registry.go index 60cfe820c3b..ae28045a36e 100644 --- a/exporters/autoexport/registry.go +++ b/exporters/autoexport/registry.go @@ -22,6 +22,7 @@ import ( "sync" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc" + "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" "go.opentelemetry.io/otel/sdk/metric" @@ -213,7 +214,7 @@ func buildOTLPMetricReader(ctx context.Context) (metric.Reader, error) { } return metric.NewPeriodicReader(r), nil case "http/protobuf": - r, err := otlpmetricgrpc.New(ctx) + r, err := otlpmetrichttp.New(ctx) if err != nil { return nil, err } From 0b6c412f0a4cabad6c88f6b64cc7da6a5dd01728 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 22 Aug 2023 10:05:07 -0400 Subject: [PATCH 03/26] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1af4e337007..5c9a68a1ae3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add `WithSpanOptions` option in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (#3768) - Add testing support for Go 1.21. (#4233) - Add `WithFilter` option to `go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux`. (#4230) +- Add metrics support to `go.opentelemetry.io/contrib/exporters/autoexport`. (#4229) ### Changed From 3ea1366fa071c9ad67e595f35e38089af5fd0d06 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 22 Aug 2023 13:47:48 -0400 Subject: [PATCH 04/26] Factor registry into metrics/spans/shared --- exporters/autoexport/registry.go | 148 +---------------------- exporters/autoexport/registry_metrics.go | 96 +++++++++++++++ exporters/autoexport/registry_spans.go | 88 ++++++++++++++ 3 files changed, 185 insertions(+), 147 deletions(-) create mode 100644 exporters/autoexport/registry_metrics.go create mode 100644 exporters/autoexport/registry_spans.go diff --git a/exporters/autoexport/registry.go b/exporters/autoexport/registry.go index ae28045a36e..8f0c619ce52 100644 --- a/exporters/autoexport/registry.go +++ b/exporters/autoexport/registry.go @@ -18,20 +18,10 @@ import ( "context" "errors" "fmt" - "os" "sync" - - "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc" - "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp" - "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" - "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" - "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/trace" ) -const ( - otelExporterOTLPProtoEnvKey = "OTEL_EXPORTER_OTLP_PROTOCOL" -) +const otelExporterOTLPProtoEnvKey = "OTEL_EXPORTER_OTLP_PROTOCOL" // registry maintains a map of exporter names to exporter factories // func(context.Context) (T, error) that is safe for concurrent use by multiple @@ -41,35 +31,7 @@ type registry[T any] struct { names map[string]func(context.Context) (T, error) } -func newSpanExporterRegistry() registry[trace.SpanExporter] { - return registry[trace.SpanExporter]{ - names: map[string]func(context.Context) (trace.SpanExporter, error){ - "": buildOTLPSpanExporter, - "otlp": buildOTLPSpanExporter, - "none": func(ctx context.Context) (trace.SpanExporter, error) { return noop{}, nil }, - }, - } -} - -func newMetricReaderRegistry() registry[metric.Reader] { - return registry[metric.Reader]{ - names: map[string]func(context.Context) (metric.Reader, error){ - "": buildOTLPMetricReader, - "otlp": buildOTLPMetricReader, - "none": func(ctx context.Context) (metric.Reader, error) { return metric.NewManualReader(), nil }, - }, - } -} - var ( - // spanExporterRegistry is the package level registry of exporter registrations - // and their mapping to a SpanExporter factory func(context.Context) (trace.SpanExporter, error). - spanExporterRegistry = newSpanExporterRegistry() - - // metricReaderRegistry is the package level registry of exporter registrations - // and their mapping to a MetricReader factory func(context.Context) (metric.MetricReader, error). - metricReaderRegistry = newMetricReaderRegistry() - // errUnknownExporter is returned when an unknown exporter name is used in // the OTEL_*_EXPORTER environment variables. errUnknownExporter = errors.New("unknown exporter") @@ -115,111 +77,3 @@ func (r *registry[T]) drop(key string) { defer r.mu.Unlock() delete(r.names, key) } - -// RegisterSpanExporter sets the SpanExporter factory to be used when the -// OTEL_TRACES_EXPORTERS environment variable contains the exporter name. This -// will panic if name has already been registered. -func RegisterSpanExporter(name string, factory func(context.Context) (trace.SpanExporter, error)) { - if err := spanExporterRegistry.store(name, factory); err != nil { - // registry.store will return errDuplicateRegistration if name is already - // registered. Panic here so the user is made aware of the duplicate - // registration, which could be done by malicious code trying to - // intercept cross-cutting concerns. - // - // Panic for all other errors as well. At this point there should not - // be any other errors returned from the store operation. If there - // are, alert the developer that adding them as soon as possible that - // they need to be handled here. - panic(err) - } -} - -// RegisterMetricReader sets the MetricReader factory to be used when the -// OTEL_METRICS_EXPORTERS environment variable contains the exporter name. This -// will panic if name has already been registered. -func RegisterMetricReader(name string, factory func(context.Context) (metric.Reader, error)) { - if err := metricReaderRegistry.store(name, factory); err != nil { - // registry.store will return errDuplicateRegistration if name is already - // registered. Panic here so the user is made aware of the duplicate - // registration, which could be done by malicious code trying to - // intercept cross-cutting concerns. - // - // Panic for all other errors as well. At this point there should not - // be any other errors returned from the store operation. If there - // are, alert the developer that adding them as soon as possible that - // they need to be handled here. - panic(err) - } -} - -// spanExporter returns a span exporter using the passed in name -// from the list of registered SpanExporters. Each name must match an -// already registered SpanExporter. A default OTLP exporter is registered -// under both an empty string "" and "otlp". -// An error is returned for any unknown exporters. -func spanExporter(ctx context.Context, name string) (trace.SpanExporter, error) { - exp, err := spanExporterRegistry.load(ctx, name) - if err != nil { - return nil, err - } - return exp, nil -} - -// metricReader returns a metric reader using the passed in name -// from the list of registered metric.Readers. Each name must match an -// already registered metric.Reader. A default OTLP exporter is registered -// under both an empty string "" and "otlp". -// An error is returned for any unknown exporters. -func metricReader(ctx context.Context, name string) (metric.Reader, error) { - exp, err := metricReaderRegistry.load(ctx, name) - if err != nil { - return nil, err - } - return exp, nil -} - -// buildOTLPSpanExporter creates an OTLP span exporter using the environment variable -// OTEL_EXPORTER_OTLP_PROTOCOL to determine the exporter protocol. -// Defaults to http/protobuf protocol. -func buildOTLPSpanExporter(ctx context.Context) (trace.SpanExporter, error) { - proto := os.Getenv(otelExporterOTLPProtoEnvKey) - if proto == "" { - proto = "http/protobuf" - } - - switch proto { - case "grpc": - return otlptracegrpc.New(ctx) - case "http/protobuf": - return otlptracehttp.New(ctx) - default: - return nil, errInvalidOTLPProtocol - } -} - -// buildOTLPMetricReader creates an OTLP metric reader using the environment variable -// OTEL_EXPORTER_OTLP_PROTOCOL to determine the exporter protocol. -// Defaults to http/protobuf protocol. -func buildOTLPMetricReader(ctx context.Context) (metric.Reader, error) { - proto := os.Getenv(otelExporterOTLPProtoEnvKey) - if proto == "" { - proto = "http/protobuf" - } - - switch proto { - case "grpc": - r, err := otlpmetricgrpc.New(ctx) - if err != nil { - return nil, err - } - return metric.NewPeriodicReader(r), nil - case "http/protobuf": - r, err := otlpmetrichttp.New(ctx) - if err != nil { - return nil, err - } - return metric.NewPeriodicReader(r), nil - default: - return nil, errInvalidOTLPProtocol - } -} diff --git a/exporters/autoexport/registry_metrics.go b/exporters/autoexport/registry_metrics.go new file mode 100644 index 00000000000..dc937d2a440 --- /dev/null +++ b/exporters/autoexport/registry_metrics.go @@ -0,0 +1,96 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" + +import ( + "context" + "os" + + "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc" + "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp" + "go.opentelemetry.io/otel/sdk/metric" +) + +func newMetricReaderRegistry() registry[metric.Reader] { + return registry[metric.Reader]{ + names: map[string]func(context.Context) (metric.Reader, error){ + "": buildOTLPMetricReader, + "otlp": buildOTLPMetricReader, + "none": func(ctx context.Context) (metric.Reader, error) { return metric.NewManualReader(), nil }, + }, + } +} + +// metricReaderRegistry is the package level registry of exporter registrations +// and their mapping to a MetricReader factory func(context.Context) (metric.MetricReader, error). +var metricReaderRegistry = newMetricReaderRegistry() + +// RegisterMetricReader sets the MetricReader factory to be used when the +// OTEL_METRICS_EXPORTERS environment variable contains the exporter name. This +// will panic if name has already been registered. +func RegisterMetricReader(name string, factory func(context.Context) (metric.Reader, error)) { + if err := metricReaderRegistry.store(name, factory); err != nil { + // registry.store will return errDuplicateRegistration if name is already + // registered. Panic here so the user is made aware of the duplicate + // registration, which could be done by malicious code trying to + // intercept cross-cutting concerns. + // + // Panic for all other errors as well. At this point there should not + // be any other errors returned from the store operation. If there + // are, alert the developer that adding them as soon as possible that + // they need to be handled here. + panic(err) + } +} + +// metricReader returns a metric reader using the passed in name +// from the list of registered metric.Readers. Each name must match an +// already registered metric.Reader. A default OTLP exporter is registered +// under both an empty string "" and "otlp". +// An error is returned for any unknown exporters. +func metricReader(ctx context.Context, name string) (metric.Reader, error) { + exp, err := metricReaderRegistry.load(ctx, name) + if err != nil { + return nil, err + } + return exp, nil +} + +// buildOTLPMetricReader creates an OTLP metric reader using the environment variable +// OTEL_EXPORTER_OTLP_PROTOCOL to determine the exporter protocol. +// Defaults to http/protobuf protocol. +func buildOTLPMetricReader(ctx context.Context) (metric.Reader, error) { + proto := os.Getenv(otelExporterOTLPProtoEnvKey) + if proto == "" { + proto = "http/protobuf" + } + + switch proto { + case "grpc": + r, err := otlpmetricgrpc.New(ctx) + if err != nil { + return nil, err + } + return metric.NewPeriodicReader(r), nil + case "http/protobuf": + r, err := otlpmetrichttp.New(ctx) + if err != nil { + return nil, err + } + return metric.NewPeriodicReader(r), nil + default: + return nil, errInvalidOTLPProtocol + } +} diff --git a/exporters/autoexport/registry_spans.go b/exporters/autoexport/registry_spans.go new file mode 100644 index 00000000000..5f82ae6f7e8 --- /dev/null +++ b/exporters/autoexport/registry_spans.go @@ -0,0 +1,88 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" + +import ( + "context" + "os" + + "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" + "go.opentelemetry.io/otel/sdk/trace" +) + +func newSpanExporterRegistry() registry[trace.SpanExporter] { + return registry[trace.SpanExporter]{ + names: map[string]func(context.Context) (trace.SpanExporter, error){ + "": buildOTLPSpanExporter, + "otlp": buildOTLPSpanExporter, + "none": func(ctx context.Context) (trace.SpanExporter, error) { return noop{}, nil }, + }, + } +} + +// spanExporterRegistry is the package level registry of exporter registrations +// and their mapping to a SpanExporter factory func(context.Context) (trace.SpanExporter, error). +var spanExporterRegistry = newSpanExporterRegistry() + +// RegisterSpanExporter sets the SpanExporter factory to be used when the +// OTEL_TRACES_EXPORTERS environment variable contains the exporter name. This +// will panic if name has already been registered. +func RegisterSpanExporter(name string, factory func(context.Context) (trace.SpanExporter, error)) { + if err := spanExporterRegistry.store(name, factory); err != nil { + // registry.store will return errDuplicateRegistration if name is already + // registered. Panic here so the user is made aware of the duplicate + // registration, which could be done by malicious code trying to + // intercept cross-cutting concerns. + // + // Panic for all other errors as well. At this point there should not + // be any other errors returned from the store operation. If there + // are, alert the developer that adding them as soon as possible that + // they need to be handled here. + panic(err) + } +} + +// spanExporter returns a span exporter using the passed in name +// from the list of registered SpanExporters. Each name must match an +// already registered SpanExporter. A default OTLP exporter is registered +// under both an empty string "" and "otlp". +// An error is returned for any unknown exporters. +func spanExporter(ctx context.Context, name string) (trace.SpanExporter, error) { + exp, err := spanExporterRegistry.load(ctx, name) + if err != nil { + return nil, err + } + return exp, nil +} + +// buildOTLPSpanExporter creates an OTLP span exporter using the environment variable +// OTEL_EXPORTER_OTLP_PROTOCOL to determine the exporter protocol. +// Defaults to http/protobuf protocol. +func buildOTLPSpanExporter(ctx context.Context) (trace.SpanExporter, error) { + proto := os.Getenv(otelExporterOTLPProtoEnvKey) + if proto == "" { + proto = "http/protobuf" + } + + switch proto { + case "grpc": + return otlptracegrpc.New(ctx) + case "http/protobuf": + return otlptracehttp.New(ctx) + default: + return nil, errInvalidOTLPProtocol + } +} From 3ae284abe53fd73597a6e1d097fbdc955dc20409 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Wed, 23 Aug 2023 10:38:22 -0400 Subject: [PATCH 05/26] Start adding metrics-related tests --- exporters/autoexport/go.mod | 1 + exporters/autoexport/go.sum | 2 + exporters/autoexport/registry_test.go | 56 ++++++++++++++++++++------- 3 files changed, 45 insertions(+), 14 deletions(-) diff --git a/exporters/autoexport/go.mod b/exporters/autoexport/go.mod index 9535cf970bc..6105e7d16d5 100644 --- a/exporters/autoexport/go.mod +++ b/exporters/autoexport/go.mod @@ -33,6 +33,7 @@ require ( go.opentelemetry.io/otel v1.16.0 // indirect go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.16.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.39.0 // indirect + go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v0.39.0 go.opentelemetry.io/otel/metric v1.16.0 // indirect go.opentelemetry.io/otel/sdk/metric v0.39.0 go.opentelemetry.io/otel/trace v1.16.0 // indirect diff --git a/exporters/autoexport/go.sum b/exporters/autoexport/go.sum index 71dca04a33c..b05e9faa017 100644 --- a/exporters/autoexport/go.sum +++ b/exporters/autoexport/go.sum @@ -54,6 +54,8 @@ go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.16.0 h1:TVQp/ go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.16.0/go.mod h1:I33vtIe0sR96wfrUcilIzLoA3mLHhRmz9S9Te0S3gDo= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.16.0 h1:iqjq9LAB8aK++sKVcELezzn655JnBNdsDhghU4G/So8= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.16.0/go.mod h1:hGXzO5bhhSHZnKvrDaXB82Y9DRFour0Nz/KrBh7reWw= +go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v0.39.0 h1:fl2WmyenEf6LYYlfHAtCUEDyGcpwJNqD4dHGO7PVm4w= +go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v0.39.0/go.mod h1:csyQxQ0UHHKVA8KApS7eUO/klMO5sd/av5CNZNU4O6w= go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.16.0 h1:+XWJd3jf75RXJq29mxbuXhCXFDG3S3R4vBUeSI2P7tE= go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.16.0/go.mod h1:hqgzBPTf4yONMFgdZvL/bK42R/iinTyVQtiWihs3SZc= go.opentelemetry.io/otel/metric v1.16.0 h1:RbrpwVG1Hfv85LgnZ7+txXioPDoh6EdbZHo26Q3hqOo= diff --git a/exporters/autoexport/registry_test.go b/exporters/autoexport/registry_test.go index f1faa10bd7b..96bf32d8a19 100644 --- a/exporters/autoexport/registry_test.go +++ b/exporters/autoexport/registry_test.go @@ -23,11 +23,21 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/exporters/stdout/stdoutmetric" "go.opentelemetry.io/otel/exporters/stdout/stdouttrace" + "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/trace" ) -var stdoutFactory = func(ctx context.Context) (trace.SpanExporter, error) { +var stdoutMetricFactory = func(ctx context.Context) (metric.Reader, error) { + exp, err := stdoutmetric.New() + if err != nil { + return nil, err + } + return metric.NewPeriodicReader(exp), nil +} + +var stdoutSpanFactory = func(ctx context.Context) (trace.SpanExporter, error) { exp, err := stdouttrace.New() if err != nil { return nil, err @@ -36,18 +46,36 @@ var stdoutFactory = func(ctx context.Context) (trace.SpanExporter, error) { } func TestCanStoreExporterFactory(t *testing.T) { - r := newSpanExporterRegistry() - assert.NotPanics(t, func() { - require.NoError(t, r.store("first", stdoutFactory)) + t.Run("spans", func(t *testing.T) { + r := newSpanExporterRegistry() + assert.NotPanics(t, func() { + require.NoError(t, r.store("first", stdoutSpanFactory)) + }) + }) + t.Run("metrics", func(t *testing.T) { + r := newMetricReaderRegistry() + assert.NotPanics(t, func() { + require.NoError(t, r.store("first", stdoutMetricFactory)) + }) }) } func TestLoadOfUnknownExporterReturnsError(t *testing.T) { - r := newSpanExporterRegistry() - assert.NotPanics(t, func() { - exp, err := r.load(context.Background(), "non-existent") - assert.Equal(t, err, errUnknownExporter, "empty registry should hold nothing") - assert.Nil(t, exp, "non-nil exporter returned") + t.Run("spans", func(t *testing.T) { + r := newSpanExporterRegistry() + assert.NotPanics(t, func() { + exp, err := r.load(context.Background(), "non-existent") + assert.Equal(t, err, errUnknownExporter, "empty registry should hold nothing") + assert.Nil(t, exp, "non-nil exporter returned") + }) + }) + t.Run("metrics", func(t *testing.T) { + r := newMetricReaderRegistry() + assert.NotPanics(t, func() { + exp, err := r.load(context.Background(), "non-existent") + assert.Equal(t, err, errUnknownExporter, "empty registry should hold nothing") + assert.Nil(t, exp, "non-nil exporter returned") + }) }) } @@ -56,7 +84,7 @@ func TestRegistryIsConcurrentSafe(t *testing.T) { r := newSpanExporterRegistry() assert.NotPanics(t, func() { - require.NoError(t, r.store(exporterName, stdoutFactory)) + require.NoError(t, r.store(exporterName, stdoutSpanFactory)) }) var wg sync.WaitGroup @@ -65,7 +93,7 @@ func TestRegistryIsConcurrentSafe(t *testing.T) { go func() { defer wg.Done() assert.NotPanics(t, func() { - require.ErrorIs(t, r.store(exporterName, stdoutFactory), errDuplicateRegistration) + require.ErrorIs(t, r.store(exporterName, stdoutSpanFactory), errDuplicateRegistration) }) }() @@ -107,7 +135,7 @@ func TestDefaultOTLPExporterFactoriesAreAutomaticallyRegistered(t *testing.T) { func TestEnvRegistryCanRegisterExporterFactory(t *testing.T) { const exporterName = "custom" - RegisterSpanExporter(exporterName, stdoutFactory) + RegisterSpanExporter(exporterName, stdoutSpanFactory) t.Cleanup(func() { spanExporterRegistry.drop(exporterName) }) exp, err := spanExporterRegistry.load(context.Background(), exporterName) @@ -117,11 +145,11 @@ func TestEnvRegistryCanRegisterExporterFactory(t *testing.T) { func TestEnvRegistryPanicsOnDuplicateRegisterCalls(t *testing.T) { const exporterName = "custom" - RegisterSpanExporter(exporterName, stdoutFactory) + RegisterSpanExporter(exporterName, stdoutSpanFactory) t.Cleanup(func() { spanExporterRegistry.drop(exporterName) }) errString := fmt.Sprintf("%s: %q", errDuplicateRegistration, exporterName) assert.PanicsWithError(t, errString, func() { - RegisterSpanExporter(exporterName, stdoutFactory) + RegisterSpanExporter(exporterName, stdoutSpanFactory) }) } From 155c74e538a432d07cc64506088426c64cbe7fac Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Wed, 23 Aug 2023 14:30:16 -0400 Subject: [PATCH 06/26] Generalize tests to metrics --- exporters/autoexport/exporter_test.go | 27 +++-- exporters/autoexport/registry_test.go | 141 +++++++++++++++++--------- 2 files changed, 112 insertions(+), 56 deletions(-) diff --git a/exporters/autoexport/exporter_test.go b/exporters/autoexport/exporter_test.go index 1e39d363d6e..9e7b0d0b500 100644 --- a/exporters/autoexport/exporter_test.go +++ b/exporters/autoexport/exporter_test.go @@ -22,13 +22,14 @@ import ( "github.com/stretchr/testify/assert" "go.opentelemetry.io/otel/exporters/otlp/otlptrace" + "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/trace" ) func TestOTLPExporterReturnedWhenNoEnvOrFallbackExporterConfigured(t *testing.T) { exporter, err := NewSpanExporter(context.Background()) assert.NoError(t, err) - assertOTLPHTTPExporter(t, exporter) + assertOTLPHTTPSpanExporter(t, exporter) } func TestFallbackExporterReturnedWhenNoEnvExporterConfigured(t *testing.T) { @@ -51,7 +52,7 @@ func TestEnvExporterIsPreferredOverFallbackExporter(t *testing.T) { WithFallbackSpanExporter(testExporter), ) assert.NoError(t, err) - assertOTLPHTTPExporter(t, exporter) + assertOTLPHTTPSpanExporter(t, exporter) } func TestEnvExporterOTLPOverHTTP(t *testing.T) { @@ -60,7 +61,7 @@ func TestEnvExporterOTLPOverHTTP(t *testing.T) { exporter, err := NewSpanExporter(context.Background()) assert.NoError(t, err) - assertOTLPHTTPExporter(t, exporter) + assertOTLPHTTPSpanExporter(t, exporter) } func TestEnvExporterOTLPOverGRPC(t *testing.T) { @@ -69,7 +70,7 @@ func TestEnvExporterOTLPOverGRPC(t *testing.T) { exporter, err := NewSpanExporter(context.Background()) assert.NoError(t, err) - assertOTLPGRPCExporter(t, exporter) + assertOTLPGRPCSpanExporter(t, exporter) } func TestEnvExporterOTLPOverGRPCOnlyProtocol(t *testing.T) { @@ -77,7 +78,7 @@ func TestEnvExporterOTLPOverGRPCOnlyProtocol(t *testing.T) { exporter, err := NewSpanExporter(context.Background()) assert.NoError(t, err) - assertOTLPGRPCExporter(t, exporter) + assertOTLPGRPCSpanExporter(t, exporter) } func TestEnvExporterOTLPInvalidProtocol(t *testing.T) { @@ -97,7 +98,19 @@ func TestEnvExporterNone(t *testing.T) { assert.True(t, IsNoneSpanExporter(exporter)) } -func assertOTLPHTTPExporter(t *testing.T, got trace.SpanExporter) { +func assertOTLPHTTPMetricReader(t *testing.T, got metric.Reader) { + t.Helper() + + if !assert.IsType(t, metric.NewPeriodicReader(nil), got) { + return + } + + // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. + clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("exporter").Elem().Type().String() + assert.Equal(t, "*internal.exporter", clientType) +} + +func assertOTLPHTTPSpanExporter(t *testing.T, got trace.SpanExporter) { t.Helper() if !assert.IsType(t, &otlptrace.Exporter{}, got) { @@ -111,7 +124,7 @@ func assertOTLPHTTPExporter(t *testing.T, got trace.SpanExporter) { assert.False(t, IsNoneSpanExporter(got)) } -func assertOTLPGRPCExporter(t *testing.T, got trace.SpanExporter) { +func assertOTLPGRPCSpanExporter(t *testing.T, got trace.SpanExporter) { t.Helper() if !assert.IsType(t, &otlptrace.Exporter{}, got) { diff --git a/exporters/autoexport/registry_test.go b/exporters/autoexport/registry_test.go index 96bf32d8a19..6dc8c084319 100644 --- a/exporters/autoexport/registry_test.go +++ b/exporters/autoexport/registry_test.go @@ -29,6 +29,22 @@ import ( "go.opentelemetry.io/otel/sdk/trace" ) +type testType struct{ string } + +func factory(val string) func(ctx context.Context) (*testType, error) { + return func(ctx context.Context) (*testType, error) { return &testType{val}, nil } +} + +func newTestRegistry() registry[*testType] { + return registry[*testType]{ + names: map[string]func(context.Context) (*testType, error){ + "": factory(""), + "otlp": factory("otlp"), + "none": factory("none"), + }, + } +} + var stdoutMetricFactory = func(ctx context.Context) (metric.Reader, error) { exp, err := stdoutmetric.New() if err != nil { @@ -46,45 +62,27 @@ var stdoutSpanFactory = func(ctx context.Context) (trace.SpanExporter, error) { } func TestCanStoreExporterFactory(t *testing.T) { - t.Run("spans", func(t *testing.T) { - r := newSpanExporterRegistry() - assert.NotPanics(t, func() { - require.NoError(t, r.store("first", stdoutSpanFactory)) - }) - }) - t.Run("metrics", func(t *testing.T) { - r := newMetricReaderRegistry() - assert.NotPanics(t, func() { - require.NoError(t, r.store("first", stdoutMetricFactory)) - }) + r := newTestRegistry() + assert.NotPanics(t, func() { + require.NoError(t, r.store("first", factory("first"))) }) } func TestLoadOfUnknownExporterReturnsError(t *testing.T) { - t.Run("spans", func(t *testing.T) { - r := newSpanExporterRegistry() - assert.NotPanics(t, func() { - exp, err := r.load(context.Background(), "non-existent") - assert.Equal(t, err, errUnknownExporter, "empty registry should hold nothing") - assert.Nil(t, exp, "non-nil exporter returned") - }) - }) - t.Run("metrics", func(t *testing.T) { - r := newMetricReaderRegistry() - assert.NotPanics(t, func() { - exp, err := r.load(context.Background(), "non-existent") - assert.Equal(t, err, errUnknownExporter, "empty registry should hold nothing") - assert.Nil(t, exp, "non-nil exporter returned") - }) + r := newTestRegistry() + assert.NotPanics(t, func() { + exp, err := r.load(context.Background(), "non-existent") + assert.Equal(t, err, errUnknownExporter, "empty registry should hold nothing") + assert.Nil(t, exp, "non-nil exporter returned") }) } func TestRegistryIsConcurrentSafe(t *testing.T) { const exporterName = "stdout" - r := newSpanExporterRegistry() + r := newTestRegistry() assert.NotPanics(t, func() { - require.NoError(t, r.store(exporterName, stdoutSpanFactory)) + require.NoError(t, r.store(exporterName, factory("stdout"))) }) var wg sync.WaitGroup @@ -93,7 +91,7 @@ func TestRegistryIsConcurrentSafe(t *testing.T) { go func() { defer wg.Done() assert.NotPanics(t, func() { - require.ErrorIs(t, r.store(exporterName, stdoutSpanFactory), errDuplicateRegistration) + require.ErrorIs(t, r.store(exporterName, factory("stdout")), errDuplicateRegistration) }) }() @@ -101,55 +99,100 @@ func TestRegistryIsConcurrentSafe(t *testing.T) { go func() { defer wg.Done() assert.NotPanics(t, func() { - exp, err := r.load(context.Background(), exporterName) + _, err := r.load(context.Background(), exporterName) assert.NoError(t, err, "missing exporter in registry") - assert.IsType(t, &stdouttrace.Exporter{}, exp) }) }() wg.Wait() } -func TestSubsequentCallsToGetExporterReturnsNewInstances(t *testing.T) { +type funcs[T any] struct { + makeExporter func(context.Context, string) (T, error) + assertOTLPHTTP func(*testing.T, T) + registerFactory func(string, func(context.Context) (T, error)) + stdoutFactory func(ctx context.Context) (T, error) + registry *registry[T] +} + +var ( + spanFuncs = funcs[trace.SpanExporter]{ + makeExporter: spanExporter, + assertOTLPHTTP: assertOTLPHTTPSpanExporter, + registerFactory: RegisterSpanExporter, + stdoutFactory: stdoutSpanFactory, + registry: &spanExporterRegistry, + } + + metricFuncs = funcs[metric.Reader]{ + makeExporter: metricReader, + assertOTLPHTTP: assertOTLPHTTPMetricReader, + registerFactory: RegisterMetricReader, + stdoutFactory: stdoutMetricFactory, + registry: &metricReaderRegistry, + } +) + +func (f funcs[T]) testSubsequentCallsToGetExporterReturnsNewInstances(t *testing.T) { const exporterType = "otlp" - exp1, err := spanExporter(context.Background(), exporterType) + + exp1, err := f.makeExporter(context.Background(), exporterType) assert.NoError(t, err) - assertOTLPHTTPExporter(t, exp1) + f.assertOTLPHTTP(t, exp1) exp2, err := spanExporter(context.Background(), exporterType) assert.NoError(t, err) - assertOTLPHTTPExporter(t, exp2) + assertOTLPHTTPSpanExporter(t, exp2) assert.NotSame(t, exp1, exp2) } -func TestDefaultOTLPExporterFactoriesAreAutomaticallyRegistered(t *testing.T) { - exp1, err := spanExporter(context.Background(), "") +func TestSubsequentCallsToGetExporterReturnsNewInstances(t *testing.T) { + t.Run("spans", spanFuncs.testSubsequentCallsToGetExporterReturnsNewInstances) + t.Run("metrics", metricFuncs.testSubsequentCallsToGetExporterReturnsNewInstances) +} + +func (f funcs[T]) testDefaultOTLPExporterFactoriesAreAutomaticallyRegistered(t *testing.T) { + exp1, err := f.makeExporter(context.Background(), "") assert.Nil(t, err) - assertOTLPHTTPExporter(t, exp1) + f.assertOTLPHTTP(t, exp1) - exp2, err := spanExporter(context.Background(), "otlp") + exp2, err := f.makeExporter(context.Background(), "otlp") assert.Nil(t, err) - assertOTLPHTTPExporter(t, exp2) + f.assertOTLPHTTP(t, exp2) } -func TestEnvRegistryCanRegisterExporterFactory(t *testing.T) { +func TestDefaultOTLPExporterFactoriesAreAutomaticallyRegistered(t *testing.T) { + t.Run("spans", spanFuncs.testDefaultOTLPExporterFactoriesAreAutomaticallyRegistered) + t.Run("metrics", metricFuncs.testDefaultOTLPExporterFactoriesAreAutomaticallyRegistered) +} + +func (f funcs[T]) testEnvRegistryCanRegisterExporterFactory(t *testing.T) { const exporterName = "custom" - RegisterSpanExporter(exporterName, stdoutSpanFactory) - t.Cleanup(func() { spanExporterRegistry.drop(exporterName) }) + f.registerFactory(exporterName, f.stdoutFactory) + t.Cleanup(func() { f.registry.drop(exporterName) }) - exp, err := spanExporterRegistry.load(context.Background(), exporterName) + _, err := f.registry.load(context.Background(), exporterName) assert.Nil(t, err, "missing exporter in envRegistry") - assert.IsType(t, &stdouttrace.Exporter{}, exp) } -func TestEnvRegistryPanicsOnDuplicateRegisterCalls(t *testing.T) { +func TestEnvRegistryCanRegisterExporterFactory(t *testing.T) { + t.Run("spans", spanFuncs.testEnvRegistryCanRegisterExporterFactory) + t.Run("metrics", metricFuncs.testEnvRegistryCanRegisterExporterFactory) +} + +func (f funcs[T]) testEnvRegistryPanicsOnDuplicateRegisterCalls(t *testing.T) { const exporterName = "custom" - RegisterSpanExporter(exporterName, stdoutSpanFactory) - t.Cleanup(func() { spanExporterRegistry.drop(exporterName) }) + f.registerFactory(exporterName, f.stdoutFactory) + t.Cleanup(func() { f.registry.drop(exporterName) }) errString := fmt.Sprintf("%s: %q", errDuplicateRegistration, exporterName) assert.PanicsWithError(t, errString, func() { - RegisterSpanExporter(exporterName, stdoutSpanFactory) + f.registerFactory(exporterName, f.stdoutFactory) }) } + +func TestEnvRegistryPanicsOnDuplicateRegisterCalls(t *testing.T) { + t.Run("spans", spanFuncs.testEnvRegistryPanicsOnDuplicateRegisterCalls) + t.Run("metrics", metricFuncs.testEnvRegistryPanicsOnDuplicateRegisterCalls) +} From 2999fd48aa69f9056872af114051d4c1500dc3be Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Mon, 16 Oct 2023 11:12:23 -0400 Subject: [PATCH 07/26] Fix deps --- exporters/autoexport/go.mod | 20 +++++-------------- exporters/autoexport/go.sum | 38 ++++++++++--------------------------- 2 files changed, 15 insertions(+), 43 deletions(-) diff --git a/exporters/autoexport/go.mod b/exporters/autoexport/go.mod index 6105e7d16d5..d640a4dfd58 100644 --- a/exporters/autoexport/go.mod +++ b/exporters/autoexport/go.mod @@ -4,18 +4,15 @@ go 1.20 require ( github.com/stretchr/testify v1.8.4 + go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.42.0 + go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.42.0 go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.19.0 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.19.0 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0 + go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v0.42.0 go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.19.0 go.opentelemetry.io/otel/sdk v1.19.0 - go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.39.0 - go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.39.0 - go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.16.0 - go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.16.0 - go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.16.0 - go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.16.0 - go.opentelemetry.io/otel/sdk v1.16.0 + go.opentelemetry.io/otel/sdk/metric v1.19.0 ) require ( @@ -27,16 +24,9 @@ require ( github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect go.opentelemetry.io/otel v1.19.0 // indirect + go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.42.0 // indirect go.opentelemetry.io/otel/metric v1.19.0 // indirect go.opentelemetry.io/otel/trace v1.19.0 // indirect - github.com/rogpeppe/go-internal v1.10.0 // indirect - go.opentelemetry.io/otel v1.16.0 // indirect - go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.16.0 // indirect - go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.39.0 // indirect - go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v0.39.0 - go.opentelemetry.io/otel/metric v1.16.0 // indirect - go.opentelemetry.io/otel/sdk/metric v0.39.0 - go.opentelemetry.io/otel/trace v1.16.0 // indirect go.opentelemetry.io/proto/otlp v1.0.0 // indirect golang.org/x/net v0.17.0 // indirect golang.org/x/sys v0.13.0 // indirect diff --git a/exporters/autoexport/go.sum b/exporters/autoexport/go.sum index b05e9faa017..7476b594f9c 100644 --- a/exporters/autoexport/go.sum +++ b/exporters/autoexport/go.sum @@ -24,48 +24,30 @@ github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcU github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= go.opentelemetry.io/otel v1.19.0 h1:MuS/TNf4/j4IXsZuJegVzI1cwut7Qc00344rgH7p8bs= go.opentelemetry.io/otel v1.19.0/go.mod h1:i0QyjOq3UPoTzff0PJB2N66fb4S0+rSbSB15/oyH9fY= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.42.0 h1:ZtfnDL+tUrs1F0Pzfwbg2d59Gru9NCH3bgSHBM6LDwU= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.42.0/go.mod h1:hG4Fj/y8TR/tlEDREo8tWstl9fO9gcFkn4xrx0Io8xU= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.42.0 h1:NmnYCiR0qNufkldjVvyQfZTHSdzeHoZ41zggMsdMcLM= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.42.0/go.mod h1:UVAO61+umUsHLtYb8KXXRoHtxUkdOPkYidzW3gipRLQ= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.42.0 h1:wNMDy/LVGLj2h3p6zg4d0gypKfWKSWI14E1C4smOgl8= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.42.0/go.mod h1:YfbDdXAAkemWJK3H/DshvlrxqFB2rtW4rY6ky/3x/H0= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.19.0 h1:Mne5On7VWdx7omSrSSZvM4Kw7cS7NQkOOmLcgscI51U= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.19.0/go.mod h1:IPtUMKL4O3tH5y+iXVyAXqpAwMuzC1IrxVS81rummfE= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.19.0 h1:3d+S281UTjM+AbF31XSOYn1qXn3BgIdWl8HNEpx08Jk= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.19.0/go.mod h1:0+KuTDyKL4gjKCF75pHOX4wuzYDUZYfAQdSu43o+Z2I= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0 h1:IeMeyr1aBvBiPVYihXIaeIZba6b8E1bYp7lbdxK8CQg= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0/go.mod h1:oVdCUtjq9MK9BlS7TtucsQwUcXcymNiEDjgDD2jMtZU= +go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v0.42.0 h1:4jJuoeOo9W6hZnz+r046fyoH5kykZPRvKfUXJVfMpB0= +go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v0.42.0/go.mod h1:/MtYTE1SfC2QIcE0bDot6fIX+h+WvXjgTqgn9P0LNPE= go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.19.0 h1:Nw7Dv4lwvGrI68+wULbcq7su9K2cebeCUrDjVrUJHxM= go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.19.0/go.mod h1:1MsF6Y7gTqosgoZvHlzcaaM8DIMNZgJh87ykokoNH7Y= go.opentelemetry.io/otel/metric v1.19.0 h1:aTzpGtV0ar9wlV4Sna9sdJyII5jTVJEvKETPiOKwvpE= go.opentelemetry.io/otel/metric v1.19.0/go.mod h1:L5rUsV9kM1IxCj1MmSdS+JQAcVm319EUrDVLrt7jqt8= go.opentelemetry.io/otel/sdk v1.19.0 h1:6USY6zH+L8uMH8L3t1enZPR3WFEmSTADlqldyHtJi3o= go.opentelemetry.io/otel/sdk v1.19.0/go.mod h1:NedEbbS4w3C6zElbLdPJKOpJQOrGUJ+GfzpjUvI0v1A= +go.opentelemetry.io/otel/sdk/metric v1.19.0 h1:EJoTO5qysMsYCa+w4UghwFV/ptQgqSL/8Ni+hx+8i1k= +go.opentelemetry.io/otel/sdk/metric v1.19.0/go.mod h1:XjG0jQyFJrv2PbMvwND7LwCEhsJzCzV5210euduKcKY= go.opentelemetry.io/otel/trace v1.19.0 h1:DFVQmlVbfVeOuBRrwdtaehRrWiL1JoVs9CPIQ1Dzxpg= go.opentelemetry.io/otel/trace v1.19.0/go.mod h1:mfaSyvGyEJEI0nyV2I4qhNQnbBOUUmYZpYojqMnX2vo= -go.opentelemetry.io/otel v1.16.0 h1:Z7GVAX/UkAXPKsy94IU+i6thsQS4nb7LviLpnaNeW8s= -go.opentelemetry.io/otel v1.16.0/go.mod h1:vl0h9NUa1D5s1nv3A5vZOYWn8av4K8Ml6JDeHrT/bx4= -go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.16.0 h1:t4ZwRPU+emrcvM2e9DHd0Fsf0JTPVcbfa/BhTDF03d0= -go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.16.0/go.mod h1:vLarbg68dH2Wa77g71zmKQqlQ8+8Rq3GRG31uc0WcWI= -go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.39.0 h1:f6BwB2OACc3FCbYVznctQ9V6KK7Vq6CjmYXJ7DeSs4E= -go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.39.0/go.mod h1:UqL5mZ3qs6XYhDnZaW1Ps4upD+PX6LipH40AoeuIlwU= -go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.39.0 h1:rm+Fizi7lTM2UefJ1TO347fSRcwmIsUAaZmYmIGBRAo= -go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.39.0/go.mod h1:sWFbI3jJ+6JdjOVepA5blpv/TJ20Hw+26561iMbWcwU= -go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.39.0 h1:IZXpCEtI7BbX01DRQEWTGDkvjMB6hEhiEZXS+eg2YqY= -go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.39.0/go.mod h1:xY111jIZtWb+pUUgT4UiiSonAaY2cD2Ts5zvuKLki3o= -go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.16.0 h1:cbsD4cUcviQGXdw8+bo5x2wazq10SKz8hEbtCRPcU78= -go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.16.0/go.mod h1:JgXSGah17croqhJfhByOLVY719k1emAXC8MVhCIJlRs= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.16.0 h1:TVQp/bboR4mhZSav+MdgXB8FaRho1RC8UwVn3T0vjVc= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.16.0/go.mod h1:I33vtIe0sR96wfrUcilIzLoA3mLHhRmz9S9Te0S3gDo= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.16.0 h1:iqjq9LAB8aK++sKVcELezzn655JnBNdsDhghU4G/So8= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.16.0/go.mod h1:hGXzO5bhhSHZnKvrDaXB82Y9DRFour0Nz/KrBh7reWw= -go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v0.39.0 h1:fl2WmyenEf6LYYlfHAtCUEDyGcpwJNqD4dHGO7PVm4w= -go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v0.39.0/go.mod h1:csyQxQ0UHHKVA8KApS7eUO/klMO5sd/av5CNZNU4O6w= -go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.16.0 h1:+XWJd3jf75RXJq29mxbuXhCXFDG3S3R4vBUeSI2P7tE= -go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.16.0/go.mod h1:hqgzBPTf4yONMFgdZvL/bK42R/iinTyVQtiWihs3SZc= -go.opentelemetry.io/otel/metric v1.16.0 h1:RbrpwVG1Hfv85LgnZ7+txXioPDoh6EdbZHo26Q3hqOo= -go.opentelemetry.io/otel/metric v1.16.0/go.mod h1:QE47cpOmkwipPiefDwo2wDzwJrlfxxNYodqc4xnGCo4= -go.opentelemetry.io/otel/sdk v1.16.0 h1:Z1Ok1YsijYL0CSJpHt4cS3wDDh7p572grzNrBMiMWgE= -go.opentelemetry.io/otel/sdk v1.16.0/go.mod h1:tMsIuKXuuIWPBAOrH+eHtvhTL+SntFtXF9QD68aP6p4= -go.opentelemetry.io/otel/sdk/metric v0.39.0 h1:Kun8i1eYf48kHH83RucG93ffz0zGV1sh46FAScOTuDI= -go.opentelemetry.io/otel/sdk/metric v0.39.0/go.mod h1:piDIRgjcK7u0HCL5pCA4e74qpK/jk3NiUoAHATVAmiI= -go.opentelemetry.io/otel/trace v1.16.0 h1:8JRpaObFoW0pxuVPapkgH8UhHQj+bJW8jJsCZEu5MQs= -go.opentelemetry.io/otel/trace v1.16.0/go.mod h1:Yt9vYq1SdNz3xdjZZK7wcXv1qv2pwLkqr2QVwea0ef0= go.opentelemetry.io/proto/otlp v1.0.0 h1:T0TX0tmXU8a3CbNXzEKGeU5mIVOdf0oykP+u2lIVU/I= go.opentelemetry.io/proto/otlp v1.0.0/go.mod h1:Sy6pihPLfYHkr3NkUbEhGHFhINUSI/v80hjKIs5JXpM= go.uber.org/goleak v1.2.1 h1:NBol2c7O1ZokfZ0LEU9K6Whx/KnwvepVetCUhtKja4A= From d498e8efcda4450a1a0e4da4464b0bb4592ba42a Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Wed, 18 Oct 2023 07:23:54 -0400 Subject: [PATCH 08/26] Tests --- exporters/autoexport/exporter.go | 9 + exporters/autoexport/exporter_test.go | 210 ++++++++++++++--------- exporters/autoexport/noop.go | 9 + exporters/autoexport/registry_metrics.go | 2 +- 4 files changed, 147 insertions(+), 83 deletions(-) diff --git a/exporters/autoexport/exporter.go b/exporters/autoexport/exporter.go index 1abfcde4f20..f3a066c117b 100644 --- a/exporters/autoexport/exporter.go +++ b/exporters/autoexport/exporter.go @@ -157,6 +157,15 @@ func NewMetricReader(ctx context.Context, opts ...Option) (metric.Reader, error) return config.fallbackMetricReader, nil } +// WithFallbackMetricReader sets the fallback exporter to use when no exporter +// is configured through the OTEL_METRICS_EXPORTER environment variable. +func WithFallbackMetricReader(reader metric.Reader) Option { + return optionFunc(func(cfg config) config { + cfg.fallbackMetricReader = reader + return cfg + }) +} + // makeSpanExporterFromEnv returns a configured SpanExporter defined by the OTEL_TRACES_EXPORTER // environment variable. // nil is returned if no exporter is defined for the environment variable. diff --git a/exporters/autoexport/exporter_test.go b/exporters/autoexport/exporter_test.go index 9e7b0d0b500..06b757d1117 100644 --- a/exporters/autoexport/exporter_test.go +++ b/exporters/autoexport/exporter_test.go @@ -26,76 +26,136 @@ import ( "go.opentelemetry.io/otel/sdk/trace" ) -func TestOTLPExporterReturnedWhenNoEnvOrFallbackExporterConfigured(t *testing.T) { - exporter, err := NewSpanExporter(context.Background()) - assert.NoError(t, err) - assertOTLPHTTPSpanExporter(t, exporter) +func TestSpans(t *testing.T) { + var fallbackExporter testSpanExporter + + signal[trace.SpanExporter]{ + newExporter: func() (trace.SpanExporter, error) { + return NewSpanExporter(context.Background()) + }, + fallbackExporter: &fallbackExporter, + newExporterWithFallback: func() (trace.SpanExporter, error) { + return NewSpanExporter(context.Background(), WithFallbackSpanExporter(&fallbackExporter)) + }, + assertOTLPHTTP: assertOTLPHTTPSpanExporter, + assertOTLPGRPC: func(t *testing.T, got trace.SpanExporter) { + t.Helper() + + if !assert.IsType(t, &otlptrace.Exporter{}, got) { + return + } + + // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. + clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("client").Elem().Type().String() + assert.Equal(t, "*otlptracegrpc.client", clientType) + + assert.False(t, IsNoneSpanExporter(got)) + }, + isNoneExporter: IsNoneSpanExporter, + envVariable: "OTEL_TRACES_EXPORTER", + }.testAll(t) } -func TestFallbackExporterReturnedWhenNoEnvExporterConfigured(t *testing.T) { - testExporter := &testExporter{} - exporter, err := NewSpanExporter( - context.Background(), - WithFallbackSpanExporter(testExporter), - ) - assert.NoError(t, err) - assert.Equal(t, testExporter, exporter) - assert.False(t, IsNoneSpanExporter(exporter)) +func TestMetrics(t *testing.T) { + fallbackExporter := metric.NewManualReader() + + signal[metric.Reader]{ + newExporter: func() (metric.Reader, error) { + return NewMetricReader(context.Background()) + }, + fallbackExporter: fallbackExporter, + newExporterWithFallback: func() (metric.Reader, error) { + return NewMetricReader(context.Background(), WithFallbackMetricReader(fallbackExporter)) + }, + assertOTLPHTTP: assertOTLPHTTPMetricReader, + assertOTLPGRPC: func(t *testing.T, got metric.Reader) { + t.Helper() + + if !assert.IsType(t, metric.NewPeriodicReader(nil), got) { + return + } + + // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. + clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("exporter").Elem().Type().String() + assert.Equal(t, "*otlpmetricgrpc.Exporter", clientType) + }, + isNoneExporter: IsNoneMetricReader, + envVariable: "OTEL_METRICS_EXPORTER", + }.testAll(t) } -func TestEnvExporterIsPreferredOverFallbackExporter(t *testing.T) { - t.Setenv("OTEL_TRACES_EXPORTER", "otlp") - - testExporter := &testExporter{} - exporter, err := NewSpanExporter( - context.Background(), - WithFallbackSpanExporter(testExporter), - ) - assert.NoError(t, err) - assertOTLPHTTPSpanExporter(t, exporter) -} - -func TestEnvExporterOTLPOverHTTP(t *testing.T) { - t.Setenv("OTEL_TRACES_EXPORTER", "otlp") - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf") - - exporter, err := NewSpanExporter(context.Background()) - assert.NoError(t, err) - assertOTLPHTTPSpanExporter(t, exporter) -} - -func TestEnvExporterOTLPOverGRPC(t *testing.T) { - t.Setenv("OTEL_TRACES_EXPORTER", "otlp") - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") - - exporter, err := NewSpanExporter(context.Background()) - assert.NoError(t, err) - assertOTLPGRPCSpanExporter(t, exporter) -} - -func TestEnvExporterOTLPOverGRPCOnlyProtocol(t *testing.T) { - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") - - exporter, err := NewSpanExporter(context.Background()) - assert.NoError(t, err) - assertOTLPGRPCSpanExporter(t, exporter) -} - -func TestEnvExporterOTLPInvalidProtocol(t *testing.T) { - t.Setenv("OTEL_TRACES_EXPORTER", "otlp") - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "invalid") - - exporter, err := NewSpanExporter(context.Background()) - assert.Error(t, err) - assert.Nil(t, exporter) +type signal[T any] struct { + newExporter, newExporterWithFallback func() (T, error) + fallbackExporter T + assertOTLPHTTP, assertOTLPGRPC func(t *testing.T, got T) + isNoneExporter func(exporter T) bool + envVariable string } -func TestEnvExporterNone(t *testing.T) { - t.Setenv("OTEL_TRACES_EXPORTER", "none") - - exporter, err := NewSpanExporter(context.Background()) - assert.NoError(t, err) - assert.True(t, IsNoneSpanExporter(exporter)) +func (s signal[T]) testAll(t *testing.T) { + t.Run("OTLPExporterReturnedWhenNoEnvOrFallbackExporterConfigured", func(t *testing.T) { + exporter, err := s.newExporter() + assert.NoError(t, err) + s.assertOTLPHTTP(t, exporter) + }) + + t.Run("FallbackExporterReturnedWhenNoEnvExporterConfigured", func(t *testing.T) { + exporter, err := s.newExporterWithFallback() + assert.NoError(t, err) + assert.Equal(t, s.fallbackExporter, exporter) + assert.False(t, s.isNoneExporter(exporter)) + }) + + t.Run("EnvExporterIsPreferredOverFallbackExporter", func(t *testing.T) { + t.Setenv(s.envVariable, "otlp") + + exporter, err := s.newExporterWithFallback() + assert.NoError(t, err) + s.assertOTLPHTTP(t, exporter) + }) + + t.Run("EnvExporterOTLPOverHTTP", func(t *testing.T) { + t.Setenv(s.envVariable, "otlp") + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf") + + exporter, err := s.newExporter() + assert.NoError(t, err) + s.assertOTLPHTTP(t, exporter) + }) + + t.Run("EnvExporterOTLPOverGRPC", func(t *testing.T) { + t.Setenv(s.envVariable, "otlp") + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") + + exporter, err := s.newExporter() + assert.NoError(t, err) + s.assertOTLPGRPC(t, exporter) + }) + + t.Run("EnvExporterOTLPOverGRPCOnlyProtocol", func(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") + + exporter, err := s.newExporter() + assert.NoError(t, err) + s.assertOTLPGRPC(t, exporter) + }) + + t.Run("EnvExporterOTLPInvalidProtocol", func(t *testing.T) { + t.Setenv(s.envVariable, "otlp") + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "invalid") + + exporter, err := s.newExporter() + assert.Error(t, err) + assert.Nil(t, exporter) + }) + + t.Run("EnvExporterNone", func(t *testing.T) { + t.Setenv(s.envVariable, "none") + + exporter, err := s.newExporter() + assert.NoError(t, err) + assert.True(t, s.isNoneExporter(exporter)) + }) } func assertOTLPHTTPMetricReader(t *testing.T, got metric.Reader) { @@ -107,7 +167,7 @@ func assertOTLPHTTPMetricReader(t *testing.T, got metric.Reader) { // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("exporter").Elem().Type().String() - assert.Equal(t, "*internal.exporter", clientType) + assert.Equal(t, "*otlpmetrichttp.Exporter", clientType) } func assertOTLPHTTPSpanExporter(t *testing.T, got trace.SpanExporter) { @@ -124,26 +184,12 @@ func assertOTLPHTTPSpanExporter(t *testing.T, got trace.SpanExporter) { assert.False(t, IsNoneSpanExporter(got)) } -func assertOTLPGRPCSpanExporter(t *testing.T, got trace.SpanExporter) { - t.Helper() - - if !assert.IsType(t, &otlptrace.Exporter{}, got) { - return - } - - // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. - clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("client").Elem().Type().String() - assert.Equal(t, "*otlptracegrpc.client", clientType) - - assert.False(t, IsNoneSpanExporter(got)) -} - -type testExporter struct{} +type testSpanExporter struct{} -func (e *testExporter) ExportSpans(ctx context.Context, ss []trace.ReadOnlySpan) error { +func (e *testSpanExporter) ExportSpans(ctx context.Context, ss []trace.ReadOnlySpan) error { return nil } -func (e *testExporter) Shutdown(ctx context.Context) error { +func (e *testSpanExporter) Shutdown(ctx context.Context) error { return nil } diff --git a/exporters/autoexport/noop.go b/exporters/autoexport/noop.go index 95797692a01..38473613800 100644 --- a/exporters/autoexport/noop.go +++ b/exporters/autoexport/noop.go @@ -17,6 +17,7 @@ package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" import ( "context" + "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/trace" ) @@ -41,3 +42,11 @@ func IsNoneSpanExporter(e trace.SpanExporter) bool { _, ok := e.(noop) return ok } + +var noopMetricReader = metric.NewManualReader() + +// IsNoneMetricReader returns true for the exporter returned by [NewMetricReader] +// when OTEL_METRICS_EXPORTER environment variable is set to "none". +func IsNoneMetricReader(e metric.Reader) bool { + return e == noopMetricReader +} diff --git a/exporters/autoexport/registry_metrics.go b/exporters/autoexport/registry_metrics.go index dc937d2a440..9a81aa47a06 100644 --- a/exporters/autoexport/registry_metrics.go +++ b/exporters/autoexport/registry_metrics.go @@ -28,7 +28,7 @@ func newMetricReaderRegistry() registry[metric.Reader] { names: map[string]func(context.Context) (metric.Reader, error){ "": buildOTLPMetricReader, "otlp": buildOTLPMetricReader, - "none": func(ctx context.Context) (metric.Reader, error) { return metric.NewManualReader(), nil }, + "none": func(ctx context.Context) (metric.Reader, error) { return noopMetricReader, nil }, }, } } From c33d73fadc34dfc89510073f2de27394d81b3368 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Thu, 19 Oct 2023 05:23:57 -0400 Subject: [PATCH 09/26] Add support for Prometheus exporter --- exporters/autoexport/exporter.go | 7 ++++--- exporters/autoexport/go.mod | 11 +++++++++++ exporters/autoexport/go.sum | 18 ++++++++++++++++++ exporters/autoexport/registry_metrics.go | 8 +++++--- 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/exporters/autoexport/exporter.go b/exporters/autoexport/exporter.go index f3a066c117b..f19e920ff0c 100644 --- a/exporters/autoexport/exporter.go +++ b/exporters/autoexport/exporter.go @@ -123,14 +123,15 @@ func NewSpanExporter(ctx context.Context, opts ...Option) (trace.SpanExporter, e // // OTEL_METRICS_EXPORTER defines the metrics exporter; supported values: // - "none" - "no operation" exporter -// - "otlp" (default) - OTLP exporter; see [go.opentelemetry.io/otel/exporters/otlp/otlptrace] +// - "otlp" (default) - OTLP exporter; see [go.opentelemetry.io/otel/exporters/otlp/otlpmetric] +// - "prometheus" - Prometheus exporter; see [go.opentelemetry.io/otel/exporters/prometheus] // // OTEL_EXPORTER_OTLP_PROTOCOL defines OTLP exporter's transport protocol; // supported values: // - "grpc" - protobuf-encoded data using gRPC wire format over HTTP/2 connection; -// see: [go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc] +// see: [go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc] // - "http/protobuf" (default) - protobuf-encoded data over HTTP connection; -// see: [go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp] +// see: [go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp] // // An error is returned if an environment value is set to an unhandled value. // diff --git a/exporters/autoexport/go.mod b/exporters/autoexport/go.mod index d640a4dfd58..6bd6cfdfdca 100644 --- a/exporters/autoexport/go.mod +++ b/exporters/autoexport/go.mod @@ -15,6 +15,16 @@ require ( go.opentelemetry.io/otel/sdk/metric v1.19.0 ) +require ( + github.com/beorn7/perks v1.0.1 // indirect + github.com/cespare/xxhash/v2 v2.2.0 // indirect + github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect + github.com/prometheus/client_golang v1.16.0 // indirect + github.com/prometheus/client_model v0.4.0 // indirect + github.com/prometheus/common v0.42.0 // indirect + github.com/prometheus/procfs v0.10.1 // indirect +) + require ( github.com/cenkalti/backoff/v4 v4.2.1 // indirect github.com/davecgh/go-spew v1.1.1 // indirect @@ -25,6 +35,7 @@ require ( github.com/pmezard/go-difflib v1.0.0 // indirect go.opentelemetry.io/otel v1.19.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.42.0 // indirect + go.opentelemetry.io/otel/exporters/prometheus v0.42.0 go.opentelemetry.io/otel/metric v1.19.0 // indirect go.opentelemetry.io/otel/trace v1.19.0 // indirect go.opentelemetry.io/proto/otlp v1.0.0 // indirect diff --git a/exporters/autoexport/go.sum b/exporters/autoexport/go.sum index 7476b594f9c..022495a5fd1 100644 --- a/exporters/autoexport/go.sum +++ b/exporters/autoexport/go.sum @@ -1,5 +1,9 @@ +github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= +github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/cenkalti/backoff/v4 v4.2.1 h1:y4OZtCnogmCPw98Zjyt5a6+QwPLGkiQsYW5oUqylYbM= github.com/cenkalti/backoff/v4 v4.2.1/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= +github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44= +github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= 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/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= @@ -8,6 +12,7 @@ github.com/go-logr/logr v1.2.4/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbV 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/glog v1.1.0 h1:/d3pCKDPWNnvIWe0vVUpNP32qc8U3PDVxySP/y360qE= +github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= @@ -17,8 +22,18 @@ github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.0 h1:YBftPWNWd4WwGqtY2yeZL2ef8rH github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.0/go.mod h1:YN5jB8ie0yfIUg6VvR9Kz84aCaG7AsGZnLjhHbUqwPg= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zkfA9PSy5pEvNWRP0ET0TIVo= +github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4= 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/prometheus/client_golang v1.16.0 h1:yk/hx9hDbrGHovbci4BY+pRMfSuuat626eFsHb7tmT8= +github.com/prometheus/client_golang v1.16.0/go.mod h1:Zsulrv/L9oM40tJ7T815tM89lFEugiJ9HzIqaAx4LKc= +github.com/prometheus/client_model v0.4.0 h1:5lQXD3cAg1OXBf4Wq03gTrXHeaV0TQvGfUooCfx1yqY= +github.com/prometheus/client_model v0.4.0/go.mod h1:oMQmHW1/JoDwqLtg57MGgP/Fb1CJEYF2imWWhWtMkYU= +github.com/prometheus/common v0.42.0 h1:EKsfXEYo4JpWMHH5cg+KOUWeuJSov1Id8zGR8eeI1YM= +github.com/prometheus/common v0.42.0/go.mod h1:xBwqVerjNdUDjgODMpudtOMwlOwf2SaTr1yjz4b7Zbc= +github.com/prometheus/procfs v0.10.1 h1:kYK1Va/YMlutzCGazswoHKo//tZVlFpKYh+PymziUAg= +github.com/prometheus/procfs v0.10.1/go.mod h1:nwNm2aOCAYw8uTR/9bWRREkZFxAUcWzPHWJq+XBB/FM= github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= @@ -36,6 +51,8 @@ go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.19.0 h1:3d+S2 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.19.0/go.mod h1:0+KuTDyKL4gjKCF75pHOX4wuzYDUZYfAQdSu43o+Z2I= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0 h1:IeMeyr1aBvBiPVYihXIaeIZba6b8E1bYp7lbdxK8CQg= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0/go.mod h1:oVdCUtjq9MK9BlS7TtucsQwUcXcymNiEDjgDD2jMtZU= +go.opentelemetry.io/otel/exporters/prometheus v0.42.0 h1:jwV9iQdvp38fxXi8ZC+lNpxjK16MRcZlpDYvbuO1FiA= +go.opentelemetry.io/otel/exporters/prometheus v0.42.0/go.mod h1:f3bYiqNqhoPxkvI2LrXqQVC546K7BuRDL/kKuxkujhA= go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v0.42.0 h1:4jJuoeOo9W6hZnz+r046fyoH5kykZPRvKfUXJVfMpB0= go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v0.42.0/go.mod h1:/MtYTE1SfC2QIcE0bDot6fIX+h+WvXjgTqgn9P0LNPE= go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.19.0 h1:Nw7Dv4lwvGrI68+wULbcq7su9K2cebeCUrDjVrUJHxM= @@ -53,6 +70,7 @@ go.opentelemetry.io/proto/otlp v1.0.0/go.mod h1:Sy6pihPLfYHkr3NkUbEhGHFhINUSI/v8 go.uber.org/goleak v1.2.1 h1:NBol2c7O1ZokfZ0LEU9K6Whx/KnwvepVetCUhtKja4A= golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM= golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= +golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k= diff --git a/exporters/autoexport/registry_metrics.go b/exporters/autoexport/registry_metrics.go index 9a81aa47a06..8525bc78ecd 100644 --- a/exporters/autoexport/registry_metrics.go +++ b/exporters/autoexport/registry_metrics.go @@ -20,15 +20,17 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp" + "go.opentelemetry.io/otel/exporters/prometheus" "go.opentelemetry.io/otel/sdk/metric" ) func newMetricReaderRegistry() registry[metric.Reader] { return registry[metric.Reader]{ names: map[string]func(context.Context) (metric.Reader, error){ - "": buildOTLPMetricReader, - "otlp": buildOTLPMetricReader, - "none": func(ctx context.Context) (metric.Reader, error) { return noopMetricReader, nil }, + "": buildOTLPMetricReader, + "otlp": buildOTLPMetricReader, + "prometheus": func(ctx context.Context) (metric.Reader, error) { return prometheus.New() }, + "none": func(ctx context.Context) (metric.Reader, error) { return noopMetricReader, nil }, }, } } From 8c40f0af5f2668bc00afd4a46b530fb23dc6c6cf Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Thu, 19 Oct 2023 06:05:05 -0400 Subject: [PATCH 10/26] Rename signal -> fixture in tests --- exporters/autoexport/exporter_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/exporters/autoexport/exporter_test.go b/exporters/autoexport/exporter_test.go index 06b757d1117..1a812dc200c 100644 --- a/exporters/autoexport/exporter_test.go +++ b/exporters/autoexport/exporter_test.go @@ -29,7 +29,7 @@ import ( func TestSpans(t *testing.T) { var fallbackExporter testSpanExporter - signal[trace.SpanExporter]{ + fixture[trace.SpanExporter]{ newExporter: func() (trace.SpanExporter, error) { return NewSpanExporter(context.Background()) }, @@ -59,7 +59,7 @@ func TestSpans(t *testing.T) { func TestMetrics(t *testing.T) { fallbackExporter := metric.NewManualReader() - signal[metric.Reader]{ + fixture[metric.Reader]{ newExporter: func() (metric.Reader, error) { return NewMetricReader(context.Background()) }, @@ -84,7 +84,7 @@ func TestMetrics(t *testing.T) { }.testAll(t) } -type signal[T any] struct { +type fixture[T any] struct { newExporter, newExporterWithFallback func() (T, error) fallbackExporter T assertOTLPHTTP, assertOTLPGRPC func(t *testing.T, got T) @@ -92,7 +92,7 @@ type signal[T any] struct { envVariable string } -func (s signal[T]) testAll(t *testing.T) { +func (s fixture[T]) testAll(t *testing.T) { t.Run("OTLPExporterReturnedWhenNoEnvOrFallbackExporterConfigured", func(t *testing.T) { exporter, err := s.newExporter() assert.NoError(t, err) From ffd185b2aa1cace6c9274544e38bca6ae3cbc777 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Thu, 19 Oct 2023 10:55:57 -0400 Subject: [PATCH 11/26] Rewrite + respond to PR feedback --- exporters/autoexport/exporter.go | 190 ----------------------- exporters/autoexport/metrics.go | 113 ++++++++++++++ exporters/autoexport/noop.go | 11 +- exporters/autoexport/registry_metrics.go | 98 ------------ exporters/autoexport/registry_spans.go | 88 ----------- exporters/autoexport/registry_test.go | 133 +++++----------- exporters/autoexport/signal.go | 73 +++++++++ exporters/autoexport/signal_test.go | 30 ++++ exporters/autoexport/spans.go | 105 +++++++++++++ 9 files changed, 367 insertions(+), 474 deletions(-) delete mode 100644 exporters/autoexport/exporter.go create mode 100644 exporters/autoexport/metrics.go delete mode 100644 exporters/autoexport/registry_metrics.go delete mode 100644 exporters/autoexport/registry_spans.go create mode 100644 exporters/autoexport/signal.go create mode 100644 exporters/autoexport/signal_test.go create mode 100644 exporters/autoexport/spans.go diff --git a/exporters/autoexport/exporter.go b/exporters/autoexport/exporter.go deleted file mode 100644 index f19e920ff0c..00000000000 --- a/exporters/autoexport/exporter.go +++ /dev/null @@ -1,190 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" - -import ( - "context" - "os" - - "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/trace" -) - -const ( - otelTracesExportersEnvKey = "OTEL_TRACES_EXPORTER" - otelMetricsExportersEnvKey = "OTEL_METRICS_EXPORTER" -) - -type config struct { - fallbackSpanExporter trace.SpanExporter - fallbackMetricReader metric.Reader -} - -func newConfig(ctx context.Context, opts ...Option) (config, error) { - cfg := config{} - for _, opt := range opts { - cfg = opt.apply(cfg) - } - - // if no fallback span exporter is configured, use otlp exporter - if cfg.fallbackSpanExporter == nil { - exp, err := spanExporter(ctx, "otlp") - if err != nil { - return cfg, err - } - cfg.fallbackSpanExporter = exp - } - - // if no fallback metric reader is configured, use otlp exporter - if cfg.fallbackMetricReader == nil { - r, err := metricReader(ctx, "otlp") - if err != nil { - return cfg, err - } - cfg.fallbackMetricReader = r - } - - return cfg, nil -} - -// Option applies an autoexport configuration option. -type Option interface { - apply(config) config -} - -type optionFunc func(config) config - -func (fn optionFunc) apply(cfg config) config { - return fn(cfg) -} - -// WithFallbackSpanExporter sets the fallback exporter to use when no exporter -// is configured through the OTEL_TRACES_EXPORTER environment variable. -func WithFallbackSpanExporter(exporter trace.SpanExporter) Option { - return optionFunc(func(cfg config) config { - cfg.fallbackSpanExporter = exporter - return cfg - }) -} - -// NewSpanExporter returns a configured [go.opentelemetry.io/otel/sdk/trace.SpanExporter] -// defined using the environment variables described below. -// -// OTEL_TRACES_EXPORTER defines the traces exporter; supported values: -// - "none" - "no operation" exporter -// - "otlp" (default) - OTLP exporter; see [go.opentelemetry.io/otel/exporters/otlp/otlptrace] -// -// OTEL_EXPORTER_OTLP_PROTOCOL defines OTLP exporter's transport protocol; -// supported values: -// - "grpc" - protobuf-encoded data using gRPC wire format over HTTP/2 connection; -// see: [go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc] -// - "http/protobuf" (default) - protobuf-encoded data over HTTP connection; -// see: [go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp] -// -// An error is returned if an environment value is set to an unhandled value. -// -// Use [RegisterSpanExporter] to handle more values of OTEL_TRACES_EXPORTER. -// -// Use [WithFallbackSpanExporter] option to change the returned exporter -// when OTEL_TRACES_EXPORTER is unset or empty. -// -// Use [IsNoneSpanExporter] to check if the retured exporter is a "no operation" exporter. -func NewSpanExporter(ctx context.Context, opts ...Option) (trace.SpanExporter, error) { - // prefer exporter configured via environment variables over exporter - // passed in via exporter parameter - envExporter, err := makeSpanExporterFromEnv(ctx) - if err != nil { - return nil, err - } - if envExporter != nil { - return envExporter, nil - } - config, err := newConfig(ctx, opts...) - if err != nil { - return nil, err - } - return config.fallbackSpanExporter, nil -} - -// NewMetricReader returns a configured [go.opentelemetry.io/otel/sdk/metric.Reader] -// defined using the environment variables described below. -// -// OTEL_METRICS_EXPORTER defines the metrics exporter; supported values: -// - "none" - "no operation" exporter -// - "otlp" (default) - OTLP exporter; see [go.opentelemetry.io/otel/exporters/otlp/otlpmetric] -// - "prometheus" - Prometheus exporter; see [go.opentelemetry.io/otel/exporters/prometheus] -// -// OTEL_EXPORTER_OTLP_PROTOCOL defines OTLP exporter's transport protocol; -// supported values: -// - "grpc" - protobuf-encoded data using gRPC wire format over HTTP/2 connection; -// see: [go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc] -// - "http/protobuf" (default) - protobuf-encoded data over HTTP connection; -// see: [go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp] -// -// An error is returned if an environment value is set to an unhandled value. -// -// Use [RegisterMetricReader] to handle more values of OTEL_METRICS_EXPORTER. -// -// Use [WithFallbackMetricReader] option to change the returned exporter -// when OTEL_TRACES_EXPORTER is unset or empty. -// -// Use [IsNoneSpanExporter] to check if the retured exporter is a "no operation" exporter. -func NewMetricReader(ctx context.Context, opts ...Option) (metric.Reader, error) { - // prefer exporter configured via environment variables over exporter - // passed in via exporter parameter - envReader, err := makeMetricReaderFromEnv(ctx) - if err != nil { - return nil, err - } - if envReader != nil { - return envReader, nil - } - config, err := newConfig(ctx, opts...) - if err != nil { - return nil, err - } - return config.fallbackMetricReader, nil -} - -// WithFallbackMetricReader sets the fallback exporter to use when no exporter -// is configured through the OTEL_METRICS_EXPORTER environment variable. -func WithFallbackMetricReader(reader metric.Reader) Option { - return optionFunc(func(cfg config) config { - cfg.fallbackMetricReader = reader - return cfg - }) -} - -// makeSpanExporterFromEnv returns a configured SpanExporter defined by the OTEL_TRACES_EXPORTER -// environment variable. -// nil is returned if no exporter is defined for the environment variable. -func makeSpanExporterFromEnv(ctx context.Context) (trace.SpanExporter, error) { - expType := os.Getenv(otelTracesExportersEnvKey) - if expType == "" { - return nil, nil - } - return spanExporter(ctx, expType) -} - -// makeMetricReaderFromEnv returns a configured metric.Reader defined by the OTEL_METRICS_EXPORTER -// environment variable. -// nil is returned if no exporter is defined for the environment variable. -func makeMetricReaderFromEnv(ctx context.Context) (metric.Reader, error) { - expType := os.Getenv(otelMetricsExportersEnvKey) - if expType == "" { - return nil, nil - } - return metricReader(ctx, expType) -} diff --git a/exporters/autoexport/metrics.go b/exporters/autoexport/metrics.go new file mode 100644 index 00000000000..6e5b355254b --- /dev/null +++ b/exporters/autoexport/metrics.go @@ -0,0 +1,113 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" + +import ( + "context" + "os" + + "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc" + "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp" + "go.opentelemetry.io/otel/exporters/prometheus" + "go.opentelemetry.io/otel/sdk/metric" +) + +// MetricOption applies an autoexport configuration option. +type MetricOption = option[metric.Reader] + +// WithFallbackMetricReader sets the fallback exporter to use when no exporter +// is configured through the OTEL_METRICS_EXPORTER environment variable. +func WithFallbackMetricReader(exporter metric.Reader) MetricOption { + return withFallback[metric.Reader](exporter) +} + +// NewMetricReader returns a configured [go.opentelemetry.io/otel/sdk/metric.Reader] +// defined using the environment variables described below. +// +// OTEL_METRICS_EXPORTER defines the metrics exporter; supported values: +// - "none" - "no operation" exporter +// - "otlp" (default) - OTLP exporter; see [go.opentelemetry.io/otel/exporters/otlp/otlpmetric] +// - "prometheus" - Prometheus exporter; see [go.opentelemetry.io/otel/exporters/prometheus] +// +// OTEL_EXPORTER_OTLP_PROTOCOL defines OTLP exporter's transport protocol; +// supported values: +// - "grpc" - protobuf-encoded data using gRPC wire format over HTTP/2 connection; +// see: [go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc] +// - "http/protobuf" (default) - protobuf-encoded data over HTTP connection; +// see: [go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp] +// +// An error is returned if an environment value is set to an unhandled value. +// +// Use [RegisterMetricReader] to handle more values of OTEL_METRICS_EXPORTER. +// +// Use [WithFallbackMetricReader] option to change the returned exporter +// when OTEL_TRACES_EXPORTER is unset or empty. +// +// Use [IsNoneSpanExporter] to check if the retured exporter is a "no operation" exporter. +func NewMetricReader(ctx context.Context, opts ...MetricOption) (metric.Reader, error) { + return metricsSignal.create(ctx, opts...) +} + +// RegisterMetricReader sets the MetricReader factory to be used when the +// OTEL_METRICS_EXPORTERS environment variable contains the exporter name. This +// will panic if name has already been registered. +func RegisterMetricReader(name string, factory func(context.Context) (metric.Reader, error)) { + if err := metricsSignal.registry.store(name, factory); err != nil { + // registry.store will return errDuplicateRegistration if name is already + // registered. Panic here so the user is made aware of the duplicate + // registration, which could be done by malicious code trying to + // intercept cross-cutting concerns. + // + // Panic for all other errors as well. At this point there should not + // be any other errors returned from the store operation. If there + // are, alert the developer that adding them as soon as possible that + // they need to be handled here. + panic(err) + } +} + +var metricsSignal = newSignal[metric.Reader]("OTEL_METRICS_EXPORTER") + +func init() { + RegisterMetricReader("otlp", func(ctx context.Context) (metric.Reader, error) { + proto := os.Getenv(otelExporterOTLPProtoEnvKey) + if proto == "" { + proto = "http/protobuf" + } + + switch proto { + case "grpc": + r, err := otlpmetricgrpc.New(ctx) + if err != nil { + return nil, err + } + return metric.NewPeriodicReader(r), nil + case "http/protobuf": + r, err := otlpmetrichttp.New(ctx) + if err != nil { + return nil, err + } + return metric.NewPeriodicReader(r), nil + default: + return nil, errInvalidOTLPProtocol + } + }) + RegisterMetricReader("prometheus", func(ctx context.Context) (metric.Reader, error) { + return prometheus.New() + }) + RegisterMetricReader("none", func(ctx context.Context) (metric.Reader, error) { + return newNoopMetricReader(), nil + }) +} diff --git a/exporters/autoexport/noop.go b/exporters/autoexport/noop.go index 38473613800..a0736523ba9 100644 --- a/exporters/autoexport/noop.go +++ b/exporters/autoexport/noop.go @@ -43,10 +43,17 @@ func IsNoneSpanExporter(e trace.SpanExporter) bool { return ok } -var noopMetricReader = metric.NewManualReader() +type noopMetricReader struct { + *metric.ManualReader +} + +func newNoopMetricReader() noopMetricReader { + return noopMetricReader{metric.NewManualReader()} +} // IsNoneMetricReader returns true for the exporter returned by [NewMetricReader] // when OTEL_METRICS_EXPORTER environment variable is set to "none". func IsNoneMetricReader(e metric.Reader) bool { - return e == noopMetricReader + _, ok := e.(noopMetricReader) + return ok } diff --git a/exporters/autoexport/registry_metrics.go b/exporters/autoexport/registry_metrics.go deleted file mode 100644 index 8525bc78ecd..00000000000 --- a/exporters/autoexport/registry_metrics.go +++ /dev/null @@ -1,98 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" - -import ( - "context" - "os" - - "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc" - "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp" - "go.opentelemetry.io/otel/exporters/prometheus" - "go.opentelemetry.io/otel/sdk/metric" -) - -func newMetricReaderRegistry() registry[metric.Reader] { - return registry[metric.Reader]{ - names: map[string]func(context.Context) (metric.Reader, error){ - "": buildOTLPMetricReader, - "otlp": buildOTLPMetricReader, - "prometheus": func(ctx context.Context) (metric.Reader, error) { return prometheus.New() }, - "none": func(ctx context.Context) (metric.Reader, error) { return noopMetricReader, nil }, - }, - } -} - -// metricReaderRegistry is the package level registry of exporter registrations -// and their mapping to a MetricReader factory func(context.Context) (metric.MetricReader, error). -var metricReaderRegistry = newMetricReaderRegistry() - -// RegisterMetricReader sets the MetricReader factory to be used when the -// OTEL_METRICS_EXPORTERS environment variable contains the exporter name. This -// will panic if name has already been registered. -func RegisterMetricReader(name string, factory func(context.Context) (metric.Reader, error)) { - if err := metricReaderRegistry.store(name, factory); err != nil { - // registry.store will return errDuplicateRegistration if name is already - // registered. Panic here so the user is made aware of the duplicate - // registration, which could be done by malicious code trying to - // intercept cross-cutting concerns. - // - // Panic for all other errors as well. At this point there should not - // be any other errors returned from the store operation. If there - // are, alert the developer that adding them as soon as possible that - // they need to be handled here. - panic(err) - } -} - -// metricReader returns a metric reader using the passed in name -// from the list of registered metric.Readers. Each name must match an -// already registered metric.Reader. A default OTLP exporter is registered -// under both an empty string "" and "otlp". -// An error is returned for any unknown exporters. -func metricReader(ctx context.Context, name string) (metric.Reader, error) { - exp, err := metricReaderRegistry.load(ctx, name) - if err != nil { - return nil, err - } - return exp, nil -} - -// buildOTLPMetricReader creates an OTLP metric reader using the environment variable -// OTEL_EXPORTER_OTLP_PROTOCOL to determine the exporter protocol. -// Defaults to http/protobuf protocol. -func buildOTLPMetricReader(ctx context.Context) (metric.Reader, error) { - proto := os.Getenv(otelExporterOTLPProtoEnvKey) - if proto == "" { - proto = "http/protobuf" - } - - switch proto { - case "grpc": - r, err := otlpmetricgrpc.New(ctx) - if err != nil { - return nil, err - } - return metric.NewPeriodicReader(r), nil - case "http/protobuf": - r, err := otlpmetrichttp.New(ctx) - if err != nil { - return nil, err - } - return metric.NewPeriodicReader(r), nil - default: - return nil, errInvalidOTLPProtocol - } -} diff --git a/exporters/autoexport/registry_spans.go b/exporters/autoexport/registry_spans.go deleted file mode 100644 index 5f82ae6f7e8..00000000000 --- a/exporters/autoexport/registry_spans.go +++ /dev/null @@ -1,88 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" - -import ( - "context" - "os" - - "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" - "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" - "go.opentelemetry.io/otel/sdk/trace" -) - -func newSpanExporterRegistry() registry[trace.SpanExporter] { - return registry[trace.SpanExporter]{ - names: map[string]func(context.Context) (trace.SpanExporter, error){ - "": buildOTLPSpanExporter, - "otlp": buildOTLPSpanExporter, - "none": func(ctx context.Context) (trace.SpanExporter, error) { return noop{}, nil }, - }, - } -} - -// spanExporterRegistry is the package level registry of exporter registrations -// and their mapping to a SpanExporter factory func(context.Context) (trace.SpanExporter, error). -var spanExporterRegistry = newSpanExporterRegistry() - -// RegisterSpanExporter sets the SpanExporter factory to be used when the -// OTEL_TRACES_EXPORTERS environment variable contains the exporter name. This -// will panic if name has already been registered. -func RegisterSpanExporter(name string, factory func(context.Context) (trace.SpanExporter, error)) { - if err := spanExporterRegistry.store(name, factory); err != nil { - // registry.store will return errDuplicateRegistration if name is already - // registered. Panic here so the user is made aware of the duplicate - // registration, which could be done by malicious code trying to - // intercept cross-cutting concerns. - // - // Panic for all other errors as well. At this point there should not - // be any other errors returned from the store operation. If there - // are, alert the developer that adding them as soon as possible that - // they need to be handled here. - panic(err) - } -} - -// spanExporter returns a span exporter using the passed in name -// from the list of registered SpanExporters. Each name must match an -// already registered SpanExporter. A default OTLP exporter is registered -// under both an empty string "" and "otlp". -// An error is returned for any unknown exporters. -func spanExporter(ctx context.Context, name string) (trace.SpanExporter, error) { - exp, err := spanExporterRegistry.load(ctx, name) - if err != nil { - return nil, err - } - return exp, nil -} - -// buildOTLPSpanExporter creates an OTLP span exporter using the environment variable -// OTEL_EXPORTER_OTLP_PROTOCOL to determine the exporter protocol. -// Defaults to http/protobuf protocol. -func buildOTLPSpanExporter(ctx context.Context) (trace.SpanExporter, error) { - proto := os.Getenv(otelExporterOTLPProtoEnvKey) - if proto == "" { - proto = "http/protobuf" - } - - switch proto { - case "grpc": - return otlptracegrpc.New(ctx) - case "http/protobuf": - return otlptracehttp.New(ctx) - default: - return nil, errInvalidOTLPProtocol - } -} diff --git a/exporters/autoexport/registry_test.go b/exporters/autoexport/registry_test.go index 6dc8c084319..e241b0dfbfc 100644 --- a/exporters/autoexport/registry_test.go +++ b/exporters/autoexport/registry_test.go @@ -22,11 +22,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - - "go.opentelemetry.io/otel/exporters/stdout/stdoutmetric" - "go.opentelemetry.io/otel/exporters/stdout/stdouttrace" - "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/trace" ) type testType struct{ string } @@ -37,29 +32,25 @@ func factory(val string) func(ctx context.Context) (*testType, error) { func newTestRegistry() registry[*testType] { return registry[*testType]{ - names: map[string]func(context.Context) (*testType, error){ - "": factory(""), - "otlp": factory("otlp"), - "none": factory("none"), - }, + names: make(map[string]func(context.Context) (*testType, error)), } } -var stdoutMetricFactory = func(ctx context.Context) (metric.Reader, error) { - exp, err := stdoutmetric.New() - if err != nil { - return nil, err - } - return metric.NewPeriodicReader(exp), nil -} +// var stdoutMetricFactory = func(ctx context.Context) (metric.Reader, error) { +// exp, err := stdoutmetric.New() +// if err != nil { +// return nil, err +// } +// return metric.NewPeriodicReader(exp), nil +// } -var stdoutSpanFactory = func(ctx context.Context) (trace.SpanExporter, error) { - exp, err := stdouttrace.New() - if err != nil { - return nil, err - } - return exp, nil -} +// var stdoutSpanFactory = func(ctx context.Context) (trace.SpanExporter, error) { +// exp, err := stdouttrace.New() +// if err != nil { +// return nil, err +// } +// return exp, nil +// } func TestCanStoreExporterFactory(t *testing.T) { r := newTestRegistry() @@ -107,92 +98,42 @@ func TestRegistryIsConcurrentSafe(t *testing.T) { wg.Wait() } -type funcs[T any] struct { - makeExporter func(context.Context, string) (T, error) - assertOTLPHTTP func(*testing.T, T) - registerFactory func(string, func(context.Context) (T, error)) - stdoutFactory func(ctx context.Context) (T, error) - registry *registry[T] -} - -var ( - spanFuncs = funcs[trace.SpanExporter]{ - makeExporter: spanExporter, - assertOTLPHTTP: assertOTLPHTTPSpanExporter, - registerFactory: RegisterSpanExporter, - stdoutFactory: stdoutSpanFactory, - registry: &spanExporterRegistry, - } - - metricFuncs = funcs[metric.Reader]{ - makeExporter: metricReader, - assertOTLPHTTP: assertOTLPHTTPMetricReader, - registerFactory: RegisterMetricReader, - stdoutFactory: stdoutMetricFactory, - registry: &metricReaderRegistry, - } -) +func TestSubsequentCallsToGetExporterReturnsNewInstances(t *testing.T) { + r := newTestRegistry() -func (f funcs[T]) testSubsequentCallsToGetExporterReturnsNewInstances(t *testing.T) { - const exporterType = "otlp" + const key = "key" + r.store(key, factory(key)) - exp1, err := f.makeExporter(context.Background(), exporterType) + exp1, err := r.load(context.Background(), key) assert.NoError(t, err) - f.assertOTLPHTTP(t, exp1) - exp2, err := spanExporter(context.Background(), exporterType) + exp2, err := r.load(context.Background(), key) assert.NoError(t, err) - assertOTLPHTTPSpanExporter(t, exp2) assert.NotSame(t, exp1, exp2) } -func TestSubsequentCallsToGetExporterReturnsNewInstances(t *testing.T) { - t.Run("spans", spanFuncs.testSubsequentCallsToGetExporterReturnsNewInstances) - t.Run("metrics", metricFuncs.testSubsequentCallsToGetExporterReturnsNewInstances) -} - -func (f funcs[T]) testDefaultOTLPExporterFactoriesAreAutomaticallyRegistered(t *testing.T) { - exp1, err := f.makeExporter(context.Background(), "") - assert.Nil(t, err) - f.assertOTLPHTTP(t, exp1) +// func (f funcs[T]) testDefaultOTLPExporterFactoriesAreAutomaticallyRegistered(t *testing.T) { +// exp1, err := f.makeExporter(context.Background(), "") +// assert.Nil(t, err) +// f.assertOTLPHTTP(t, exp1) - exp2, err := f.makeExporter(context.Background(), "otlp") - assert.Nil(t, err) - f.assertOTLPHTTP(t, exp2) -} +// exp2, err := f.makeExporter(context.Background(), "otlp") +// assert.Nil(t, err) +// f.assertOTLPHTTP(t, exp2) +// } -func TestDefaultOTLPExporterFactoriesAreAutomaticallyRegistered(t *testing.T) { - t.Run("spans", spanFuncs.testDefaultOTLPExporterFactoriesAreAutomaticallyRegistered) - t.Run("metrics", metricFuncs.testDefaultOTLPExporterFactoriesAreAutomaticallyRegistered) -} +// func TestDefaultOTLPExporterFactoriesAreAutomaticallyRegistered(t *testing.T) { +// t.Run("spans", spanFuncs.testDefaultOTLPExporterFactoriesAreAutomaticallyRegistered) +// t.Run("metrics", metricFuncs.testDefaultOTLPExporterFactoriesAreAutomaticallyRegistered) +// } -func (f funcs[T]) testEnvRegistryCanRegisterExporterFactory(t *testing.T) { - const exporterName = "custom" - f.registerFactory(exporterName, f.stdoutFactory) - t.Cleanup(func() { f.registry.drop(exporterName) }) - - _, err := f.registry.load(context.Background(), exporterName) - assert.Nil(t, err, "missing exporter in envRegistry") -} - -func TestEnvRegistryCanRegisterExporterFactory(t *testing.T) { - t.Run("spans", spanFuncs.testEnvRegistryCanRegisterExporterFactory) - t.Run("metrics", metricFuncs.testEnvRegistryCanRegisterExporterFactory) -} +func TestRegistryErrorsOnDuplicateRegisterCalls(t *testing.T) { + r := newTestRegistry() -func (f funcs[T]) testEnvRegistryPanicsOnDuplicateRegisterCalls(t *testing.T) { const exporterName = "custom" - f.registerFactory(exporterName, f.stdoutFactory) - t.Cleanup(func() { f.registry.drop(exporterName) }) + assert.NoError(t, r.store(exporterName, factory(exporterName))) errString := fmt.Sprintf("%s: %q", errDuplicateRegistration, exporterName) - assert.PanicsWithError(t, errString, func() { - f.registerFactory(exporterName, f.stdoutFactory) - }) -} - -func TestEnvRegistryPanicsOnDuplicateRegisterCalls(t *testing.T) { - t.Run("spans", spanFuncs.testEnvRegistryPanicsOnDuplicateRegisterCalls) - t.Run("metrics", metricFuncs.testEnvRegistryPanicsOnDuplicateRegisterCalls) + assert.ErrorContains(t, r.store(exporterName, factory(exporterName)), errString) } diff --git a/exporters/autoexport/signal.go b/exporters/autoexport/signal.go new file mode 100644 index 00000000000..3528be990cb --- /dev/null +++ b/exporters/autoexport/signal.go @@ -0,0 +1,73 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" + +import ( + "context" + "os" +) + +type signal[T any] struct { + envKey string + registry *registry[T] +} + +func newSignal[T any](envKey string) signal[T] { + return signal[T]{ + envKey: envKey, + registry: ®istry[T]{ + names: make(map[string]func(context.Context) (T, error)), + }, + } +} + +func (s signal[T]) create(ctx context.Context, opts ...option[T]) (T, error) { + var cfg config[T] + for _, opt := range opts { + opt.apply(&cfg) + } + + expType := os.Getenv(s.envKey) + if expType == "" { + if cfg.hasFallback { + return cfg.fallback, nil + } + expType = "otlp" + } + + return s.registry.load(ctx, expType) +} + +type config[T any] struct { + hasFallback bool + fallback T +} + +type option[T any] interface { + apply(cfg *config[T]) +} + +type optionFunc[T any] func(cfg *config[T]) + +func (fn optionFunc[T]) apply(cfg *config[T]) { + fn(cfg) +} + +func withFallback[T any](fallback T) option[T] { + return optionFunc[T](func(cfg *config[T]) { + cfg.hasFallback = true + cfg.fallback = fallback + }) +} diff --git a/exporters/autoexport/signal_test.go b/exporters/autoexport/signal_test.go new file mode 100644 index 00000000000..f600e7296b2 --- /dev/null +++ b/exporters/autoexport/signal_test.go @@ -0,0 +1,30 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestOTLPExporterReturnedWhenNoEnvOrFallbackExporterConfigured(t *testing.T) { + ts := newSignal[*testType]("TEST_TYPE_KEY") + ts.registry.store("otlp", factory("test-otlp-exporter")) + exp, err := ts.create(context.Background()) + assert.NoError(t, err) + assert.Equal(t, exp.string, "test-otlp-exporter") +} diff --git a/exporters/autoexport/spans.go b/exporters/autoexport/spans.go new file mode 100644 index 00000000000..34e48b3b020 --- /dev/null +++ b/exporters/autoexport/spans.go @@ -0,0 +1,105 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" + +import ( + "context" + "os" + + "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" + "go.opentelemetry.io/otel/sdk/trace" +) + +// SpanOption applies an autoexport configuration option. +type SpanOption = option[trace.SpanExporter] + +// Option applies an autoexport configuration option. +// +// Deprecated: Use SpanOption +type Option = SpanOption + +// WithFallbackSpanExporter sets the fallback exporter to use when no exporter +// is configured through the OTEL_TRACES_EXPORTER environment variable. +func WithFallbackSpanExporter(exporter trace.SpanExporter) SpanOption { + return withFallback[trace.SpanExporter](exporter) +} + +// NewSpanExporter returns a configured [go.opentelemetry.io/otel/sdk/trace.SpanExporter] +// defined using the environment variables described below. +// +// OTEL_TRACES_EXPORTER defines the traces exporter; supported values: +// - "none" - "no operation" exporter +// - "otlp" (default) - OTLP exporter; see [go.opentelemetry.io/otel/exporters/otlp/otlptrace] +// +// OTEL_EXPORTER_OTLP_PROTOCOL defines OTLP exporter's transport protocol; +// supported values: +// - "grpc" - protobuf-encoded data using gRPC wire format over HTTP/2 connection; +// see: [go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc] +// - "http/protobuf" (default) - protobuf-encoded data over HTTP connection; +// see: [go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp] +// +// An error is returned if an environment value is set to an unhandled value. +// +// Use [RegisterSpanExporter] to handle more values of OTEL_TRACES_EXPORTER. +// +// Use [WithFallbackSpanExporter] option to change the returned exporter +// when OTEL_TRACES_EXPORTER is unset or empty. +// +// Use [IsNoneSpanExporter] to check if the retured exporter is a "no operation" exporter. +func NewSpanExporter(ctx context.Context, opts ...Option) (trace.SpanExporter, error) { + return tracesSignal.create(ctx, opts...) +} + +// RegisterSpanExporter sets the SpanExporter factory to be used when the +// OTEL_TRACES_EXPORTERS environment variable contains the exporter name. This +// will panic if name has already been registered. +func RegisterSpanExporter(name string, factory func(context.Context) (trace.SpanExporter, error)) { + if err := tracesSignal.registry.store(name, factory); err != nil { + // registry.store will return errDuplicateRegistration if name is already + // registered. Panic here so the user is made aware of the duplicate + // registration, which could be done by malicious code trying to + // intercept cross-cutting concerns. + // + // Panic for all other errors as well. At this point there should not + // be any other errors returned from the store operation. If there + // are, alert the developer that adding them as soon as possible that + // they need to be handled here. + panic(err) + } +} + +var tracesSignal = newSignal[trace.SpanExporter]("OTEL_TRACES_EXPORTER") + +func init() { + RegisterSpanExporter("otlp", func(ctx context.Context) (trace.SpanExporter, error) { + proto := os.Getenv(otelExporterOTLPProtoEnvKey) + if proto == "" { + proto = "http/protobuf" + } + + switch proto { + case "grpc": + return otlptracegrpc.New(ctx) + case "http/protobuf": + return otlptracehttp.New(ctx) + default: + return nil, errInvalidOTLPProtocol + } + }) + RegisterSpanExporter("none", func(ctx context.Context) (trace.SpanExporter, error) { + return noop{}, nil + }) +} From 0f350db3644d259544e4c0170291a4ece98a18a0 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Thu, 19 Oct 2023 11:33:28 -0400 Subject: [PATCH 12/26] Reduce unnecessarily fancy generics in tests --- exporters/autoexport/exporter_test.go | 52 ++-------------------- exporters/autoexport/metrics_test.go | 64 +++++++++++++++++++++++++++ exporters/autoexport/signal_test.go | 22 +++++++++ exporters/autoexport/spans_test.go | 15 +++++++ 4 files changed, 105 insertions(+), 48 deletions(-) create mode 100644 exporters/autoexport/metrics_test.go create mode 100644 exporters/autoexport/spans_test.go diff --git a/exporters/autoexport/exporter_test.go b/exporters/autoexport/exporter_test.go index 1a812dc200c..7b1dfb28308 100644 --- a/exporters/autoexport/exporter_test.go +++ b/exporters/autoexport/exporter_test.go @@ -27,16 +27,10 @@ import ( ) func TestSpans(t *testing.T) { - var fallbackExporter testSpanExporter - fixture[trace.SpanExporter]{ newExporter: func() (trace.SpanExporter, error) { return NewSpanExporter(context.Background()) }, - fallbackExporter: &fallbackExporter, - newExporterWithFallback: func() (trace.SpanExporter, error) { - return NewSpanExporter(context.Background(), WithFallbackSpanExporter(&fallbackExporter)) - }, assertOTLPHTTP: assertOTLPHTTPSpanExporter, assertOTLPGRPC: func(t *testing.T, got trace.SpanExporter) { t.Helper() @@ -57,16 +51,10 @@ func TestSpans(t *testing.T) { } func TestMetrics(t *testing.T) { - fallbackExporter := metric.NewManualReader() - fixture[metric.Reader]{ newExporter: func() (metric.Reader, error) { return NewMetricReader(context.Background()) }, - fallbackExporter: fallbackExporter, - newExporterWithFallback: func() (metric.Reader, error) { - return NewMetricReader(context.Background(), WithFallbackMetricReader(fallbackExporter)) - }, assertOTLPHTTP: assertOTLPHTTPMetricReader, assertOTLPGRPC: func(t *testing.T, got metric.Reader) { t.Helper() @@ -85,35 +73,13 @@ func TestMetrics(t *testing.T) { } type fixture[T any] struct { - newExporter, newExporterWithFallback func() (T, error) - fallbackExporter T - assertOTLPHTTP, assertOTLPGRPC func(t *testing.T, got T) - isNoneExporter func(exporter T) bool - envVariable string + newExporter func() (T, error) + assertOTLPHTTP, assertOTLPGRPC func(t *testing.T, got T) + isNoneExporter func(exporter T) bool + envVariable string } func (s fixture[T]) testAll(t *testing.T) { - t.Run("OTLPExporterReturnedWhenNoEnvOrFallbackExporterConfigured", func(t *testing.T) { - exporter, err := s.newExporter() - assert.NoError(t, err) - s.assertOTLPHTTP(t, exporter) - }) - - t.Run("FallbackExporterReturnedWhenNoEnvExporterConfigured", func(t *testing.T) { - exporter, err := s.newExporterWithFallback() - assert.NoError(t, err) - assert.Equal(t, s.fallbackExporter, exporter) - assert.False(t, s.isNoneExporter(exporter)) - }) - - t.Run("EnvExporterIsPreferredOverFallbackExporter", func(t *testing.T) { - t.Setenv(s.envVariable, "otlp") - - exporter, err := s.newExporterWithFallback() - assert.NoError(t, err) - s.assertOTLPHTTP(t, exporter) - }) - t.Run("EnvExporterOTLPOverHTTP", func(t *testing.T) { t.Setenv(s.envVariable, "otlp") t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf") @@ -183,13 +149,3 @@ func assertOTLPHTTPSpanExporter(t *testing.T, got trace.SpanExporter) { assert.False(t, IsNoneSpanExporter(got)) } - -type testSpanExporter struct{} - -func (e *testSpanExporter) ExportSpans(ctx context.Context, ss []trace.ReadOnlySpan) error { - return nil -} - -func (e *testSpanExporter) Shutdown(ctx context.Context) error { - return nil -} diff --git a/exporters/autoexport/metrics_test.go b/exporters/autoexport/metrics_test.go new file mode 100644 index 00000000000..31975ea7a51 --- /dev/null +++ b/exporters/autoexport/metrics_test.go @@ -0,0 +1,64 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" + +import ( + "context" + "reflect" + "testing" + + "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel/sdk/metric" +) + +func TestMetricOTLPOverHTTP(t *testing.T) { + t.Setenv("OTEL_METRICS_EXPORTER", "otlp") + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf") + + got, err := NewMetricReader(context.Background()) + assert.NoError(t, err) + + if !assert.IsType(t, metric.NewPeriodicReader(nil), got) { + return + } + + // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. + clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("exporter").Elem().Type().String() + assert.Equal(t, "*otlpmetrichttp.Exporter", clientType) +} + +func TestMetricOTLPOverGRPC(t *testing.T) { + t.Setenv("OTEL_METRICS_EXPORTER", "otlp") + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") + + got, err := NewMetricReader(context.Background()) + assert.NoError(t, err) + + if !assert.IsType(t, metric.NewPeriodicReader(nil), got) { + return + } + + // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. + clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("exporter").Elem().Type().String() + assert.Equal(t, "*otlpmetricgrpc.Exporter", clientType) +} + +func TestMetricOTLPOverInvalidProtocol(t *testing.T) { + t.Setenv("OTEL_METRICS_EXPORTER", "otlp") + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") + + _, err := NewMetricReader(context.Background()) + assert.Error(t, err) +} diff --git a/exporters/autoexport/signal_test.go b/exporters/autoexport/signal_test.go index f600e7296b2..9d314fdbd4f 100644 --- a/exporters/autoexport/signal_test.go +++ b/exporters/autoexport/signal_test.go @@ -28,3 +28,25 @@ func TestOTLPExporterReturnedWhenNoEnvOrFallbackExporterConfigured(t *testing.T) assert.NoError(t, err) assert.Equal(t, exp.string, "test-otlp-exporter") } + +func TestFallbackExporterReturnedWhenNoEnvExporterConfigured(t *testing.T) { + ts := newSignal[*testType]("TEST_TYPE_KEY") + fallback := testType{"test-fallback-exporter"} + exp, err := ts.create(context.Background(), withFallback(&fallback)) + assert.NoError(t, err) + assert.Same(t, &fallback, exp) +} + +func TestEnvExporterIsPreferredOverFallbackExporter(t *testing.T) { + envVariable := "TEST_TYPE_KEY" + ts := newSignal[*testType](envVariable) + + expName := "test-env-exporter-name" + t.Setenv(envVariable, expName) + fallback := testType{"test-fallback-exporter"} + ts.registry.store(expName, factory("test-env-exporter")) + + exp, err := ts.create(context.Background(), withFallback(&fallback)) + assert.NoError(t, err) + assert.Equal(t, exp.string, "test-env-exporter") +} diff --git a/exporters/autoexport/spans_test.go b/exporters/autoexport/spans_test.go new file mode 100644 index 00000000000..836d9711a5b --- /dev/null +++ b/exporters/autoexport/spans_test.go @@ -0,0 +1,15 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" From 6d04d4ceb42cc6357302f25877dd9c8b9e4cb14f Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Thu, 19 Oct 2023 13:09:28 -0400 Subject: [PATCH 13/26] Remove unnecessary generics in tests --- exporters/autoexport/exporter_test.go | 151 -------------------------- exporters/autoexport/metrics_test.go | 47 +++++--- exporters/autoexport/noop.go | 12 +- exporters/autoexport/spans.go | 2 +- exporters/autoexport/spans_test.go | 52 +++++++++ 5 files changed, 88 insertions(+), 176 deletions(-) delete mode 100644 exporters/autoexport/exporter_test.go diff --git a/exporters/autoexport/exporter_test.go b/exporters/autoexport/exporter_test.go deleted file mode 100644 index 7b1dfb28308..00000000000 --- a/exporters/autoexport/exporter_test.go +++ /dev/null @@ -1,151 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package autoexport - -import ( - "context" - "reflect" - "testing" - - "github.com/stretchr/testify/assert" - - "go.opentelemetry.io/otel/exporters/otlp/otlptrace" - "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/trace" -) - -func TestSpans(t *testing.T) { - fixture[trace.SpanExporter]{ - newExporter: func() (trace.SpanExporter, error) { - return NewSpanExporter(context.Background()) - }, - assertOTLPHTTP: assertOTLPHTTPSpanExporter, - assertOTLPGRPC: func(t *testing.T, got trace.SpanExporter) { - t.Helper() - - if !assert.IsType(t, &otlptrace.Exporter{}, got) { - return - } - - // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. - clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("client").Elem().Type().String() - assert.Equal(t, "*otlptracegrpc.client", clientType) - - assert.False(t, IsNoneSpanExporter(got)) - }, - isNoneExporter: IsNoneSpanExporter, - envVariable: "OTEL_TRACES_EXPORTER", - }.testAll(t) -} - -func TestMetrics(t *testing.T) { - fixture[metric.Reader]{ - newExporter: func() (metric.Reader, error) { - return NewMetricReader(context.Background()) - }, - assertOTLPHTTP: assertOTLPHTTPMetricReader, - assertOTLPGRPC: func(t *testing.T, got metric.Reader) { - t.Helper() - - if !assert.IsType(t, metric.NewPeriodicReader(nil), got) { - return - } - - // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. - clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("exporter").Elem().Type().String() - assert.Equal(t, "*otlpmetricgrpc.Exporter", clientType) - }, - isNoneExporter: IsNoneMetricReader, - envVariable: "OTEL_METRICS_EXPORTER", - }.testAll(t) -} - -type fixture[T any] struct { - newExporter func() (T, error) - assertOTLPHTTP, assertOTLPGRPC func(t *testing.T, got T) - isNoneExporter func(exporter T) bool - envVariable string -} - -func (s fixture[T]) testAll(t *testing.T) { - t.Run("EnvExporterOTLPOverHTTP", func(t *testing.T) { - t.Setenv(s.envVariable, "otlp") - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf") - - exporter, err := s.newExporter() - assert.NoError(t, err) - s.assertOTLPHTTP(t, exporter) - }) - - t.Run("EnvExporterOTLPOverGRPC", func(t *testing.T) { - t.Setenv(s.envVariable, "otlp") - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") - - exporter, err := s.newExporter() - assert.NoError(t, err) - s.assertOTLPGRPC(t, exporter) - }) - - t.Run("EnvExporterOTLPOverGRPCOnlyProtocol", func(t *testing.T) { - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") - - exporter, err := s.newExporter() - assert.NoError(t, err) - s.assertOTLPGRPC(t, exporter) - }) - - t.Run("EnvExporterOTLPInvalidProtocol", func(t *testing.T) { - t.Setenv(s.envVariable, "otlp") - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "invalid") - - exporter, err := s.newExporter() - assert.Error(t, err) - assert.Nil(t, exporter) - }) - - t.Run("EnvExporterNone", func(t *testing.T) { - t.Setenv(s.envVariable, "none") - - exporter, err := s.newExporter() - assert.NoError(t, err) - assert.True(t, s.isNoneExporter(exporter)) - }) -} - -func assertOTLPHTTPMetricReader(t *testing.T, got metric.Reader) { - t.Helper() - - if !assert.IsType(t, metric.NewPeriodicReader(nil), got) { - return - } - - // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. - clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("exporter").Elem().Type().String() - assert.Equal(t, "*otlpmetrichttp.Exporter", clientType) -} - -func assertOTLPHTTPSpanExporter(t *testing.T, got trace.SpanExporter) { - t.Helper() - - if !assert.IsType(t, &otlptrace.Exporter{}, got) { - return - } - - // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. - clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("client").Elem().Type().String() - assert.Equal(t, "*otlptracehttp.client", clientType) - - assert.False(t, IsNoneSpanExporter(got)) -} diff --git a/exporters/autoexport/metrics_test.go b/exporters/autoexport/metrics_test.go index 31975ea7a51..89049a155e7 100644 --- a/exporters/autoexport/metrics_test.go +++ b/exporters/autoexport/metrics_test.go @@ -20,45 +20,56 @@ import ( "testing" "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel/exporters/prometheus" "go.opentelemetry.io/otel/sdk/metric" ) -func TestMetricOTLPOverHTTP(t *testing.T) { - t.Setenv("OTEL_METRICS_EXPORTER", "otlp") - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf") - +func TestMetricExporterNone(t *testing.T) { + t.Setenv("OTEL_METRICS_EXPORTER", "none") got, err := NewMetricReader(context.Background()) assert.NoError(t, err) + assert.True(t, IsNoneMetricReader(got)) +} - if !assert.IsType(t, metric.NewPeriodicReader(nil), got) { - return - } +func assertPeriodicReaderWithExporter(t *testing.T, got metric.Reader, exporterTypeName string) { + t.Helper() + assert.IsType(t, &metric.PeriodicReader{}, got) // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("exporter").Elem().Type().String() - assert.Equal(t, "*otlpmetrichttp.Exporter", clientType) + assert.Equal(t, exporterTypeName, clientType) } -func TestMetricOTLPOverGRPC(t *testing.T) { +func TestMetricExporterOTLPOverHTTP(t *testing.T) { t.Setenv("OTEL_METRICS_EXPORTER", "otlp") - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf") got, err := NewMetricReader(context.Background()) assert.NoError(t, err) + assertPeriodicReaderWithExporter(t, got, "*otlpmetrichttp.Exporter") +} - if !assert.IsType(t, metric.NewPeriodicReader(nil), got) { - return - } +func TestMetricExporterOTLPOverGRPC(t *testing.T) { + t.Setenv("OTEL_METRICS_EXPORTER", "otlp") + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") - // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. - clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("exporter").Elem().Type().String() - assert.Equal(t, "*otlpmetricgrpc.Exporter", clientType) + got, err := NewMetricReader(context.Background()) + assert.NoError(t, err) + assertPeriodicReaderWithExporter(t, got, "*otlpmetricgrpc.Exporter") } -func TestMetricOTLPOverInvalidProtocol(t *testing.T) { +func TestMetricExporterOTLPOverInvalidProtocol(t *testing.T) { t.Setenv("OTEL_METRICS_EXPORTER", "otlp") - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "invalid-protocol") _, err := NewMetricReader(context.Background()) assert.Error(t, err) } + +func TestMetricExporterPrometheus(t *testing.T) { + t.Setenv("OTEL_METRICS_EXPORTER", "prometheus") + + got, err := NewMetricReader(context.Background()) + assert.NoError(t, err) + assert.IsType(t, &prometheus.Exporter{}, got) +} diff --git a/exporters/autoexport/noop.go b/exporters/autoexport/noop.go index a0736523ba9..31df837aba6 100644 --- a/exporters/autoexport/noop.go +++ b/exporters/autoexport/noop.go @@ -21,25 +21,25 @@ import ( "go.opentelemetry.io/otel/sdk/trace" ) -// noop is an implementation of trace.SpanExporter that performs no operations. -type noop struct{} +// noopSpanExporter is an implementation of trace.SpanExporter that performs no operations. +type noopSpanExporter struct{} -var _ trace.SpanExporter = noop{} +var _ trace.SpanExporter = noopSpanExporter{} // ExportSpans is part of trace.SpanExporter interface. -func (e noop) ExportSpans(ctx context.Context, spans []trace.ReadOnlySpan) error { +func (e noopSpanExporter) ExportSpans(ctx context.Context, spans []trace.ReadOnlySpan) error { return nil } // Shutdown is part of trace.SpanExporter interface. -func (e noop) Shutdown(ctx context.Context) error { +func (e noopSpanExporter) Shutdown(ctx context.Context) error { return nil } // IsNoneSpanExporter returns true for the exporter returned by [NewSpanExporter] // when OTEL_TRACES_EXPORTER environment variable is set to "none". func IsNoneSpanExporter(e trace.SpanExporter) bool { - _, ok := e.(noop) + _, ok := e.(noopSpanExporter) return ok } diff --git a/exporters/autoexport/spans.go b/exporters/autoexport/spans.go index 34e48b3b020..667ae9aaf86 100644 --- a/exporters/autoexport/spans.go +++ b/exporters/autoexport/spans.go @@ -100,6 +100,6 @@ func init() { } }) RegisterSpanExporter("none", func(ctx context.Context) (trace.SpanExporter, error) { - return noop{}, nil + return noopSpanExporter{}, nil }) } diff --git a/exporters/autoexport/spans_test.go b/exporters/autoexport/spans_test.go index 836d9711a5b..1d10f9d305d 100644 --- a/exporters/autoexport/spans_test.go +++ b/exporters/autoexport/spans_test.go @@ -13,3 +13,55 @@ // limitations under the License. package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" + +import ( + "context" + "reflect" + "testing" + + "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace" + "go.opentelemetry.io/otel/sdk/trace" +) + +func TestSpanExporterNone(t *testing.T) { + t.Setenv("OTEL_TRACES_EXPORTER", "none") + got, err := NewSpanExporter(context.Background()) + assert.NoError(t, err) + assert.True(t, IsNoneSpanExporter(got)) +} + +func assertOTLPExporterWithClientTypeName(t *testing.T, got trace.SpanExporter, clientTypeName string) { + t.Helper() + assert.IsType(t, &otlptrace.Exporter{}, got) + + // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. + clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("client").Elem().Type().String() + assert.Equal(t, clientTypeName, clientType) +} + +func TestSpanExporterOTLPOverHTTP(t *testing.T) { + t.Setenv("OTEL_TRACES_EXPORTER", "otlp") + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf") + + got, err := NewSpanExporter(context.Background()) + assert.NoError(t, err) + assertOTLPExporterWithClientTypeName(t, got, "*otlptracehttp.client") +} + +func TestSpanExporterOTLPOverGRPC(t *testing.T) { + t.Setenv("OTEL_TRACES_EXPORTER", "otlp") + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") + + got, err := NewSpanExporter(context.Background()) + assert.NoError(t, err) + assertOTLPExporterWithClientTypeName(t, got, "*otlptracegrpc.client") +} + +func TestSpanExporterOTLPOverInvalidProtocol(t *testing.T) { + t.Setenv("OTEL_TRACES_EXPORTER", "otlp") + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "invalid-protocol") + + _, err := NewSpanExporter(context.Background()) + assert.Error(t, err) +} From 5cf5027d42c4a2a5314d84f8c5df73b516567a4c Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Thu, 19 Oct 2023 13:11:42 -0400 Subject: [PATCH 14/26] Remove unused deps --- exporters/autoexport/go.mod | 2 -- exporters/autoexport/go.sum | 4 ---- 2 files changed, 6 deletions(-) diff --git a/exporters/autoexport/go.mod b/exporters/autoexport/go.mod index 6bd6cfdfdca..7b0ed679c4a 100644 --- a/exporters/autoexport/go.mod +++ b/exporters/autoexport/go.mod @@ -9,8 +9,6 @@ require ( go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.19.0 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.19.0 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0 - go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v0.42.0 - go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.19.0 go.opentelemetry.io/otel/sdk v1.19.0 go.opentelemetry.io/otel/sdk/metric v1.19.0 ) diff --git a/exporters/autoexport/go.sum b/exporters/autoexport/go.sum index 022495a5fd1..9915420e288 100644 --- a/exporters/autoexport/go.sum +++ b/exporters/autoexport/go.sum @@ -53,10 +53,6 @@ go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0 h1:IeMey go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0/go.mod h1:oVdCUtjq9MK9BlS7TtucsQwUcXcymNiEDjgDD2jMtZU= go.opentelemetry.io/otel/exporters/prometheus v0.42.0 h1:jwV9iQdvp38fxXi8ZC+lNpxjK16MRcZlpDYvbuO1FiA= go.opentelemetry.io/otel/exporters/prometheus v0.42.0/go.mod h1:f3bYiqNqhoPxkvI2LrXqQVC546K7BuRDL/kKuxkujhA= -go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v0.42.0 h1:4jJuoeOo9W6hZnz+r046fyoH5kykZPRvKfUXJVfMpB0= -go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v0.42.0/go.mod h1:/MtYTE1SfC2QIcE0bDot6fIX+h+WvXjgTqgn9P0LNPE= -go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.19.0 h1:Nw7Dv4lwvGrI68+wULbcq7su9K2cebeCUrDjVrUJHxM= -go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.19.0/go.mod h1:1MsF6Y7gTqosgoZvHlzcaaM8DIMNZgJh87ykokoNH7Y= go.opentelemetry.io/otel/metric v1.19.0 h1:aTzpGtV0ar9wlV4Sna9sdJyII5jTVJEvKETPiOKwvpE= go.opentelemetry.io/otel/metric v1.19.0/go.mod h1:L5rUsV9kM1IxCj1MmSdS+JQAcVm319EUrDVLrt7jqt8= go.opentelemetry.io/otel/sdk v1.19.0 h1:6USY6zH+L8uMH8L3t1enZPR3WFEmSTADlqldyHtJi3o= From 4f0e614ebf3e6bd774fa0e0b28b6669acde336c5 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Thu, 19 Oct 2023 13:23:53 -0400 Subject: [PATCH 15/26] Cleanup --- exporters/autoexport/metrics_test.go | 3 ++- exporters/autoexport/registry_test.go | 33 +-------------------------- exporters/autoexport/signal_test.go | 2 +- exporters/autoexport/spans.go | 2 +- exporters/autoexport/spans_test.go | 3 ++- 5 files changed, 7 insertions(+), 36 deletions(-) diff --git a/exporters/autoexport/metrics_test.go b/exporters/autoexport/metrics_test.go index 89049a155e7..4e9fe57c4b2 100644 --- a/exporters/autoexport/metrics_test.go +++ b/exporters/autoexport/metrics_test.go @@ -19,9 +19,10 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" "go.opentelemetry.io/otel/exporters/prometheus" "go.opentelemetry.io/otel/sdk/metric" + + "github.com/stretchr/testify/assert" ) func TestMetricExporterNone(t *testing.T) { diff --git a/exporters/autoexport/registry_test.go b/exporters/autoexport/registry_test.go index e241b0dfbfc..6b3f38dbb35 100644 --- a/exporters/autoexport/registry_test.go +++ b/exporters/autoexport/registry_test.go @@ -36,22 +36,6 @@ func newTestRegistry() registry[*testType] { } } -// var stdoutMetricFactory = func(ctx context.Context) (metric.Reader, error) { -// exp, err := stdoutmetric.New() -// if err != nil { -// return nil, err -// } -// return metric.NewPeriodicReader(exp), nil -// } - -// var stdoutSpanFactory = func(ctx context.Context) (trace.SpanExporter, error) { -// exp, err := stdouttrace.New() -// if err != nil { -// return nil, err -// } -// return exp, nil -// } - func TestCanStoreExporterFactory(t *testing.T) { r := newTestRegistry() assert.NotPanics(t, func() { @@ -102,7 +86,7 @@ func TestSubsequentCallsToGetExporterReturnsNewInstances(t *testing.T) { r := newTestRegistry() const key = "key" - r.store(key, factory(key)) + assert.NoError(t, r.store(key, factory(key))) exp1, err := r.load(context.Background(), key) assert.NoError(t, err) @@ -113,21 +97,6 @@ func TestSubsequentCallsToGetExporterReturnsNewInstances(t *testing.T) { assert.NotSame(t, exp1, exp2) } -// func (f funcs[T]) testDefaultOTLPExporterFactoriesAreAutomaticallyRegistered(t *testing.T) { -// exp1, err := f.makeExporter(context.Background(), "") -// assert.Nil(t, err) -// f.assertOTLPHTTP(t, exp1) - -// exp2, err := f.makeExporter(context.Background(), "otlp") -// assert.Nil(t, err) -// f.assertOTLPHTTP(t, exp2) -// } - -// func TestDefaultOTLPExporterFactoriesAreAutomaticallyRegistered(t *testing.T) { -// t.Run("spans", spanFuncs.testDefaultOTLPExporterFactoriesAreAutomaticallyRegistered) -// t.Run("metrics", metricFuncs.testDefaultOTLPExporterFactoriesAreAutomaticallyRegistered) -// } - func TestRegistryErrorsOnDuplicateRegisterCalls(t *testing.T) { r := newTestRegistry() diff --git a/exporters/autoexport/signal_test.go b/exporters/autoexport/signal_test.go index 9d314fdbd4f..b6745321f6d 100644 --- a/exporters/autoexport/signal_test.go +++ b/exporters/autoexport/signal_test.go @@ -23,7 +23,7 @@ import ( func TestOTLPExporterReturnedWhenNoEnvOrFallbackExporterConfigured(t *testing.T) { ts := newSignal[*testType]("TEST_TYPE_KEY") - ts.registry.store("otlp", factory("test-otlp-exporter")) + assert.NoError(t, ts.registry.store("otlp", factory("test-otlp-exporter"))) exp, err := ts.create(context.Background()) assert.NoError(t, err) assert.Equal(t, exp.string, "test-otlp-exporter") diff --git a/exporters/autoexport/spans.go b/exporters/autoexport/spans.go index 667ae9aaf86..cd8b4c9bf88 100644 --- a/exporters/autoexport/spans.go +++ b/exporters/autoexport/spans.go @@ -28,7 +28,7 @@ type SpanOption = option[trace.SpanExporter] // Option applies an autoexport configuration option. // -// Deprecated: Use SpanOption +// Deprecated: Use SpanOption. type Option = SpanOption // WithFallbackSpanExporter sets the fallback exporter to use when no exporter diff --git a/exporters/autoexport/spans_test.go b/exporters/autoexport/spans_test.go index 1d10f9d305d..895c502b250 100644 --- a/exporters/autoexport/spans_test.go +++ b/exporters/autoexport/spans_test.go @@ -19,9 +19,10 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" "go.opentelemetry.io/otel/exporters/otlp/otlptrace" "go.opentelemetry.io/otel/sdk/trace" + + "github.com/stretchr/testify/assert" ) func TestSpanExporterNone(t *testing.T) { From 90eeff5436923b20e07d6a30fcb7067223258489 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Thu, 19 Oct 2023 13:28:20 -0400 Subject: [PATCH 16/26] Cleanup and lint fixes --- exporters/autoexport/registry.go | 7 ------- exporters/autoexport/signal.go | 1 + 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/exporters/autoexport/registry.go b/exporters/autoexport/registry.go index 8f0c619ce52..3a4658cd534 100644 --- a/exporters/autoexport/registry.go +++ b/exporters/autoexport/registry.go @@ -70,10 +70,3 @@ func (r *registry[T]) store(key string, factory func(context.Context) (T, error) r.names[key] = factory return nil } - -// drop removes key from the registry if it exists, otherwise nothing. -func (r *registry[T]) drop(key string) { - r.mu.Lock() - defer r.mu.Unlock() - delete(r.names, key) -} diff --git a/exporters/autoexport/signal.go b/exporters/autoexport/signal.go index 3528be990cb..1103a65ae66 100644 --- a/exporters/autoexport/signal.go +++ b/exporters/autoexport/signal.go @@ -61,6 +61,7 @@ type option[T any] interface { type optionFunc[T any] func(cfg *config[T]) +//lint:ignore U1000 https://github.com/dominikh/go-tools/issues/1440 func (fn optionFunc[T]) apply(cfg *config[T]) { fn(cfg) } From 6c1e67ccb2b3612fe2c9ec6d5ee400c9baf9e162 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Thu, 19 Oct 2023 13:32:23 -0400 Subject: [PATCH 17/26] Fix copypasta and improve names --- exporters/autoexport/metrics_test.go | 4 ++-- exporters/autoexport/spans_test.go | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/exporters/autoexport/metrics_test.go b/exporters/autoexport/metrics_test.go index 4e9fe57c4b2..d79667f3d66 100644 --- a/exporters/autoexport/metrics_test.go +++ b/exporters/autoexport/metrics_test.go @@ -37,8 +37,8 @@ func assertPeriodicReaderWithExporter(t *testing.T, got metric.Reader, exporterT assert.IsType(t, &metric.PeriodicReader{}, got) // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. - clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("exporter").Elem().Type().String() - assert.Equal(t, exporterTypeName, clientType) + exporterType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("exporter").Elem().Type() + assert.Equal(t, exporterTypeName, exporterType.String()) } func TestMetricExporterOTLPOverHTTP(t *testing.T) { diff --git a/exporters/autoexport/spans_test.go b/exporters/autoexport/spans_test.go index 895c502b250..58ef73e1898 100644 --- a/exporters/autoexport/spans_test.go +++ b/exporters/autoexport/spans_test.go @@ -32,13 +32,13 @@ func TestSpanExporterNone(t *testing.T) { assert.True(t, IsNoneSpanExporter(got)) } -func assertOTLPExporterWithClientTypeName(t *testing.T, got trace.SpanExporter, clientTypeName string) { +func assertOTLPExporterWithClient(t *testing.T, got trace.SpanExporter, clientTypeName string) { t.Helper() assert.IsType(t, &otlptrace.Exporter{}, got) // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. - clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("client").Elem().Type().String() - assert.Equal(t, clientTypeName, clientType) + clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("client").Elem().Type() + assert.Equal(t, clientTypeName, clientType.String()) } func TestSpanExporterOTLPOverHTTP(t *testing.T) { @@ -47,7 +47,7 @@ func TestSpanExporterOTLPOverHTTP(t *testing.T) { got, err := NewSpanExporter(context.Background()) assert.NoError(t, err) - assertOTLPExporterWithClientTypeName(t, got, "*otlptracehttp.client") + assertOTLPExporterWithClient(t, got, "*otlptracehttp.client") } func TestSpanExporterOTLPOverGRPC(t *testing.T) { @@ -56,7 +56,7 @@ func TestSpanExporterOTLPOverGRPC(t *testing.T) { got, err := NewSpanExporter(context.Background()) assert.NoError(t, err) - assertOTLPExporterWithClientTypeName(t, got, "*otlptracegrpc.client") + assertOTLPExporterWithClient(t, got, "*otlptracegrpc.client") } func TestSpanExporterOTLPOverInvalidProtocol(t *testing.T) { From 1e68beb5325a797255843d7c250a08f57e61607f Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Thu, 19 Oct 2023 14:57:43 -0400 Subject: [PATCH 18/26] Tests for unspecified protocol --- exporters/autoexport/metrics_test.go | 40 +++++++++++++------------- exporters/autoexport/signal_test.go | 2 +- exporters/autoexport/spans_test.go | 42 +++++++++++++--------------- 3 files changed, 39 insertions(+), 45 deletions(-) diff --git a/exporters/autoexport/metrics_test.go b/exporters/autoexport/metrics_test.go index d79667f3d66..5a560c15399 100644 --- a/exporters/autoexport/metrics_test.go +++ b/exporters/autoexport/metrics_test.go @@ -16,6 +16,7 @@ package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" import ( "context" + "fmt" "reflect" "testing" @@ -32,31 +33,28 @@ func TestMetricExporterNone(t *testing.T) { assert.True(t, IsNoneMetricReader(got)) } -func assertPeriodicReaderWithExporter(t *testing.T, got metric.Reader, exporterTypeName string) { - t.Helper() - assert.IsType(t, &metric.PeriodicReader{}, got) - - // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. - exporterType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("exporter").Elem().Type() - assert.Equal(t, exporterTypeName, exporterType.String()) -} - -func TestMetricExporterOTLPOverHTTP(t *testing.T) { +func TestMetricExporterOTLP(t *testing.T) { t.Setenv("OTEL_METRICS_EXPORTER", "otlp") - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf") - got, err := NewMetricReader(context.Background()) - assert.NoError(t, err) - assertPeriodicReaderWithExporter(t, got, "*otlpmetrichttp.Exporter") -} + for _, tc := range []struct { + protocol, exporterType string + }{ + {"http/protobuf", "*otlpmetrichttp.Exporter"}, + {"", "*otlpmetrichttp.Exporter"}, + {"grpc", "*otlpmetricgrpc.Exporter"}, + } { + t.Run(fmt.Sprintf("protocol=%q", tc.protocol), func(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", tc.protocol) -func TestMetricExporterOTLPOverGRPC(t *testing.T) { - t.Setenv("OTEL_METRICS_EXPORTER", "otlp") - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") + got, err := NewMetricReader(context.Background()) + assert.NoError(t, err) + assert.IsType(t, &metric.PeriodicReader{}, got) - got, err := NewMetricReader(context.Background()) - assert.NoError(t, err) - assertPeriodicReaderWithExporter(t, got, "*otlpmetricgrpc.Exporter") + // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. + exporterType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("exporter").Elem().Type() + assert.Equal(t, tc.exporterType, exporterType.String()) + }) + } } func TestMetricExporterOTLPOverInvalidProtocol(t *testing.T) { diff --git a/exporters/autoexport/signal_test.go b/exporters/autoexport/signal_test.go index b6745321f6d..10bb6e62e96 100644 --- a/exporters/autoexport/signal_test.go +++ b/exporters/autoexport/signal_test.go @@ -44,7 +44,7 @@ func TestEnvExporterIsPreferredOverFallbackExporter(t *testing.T) { expName := "test-env-exporter-name" t.Setenv(envVariable, expName) fallback := testType{"test-fallback-exporter"} - ts.registry.store(expName, factory("test-env-exporter")) + assert.NoError(t, ts.registry.store(expName, factory("test-env-exporter"))) exp, err := ts.create(context.Background(), withFallback(&fallback)) assert.NoError(t, err) diff --git a/exporters/autoexport/spans_test.go b/exporters/autoexport/spans_test.go index 58ef73e1898..d958f77c469 100644 --- a/exporters/autoexport/spans_test.go +++ b/exporters/autoexport/spans_test.go @@ -16,11 +16,11 @@ package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" import ( "context" + "fmt" "reflect" "testing" "go.opentelemetry.io/otel/exporters/otlp/otlptrace" - "go.opentelemetry.io/otel/sdk/trace" "github.com/stretchr/testify/assert" ) @@ -32,33 +32,29 @@ func TestSpanExporterNone(t *testing.T) { assert.True(t, IsNoneSpanExporter(got)) } -func assertOTLPExporterWithClient(t *testing.T, got trace.SpanExporter, clientTypeName string) { - t.Helper() - assert.IsType(t, &otlptrace.Exporter{}, got) - - // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. - clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("client").Elem().Type() - assert.Equal(t, clientTypeName, clientType.String()) -} - -func TestSpanExporterOTLPOverHTTP(t *testing.T) { +func TestSpanExporterOTLP(t *testing.T) { t.Setenv("OTEL_TRACES_EXPORTER", "otlp") - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf") - got, err := NewSpanExporter(context.Background()) - assert.NoError(t, err) - assertOTLPExporterWithClient(t, got, "*otlptracehttp.client") -} + for _, tc := range []struct { + protocol, clientType string + }{ + {"http/protobuf", "*otlptracehttp.client"}, + {"", "*otlptracehttp.client"}, + {"grpc", "*otlptracegrpc.client"}, + } { + t.Run(fmt.Sprintf("protocol=%q", tc.protocol), func(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", tc.protocol) -func TestSpanExporterOTLPOverGRPC(t *testing.T) { - t.Setenv("OTEL_TRACES_EXPORTER", "otlp") - t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc") + got, err := NewSpanExporter(context.Background()) + assert.NoError(t, err) + assert.IsType(t, &otlptrace.Exporter{}, got) - got, err := NewSpanExporter(context.Background()) - assert.NoError(t, err) - assertOTLPExporterWithClient(t, got, "*otlptracegrpc.client") + // Implementation detail hack. This may break when bumping OTLP exporter modules as it uses unexported API. + clientType := reflect.Indirect(reflect.ValueOf(got)).FieldByName("client").Elem().Type() + assert.Equal(t, tc.clientType, clientType.String()) + }) + } } - func TestSpanExporterOTLPOverInvalidProtocol(t *testing.T) { t.Setenv("OTEL_TRACES_EXPORTER", "otlp") t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "invalid-protocol") From b8c8b35e4dad45138cfa83fdacf304a34095ace2 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Thu, 19 Oct 2023 15:47:05 -0400 Subject: [PATCH 19/26] Appease gofumpt --- exporters/autoexport/spans_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/exporters/autoexport/spans_test.go b/exporters/autoexport/spans_test.go index d958f77c469..35ec975b301 100644 --- a/exporters/autoexport/spans_test.go +++ b/exporters/autoexport/spans_test.go @@ -55,6 +55,7 @@ func TestSpanExporterOTLP(t *testing.T) { }) } } + func TestSpanExporterOTLPOverInvalidProtocol(t *testing.T) { t.Setenv("OTEL_TRACES_EXPORTER", "otlp") t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "invalid-protocol") From 1af21f1f207ce9f9745018ccabaff0b5f86ee9ad Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Thu, 19 Oct 2023 16:01:59 -0400 Subject: [PATCH 20/26] DRY panic logic --- exporters/autoexport/metrics.go | 13 +---------- exporters/autoexport/registry.go | 6 +++++ exporters/autoexport/registry_test.go | 32 ++++++++++++--------------- exporters/autoexport/spans.go | 13 +---------- 4 files changed, 22 insertions(+), 42 deletions(-) diff --git a/exporters/autoexport/metrics.go b/exporters/autoexport/metrics.go index 6e5b355254b..de770420b38 100644 --- a/exporters/autoexport/metrics.go +++ b/exporters/autoexport/metrics.go @@ -64,18 +64,7 @@ func NewMetricReader(ctx context.Context, opts ...MetricOption) (metric.Reader, // OTEL_METRICS_EXPORTERS environment variable contains the exporter name. This // will panic if name has already been registered. func RegisterMetricReader(name string, factory func(context.Context) (metric.Reader, error)) { - if err := metricsSignal.registry.store(name, factory); err != nil { - // registry.store will return errDuplicateRegistration if name is already - // registered. Panic here so the user is made aware of the duplicate - // registration, which could be done by malicious code trying to - // intercept cross-cutting concerns. - // - // Panic for all other errors as well. At this point there should not - // be any other errors returned from the store operation. If there - // are, alert the developer that adding them as soon as possible that - // they need to be handled here. - panic(err) - } + must(metricsSignal.registry.store(name, factory)) } var metricsSignal = newSignal[metric.Reader]("OTEL_METRICS_EXPORTER") diff --git a/exporters/autoexport/registry.go b/exporters/autoexport/registry.go index 3a4658cd534..03eb63d11a3 100644 --- a/exporters/autoexport/registry.go +++ b/exporters/autoexport/registry.go @@ -70,3 +70,9 @@ func (r *registry[T]) store(key string, factory func(context.Context) (T, error) r.names[key] = factory return nil } + +func must(err error) { + if err != nil { + panic(err) + } +} diff --git a/exporters/autoexport/registry_test.go b/exporters/autoexport/registry_test.go index 6b3f38dbb35..c1b633aed3b 100644 --- a/exporters/autoexport/registry_test.go +++ b/exporters/autoexport/registry_test.go @@ -16,6 +16,7 @@ package autoexport import ( "context" + "errors" "fmt" "sync" "testing" @@ -38,45 +39,35 @@ func newTestRegistry() registry[*testType] { func TestCanStoreExporterFactory(t *testing.T) { r := newTestRegistry() - assert.NotPanics(t, func() { - require.NoError(t, r.store("first", factory("first"))) - }) + require.NoError(t, r.store("first", factory("first"))) } func TestLoadOfUnknownExporterReturnsError(t *testing.T) { r := newTestRegistry() - assert.NotPanics(t, func() { - exp, err := r.load(context.Background(), "non-existent") - assert.Equal(t, err, errUnknownExporter, "empty registry should hold nothing") - assert.Nil(t, exp, "non-nil exporter returned") - }) + exp, err := r.load(context.Background(), "non-existent") + assert.Equal(t, err, errUnknownExporter, "empty registry should hold nothing") + assert.Nil(t, exp, "non-nil exporter returned") } func TestRegistryIsConcurrentSafe(t *testing.T) { const exporterName = "stdout" r := newTestRegistry() - assert.NotPanics(t, func() { - require.NoError(t, r.store(exporterName, factory("stdout"))) - }) + require.NoError(t, r.store(exporterName, factory("stdout"))) var wg sync.WaitGroup wg.Add(1) go func() { defer wg.Done() - assert.NotPanics(t, func() { - require.ErrorIs(t, r.store(exporterName, factory("stdout")), errDuplicateRegistration) - }) + require.ErrorIs(t, r.store(exporterName, factory("stdout")), errDuplicateRegistration) }() wg.Add(1) go func() { defer wg.Done() - assert.NotPanics(t, func() { - _, err := r.load(context.Background(), exporterName) - assert.NoError(t, err, "missing exporter in registry") - }) + _, err := r.load(context.Background(), exporterName) + assert.NoError(t, err, "missing exporter in registry") }() wg.Wait() @@ -106,3 +97,8 @@ func TestRegistryErrorsOnDuplicateRegisterCalls(t *testing.T) { errString := fmt.Sprintf("%s: %q", errDuplicateRegistration, exporterName) assert.ErrorContains(t, r.store(exporterName, factory(exporterName)), errString) } + +func TestMust(t *testing.T) { + assert.Panics(t, func() { must(errors.New("test")) }) + assert.NotPanics(t, func() { must(nil) }) +} diff --git a/exporters/autoexport/spans.go b/exporters/autoexport/spans.go index cd8b4c9bf88..9673f263276 100644 --- a/exporters/autoexport/spans.go +++ b/exporters/autoexport/spans.go @@ -67,18 +67,7 @@ func NewSpanExporter(ctx context.Context, opts ...Option) (trace.SpanExporter, e // OTEL_TRACES_EXPORTERS environment variable contains the exporter name. This // will panic if name has already been registered. func RegisterSpanExporter(name string, factory func(context.Context) (trace.SpanExporter, error)) { - if err := tracesSignal.registry.store(name, factory); err != nil { - // registry.store will return errDuplicateRegistration if name is already - // registered. Panic here so the user is made aware of the duplicate - // registration, which could be done by malicious code trying to - // intercept cross-cutting concerns. - // - // Panic for all other errors as well. At this point there should not - // be any other errors returned from the store operation. If there - // are, alert the developer that adding them as soon as possible that - // they need to be handled here. - panic(err) - } + must(tracesSignal.registry.store(name, factory)) } var tracesSignal = newSignal[trace.SpanExporter]("OTEL_TRACES_EXPORTER") From 2b08d9fa23ba8507d44d1e26c09a881a1e84248a Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Thu, 19 Oct 2023 16:03:59 -0400 Subject: [PATCH 21/26] Avoid unnecessary require block --- exporters/autoexport/go.mod | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/exporters/autoexport/go.mod b/exporters/autoexport/go.mod index 7b0ed679c4a..11da0fb35e3 100644 --- a/exporters/autoexport/go.mod +++ b/exporters/autoexport/go.mod @@ -15,22 +15,19 @@ require ( require ( github.com/beorn7/perks v1.0.1 // indirect - github.com/cespare/xxhash/v2 v2.2.0 // indirect - github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect - github.com/prometheus/client_golang v1.16.0 // indirect - github.com/prometheus/client_model v0.4.0 // indirect - github.com/prometheus/common v0.42.0 // indirect - github.com/prometheus/procfs v0.10.1 // indirect -) - -require ( github.com/cenkalti/backoff/v4 v4.2.1 // indirect + github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/go-logr/logr v1.2.4 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.0 // indirect + github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/prometheus/client_golang v1.16.0 // indirect + github.com/prometheus/client_model v0.4.0 // indirect + github.com/prometheus/common v0.42.0 // indirect + github.com/prometheus/procfs v0.10.1 // indirect go.opentelemetry.io/otel v1.19.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.42.0 // indirect go.opentelemetry.io/otel/exporters/prometheus v0.42.0 From 37ac9d5fc87e4734f36d1f45db6612108882f836 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Fri, 20 Oct 2023 07:05:26 -0400 Subject: [PATCH 22/26] Fix documentation --- exporters/autoexport/metrics.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/autoexport/metrics.go b/exporters/autoexport/metrics.go index de770420b38..13597bc557f 100644 --- a/exporters/autoexport/metrics.go +++ b/exporters/autoexport/metrics.go @@ -55,7 +55,7 @@ func WithFallbackMetricReader(exporter metric.Reader) MetricOption { // Use [WithFallbackMetricReader] option to change the returned exporter // when OTEL_TRACES_EXPORTER is unset or empty. // -// Use [IsNoneSpanExporter] to check if the retured exporter is a "no operation" exporter. +// Use [IsNoneMetricReader] to check if the retured exporter is a "no operation" exporter. func NewMetricReader(ctx context.Context, opts ...MetricOption) (metric.Reader, error) { return metricsSignal.create(ctx, opts...) } From 8f6c1bbaca1af5e83472320b71275727dbe53c0e Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Fri, 20 Oct 2023 10:36:18 -0400 Subject: [PATCH 23/26] Use SpanOption rather than deprecated alias --- exporters/autoexport/spans.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/autoexport/spans.go b/exporters/autoexport/spans.go index 9673f263276..e7de52e28a7 100644 --- a/exporters/autoexport/spans.go +++ b/exporters/autoexport/spans.go @@ -59,7 +59,7 @@ func WithFallbackSpanExporter(exporter trace.SpanExporter) SpanOption { // when OTEL_TRACES_EXPORTER is unset or empty. // // Use [IsNoneSpanExporter] to check if the retured exporter is a "no operation" exporter. -func NewSpanExporter(ctx context.Context, opts ...Option) (trace.SpanExporter, error) { +func NewSpanExporter(ctx context.Context, opts ...SpanOption) (trace.SpanExporter, error) { return tracesSignal.create(ctx, opts...) } From b1a55843d125e37e0fa3a6663ebb2ca18b0f24e8 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Mon, 23 Oct 2023 10:40:31 -0400 Subject: [PATCH 24/26] Remove Prometheus support - deferred to #4472 --- exporters/autoexport/go.mod | 8 -------- exporters/autoexport/go.sum | 18 ------------------ exporters/autoexport/metrics.go | 5 ----- exporters/autoexport/metrics_test.go | 9 --------- 4 files changed, 40 deletions(-) diff --git a/exporters/autoexport/go.mod b/exporters/autoexport/go.mod index 11da0fb35e3..8d7ad6421a4 100644 --- a/exporters/autoexport/go.mod +++ b/exporters/autoexport/go.mod @@ -14,23 +14,15 @@ require ( ) require ( - github.com/beorn7/perks v1.0.1 // indirect github.com/cenkalti/backoff/v4 v4.2.1 // indirect - github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/go-logr/logr v1.2.4 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.0 // indirect - github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/prometheus/client_golang v1.16.0 // indirect - github.com/prometheus/client_model v0.4.0 // indirect - github.com/prometheus/common v0.42.0 // indirect - github.com/prometheus/procfs v0.10.1 // indirect go.opentelemetry.io/otel v1.19.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.42.0 // indirect - go.opentelemetry.io/otel/exporters/prometheus v0.42.0 go.opentelemetry.io/otel/metric v1.19.0 // indirect go.opentelemetry.io/otel/trace v1.19.0 // indirect go.opentelemetry.io/proto/otlp v1.0.0 // indirect diff --git a/exporters/autoexport/go.sum b/exporters/autoexport/go.sum index 9915420e288..0bfd1b46dda 100644 --- a/exporters/autoexport/go.sum +++ b/exporters/autoexport/go.sum @@ -1,9 +1,5 @@ -github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= -github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/cenkalti/backoff/v4 v4.2.1 h1:y4OZtCnogmCPw98Zjyt5a6+QwPLGkiQsYW5oUqylYbM= github.com/cenkalti/backoff/v4 v4.2.1/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= -github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44= -github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= 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/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= @@ -12,7 +8,6 @@ github.com/go-logr/logr v1.2.4/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbV 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/glog v1.1.0 h1:/d3pCKDPWNnvIWe0vVUpNP32qc8U3PDVxySP/y360qE= -github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= @@ -22,18 +17,8 @@ github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.0 h1:YBftPWNWd4WwGqtY2yeZL2ef8rH github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.0/go.mod h1:YN5jB8ie0yfIUg6VvR9Kz84aCaG7AsGZnLjhHbUqwPg= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= -github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zkfA9PSy5pEvNWRP0ET0TIVo= -github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4= 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/prometheus/client_golang v1.16.0 h1:yk/hx9hDbrGHovbci4BY+pRMfSuuat626eFsHb7tmT8= -github.com/prometheus/client_golang v1.16.0/go.mod h1:Zsulrv/L9oM40tJ7T815tM89lFEugiJ9HzIqaAx4LKc= -github.com/prometheus/client_model v0.4.0 h1:5lQXD3cAg1OXBf4Wq03gTrXHeaV0TQvGfUooCfx1yqY= -github.com/prometheus/client_model v0.4.0/go.mod h1:oMQmHW1/JoDwqLtg57MGgP/Fb1CJEYF2imWWhWtMkYU= -github.com/prometheus/common v0.42.0 h1:EKsfXEYo4JpWMHH5cg+KOUWeuJSov1Id8zGR8eeI1YM= -github.com/prometheus/common v0.42.0/go.mod h1:xBwqVerjNdUDjgODMpudtOMwlOwf2SaTr1yjz4b7Zbc= -github.com/prometheus/procfs v0.10.1 h1:kYK1Va/YMlutzCGazswoHKo//tZVlFpKYh+PymziUAg= -github.com/prometheus/procfs v0.10.1/go.mod h1:nwNm2aOCAYw8uTR/9bWRREkZFxAUcWzPHWJq+XBB/FM= github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= @@ -51,8 +36,6 @@ go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.19.0 h1:3d+S2 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.19.0/go.mod h1:0+KuTDyKL4gjKCF75pHOX4wuzYDUZYfAQdSu43o+Z2I= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0 h1:IeMeyr1aBvBiPVYihXIaeIZba6b8E1bYp7lbdxK8CQg= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0/go.mod h1:oVdCUtjq9MK9BlS7TtucsQwUcXcymNiEDjgDD2jMtZU= -go.opentelemetry.io/otel/exporters/prometheus v0.42.0 h1:jwV9iQdvp38fxXi8ZC+lNpxjK16MRcZlpDYvbuO1FiA= -go.opentelemetry.io/otel/exporters/prometheus v0.42.0/go.mod h1:f3bYiqNqhoPxkvI2LrXqQVC546K7BuRDL/kKuxkujhA= go.opentelemetry.io/otel/metric v1.19.0 h1:aTzpGtV0ar9wlV4Sna9sdJyII5jTVJEvKETPiOKwvpE= go.opentelemetry.io/otel/metric v1.19.0/go.mod h1:L5rUsV9kM1IxCj1MmSdS+JQAcVm319EUrDVLrt7jqt8= go.opentelemetry.io/otel/sdk v1.19.0 h1:6USY6zH+L8uMH8L3t1enZPR3WFEmSTADlqldyHtJi3o= @@ -66,7 +49,6 @@ go.opentelemetry.io/proto/otlp v1.0.0/go.mod h1:Sy6pihPLfYHkr3NkUbEhGHFhINUSI/v8 go.uber.org/goleak v1.2.1 h1:NBol2c7O1ZokfZ0LEU9K6Whx/KnwvepVetCUhtKja4A= golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM= golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= -golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k= diff --git a/exporters/autoexport/metrics.go b/exporters/autoexport/metrics.go index 13597bc557f..f3d44d783d1 100644 --- a/exporters/autoexport/metrics.go +++ b/exporters/autoexport/metrics.go @@ -20,7 +20,6 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp" - "go.opentelemetry.io/otel/exporters/prometheus" "go.opentelemetry.io/otel/sdk/metric" ) @@ -39,7 +38,6 @@ func WithFallbackMetricReader(exporter metric.Reader) MetricOption { // OTEL_METRICS_EXPORTER defines the metrics exporter; supported values: // - "none" - "no operation" exporter // - "otlp" (default) - OTLP exporter; see [go.opentelemetry.io/otel/exporters/otlp/otlpmetric] -// - "prometheus" - Prometheus exporter; see [go.opentelemetry.io/otel/exporters/prometheus] // // OTEL_EXPORTER_OTLP_PROTOCOL defines OTLP exporter's transport protocol; // supported values: @@ -93,9 +91,6 @@ func init() { return nil, errInvalidOTLPProtocol } }) - RegisterMetricReader("prometheus", func(ctx context.Context) (metric.Reader, error) { - return prometheus.New() - }) RegisterMetricReader("none", func(ctx context.Context) (metric.Reader, error) { return newNoopMetricReader(), nil }) diff --git a/exporters/autoexport/metrics_test.go b/exporters/autoexport/metrics_test.go index 5a560c15399..4a3ecff2f68 100644 --- a/exporters/autoexport/metrics_test.go +++ b/exporters/autoexport/metrics_test.go @@ -20,7 +20,6 @@ import ( "reflect" "testing" - "go.opentelemetry.io/otel/exporters/prometheus" "go.opentelemetry.io/otel/sdk/metric" "github.com/stretchr/testify/assert" @@ -64,11 +63,3 @@ func TestMetricExporterOTLPOverInvalidProtocol(t *testing.T) { _, err := NewMetricReader(context.Background()) assert.Error(t, err) } - -func TestMetricExporterPrometheus(t *testing.T) { - t.Setenv("OTEL_METRICS_EXPORTER", "prometheus") - - got, err := NewMetricReader(context.Background()) - assert.NoError(t, err) - assert.IsType(t, &prometheus.Exporter{}, got) -} From a8b2ef75f4828cd40f27fc8da14dfeb49c4724d5 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Mon, 23 Oct 2023 14:23:10 -0400 Subject: [PATCH 25/26] Move CHANGELOG entry and add deprecation --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0e5b63a031..f60234ccd29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add `"go.opentelemetry.io/contrib/samplers/jaegerremote".WithSamplingStrategyFetcher` which sets custom fetcher implementation. (#4045) - Add `"go.opentelemetry.io/contrib/config"` package that includes configuration models generated via go-jsonschema. (#4376) - Add `NewSDK` function to `"go.opentelemetry.io/contrib/config"`. The initial implementation only returns noop providers. (#4414) +- Add metrics support to `go.opentelemetry.io/contrib/exporters/autoexport`. (#4229) + +### Deprecated + +- In `go.opentelemetry.io/contrib/exporters/autoexport`, `Option` was renamed to `SpanOption`. The old name is deprecated but continues to be supported as an alias. ### Changed @@ -74,7 +79,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add `WithSpanOptions` option in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (#3768) - Add testing support for Go 1.21. (#4233) - Add `WithFilter` option to `go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux`. (#4230) -- Add metrics support to `go.opentelemetry.io/contrib/exporters/autoexport`. (#4229) ### Changed From 503e4ff4817f9a3dd4bddab770cc7da6c0816937 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Mon, 23 Oct 2023 20:43:20 +0200 Subject: [PATCH 26/26] Update CHANGELOG.md --- CHANGELOG.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f60234ccd29..75798dfaa49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,15 +16,15 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add `NewSDK` function to `"go.opentelemetry.io/contrib/config"`. The initial implementation only returns noop providers. (#4414) - Add metrics support to `go.opentelemetry.io/contrib/exporters/autoexport`. (#4229) -### Deprecated - -- In `go.opentelemetry.io/contrib/exporters/autoexport`, `Option` was renamed to `SpanOption`. The old name is deprecated but continues to be supported as an alias. - ### Changed - Dropped compatibility testing for [Go 1.19]. The project no longer guarantees support for this version of Go. (#4352) +### Deprecated + +- In `go.opentelemetry.io/contrib/exporters/autoexport`, `Option` was renamed to `SpanOption`. The old name is deprecated but continues to be supported as an alias. (#4229) + ### Fixed - The `go.opentelemetry.io/contrib/samplers/jaegerremote` sampler does not panic when the default HTTP round-tripper (`http.DefaultTransport`) is not `*http.Transport`. (#4045)