From 7257c0a97a18ed67f21ac013e2206e809fdcb778 Mon Sep 17 00:00:00 2001 From: Huseyin Celal Oner Date: Mon, 5 Feb 2024 09:27:13 +0100 Subject: [PATCH 1/3] feat(autoexport): change WithFallback options signatures Fixes #4877. Changes the method signatures as requested in the issue. --- CHANGELOG.md | 4 ++++ exporters/autoexport/metrics.go | 4 ++-- exporters/autoexport/signal.go | 10 +++++----- exporters/autoexport/signal_test.go | 21 ++++++++++++++++----- exporters/autoexport/spans.go | 4 ++-- 5 files changed, 29 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f869f53cf8..1166aa5f09c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ 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 + +- Change the signature of the fallback options in `go.opentelemetry.io/contrib/exporters/autoexport` to accept a factory function instead. `WithFallbackMetricReader(exporter metric.Reader) MetricOption` method is now replaced with `func WithFallbackMetricReader(metricReaderFactory func(ctx context.Context) (metric.Reader, error)) MetricOption` method. `WithFallbackSpanExporter(exporter trace.SpanExporter) SpanOption` method is now replaced with `WithFallbackSpanExporter(spanExporterFactory func(ctx context.Context) (trace.SpanExporter, error)) SpanOption` method. (#4891) + ### 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..ab2f7b48bce 100644 --- a/exporters/autoexport/signal.go +++ b/exporters/autoexport/signal.go @@ -42,7 +42,7 @@ 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 + return cfg.fallbackFactory(ctx) } expType = "otlp" } @@ -51,8 +51,8 @@ func (s signal[T]) create(ctx context.Context, opts ...option[T]) (T, error) { } type config[T any] struct { - hasFallback bool - fallback T + hasFallback bool + fallbackFactory func(ctx context.Context) (T, error) } type option[T any] interface { @@ -66,9 +66,9 @@ 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] From ff11c5a0dce0ab9d7f06738db88f7feb2066add7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=BCseyin=20Celal=20=C3=96ner?= Date: Mon, 5 Feb 2024 20:01:21 +0100 Subject: [PATCH 2/3] chore: apply changelog suggestion Co-authored-by: Tyler Yahn --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1166aa5f09c..c416a72ea2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed -- Change the signature of the fallback options in `go.opentelemetry.io/contrib/exporters/autoexport` to accept a factory function instead. `WithFallbackMetricReader(exporter metric.Reader) MetricOption` method is now replaced with `func WithFallbackMetricReader(metricReaderFactory func(ctx context.Context) (metric.Reader, error)) MetricOption` method. `WithFallbackSpanExporter(exporter trace.SpanExporter) SpanOption` method is now replaced with `WithFallbackSpanExporter(spanExporterFactory func(ctx context.Context) (trace.SpanExporter, error)) SpanOption` method. (#4891) +- 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 From 697a5cad46dcae1913e651a60853ba953e5de992 Mon Sep 17 00:00:00 2001 From: Huseyin Celal Oner Date: Tue, 6 Feb 2024 13:04:08 +0100 Subject: [PATCH 3/3] chore(autoexport): get rid hasFallback flag which became extra after fallbackFactory change --- exporters/autoexport/signal.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/exporters/autoexport/signal.go b/exporters/autoexport/signal.go index ab2f7b48bce..acb0505b3c4 100644 --- a/exporters/autoexport/signal.go +++ b/exporters/autoexport/signal.go @@ -41,7 +41,7 @@ func (s signal[T]) create(ctx context.Context, opts ...option[T]) (T, error) { expType := os.Getenv(s.envKey) if expType == "" { - if cfg.hasFallback { + if cfg.fallbackFactory != nil { return cfg.fallbackFactory(ctx) } expType = "otlp" @@ -51,7 +51,6 @@ func (s signal[T]) create(ctx context.Context, opts ...option[T]) (T, error) { } type config[T any] struct { - hasFallback bool fallbackFactory func(ctx context.Context) (T, error) } @@ -68,7 +67,6 @@ func (fn optionFunc[T]) apply(cfg *config[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.fallbackFactory = fallbackFactory }) }