From 144c9332c109ce43100de7e6c74c140875500754 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=BCseyin=20Celal=20=C3=96ner?= Date: Tue, 6 Feb 2024 18:23:46 +0100 Subject: [PATCH] feat(autoexport): change WithFallback options signatures (#4891) * feat(autoexport): change WithFallback options signatures Fixes #4877. Changes the method signatures as requested in the issue. * chore: apply changelog suggestion Co-authored-by: Tyler Yahn * chore(autoexport): get rid hasFallback flag which became extra after fallbackFactory change --------- Co-authored-by: Tyler Yahn --- CHANGELOG.md | 6 ++++++ exporters/autoexport/metrics.go | 4 ++-- exporters/autoexport/signal.go | 12 +++++------- exporters/autoexport/signal_test.go | 21 ++++++++++++++++----- exporters/autoexport/spans.go | 4 ++-- 5 files changed, 31 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f869f53cf8..c416a72ea2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,12 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add client metric support to `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#4707) - Add peer attributes to spans recorded by `NewClientHandler`, `NewServerHandler` in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (#4873) +### Changed + +- The fallback options in `go.opentelemetry.io/contrib/exporters/autoexport` now accept factory functions. (#4891) + - `WithFallbackMetricReader(metric.Reader) MetricOption` is replaced with `func WithFallbackMetricReader(func(ctx context.Context) (metric.Reader, error)) MetricOption`. + - `WithFallbackSpanExporter(trace.SpanExporter) SpanOption` is replaced with `WithFallbackSpanExporter(func(ctx context.Context) (trace.SpanExporter, error)) SpanOption`. + ### Deprecated - The `RequestCount`, `RequestContentLength`, `ResponseContentLength`, `ServerLatency` constants in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` are deprecated. (#4707) diff --git a/exporters/autoexport/metrics.go b/exporters/autoexport/metrics.go index 1cfbbbc7884..217f75f34a1 100644 --- a/exporters/autoexport/metrics.go +++ b/exporters/autoexport/metrics.go @@ -39,8 +39,8 @@ 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) +func WithFallbackMetricReader(metricReaderFactory func(ctx context.Context) (metric.Reader, error)) MetricOption { + return withFallbackFactory[metric.Reader](metricReaderFactory) } // NewMetricReader returns a configured [go.opentelemetry.io/otel/sdk/metric.Reader] diff --git a/exporters/autoexport/signal.go b/exporters/autoexport/signal.go index 1103a65ae66..acb0505b3c4 100644 --- a/exporters/autoexport/signal.go +++ b/exporters/autoexport/signal.go @@ -41,8 +41,8 @@ func (s signal[T]) create(ctx context.Context, opts ...option[T]) (T, error) { expType := os.Getenv(s.envKey) if expType == "" { - if cfg.hasFallback { - return cfg.fallback, nil + if cfg.fallbackFactory != nil { + return cfg.fallbackFactory(ctx) } expType = "otlp" } @@ -51,8 +51,7 @@ func (s signal[T]) create(ctx context.Context, opts ...option[T]) (T, error) { } type config[T any] struct { - hasFallback bool - fallback T + fallbackFactory func(ctx context.Context) (T, error) } type option[T any] interface { @@ -66,9 +65,8 @@ func (fn optionFunc[T]) apply(cfg *config[T]) { fn(cfg) } -func withFallback[T any](fallback T) option[T] { +func withFallbackFactory[T any](fallbackFactory func(ctx context.Context) (T, error)) option[T] { return optionFunc[T](func(cfg *config[T]) { - cfg.hasFallback = true - cfg.fallback = fallback + cfg.fallbackFactory = fallbackFactory }) } diff --git a/exporters/autoexport/signal_test.go b/exporters/autoexport/signal_test.go index 10bb6e62e96..e4b56308d0f 100644 --- a/exporters/autoexport/signal_test.go +++ b/exporters/autoexport/signal_test.go @@ -16,6 +16,7 @@ package autoexport // import "go.opentelemetry.io/contrib/exporters/autoexport" import ( "context" + "errors" "testing" "github.com/stretchr/testify/assert" @@ -31,10 +32,21 @@ func TestOTLPExporterReturnedWhenNoEnvOrFallbackExporterConfigured(t *testing.T) func TestFallbackExporterReturnedWhenNoEnvExporterConfigured(t *testing.T) { ts := newSignal[*testType]("TEST_TYPE_KEY") - fallback := testType{"test-fallback-exporter"} - exp, err := ts.create(context.Background(), withFallback(&fallback)) + exp, err := ts.create(context.Background(), withFallbackFactory(factory("test-fallback-exporter"))) assert.NoError(t, err) - assert.Same(t, &fallback, exp) + assert.Equal(t, exp.string, "test-fallback-exporter") +} + +func TestFallbackExporterFactoryErrorReturnedWhenNoEnvExporterConfiguredAndFallbackFactoryReturnsAnError(t *testing.T) { + ts := newSignal[*testType]("TEST_TYPE_KEY") + + expectedErr := errors.New("error expected to return") + errFactory := func(ctx context.Context) (*testType, error) { + return nil, expectedErr + } + exp, err := ts.create(context.Background(), withFallbackFactory(errFactory)) + assert.ErrorIs(t, err, expectedErr) + assert.Nil(t, exp) } func TestEnvExporterIsPreferredOverFallbackExporter(t *testing.T) { @@ -43,10 +55,9 @@ func TestEnvExporterIsPreferredOverFallbackExporter(t *testing.T) { expName := "test-env-exporter-name" t.Setenv(envVariable, expName) - fallback := testType{"test-fallback-exporter"} assert.NoError(t, ts.registry.store(expName, factory("test-env-exporter"))) - exp, err := ts.create(context.Background(), withFallback(&fallback)) + exp, err := ts.create(context.Background(), withFallbackFactory(factory("test-fallback-exporter"))) assert.NoError(t, err) assert.Equal(t, exp.string, "test-env-exporter") } diff --git a/exporters/autoexport/spans.go b/exporters/autoexport/spans.go index af8c1bc3010..7d4a5b0cd64 100644 --- a/exporters/autoexport/spans.go +++ b/exporters/autoexport/spans.go @@ -34,8 +34,8 @@ 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) +func WithFallbackSpanExporter(spanExporterFactory func(ctx context.Context) (trace.SpanExporter, error)) SpanOption { + return withFallbackFactory[trace.SpanExporter](spanExporterFactory) } // NewSpanExporter returns a configured [go.opentelemetry.io/otel/sdk/trace.SpanExporter]