Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 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
59 changes: 59 additions & 0 deletions 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 @@ -38,6 +39,10 @@ func (t *basicSpanProcessor) Shutdown(context.Context) error {
func (*basicSpanProcessor) OnStart(context.Context, ReadWriteSpan) {}
func (*basicSpanProcessor) OnEnd(ReadOnlySpan) {}
func (t *basicSpanProcessor) ForceFlush(context.Context) error {
if t.injectExportError != nil {
t.flushed = false
return t.injectExportError
}
t.flushed = true
return nil
Comment thread
sawamurataxman marked this conversation as resolved.
Outdated
}
Expand Down Expand Up @@ -227,6 +232,60 @@ 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("MultiWithSPError", func(t *testing.T) {
stp := NewTracerProvider()
spErr := errors.New("basic span processor export failure")
sp1 := &basicSpanProcessor{injectExportError: spErr}
sp2 := &basicSpanProcessor{}
stp.RegisterSpanProcessor(sp1)
stp.RegisterSpanProcessor(sp2)
ctx := t.Context()

assert.ErrorIs(t, stp.ForceFlush(ctx), sp1.injectExportError, "span processor error not returned")
require.False(t, sp1.flushed, "SpanProcessor wrongly considered flushed")
require.True(t, sp2.flushed, "SpanProcessor ForceFlush not called")
})
Comment thread
sawamurataxman marked this conversation as resolved.
Outdated

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