Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ The next release will require at least [Go 1.25].

- Support testing of [Go 1.26]. (#7902)

### Changed

- `TracerProvider.ForceFlush` in `go.opentelemetry.io/otel/sdk/trace` joins errors together and continues iteration through SpanProcessors as opposed to returning the first encountered error without attempting exports on subsequent SpanProcessors. (#7856)

<!-- Released section -->
<!-- Don't change this section unless doing release -->

Expand Down
8 changes: 4 additions & 4 deletions sdk/trace/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package trace // import "go.opentelemetry.io/otel/sdk/trace"

import (
"context"
"errors"
"fmt"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -262,18 +263,17 @@ func (p *TracerProvider) ForceFlush(ctx context.Context) error {
return nil
}

var err error
for _, sps := range spss {
select {
case <-ctx.Done():
return ctx.Err()
default:
}

if err := sps.sp.ForceFlush(ctx); err != nil {
return err
}
err = errors.Join(err, sps.sp.ForceFlush(ctx))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This creates a singly-linked list of errors.joinErrors, each of them but the first containing only two errors, one of which is the errors.joinError from the previous failure in calling ForceFlush.

What I expect that you intended to create here was a flat sequence of errors. Sketching:

errs := make([]error, 0, len(spss))
for _, sps := range spss {
  // ...
  if err := sps.sp.ForceFlush(ctx); err != nil {
    errs = append(errs, err)
  }
}
return errors.Join(errs...)

}
return nil
return err
}

// Shutdown shuts down TracerProvider. All registered span processors are shut down
Expand Down
56 changes: 55 additions & 1 deletion sdk/trace/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type basicSpanProcessor struct {
flushed bool
closed bool
injectShutdownError error
injectExportError error
}

func (t *basicSpanProcessor) Shutdown(context.Context) error {
Expand All @@ -39,7 +40,7 @@ func (*basicSpanProcessor) OnStart(context.Context, ReadWriteSpan) {}
func (*basicSpanProcessor) OnEnd(ReadOnlySpan) {}
func (t *basicSpanProcessor) ForceFlush(context.Context) error {
t.flushed = true
return nil
return t.injectExportError
}

type shutdownSpanProcessor struct {
Expand Down Expand Up @@ -227,6 +228,59 @@ func TestRegisterAfterShutdownWithProcessors(t *testing.T) {
assert.Empty(t, stp.getSpanProcessors())
}

func TestTracerProviderForceFlush(t *testing.T) {
t.Run("AfterShutdown", func(t *testing.T) {
stp := NewTracerProvider()
sp1 := &basicSpanProcessor{}
stp.RegisterSpanProcessor(sp1)
ctx := t.Context()

require.NoError(t, stp.ForceFlush(ctx))
require.True(t, sp1.flushed, "SpanProcessor ForceFlush not called")

sp1.flushed = false
require.NoError(t, stp.Shutdown(ctx))

require.NoError(t, stp.ForceFlush(ctx))
assert.False(t, sp1.flushed, "SpanProcessor ForceFlush called after Shutdown")
})

t.Run("Multi", func(t *testing.T) {
stp := NewTracerProvider()
sp1 := &basicSpanProcessor{}
sp2 := &basicSpanProcessor{}
stp.RegisterSpanProcessor(sp1)
stp.RegisterSpanProcessor(sp2)
ctx := t.Context()

require.NoError(t, stp.ForceFlush(ctx))
require.True(t, sp1.flushed, "SpanProcessor ForceFlush not called")
require.True(t, sp2.flushed, "SpanProcessor ForceFlush not called")
})

t.Run("Error", func(t *testing.T) {
stp := NewTracerProvider()
sp1 := &basicSpanProcessor{injectExportError: assert.AnError}
sp2 := &basicSpanProcessor{}
stp.RegisterSpanProcessor(sp1)
stp.RegisterSpanProcessor(sp2)
ctx := t.Context()

assert.ErrorIs(t, stp.ForceFlush(ctx), assert.AnError, "span processor error not returned")
require.True(t, sp1.flushed, "SpanProcessor ForceFlush not called")
require.True(t, sp2.flushed, "SpanProcessor ForceFlush not called")
})

t.Run("WithCancel", func(t *testing.T) {
stp := NewTracerProvider()
sp1 := &basicSpanProcessor{}
stp.RegisterSpanProcessor(sp1)
ctx, cancel := context.WithCancel(t.Context())
cancel()
assert.ErrorIs(t, stp.ForceFlush(ctx), context.Canceled)
})
}

func TestTracerProviderSamplerConfigFromEnv(t *testing.T) {
type testCase struct {
sampler string
Expand Down