From f8eb29a45c818e4469399c15db9460f4dec4646d Mon Sep 17 00:00:00 2001 From: Om Kulkarni Date: Mon, 28 Sep 2026 08:04:33 +0530 Subject: [PATCH 1/2] [HTTP CLIENT] Remove dead SessionState::TimedOut No HttpClient implementation in this repo ever dispatched TimedOut. The curl client maps CURLE_OPERATION_TIMEDOUT to SendFailed like any other transport error, so the four branches handling TimedOut in the Elasticsearch and OTLP HTTP exporters were unreachable, and the doc comment claiming a timeout arrives as a TimedOut event was incorrect. Removed the dead branches and fixed the comment instead of making TimedOut reachable, since that would change behavior for any code already handling a timeout via SendFailed today. Fixes #4604 --- CHANGELOG.md | 15 +++++++++++++++ .../elasticsearch/src/es_log_record_exporter.cc | 11 ++--------- exporters/otlp/src/otlp_http_client.cc | 12 ------------ .../opentelemetry/ext/http/client/http_client.h | 1 - 4 files changed, 17 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68f5f6405f..8bd9fd23b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,21 @@ Increment the: ## [Unreleased] +* [HTTP CLIENT] Remove `SessionState::TimedOut`, which no `HttpClient` + implementation in this repo ever dispatched. The curl client maps + `CURLE_OPERATION_TIMEDOUT` to `SendFailed` like any other transport error, + so the four branches handling `TimedOut` in the Elasticsearch and OTLP HTTP + exporters were unreachable, and the doc comment claiming a timeout arrives + as a `TimedOut` event was incorrect. Removed the dead branches and fixed + the comment instead of making `TimedOut` reachable, to avoid changing + behavior for any code that already handles a timeout via `SendFailed`. + [#4604](https://github.com/open-telemetry/opentelemetry-cpp/issues/4604) + * `SessionState`'s remaining enumerators past `SSLHandshakeFailed` shift + down by one integer value. Source code that only names enumerators (the + normal way to use a scoped enum) is unaffected; only code that depends on + an enumerator's specific integer value would need attention, and no code + in this repo does. + * [LOGS] Fix undefined behavior in `Logger::EmitLogRecord()` when a logger is enabled (e.g. via `LoggerProvider::UpdateLoggerConfigurator()`) after `CreateLogRecord()` was called while it was still disabled. The disabled diff --git a/exporters/elasticsearch/src/es_log_record_exporter.cc b/exporters/elasticsearch/src/es_log_record_exporter.cc index cd3b1bdbfd..e370d4a525 100644 --- a/exporters/elasticsearch/src/es_log_record_exporter.cc +++ b/exporters/elasticsearch/src/es_log_record_exporter.cc @@ -111,7 +111,8 @@ class ResponseHandler : public http_client::EventHandler /** * A method the user calls to block their thread until the request has either produced a * response or failed. The longest duration is the timeout of the request, set by - * SetTimeoutMs(), which arrives here as a TimedOut session event. + * SetTimeoutMs(); the curl client reports that as a SendFailed session event, the same as any + * other transport failure, since it does not distinguish a timeout from other CURL errors. */ bool waitForResponse() { @@ -175,10 +176,6 @@ class ResponseHandler : public http_client::EventHandler OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Failed SSL Handshake"); recordCompletion(CompletionState::Failure); break; - case http_client::SessionState::TimedOut: - OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Request timed out"); - recordCompletion(CompletionState::Failure); - break; case http_client::SessionState::NetworkError: OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Network error"); recordCompletion(CompletionState::Failure); @@ -317,10 +314,6 @@ class AsyncResponseHandler : public http_client::EventHandler OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] SSL handshake to elasticsearch failed"); need_stop = true; break; - case http_client::SessionState::TimedOut: - OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Request to elasticsearch timed out"); - need_stop = true; - break; case http_client::SessionState::NetworkError: OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Network error to elasticsearch"); need_stop = true; diff --git a/exporters/otlp/src/otlp_http_client.cc b/exporters/otlp/src/otlp_http_client.cc index ee902e930a..7051e180a9 100644 --- a/exporters/otlp/src/otlp_http_client.cc +++ b/exporters/otlp/src/otlp_http_client.cc @@ -197,7 +197,6 @@ class ResponseHandler : public http_client::EventHandler case http_client::SessionState::ConnectFailed: case http_client::SessionState::SendFailed: case http_client::SessionState::SSLHandshakeFailed: - case http_client::SessionState::TimedOut: case http_client::SessionState::NetworkError: case http_client::SessionState::Destroyed: case http_client::SessionState::ReadError: @@ -300,17 +299,6 @@ class ResponseHandler : public http_client::EventHandler } break; - case http_client::SessionState::TimedOut: { - std::stringstream error_message; - error_message << "[OTLP HTTP Client] Session state: request time out."; - if (!reason.empty()) - { - error_message.write(reason.data(), static_cast(reason.size())); - } - OTEL_INTERNAL_LOG_ERROR(error_message.str()); - } - break; - case http_client::SessionState::NetworkError: { std::stringstream error_message; error_message << "[OTLP HTTP Client] Session state: network error."; diff --git a/ext/include/opentelemetry/ext/http/client/http_client.h b/ext/include/opentelemetry/ext/http/client/http_client.h index b477292fed..31f2e16f8b 100644 --- a/ext/include/opentelemetry/ext/http/client/http_client.h +++ b/ext/include/opentelemetry/ext/http/client/http_client.h @@ -90,7 +90,6 @@ enum class SessionState : std::uint8_t SendFailed, // request send failed Response, // response received SSLHandshakeFailed, // SSL handshake failed - TimedOut, // request time out NetworkError, // network error ReadError, // error reading response WriteError, // error writing request From b3151b9195141295b052c55a39d4b12a5959ea3a Mon Sep 17 00:00:00 2001 From: Om Kulkarni Date: Tue, 29 Sep 2026 20:40:43 +0530 Subject: [PATCH 2/2] Scope down to just the doc comment fix per review feedback Removing SessionState::TimedOut requires going through the project's formal deprecation process (docs/deprecation-process.md), since it is part of an installed public header and the event contract for any out-of-tree HttpClient implementation. Issue #4604 is also still needs-triage, not yet accepted for a specific direction. Reverted the enum removal and the four now-restored dead branches in the Elasticsearch and OTLP HTTP exporters. Kept only the doc comment fix on ResponseHandler::waitForResponse(), which was simply incorrect about a timeout arriving as a TimedOut event when it actually arrives as SendFailed, and needs no process to correct. Addresses review feedback from nikhilbhatia08. --- CHANGELOG.md | 19 ++++++------------- .../src/es_log_record_exporter.cc | 8 ++++++++ exporters/otlp/src/otlp_http_client.cc | 12 ++++++++++++ .../ext/http/client/http_client.h | 1 + 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8bd9fd23b1..af653846b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,20 +15,13 @@ Increment the: ## [Unreleased] -* [HTTP CLIENT] Remove `SessionState::TimedOut`, which no `HttpClient` - implementation in this repo ever dispatched. The curl client maps - `CURLE_OPERATION_TIMEDOUT` to `SendFailed` like any other transport error, - so the four branches handling `TimedOut` in the Elasticsearch and OTLP HTTP - exporters were unreachable, and the doc comment claiming a timeout arrives - as a `TimedOut` event was incorrect. Removed the dead branches and fixed - the comment instead of making `TimedOut` reachable, to avoid changing - behavior for any code that already handles a timeout via `SendFailed`. +* [EXPORTER] Fix a doc comment on the Elasticsearch log exporter's + `ResponseHandler::waitForResponse()` claiming a request timeout (set via + `SetTimeoutMs()`) arrives as a `SessionState::TimedOut` event. No + `HttpClient` implementation in this repo dispatches `TimedOut`; the curl + client maps a timeout to `SendFailed` like any other transport error, which + is what the comment now says. [#4604](https://github.com/open-telemetry/opentelemetry-cpp/issues/4604) - * `SessionState`'s remaining enumerators past `SSLHandshakeFailed` shift - down by one integer value. Source code that only names enumerators (the - normal way to use a scoped enum) is unaffected; only code that depends on - an enumerator's specific integer value would need attention, and no code - in this repo does. * [LOGS] Fix undefined behavior in `Logger::EmitLogRecord()` when a logger is enabled (e.g. via `LoggerProvider::UpdateLoggerConfigurator()`) after diff --git a/exporters/elasticsearch/src/es_log_record_exporter.cc b/exporters/elasticsearch/src/es_log_record_exporter.cc index e370d4a525..f5175ba56e 100644 --- a/exporters/elasticsearch/src/es_log_record_exporter.cc +++ b/exporters/elasticsearch/src/es_log_record_exporter.cc @@ -176,6 +176,10 @@ class ResponseHandler : public http_client::EventHandler OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Failed SSL Handshake"); recordCompletion(CompletionState::Failure); break; + case http_client::SessionState::TimedOut: + OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Request timed out"); + recordCompletion(CompletionState::Failure); + break; case http_client::SessionState::NetworkError: OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Network error"); recordCompletion(CompletionState::Failure); @@ -314,6 +318,10 @@ class AsyncResponseHandler : public http_client::EventHandler OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] SSL handshake to elasticsearch failed"); need_stop = true; break; + case http_client::SessionState::TimedOut: + OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Request to elasticsearch timed out"); + need_stop = true; + break; case http_client::SessionState::NetworkError: OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Network error to elasticsearch"); need_stop = true; diff --git a/exporters/otlp/src/otlp_http_client.cc b/exporters/otlp/src/otlp_http_client.cc index 7051e180a9..ee902e930a 100644 --- a/exporters/otlp/src/otlp_http_client.cc +++ b/exporters/otlp/src/otlp_http_client.cc @@ -197,6 +197,7 @@ class ResponseHandler : public http_client::EventHandler case http_client::SessionState::ConnectFailed: case http_client::SessionState::SendFailed: case http_client::SessionState::SSLHandshakeFailed: + case http_client::SessionState::TimedOut: case http_client::SessionState::NetworkError: case http_client::SessionState::Destroyed: case http_client::SessionState::ReadError: @@ -299,6 +300,17 @@ class ResponseHandler : public http_client::EventHandler } break; + case http_client::SessionState::TimedOut: { + std::stringstream error_message; + error_message << "[OTLP HTTP Client] Session state: request time out."; + if (!reason.empty()) + { + error_message.write(reason.data(), static_cast(reason.size())); + } + OTEL_INTERNAL_LOG_ERROR(error_message.str()); + } + break; + case http_client::SessionState::NetworkError: { std::stringstream error_message; error_message << "[OTLP HTTP Client] Session state: network error."; diff --git a/ext/include/opentelemetry/ext/http/client/http_client.h b/ext/include/opentelemetry/ext/http/client/http_client.h index 31f2e16f8b..b477292fed 100644 --- a/ext/include/opentelemetry/ext/http/client/http_client.h +++ b/ext/include/opentelemetry/ext/http/client/http_client.h @@ -90,6 +90,7 @@ enum class SessionState : std::uint8_t SendFailed, // request send failed Response, // response received SSLHandshakeFailed, // SSL handshake failed + TimedOut, // request time out NetworkError, // network error ReadError, // error reading response WriteError, // error writing request