Skip to content

Commit 69473bc

Browse files
committed
Keep the I/O error logs at debug
Recording a completion is first writer wins, so a read or write error that arrives after a response has already succeeded does not change the result Export() reports. Logging it at error level announced a failure the caller was never told about. Destroyed is the existing precedent in this switch: it ends the wait the same way and stays at debug for the same reason. Covered by IoErrorAfterAResponseKeepsTheSuccess.
1 parent 4f56aa3 commit 69473bc

2 files changed

Lines changed: 22 additions & 2 deletions

File tree

‎exporters/elasticsearch/src/es_log_record_exporter.cc‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,14 @@ class ResponseHandler : public http_client::EventHandler
182182
recordCompletion(CompletionState::Failure);
183183
break;
184184
case http_client::SessionState::ReadError:
185-
OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Read error");
185+
OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Read error");
186+
// Kept at debug, like Destroyed above: recording is first writer wins, so this can
187+
// arrive after a response has already succeeded, and an error line there would not
188+
// describe the outcome Export() reports.
186189
recordCompletion(CompletionState::Failure);
187190
break;
188191
case http_client::SessionState::WriteError:
189-
OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Write error");
192+
OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Write error");
190193
recordCompletion(CompletionState::Failure);
191194
break;
192195
case http_client::SessionState::Cancelled:

‎exporters/elasticsearch/test/es_log_record_exporter_test.cc‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,3 +311,20 @@ TEST(ElasticsearchLogsExporterSyncTests, SessionDestroyedAfterAResponseKeepsTheS
311311
});
312312
EXPECT_EQ(result, opentelemetry::sdk::common::ExportResult::kSuccess);
313313
}
314+
315+
// Same rule for the two states this change makes terminal. It is why they log at debug rather
316+
// than error: reaching one of them says nothing about the result Export() goes on to report.
317+
TEST(ElasticsearchLogsExporterSyncTests, IoErrorAfterAResponseKeepsTheSuccess)
318+
{
319+
SKIP_WITHOUT_SYNC_EXPORT();
320+
for (const auto state : {http_client::SessionState::ReadError,
321+
http_client::SessionState::WriteError})
322+
{
323+
const auto result = ExportWith([state](http_client::EventHandler &handler) {
324+
FakeResponse response(200, kAcceptedBody);
325+
handler.OnResponse(response);
326+
handler.OnEvent(state, "");
327+
});
328+
EXPECT_EQ(result, opentelemetry::sdk::common::ExportResult::kSuccess);
329+
}
330+
}

0 commit comments

Comments
 (0)