[BUG] A log handler that calls ForceFlush can deadlock the export that is logging

[thc1006]

Describe your environment main at 5af96de, Ubuntu, gcc 14, CMake, -DWITH_ASYNC_EXPORT_PREVIEW=ON.

LogHandler::Handle() says nothing about what it may call, and an application can replace the handler and raise the level to Debug through GlobalLogHandler. Two of the diagnostics an exporter writes are dispatched from places where calling ForceFlush() back into that exporter cannot finish.

Before the request has been handed to the client

ElasticsearchLogRecordExporter::Export() registers the export, then calls Session::SendRequest(). SendRequest builds the HttpOperation, whose constructor dispatches Created, which the exporter's handler answers with OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Session created"). The background thread that would run the request is only asked for after SendAsync() returns, in Session::SendRequest.

So a handler that calls the no deadline ForceFlush() from that line waits for an export whose request has not been given to anything yet, and SendRequest() cannot return while it waits. Nothing else can move.

From a callback the client dispatches

The bundled curl client runs one background thread per HttpClient. A terminal diagnostic written from a completion runs on that thread. A handler that flushes from there waits for every other outstanding export, and those can only be advanced by the thread it is blocking. #4402 is the same shape for FinishSession().

Not specific to this exporter

Nothing above is about Elasticsearch. Any exporter whose ForceFlush() blocks and whose diagnostics are written from an export or a callback has both shapes. The OTLP HTTP exporter logs from the same places.

Not new

main already waits without a bound: ForceFlush() there sets timeout_steady to duration::max() when the caller asks for no deadline.

Correction, 17 Aug. I wrote above that the Created debug line is already on main. It is on main in the synchronous ResponseHandler, and it is not in the asynchronous one, which is the handler this issue is about and the only one whose ForceFlush() waits for anything. On 60c3d11e that handler's OnEvent is a default: break; with no progress logging at all. So the progress diagnostics were introduced by #4337 rather than inherited, and #4337 now removes them again: the states on the way to an outcome report nothing, and the exhaustive switch stays so a new state is still a compile error.

What remains in this issue after that is the general shape and not the five progress events: a handler that flushes from any callback the HTTP client dispatches can still wait on work only that client thread can advance. #4337 changes how the wait decides it is finished, and it makes the current export's own terminal diagnostic safe by retiring before logging, but it neither introduces nor removes either shape above.

What would settle it

Two ways, and the choice is a design one.

Write the restriction down, as part of what LogHandler is: Handle() must not synchronously call ForceFlush, Shutdown or Export on a provider or exporter involved in producing that record. That costs nothing at runtime and makes the current behaviour a stated contract rather than an accident.

Or make it safe, which means a flush entered from an export or a callback of the same exporter has to fail fast rather than wait. That is per exporter machinery for a case the interface never promised, and it would have to be added to each one, so it is worth deciding whether the promise is wanted before anybody writes it.

I have not sent a patch. Which of the two you want decides whether this is a documentation change or a change to every exporter that blocks.

Found while working through the ForceFlush accounting in #4337.