diff --git a/CHANGELOG.md b/CHANGELOG.md index 8eba638ff43..aec59d05fd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed - `go.opentelemetry.io/otel/exporters/stdout/stdoutlog` won't print timestamps when `WithoutTimestamps` option is set. (#5241) +- The `Shutdown` method of `Exporter` in `go.opentelemetry.io/otel/exporters/stdout/stdouttrace` ignores the context cancellation and always returns `nil`. (#5189) +- The `ForceFlush` and `Shutdown` methods of the exporter returned by `New` in `go.opentelemetry.io/otel/exporters/stdout/stdoutmetric` ignore the context cancellation and always return `nil`. (#5189) ## [1.26.0/0.48.0/0.2.0-alpha] 2024-04-24 diff --git a/exporters/stdout/stdoutmetric/exporter.go b/exporters/stdout/stdoutmetric/exporter.go index 96006ab29dc..fc155d79f97 100644 --- a/exporters/stdout/stdoutmetric/exporter.go +++ b/exporters/stdout/stdoutmetric/exporter.go @@ -51,12 +51,8 @@ func (e *exporter) Aggregation(k metric.InstrumentKind) metric.Aggregation { } func (e *exporter) Export(ctx context.Context, data *metricdata.ResourceMetrics) error { - select { - case <-ctx.Done(): - // Don't do anything if the context has already timed out. - return ctx.Err() - default: - // Context is still valid, continue. + if err := ctx.Err(); err != nil { + return err } if e.redactTimestamps { redactTimestamps(data) @@ -67,18 +63,18 @@ func (e *exporter) Export(ctx context.Context, data *metricdata.ResourceMetrics) return e.encVal.Load().(encoderHolder).Encode(data) } -func (e *exporter) ForceFlush(ctx context.Context) error { +func (e *exporter) ForceFlush(context.Context) error { // exporter holds no state, nothing to flush. - return ctx.Err() + return nil } -func (e *exporter) Shutdown(ctx context.Context) error { +func (e *exporter) Shutdown(context.Context) error { e.shutdownOnce.Do(func() { e.encVal.Store(encoderHolder{ encoder: shutdownEncoder{}, }) }) - return ctx.Err() + return nil } func (e *exporter) MarshalLog() interface{} { diff --git a/exporters/stdout/stdoutmetric/exporter_test.go b/exporters/stdout/stdoutmetric/exporter_test.go index a8c96046da6..d7f957e63e1 100644 --- a/exporters/stdout/stdoutmetric/exporter_test.go +++ b/exporters/stdout/stdoutmetric/exporter_test.go @@ -28,8 +28,7 @@ func testEncoderOption() stdoutmetric.Option { func testCtxErrHonored(factory func(*testing.T) func(context.Context) error) func(t *testing.T) { return func(t *testing.T) { t.Helper() - ctx, cancel := context.WithCancel(context.Background()) - t.Cleanup(cancel) + ctx := context.Background() t.Run("DeadlineExceeded", func(t *testing.T) { innerCtx, innerCancel := context.WithTimeout(ctx, time.Nanosecond) @@ -55,26 +54,50 @@ func testCtxErrHonored(factory func(*testing.T) func(context.Context) error) fun } } -func TestExporterHonorsContextErrors(t *testing.T) { - t.Run("Shutdown", testCtxErrHonored(func(t *testing.T) func(context.Context) error { +func testCtxErrIgnored(factory func(*testing.T) func(context.Context) error) func(t *testing.T) { + return func(t *testing.T) { + t.Helper() + ctx := context.Background() + + t.Run("Canceled Ignored", func(t *testing.T) { + innerCtx, innerCancel := context.WithCancel(ctx) + innerCancel() + + f := factory(t) + assert.NoError(t, f(innerCtx)) + }) + + t.Run("NoError", func(t *testing.T) { + f := factory(t) + assert.NoError(t, f(ctx)) + }) + } +} + +func TestExporterExportHonorsContextErrors(t *testing.T) { + t.Run("Export", testCtxErrHonored(func(t *testing.T) func(context.Context) error { exp, err := stdoutmetric.New(testEncoderOption()) require.NoError(t, err) - return exp.Shutdown + return func(ctx context.Context) error { + data := new(metricdata.ResourceMetrics) + return exp.Export(ctx, data) + } })) +} - t.Run("ForceFlush", testCtxErrHonored(func(t *testing.T) func(context.Context) error { +func TestExporterForceFlushIgnoresContextErrors(t *testing.T) { + t.Run("ForceFlush", testCtxErrIgnored(func(t *testing.T) func(context.Context) error { exp, err := stdoutmetric.New(testEncoderOption()) require.NoError(t, err) return exp.ForceFlush })) +} - t.Run("Export", testCtxErrHonored(func(t *testing.T) func(context.Context) error { +func TestExporterShutdownIgnoresContextErrors(t *testing.T) { + t.Run("Shutdown", testCtxErrIgnored(func(t *testing.T) func(context.Context) error { exp, err := stdoutmetric.New(testEncoderOption()) require.NoError(t, err) - return func(ctx context.Context) error { - data := new(metricdata.ResourceMetrics) - return exp.Export(ctx, data) - } + return exp.Shutdown })) } diff --git a/exporters/stdout/stdouttrace/trace.go b/exporters/stdout/stdouttrace/trace.go index dcbd04dd62f..b19eb0dfbed 100644 --- a/exporters/stdout/stdouttrace/trace.go +++ b/exporters/stdout/stdouttrace/trace.go @@ -47,6 +47,9 @@ type Exporter struct { // ExportSpans writes spans in json format to stdout. func (e *Exporter) ExportSpans(ctx context.Context, spans []trace.ReadOnlySpan) error { + if err := ctx.Err(); err != nil { + return err + } e.stoppedMu.RLock() stopped := e.stopped e.stoppedMu.RUnlock() @@ -88,11 +91,6 @@ func (e *Exporter) Shutdown(ctx context.Context) error { e.stopped = true e.stoppedMu.Unlock() - select { - case <-ctx.Done(): - return ctx.Err() - default: - } return nil } diff --git a/exporters/stdout/stdouttrace/trace_test.go b/exporters/stdout/stdouttrace/trace_test.go index d308b13ed5e..a1fc44e559d 100644 --- a/exporters/stdout/stdouttrace/trace_test.go +++ b/exporters/stdout/stdouttrace/trace_test.go @@ -60,30 +60,44 @@ func TestExporterExportSpan(t *testing.T) { tests := []struct { opts []stdouttrace.Option expectNow time.Time + ctx context.Context + wantErr error }{ { opts: []stdouttrace.Option{stdouttrace.WithPrettyPrint()}, expectNow: now, + ctx: context.Background(), }, { opts: []stdouttrace.Option{stdouttrace.WithPrettyPrint(), stdouttrace.WithoutTimestamps()}, // expectNow is an empty time.Time + ctx: context.Background(), + }, + { + opts: []stdouttrace.Option{}, + ctx: func() context.Context { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + return ctx + }(), + wantErr: context.Canceled, }, } - ctx := context.Background() for _, tt := range tests { // write to buffer for testing var b bytes.Buffer ex, err := stdouttrace.New(append(tt.opts, stdouttrace.WithWriter(&b))...) require.Nil(t, err) - err = ex.ExportSpans(ctx, tracetest.SpanStubs{ss, ss}.Snapshots()) - require.Nil(t, err) + err = ex.ExportSpans(tt.ctx, tracetest.SpanStubs{ss, ss}.Snapshots()) + assert.Equal(t, tt.wantErr, err) - got := b.String() - wantone := expectedJSON(tt.expectNow) - assert.Equal(t, wantone+wantone, got) + if tt.wantErr == nil { + got := b.String() + wantone := expectedJSON(tt.expectNow) + assert.Equal(t, wantone+wantone, got) + } } } @@ -181,23 +195,7 @@ func expectedJSON(now time.Time) string { ` } -func TestExporterShutdownHonorsTimeout(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) - defer cancel() - - e, err := stdouttrace.New() - if err != nil { - t.Fatalf("failed to create exporter: %v", err) - } - - innerCtx, innerCancel := context.WithTimeout(ctx, time.Nanosecond) - defer innerCancel() - <-innerCtx.Done() - err = e.Shutdown(innerCtx) - assert.ErrorIs(t, err, context.DeadlineExceeded) -} - -func TestExporterShutdownHonorsCancel(t *testing.T) { +func TestExporterShutdownIgnoresContext(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) defer cancel() @@ -209,7 +207,7 @@ func TestExporterShutdownHonorsCancel(t *testing.T) { innerCtx, innerCancel := context.WithCancel(ctx) innerCancel() err = e.Shutdown(innerCtx) - assert.ErrorIs(t, err, context.Canceled) + assert.NoError(t, err) } func TestExporterShutdownNoError(t *testing.T) {