Opentelemetry-java: Simplify the BatchSpansProcessor

[jkwatson]

I believe this class can be re-implemented to use a simple Queue and a background thread to process it.

[iNikem]

@jkwatson Can you elaborate what parts of that class seem too complex right now? It currently has exactly what you want: a queue and a thread :)

[jkwatson]

@jkwatson Can you elaborate what parts of that class seem too complex right now? It currently has exactly what you want: a queue and a thread :)

It doesn't actually have a JDK java.util.Queue. Instead, it's implemented queue-like semantics from scratch, using old-school locks and notifications. I think we would be better served to use one of the higher level implementations that are built-in to the JDK, rather than writing and maintaining our own.

[neeraj97]

@jkwatson
BlockingQueue could be used but how would we go about delaying worker thread and also notifying it ? We could use Thread.sleep(millisecond) but it could be woken by interrupts only, but we are using interrupts to check whether we should continue running the thread or not. So I think there is a need for using monitor-lock for dealing with notification to the worker thread.
We could use Blocking Queue for making operations on the list(add,clear,..) independent of our custom locks as they implement their own lock mechanisms and we use custom monitor-lock for dealing with notifications to the worker thread.
Let me know thoughts so that I could start implementing it asap :)

[jkwatson]

If I were to do this, I think I'd step back and think about the high-level goals for this class, rather than trying to precisely replicate the existing behavior.

1) Thread-safety (obviously)
2) Queue up a set of spans for export, making sure that
a) we don't queue up more than the maxQueueSize before sending
b) we don't queue for longer than the scheduledDlayMillis before sending
c) If we can't handle the throughput, we drop spans and record that fact.

Does that help you? I don't have a precise implementation in mind; I just know that having to do manual wait/notify is a code smell in modern java where we've got much more powerful and robust concurrent primitives available to us.

[neeraj97]

@jkwatson Thanks for sharing I got a clear picture :)