From 23a33739e9ca2e5bdab39b70d25a3c01c3a05b30 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 19 Oct 2021 08:13:37 -0700 Subject: [PATCH] Fix flaky test TestSimpleSpanProcessorShutdownHonorsContextCancel (#2290) * Fix flaky test TestSimpleSpanProcessorShutdownHonorsContextCancel * Add changes to changelog * Prioritize return of exporter error * Update changelog description * Fix deadlock bug --- CHANGELOG.md | 1 + sdk/trace/simple_span_processor.go | 16 +++++++++++++++- sdk/trace/simple_span_processor_test.go | 10 ++++++++-- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb39b9e6dea..143a73bc5be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/sdk/trace/simple_span_processor.go b/sdk/trace/simple_span_processor.go index 9dc87f893b9..ecc45bc6c38 100644 --- a/sdk/trace/simple_span_processor.go +++ b/sdk/trace/simple_span_processor.go @@ -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 diff --git a/sdk/trace/simple_span_processor_test.go b/sdk/trace/simple_span_processor_test.go index 688cc4703bd..4ac7124c5fc 100644 --- a/sdk/trace/simple_span_processor_test.go +++ b/sdk/trace/simple_span_processor_test.go @@ -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)