From 87cc1e1fea5eb11919fd70d9cfc45443a2360e15 Mon Sep 17 00:00:00 2001
From: Tyler Yahn <MrAlias@users.noreply.github.com>
Date: Wed, 9 Jun 2021 21:05:10 +0000
Subject: [PATCH] Test BatchSpanProcessor export timeout directly (#1982)

Test the export timeout by waiting indefinitely for the export to
timeout instead of having a second timer, in its own goroutine, timeout.
The algorithm this replaces fails on machines that are slow and the one
meta-timer is given priority to progress over the export timer that is
being testing, resulting in a false-negative test result.

Move the testing of a BatchSpanProcessor export timeout to its own test
function. This removes the bloat this introduces to the other testing
options and allows customization that enable the testing in a
deterministic manner.
---
 sdk/trace/batch_span_processor_test.go | 67 +++++++++++++++-----------
 1 file changed, 38 insertions(+), 29 deletions(-)

diff --git a/sdk/trace/batch_span_processor_test.go b/sdk/trace/batch_span_processor_test.go
index 3da75aeaf..84cdf2e49 100644
--- a/sdk/trace/batch_span_processor_test.go
+++ b/sdk/trace/batch_span_processor_test.go
@@ -36,7 +36,6 @@ type testBatchExporter struct {
 	sizes         []int
 	batchCount    int
 	shutdownCount int
-	delay         time.Duration
 	errors        []error
 	droppedCount  int
 	idx           int
@@ -54,8 +53,6 @@ func (t *testBatchExporter) ExportSpans(ctx context.Context, spans []sdktrace.Re
 		return err
 	}
 
-	time.Sleep(t.delay)
-
 	select {
 	case <-ctx.Done():
 		t.err = ctx.Err()
@@ -109,19 +106,16 @@ func TestNewBatchSpanProcessorWithNilExporter(t *testing.T) {
 }
 
 type testOption struct {
-	name              string
-	o                 []sdktrace.BatchSpanProcessorOption
-	wantNumSpans      int
-	wantBatchCount    int
-	wantExportTimeout bool
-	genNumSpans       int
-	delayExportBy     time.Duration
-	parallel          bool
+	name           string
+	o              []sdktrace.BatchSpanProcessorOption
+	wantNumSpans   int
+	wantBatchCount int
+	genNumSpans    int
+	parallel       bool
 }
 
 func TestNewBatchSpanProcessorWithOptions(t *testing.T) {
 	schDelay := 200 * time.Millisecond
-	exportTimeout := time.Millisecond
 	options := []testOption{
 		{
 			name:           "default BatchSpanProcessorOptions",
@@ -129,15 +123,6 @@ func TestNewBatchSpanProcessorWithOptions(t *testing.T) {
 			wantBatchCount: 4,
 			genNumSpans:    2053,
 		},
-		{
-			name: "non-default ExportTimeout",
-			o: []sdktrace.BatchSpanProcessorOption{
-				sdktrace.WithExportTimeout(exportTimeout),
-			},
-			wantExportTimeout: true,
-			genNumSpans:       2053,
-			delayExportBy:     2 * exportTimeout, // to ensure export timeout
-		},
 		{
 			name: "non-default BatchTimeout",
 			o: []sdktrace.BatchSpanProcessorOption{
@@ -204,9 +189,7 @@ func TestNewBatchSpanProcessorWithOptions(t *testing.T) {
 	}
 	for _, option := range options {
 		t.Run(option.name, func(t *testing.T) {
-			te := testBatchExporter{
-				delay: option.delayExportBy,
-			}
+			te := testBatchExporter{}
 			tp := basicTracerProvider(t)
 			ssp := createAndRegisterBatchSP(option, &te)
 			if ssp == nil {
@@ -231,15 +214,41 @@ func TestNewBatchSpanProcessorWithOptions(t *testing.T) {
 					gotBatchCount, option.wantBatchCount)
 				t.Errorf("Batches %v\n", te.sizes)
 			}
-
-			if option.wantExportTimeout && te.err != context.DeadlineExceeded {
-				t.Errorf("context deadline: got err %+v, want %+v\n",
-					te.err, context.DeadlineExceeded)
-			}
 		})
 	}
 }
 
+type stuckExporter struct {
+	testBatchExporter
+}
+
+// ExportSpans waits for ctx to expire and returns that error.
+func (e *stuckExporter) ExportSpans(ctx context.Context, _ []sdktrace.ReadOnlySpan) error {
+	<-ctx.Done()
+	e.err = ctx.Err()
+	return ctx.Err()
+}
+
+func TestBatchSpanProcessorExportTimeout(t *testing.T) {
+	exp := new(stuckExporter)
+	bsp := sdktrace.NewBatchSpanProcessor(
+		exp,
+		// Set a non-zero export timeout so a deadline is set.
+		sdktrace.WithExportTimeout(1*time.Microsecond),
+		sdktrace.WithBlocking(),
+	)
+	tp := basicTracerProvider(t)
+	tp.RegisterSpanProcessor(bsp)
+
+	tr := tp.Tracer("BatchSpanProcessorExportTimeout")
+	generateSpan(t, false, tr, testOption{genNumSpans: 1})
+	tp.UnregisterSpanProcessor(bsp)
+
+	if exp.err != context.DeadlineExceeded {
+		t.Errorf("context deadline error not returned: got %+v", exp.err)
+	}
+}
+
 func createAndRegisterBatchSP(option testOption, te *testBatchExporter) sdktrace.SpanProcessor {
 	// Always use blocking queue to avoid flaky tests.
 	options := append(option.o, sdktrace.WithBlocking())