Skip to content

Commit fdd1320

Browse files
TracerProvider ForceFlush() Error Fix (#7856)
Previously upon a SpanProcessor's ForceFlush returning an error, it would return that error and not attempt to flush subsequent SpanProcessors. Now when an error is encountered, it will Join the new error with the existing errors and continue iterating through the SpanProcessors and return the consolidated error at the end of iteration. This is in line with the workflow found in LoggerProvider's ForceFlush. --------- Co-authored-by: Robert Pająk <pellared@hotmail.com>
1 parent 78f9904 commit fdd1320

3 files changed

Lines changed: 63 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
1414
The package contains semantic conventions from the `v1.40.0` version of the OpenTelemetry Semantic Conventions.
1515
See the [migration documentation](./semconv/v1.40.0/MIGRATION.md) for information on how to upgrade from `go.opentelemetry.io/otel/semconv/v1.39.0`. (#7985)
1616

17+
### Changed
18+
19+
- `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)
20+
1721
### Fixed
1822

1923
- Fix missing `request.GetBody` in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp` to correctly handle HTTP2 GOAWAY frame. (#7931)

sdk/trace/provider.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package trace // import "go.opentelemetry.io/otel/sdk/trace"
55

66
import (
77
"context"
8+
"errors"
89
"fmt"
910
"sync"
1011
"sync/atomic"
@@ -262,18 +263,17 @@ func (p *TracerProvider) ForceFlush(ctx context.Context) error {
262263
return nil
263264
}
264265

266+
var err error
265267
for _, sps := range spss {
266268
select {
267269
case <-ctx.Done():
268270
return ctx.Err()
269271
default:
270272
}
271273

272-
if err := sps.sp.ForceFlush(ctx); err != nil {
273-
return err
274-
}
274+
err = errors.Join(err, sps.sp.ForceFlush(ctx))
275275
}
276-
return nil
276+
return err
277277
}
278278

279279
// Shutdown shuts down TracerProvider. All registered span processors are shut down

sdk/trace/provider_test.go

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ type basicSpanProcessor struct {
2828
flushed bool
2929
closed bool
3030
injectShutdownError error
31+
injectExportError error
3132
}
3233

3334
func (t *basicSpanProcessor) Shutdown(context.Context) error {
@@ -39,7 +40,7 @@ func (*basicSpanProcessor) OnStart(context.Context, ReadWriteSpan) {}
3940
func (*basicSpanProcessor) OnEnd(ReadOnlySpan) {}
4041
func (t *basicSpanProcessor) ForceFlush(context.Context) error {
4142
t.flushed = true
42-
return nil
43+
return t.injectExportError
4344
}
4445

4546
type shutdownSpanProcessor struct {
@@ -227,6 +228,59 @@ func TestRegisterAfterShutdownWithProcessors(t *testing.T) {
227228
assert.Empty(t, stp.getSpanProcessors())
228229
}
229230

231+
func TestTracerProviderForceFlush(t *testing.T) {
232+
t.Run("AfterShutdown", func(t *testing.T) {
233+
stp := NewTracerProvider()
234+
sp1 := &basicSpanProcessor{}
235+
stp.RegisterSpanProcessor(sp1)
236+
ctx := t.Context()
237+
238+
require.NoError(t, stp.ForceFlush(ctx))
239+
require.True(t, sp1.flushed, "SpanProcessor ForceFlush not called")
240+
241+
sp1.flushed = false
242+
require.NoError(t, stp.Shutdown(ctx))
243+
244+
require.NoError(t, stp.ForceFlush(ctx))
245+
assert.False(t, sp1.flushed, "SpanProcessor ForceFlush called after Shutdown")
246+
})
247+
248+
t.Run("Multi", func(t *testing.T) {
249+
stp := NewTracerProvider()
250+
sp1 := &basicSpanProcessor{}
251+
sp2 := &basicSpanProcessor{}
252+
stp.RegisterSpanProcessor(sp1)
253+
stp.RegisterSpanProcessor(sp2)
254+
ctx := t.Context()
255+
256+
require.NoError(t, stp.ForceFlush(ctx))
257+
require.True(t, sp1.flushed, "SpanProcessor ForceFlush not called")
258+
require.True(t, sp2.flushed, "SpanProcessor ForceFlush not called")
259+
})
260+
261+
t.Run("Error", func(t *testing.T) {
262+
stp := NewTracerProvider()
263+
sp1 := &basicSpanProcessor{injectExportError: assert.AnError}
264+
sp2 := &basicSpanProcessor{}
265+
stp.RegisterSpanProcessor(sp1)
266+
stp.RegisterSpanProcessor(sp2)
267+
ctx := t.Context()
268+
269+
assert.ErrorIs(t, stp.ForceFlush(ctx), assert.AnError, "span processor error not returned")
270+
require.True(t, sp1.flushed, "SpanProcessor ForceFlush not called")
271+
require.True(t, sp2.flushed, "SpanProcessor ForceFlush not called")
272+
})
273+
274+
t.Run("WithCancel", func(t *testing.T) {
275+
stp := NewTracerProvider()
276+
sp1 := &basicSpanProcessor{}
277+
stp.RegisterSpanProcessor(sp1)
278+
ctx, cancel := context.WithCancel(t.Context())
279+
cancel()
280+
assert.ErrorIs(t, stp.ForceFlush(ctx), context.Canceled)
281+
})
282+
}
283+
230284
func TestTracerProviderSamplerConfigFromEnv(t *testing.T) {
231285
type testCase struct {
232286
sampler string

0 commit comments

Comments
 (0)