[EXPORTER] Fix Elasticsearch log exporter Export blocking forever on a non-responding client

[om7057]

Fixes #4362.

The synchronous export path waited on its response condition variable with no deadline of its own, trusting the injected HttpClient to always eventually deliver a terminal event via OnResponse or OnEvent. Nothing in the HttpClient interface actually guarantees that: a client is a supported public surface, not a test seam, and one that accepts a request and never calls back (a dead thread, a reused socket, a swallowed error) left Export() blocked for the life of the process, with no way for a caller's Shutdown() to release it either.

Changes

• waitForResponse() now takes an absolute std::chrono::steady_clock::time_point deadline instead of waiting unconditionally, via cv_.wait_until() in place of cv_.wait().
• The deadline is derived from the exporter's own configured response_timeout_ and captured before SendRequest() is called, so it reflects this exporter's own timeout budget rather than whatever the client does with it.
• A deadline that passes without a terminal event leaves the completion state at Pending, which reads as failure, the same outcome a terminal error event already produces today. No successful path changes.

Testing

Added SilentHttpClient/SilentSession test doubles whose SendRequest() never calls back into the handler at all (no OnResponse, no OnEvent), the exact scenario the issue describes. ExportReturnsOnTimeoutWhenClientNeverResponds constructs the exporter with a 1 second response_timeout_ and this client, and asserts Export() returns kFailure rather than hanging.

Verified the test actually catches the regression: reverted the fix locally (kept the test) and reran under a timeout wrapper, the test hung and was killed at exit code 124 instead of passing vacuously. Restored the fix and confirmed all four tests in the file pass, total runtime 1 second (the deadline in the new test), not 30 (the default response_timeout_).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.52%. Comparing base (01dfc71) to head (295ec73).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4530      +/-   ##
==========================================
+ Coverage   86.51%   86.52%   +0.01%     
==========================================
  Files         525      525              
  Lines       20464    20475      +11     
==========================================
+ Hits        17702    17713      +11     
  Misses       2762     2762              

Files with missing lines	Coverage Δ
...orters/elasticsearch/src/es_log_record_exporter.cc	47.73% <ø> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mateenali66]

Ran this against the curl client with response_timeout = 1. A server that never replies fails the export at about 1s on both main and this PR, and replies at 990 to 999ms split the same way. So the change only matters for a custom client.

For that case, Export() still calls session->FinishSession() after the deadline (:495). SilentSession::FinishSession returns at once, so the test can't see it. A client built like curl waits there on the transfer (http_operation_curl.cc:523-538), so a dead worker thread still hangs Export(). curl's CancelSession() does not block (http_client_curl.cc:255-262). Should the expired path cancel instead?

The conflict is only CHANGELOG.md.

[om7057]

Good catch, and sorry for the delay replying. This was actually fixed same-day in 385ae3c7: the expired path now calls session->CancelSession() instead of session->FinishSession(), since FinishSession() waiting on the in-flight transfer is exactly the hang the deadline exists to bound for a client like curl.

Added ExportCancelsSessionOnTimeoutInsteadOfFinishing, which asserts CancelSession() is called and FinishSession() is not on a timed-out export. Should be covered now; let me know if there's a gap I'm missing.

[om7057]

cc: @dbarker @marcalff could you please check this one?

[mateenali66]

No gap in the timeout path, that one holds. Built the head and swapped the silent session for one that delivers a prompt ConnectFailed. It cancels too: 0 ms elapsed, cancel called, finish not, with response_timeout at 30.

The branch is on write_successful, not on deadline expiry, so ConnectFailed, SendFailed and CreateFailed take it as well. Those record Failure promptly with the transfer already over, and for curl the two calls differ. FinishSession waits on the result future, CancelSession aborts and does not. So three paths stop reaping the transfer, which is more than the comment above the branch claims.

Pending is already the signal. A deadline expiry leaves completion_ there, every other failure sets Failure. Worth having waitForResponse report which of the two it left on?

[om7057]

Confirmed, the branch was keyed on write_successful rather than on the deadline itself. Fixed in `295ec739`: waitForResponse() now reports via an out-param whether completion_ was still Pending when the deadline passed, and only that case cancels; a terminal failure (ConnectFailed, SendFailed, CreateFailed) finishes the session normally since the transfer is already over.

Added ExportFinishesSessionOnTerminalFailureInsteadOfCancelling with a FailFastSession that delivers ConnectFailed immediately, asserting FinishSession() runs and CancelSession() does not, and that the export returns well under the 30s response_timeout_.

On the waitForResponse() reporting which state it left on: went with an out-param (timed_out) rather than exposing CompletionState directly, so the caller only sees the one bit it needs. Let me know if you'd rather have the full state exposed for something else.