Rewrite processQueue for better batching

[vmihailenco]

Previous discussion in #702

The tests are failing and I am not sure what to do about them. As I understand they were working by accident or buggy behavior in previous implementation. E.g.

		{
			name: "parallel span generation",
			o: []sdktrace.BatchSpanProcessorOption{
				sdktrace.WithScheduleDelayMillis(schDelay),
				sdktrace.WithMaxQueueSize(200),
			},
			wantNumSpans:   200,
			wantBatchCount: 1,
			genNumSpans:    205,
			waitTime:       waitTime,
			parallel:       true,
		},

fails with

parallel span generation: number of exported span: got 205, want 200

This result depends on call/scheduling order (the fact that consuming is stopped for ScheduledDelayMillis) and BatchSpanProcessor does not provide such guarantees in real/production environment. So I would suggest to remove these assertions instead of just updating the numbers.

I would also rename ScheduledDelayMillis -> ScheduledDelay -> BatchTimeout.

[jmacd]

I also support renaming the timeout to BatchTimeout.

[jmacd]

This result depends on call/scheduling order (the fact that consuming is stopped for ScheduledDelayMillis) and BatchSpanProcessor does not provide such guarantees in real/production environment.

I would like to see the call to time.Sleep removed from the test. I believe the correct test should assert

			wantNumSpans:   205,
			wantBatchCount: 2,

[vmihailenco]

PTAL. I've left BatchTimeout for separate PR since it is likely to affect several packages. There multiple commits to ease review.

[jmacd]

In my prior review I suggested using a context for safe shutdown, not a wait group. These comments would apply, if you take the suggestion.

[jmacd]

I believe there is still a problem that can be fixed by adding one more pair of Add(1) and Done() calls.

[jmacd]

This is very close, and I appreciate your patience!

[vmihailenco]

PTAL - I've went with read-and-throw-away goroutine as you suggested previously. Perhaps golang/go#21985 is not such a bad idea after all.

[jmacd]

I noticed near the end of golang/go#21985

Conventionally these days people use a context.Context value for this.

which is similar to the idea in #719 (comment). However I'm inclined to accept this as-is. I may file an issue to remove throwAwayFutureSends() later.

[vmihailenco]

I believe that comment suggests to use context.Context instead of close to signal that there are no more messages. That is not the case here (since there can be more messages) and that does not allow to remove throwAwayFutureSends. Otherwise I don't understand how you suggest to use that extra context channel.

[MrAlias]

Thanks for tackling this!