Skip to content

Commit

Permalink
Fix flaky test TestSimpleSpanProcessorShutdownHonorsContextCancel (#2290
Browse files Browse the repository at this point in the history
)

* Fix flaky test TestSimpleSpanProcessorShutdownHonorsContextCancel

* Add changes to changelog

* Prioritize return of exporter error

* Update changelog description

* Fix deadlock bug
  • Loading branch information
MrAlias committed Oct 19, 2021
1 parent 1f4b606 commit 23a3373
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -26,6 +26,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
### Fixed

- `semconv.NetAttributesFromHTTPRequest()` correctly handles IPv6 addresses. (#2285)
- The simple span processor shutdown method deterministically returns the exporter error status if it simultaneously finishes when the deadline is reached. (#2290, #2289)

## [1.0.1] - 2021-10-01

Expand Down
16 changes: 15 additions & 1 deletion sdk/trace/simple_span_processor.go
Expand Up @@ -88,10 +88,24 @@ func (ssp *simpleSpanProcessor) Shutdown(ctx context.Context) error {

go shutdown()

// Wait for the exporter to shut down or the deadline to expire.
select {
case err = <-done:
case <-ctx.Done():
err = ctx.Err()
// It is possible for the exporter to have immediately shut down
// and the context to be done simultaneously. In that case this
// outer select statement will randomly choose a case. This will
// result in a different returned error for similar scenarios.
// Instead, double check if the exporter shut down at the same
// time and return that error if so. This will ensure consistency
// as well as ensure the caller knows the exporter shut down
// successfully (they can already determine if the deadline is
// expired given they passed the context).
select {
case err = <-done:
default:
err = ctx.Err()
}
}
})
return err
Expand Down
10 changes: 8 additions & 2 deletions sdk/trace/simple_span_processor_test.go
Expand Up @@ -40,9 +40,15 @@ func (t *testExporter) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnl
return nil
}

func (t *testExporter) Shutdown(context.Context) error {
func (t *testExporter) Shutdown(ctx context.Context) error {
t.shutdown = true
return nil
select {
case <-ctx.Done():
// Ensure context deadline tests receive the expected error.
return ctx.Err()
default:
return nil
}
}

var _ sdktrace.SpanExporter = (*testExporter)(nil)
Expand Down

0 comments on commit 23a3373

Please sign in to comment.