Skip to content

Commit e4c72a8

Browse files
committed
[BUG] End the Elasticsearch exporter's wait on a read or write error
ResponseHandler::OnEvent records a completion for each session state that ends the session, but ReadError and WriteError only log. A client that reports either of them as its last event leaves the synchronous Export() blocked on its condition variable with nothing left to wake it. The bundled curl client dispatches neither state, so this does not change behaviour for callers who use it. It matters for the HttpClient that the exporter's constructor accepts. The cases live in the existing synchronous fixture, which skips when the exporter is built for async export, and a CTest timeout bounds them because a regression here stalls the job rather than failing an assertion. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
1 parent 489cd82 commit e4c72a8

4 files changed

Lines changed: 348 additions & 13 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ Increment the:
7979
Note that `AsyncMetricStorage`'s constructor now takes the view's
8080
`AttributesProcessor`, matching `SyncMetricStorage`.
8181
[#1724](https://github.com/open-telemetry/opentelemetry-cpp/issues/1724)
82+
* [BUG] End the Elasticsearch exporter's wait on a read or write error
83+
[#4331](https://github.com/open-telemetry/opentelemetry-cpp/pull/4331)
8284

8385
* [BUG] Keep the curl retry deadline stable within each attempt
8486
[#4452](https://github.com/open-telemetry/opentelemetry-cpp/pull/4452)

‎exporters/elasticsearch/CMakeLists.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,8 @@ if(OTELCPP_BUILD_TESTING)
5555
TARGET es_log_record_exporter_test
5656
TEST_PREFIX exporter.
5757
TEST_LIST es_log_record_exporter_test)
58+
59+
# These cases exist to catch a wait that never returns. Without a per test
60+
# bound a regression stalls the job instead of failing it.
61+
set_tests_properties(${es_log_record_exporter_test} PROPERTIES TIMEOUT 30)
5862
endif() # OTELCPP_BUILD_TESTING

‎exporters/elasticsearch/src/es_log_record_exporter.cc‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ class ResponseHandler : public http_client::EventHandler
110110

111111
/**
112112
* A method the user calls to block their thread until the request has either produced a
113-
* response or failed. The longest duration is the timeout of the request, set by
114-
* SetTimeoutMs(), which arrives here as a TimedOut session event.
113+
* response or failed. It has no deadline of its own and relies on the HTTP client
114+
* reporting one of the terminal session states.
115115
*/
116116
bool waitForResponse()
117117
{
@@ -183,11 +183,15 @@ class ResponseHandler : public http_client::EventHandler
183183
OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Network error");
184184
recordCompletion(CompletionState::Failure);
185185
break;
186+
// Both of these end the session, so each has to release the waiter. Without that a
187+
// synchronous export whose transfer fails this way blocks until the process ends.
186188
case http_client::SessionState::ReadError:
187189
OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Read error");
190+
recordCompletion(CompletionState::Failure);
188191
break;
189192
case http_client::SessionState::WriteError:
190193
OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Write error");
194+
recordCompletion(CompletionState::Failure);
191195
break;
192196
case http_client::SessionState::Cancelled:
193197
OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] (manually) cancelled");

0 commit comments

Comments
 (0)