From f889203855ac4ece92efac21a78e4b01ce7164b9 Mon Sep 17 00:00:00 2001
From: thc1006 <84045975+thc1006@users.noreply.github.com>
Date: Wed, 5 Aug 2026 05:53:37 +0800
Subject: [PATCH 01/12] [BUG] Decide Elasticsearch bulk export success from
 HTTP status and the errors flag

The exporter decided a bulk export had succeeded by searching the response body
for the substring "failed" : 0. That is a shard counter belonging to one item,
not a verdict on the batch, so a body that never contained it read as a failure
and a body that contained it anywhere read as a success.

Decide from the documented contract instead: a 2xx HTTP status, a top level
"errors" flag that is false, and one acknowledged operation result per record
submitted. Each entry of "items" has to be the result of the index operation the
exporter actually sent, carrying an integer status in the 2xx band, so a well
formed body that acknowledges nothing cannot pass. When "errors" is true the
first rejected item names the status in the log.

The status is compared in the type it was parsed as. Reading it through an int
first is not safe: the value comes from the server, is_number_integer() holds
for unsigned as well, and 2^32 + 200 narrows back into the 2xx band on a 32 bit
int. Comparing directly also removes the need for a rejected-status sentinel,
which could not tell a real status of 0 apart from "nothing was rejected".

The parser lives in detail/es_bulk_response.h, excluded from both the CMake
install set and the Bazel public headers.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
---
 CHANGELOG.md                                  |   4 +
 exporters/elasticsearch/BUILD                 |  20 +
 exporters/elasticsearch/CMakeLists.txt        |   6 +
 .../elasticsearch/detail/es_bulk_response.h   | 171 ++++++
 .../src/es_log_record_exporter.cc             |  90 ++--
 .../test/es_log_record_exporter_test.cc       | 501 ++++++++++++++++++
 6 files changed, 752 insertions(+), 40 deletions(-)
 create mode 100644 exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fc64aaef23..e7282c8052 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -150,6 +150,10 @@ Increment the:
 * [BUG] Wait on a completion state in the Elasticsearch exporter
   [#4298](https://github.com/open-telemetry/opentelemetry-cpp/pull/4298)
 
+* [BUG] Decide Elasticsearch bulk export success from the HTTP status, the
+  errors flag, and one acknowledgement per record
+  [#4297](https://github.com/open-telemetry/opentelemetry-cpp/pull/4297)
+
 * [BUG] Make SocketAddr string parsing safe and reject malformed addresses
   [#4292](https://github.com/open-telemetry/opentelemetry-cpp/pull/4292)
 
diff --git a/exporters/elasticsearch/BUILD b/exporters/elasticsearch/BUILD
index 71ff5f39d2..967aad14d8 100644
--- a/exporters/elasticsearch/BUILD
+++ b/exporters/elasticsearch/BUILD
@@ -6,6 +6,24 @@ load("@rules_cc//cc:cc_test.bzl", "cc_test")
 
 package(default_visibility = ["//visibility:public"])
 
+# The bulk response parser is an implementation detail that the CMake package
+# also leaves out. hdrs is a target's public interface, so it is kept in its own
+# target that only this package can depend on, and reached through
+# implementation_deps so the exporter does not re-export it.
+cc_library(
+    name = "es_bulk_response",
+    hdrs = [
+        "include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h",
+    ],
+    strip_include_prefix = "include",
+    tags = ["es"],
+    visibility = ["//exporters/elasticsearch:__pkg__"],
+    deps = [
+        "//api",
+        "@github_nlohmann_json//:json",
+    ],
+)
+
 cc_library(
     name = "es_log_record_exporter",
     srcs = [
@@ -16,6 +34,7 @@ cc_library(
         "include/opentelemetry/exporters/elasticsearch/es_log_record_exporter.h",
         "include/opentelemetry/exporters/elasticsearch/es_log_recordable.h",
     ],
+    implementation_deps = [":es_bulk_response"],
     linkopts = select({
         "//bazel:windows": [
             "-DEFAULTLIB:advapi32.lib",
@@ -43,6 +62,7 @@ cc_test(
         "test",
     ],
     deps = [
+        ":es_bulk_response",
         ":es_log_record_exporter",
         "@com_google_googletest//:gtest_main",
         "@curl",
diff --git a/exporters/elasticsearch/CMakeLists.txt b/exporters/elasticsearch/CMakeLists.txt
index cfd4ce01db..290691f4d9 100644
--- a/exporters/elasticsearch/CMakeLists.txt
+++ b/exporters/elasticsearch/CMakeLists.txt
@@ -34,6 +34,12 @@ otel_add_component(
   "*.h"
   PATTERN
   "es_log_recordable.h"
+  EXCLUDE
+  PATTERN
+  "es_bulk_response.h"
+  EXCLUDE
+  PATTERN
+  "detail"
   EXCLUDE)
 
 if(OPENTELEMETRY_INSTALL)
diff --git a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
new file mode 100644
index 0000000000..39a9a242f2
--- /dev/null
+++ b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
@@ -0,0 +1,171 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+#pragma once
+
+#include <cstddef>
+#include <nlohmann/json.hpp>
+#include <string>
+
+#include "opentelemetry/version.h"
+
+OPENTELEMETRY_BEGIN_NAMESPACE
+namespace exporter
+{
+namespace logs
+{
+namespace detail
+{
+
+/**
+ * Whether an acknowledged operation status is one that applied the operation. Elasticsearch
+ * answers an index operation with 200 or 201, so 2xx is the whole band.
+ *
+ * Compared in the type the number was parsed as. Narrowing to int first is not safe here: the
+ * value comes from the server, is_number_integer() is true for unsigned as well, and 2^32 + 200
+ * narrows back into the band on a 32 bit int.
+ */
+inline bool IsAcknowledgedStatus(const nlohmann::json &status) noexcept
+{
+  if (status.is_number_unsigned())
+  {
+    const auto value = status.get<nlohmann::json::number_unsigned_t>();
+    return value >= 200U && value <= 299U;
+  }
+
+  const auto value = status.get<nlohmann::json::number_integer_t>();
+  return value >= 200 && value <= 299;
+}
+
+/**
+ * Decide whether an Elasticsearch bulk response reports the whole batch as written.
+ *
+ * Callers include noexcept response handlers, so nothing may escape. Anything that stops the body
+ * being inspected counts as a failed export.
+ *
+ * @param status_code the response status, which the caller passes rather than this reading the
+ *        body alone: "errors" describes item outcomes and cannot override a transport error
+ * @param body the raw response body
+ * @param expected_items the number of index operations the request submitted
+ * @param failure_reason a best-effort explanation when this returns false
+ * @return true only when the status is 2xx, "errors" is false, and "items" holds exactly
+ *         expected_items index results that each acknowledge a 2xx status
+ */
+inline bool IsBulkResponseSuccessful(int status_code,
+                                     const std::string &body,
+                                     std::size_t expected_items,
+                                     std::string &failure_reason) noexcept
+{
+  failure_reason.clear();
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+  try
+  {
+#endif
+    // Inside the try: building a reason allocates.
+    if (status_code < 200 || status_code > 299)
+    {
+      failure_reason = "unexpected HTTP status " + std::to_string(status_code);
+      return false;
+    }
+
+    const nlohmann::json parsed = nlohmann::json::parse(body, nullptr, false);
+    if (parsed.is_discarded() || !parsed.is_object())
+    {
+      failure_reason = "the response body is not a JSON object";
+      return false;
+    }
+
+    const auto errors = parsed.find("errors");
+    if (errors == parsed.end() || !errors->is_boolean())
+    {
+      failure_reason = "the response body has no boolean \"errors\" field";
+      return false;
+    }
+
+    // The request is an unfiltered /_bulk, so "items" is always there and holds one entry per
+    // operation. The next three checks decide whether this body answers the request that was sent.
+    const auto items = parsed.find("items");
+    if (items == parsed.end() || !items->is_array())
+    {
+      failure_reason = "the response body has no \"items\" array";
+      return false;
+    }
+
+    if (items->size() != expected_items)
+    {
+      failure_reason = "the response acknowledges " + std::to_string(items->size()) + " of " +
+                       std::to_string(expected_items) + " submitted operations";
+      return false;
+    }
+
+    // Each entry is keyed by the action it answers, and every record goes out as an index
+    // operation. Read before "errors", which a body that is not this answer does not get to decide.
+    const nlohmann::json *rejected = nullptr;
+    for (const auto &item : *items)
+    {
+      if (!item.is_object() || item.size() != 1)
+      {
+        failure_reason = "the response has an \"items\" entry that is not one operation result";
+        return false;
+      }
+
+      const auto operation = item.find("index");
+      if (operation == item.end() || !operation->is_object())
+      {
+        failure_reason = "the response does not acknowledge the submitted index operation";
+        return false;
+      }
+
+      const auto status = operation->find("status");
+      if (status == operation->end() || !status->is_number_integer())
+      {
+        failure_reason = "the response acknowledges an index operation with no status";
+        return false;
+      }
+
+      if (rejected == nullptr && !IsAcknowledgedStatus(*status))
+      {
+        rejected = &(*status);
+      }
+    }
+
+    if (!errors->get<bool>())
+    {
+      // A false flag claims every operation was applied; hold the items to that claim.
+      if (rejected != nullptr)
+      {
+        failure_reason =
+            "the response reports no errors but acknowledges operation status " + rejected->dump();
+        return false;
+      }
+      return true;
+    }
+
+    // Name the first item error rather than only saying that something failed. Every entry is one
+    // index result by now, so its single member is the result to look in.
+    for (const auto &item : *items)
+    {
+      const auto &result = *item.begin();
+      const auto error   = result.find("error");
+      if (error != result.end())
+      {
+        failure_reason = "at least one item failed, first error: " + error->dump();
+        return false;
+      }
+    }
+
+    failure_reason = "the response reports errors";
+    return false;
+#if OPENTELEMETRY_HAVE_EXCEPTIONS
+  }
+  catch (...)
+  {
+    return false;
+  }
+#endif
+}
+
+}  // namespace detail
+}  // namespace logs
+}  // namespace exporter
+OPENTELEMETRY_END_NAMESPACE
diff --git a/exporters/elasticsearch/src/es_log_record_exporter.cc b/exporters/elasticsearch/src/es_log_record_exporter.cc
index af819c8eb7..2cae327dcc 100644
--- a/exporters/elasticsearch/src/es_log_record_exporter.cc
+++ b/exporters/elasticsearch/src/es_log_record_exporter.cc
@@ -14,6 +14,7 @@
 #include <utility>
 #include <vector>
 
+#include "opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h"
 #include "opentelemetry/exporters/elasticsearch/es_log_record_exporter.h"
 #include "opentelemetry/exporters/elasticsearch/es_log_recordable.h"
 #include "opentelemetry/ext/http/client/detail/default_factory.h"
@@ -75,35 +76,29 @@ class ResponseHandler : public http_client::EventHandler
    */
   void OnResponse(http_client::Response &response) noexcept override
   {
-    std::string log_message;
-
     // Lock the private members so they can't be read while being modified
     {
       std::unique_lock<std::mutex> lk(mutex_);
 
-      // Store the body of the request
-      body_ = std::string(response.GetBody().begin(), response.GetBody().end());
-
-      if (!(response.GetStatusCode() >= 200 && response.GetStatusCode() <= 299))
-      {
-        log_message = BuildResponseLogMessage(response, body_);
-
-        OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Export failed, " << log_message);
-      }
+      std::string body(response.GetBody().begin(), response.GetBody().end());
 
       if (console_debug_)
       {
-        if (log_message.empty())
-        {
-          log_message = BuildResponseLogMessage(response, body_);
-        }
-
         OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Got response from Elasticsearch, "
-                                << log_message);
+                                << BuildResponseLogMessage(response, body));
       }
 
-      // Record the outcome and notify any threads waiting on this result
-      recordCompletionLocked(CompletionState::Success);
+      // Kept with the outcome it decides, and only by the call that decides it. Export() folds the
+      // status into the ExportResult, since storing the body alone let a non-2xx response be
+      // reported as a success, and a status from one response beside a body from another is not
+      // something the server sent. Export() is the single place that logs the failure, so a
+      // non-2xx response is not logged a second time here with the full body.
+      if (completion_ == CompletionState::Pending)
+      {
+        status_code_ = response.GetStatusCode();
+        body_        = std::move(body);
+        recordCompletionLocked(CompletionState::Success);
+      }
     }
     cv_.notify_all();
   }
@@ -122,14 +117,21 @@ class ResponseHandler : public http_client::EventHandler
     return completion_ == CompletionState::Success;
   }
 
+  /// The status and body of the response the recorded outcome was decided from.
+  struct Response
+  {
+    int status_code = 0;
+    std::string body;
+  };
+
   /**
-   * Returns the body of the response
+   * Returns the response the outcome was decided from. Both halves together, since reading them
+   * one lock at a time can pair a status with a body that arrived in a different response.
    */
-  std::string GetResponseBody()
+  Response GetResponse()
   {
-    // Lock so that body_ can't be written to while returning it
     std::unique_lock<std::mutex> lk(mutex_);
-    return body_;
+    return Response{status_code_, body_};
   }
 
   // Callback method when an http event occurs
@@ -236,6 +238,9 @@ class ResponseHandler : public http_client::EventHandler
   // A string to store the response body
   std::string body_ = "";
 
+  // The HTTP status code of the response
+  int status_code_ = 0;
+
   // Whether to print the results from the callback
   bool console_debug_ = false;
 };
@@ -253,9 +258,11 @@ class AsyncResponseHandler : public http_client::EventHandler
   AsyncResponseHandler(
       std::shared_ptr<ext::http::client::Session> session,
       std::function<bool(opentelemetry::sdk::common::ExportResult)> &&result_callback,
+      std::size_t submitted_operations,
       bool console_debug = false)
       : session_{std::move(session)},
         result_callback_{std::move(result_callback)},
+        submitted_operations_{submitted_operations},
         console_debug_{console_debug}
   {}
 
@@ -275,18 +282,20 @@ class AsyncResponseHandler : public http_client::EventHandler
   void OnResponse(http_client::Response &response) noexcept override
   {
 
-    // Store the body of the response
-    body_ = std::string(response.GetBody().begin(), response.GetBody().end());
+    // Local, since nothing outside this call reads it and a member would be written twice over by
+    // a client that delivers two responses for one request.
+    const std::string body(response.GetBody().begin(), response.GetBody().end());
     if (console_debug_)
     {
       OTEL_INTERNAL_LOG_DEBUG(
-          "[ES Log Exporter] Got response from Elasticsearch,  response body: " << body_);
+          "[ES Log Exporter] Got response from Elasticsearch,  response body: " << body);
     }
-    if (body_.find("\"failed\" : 0") == std::string::npos)
+    std::string failure_reason;
+    if (!detail::IsBulkResponseSuccessful(response.GetStatusCode(), body, submitted_operations_,
+                                          failure_reason))
     {
-      OTEL_INTERNAL_LOG_ERROR(
-          "[ES Log Exporter] Logs were not written to Elasticsearch correctly, response body: "
-          << body_);
+      OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Logs were not written to Elasticsearch correctly, "
+                              << failure_reason << ", response body: " << body);
       result_callback_(sdk::common::ExportResult::kFailure);
     }
     else
@@ -344,8 +353,8 @@ class AsyncResponseHandler : public http_client::EventHandler
   // Callback to call to on receiving events
   std::function<bool(opentelemetry::sdk::common::ExportResult)> result_callback_;
 
-  // A string to store the response body
-  std::string body_ = "";
+  // How many operations this request submitted, which is how many the response has to answer
+  std::size_t submitted_operations_ = 0;
 
   // Whether to print the results from the callback
   bool console_debug_ = false;
@@ -406,7 +415,7 @@ sdk::common::ExportResult ElasticsearchLogRecordExporter::Export(
   auto request = session->CreateRequest();
 
   // Populate the request with headers and methods
-  request->SetUri(options_.index_ + "/_bulk?pretty");
+  request->SetUri(options_.index_ + "/_bulk");
   request->SetMethod(http_client::Method::Post);
   request->AddHeader("Content-Type", "application/json");
 
@@ -423,7 +432,7 @@ sdk::common::ExportResult ElasticsearchLogRecordExporter::Export(
   for (auto &record : records)
   {
     // Append {"index":{}} before JSON body, which tells Elasticsearch to write to index specified
-    // in URI
+    // in URI. detail/es_bulk_response.h requires each acknowledgement to name this same action.
     body += "{\"index\" : {}}\n";
 
     // Add the context of the Recordable
@@ -458,7 +467,7 @@ sdk::common::ExportResult ElasticsearchLogRecordExporter::Export(
         synchronization_data->force_flush_cv.notify_all();
         return true;
       },
-      options_.console_debug_);
+      span_count, options_.console_debug_);
   session->SendRequest(handler);
   return sdk::common::ExportResult::kSuccess;
 #else
@@ -485,12 +494,13 @@ sdk::common::ExportResult ElasticsearchLogRecordExporter::Export(
   }
 
   // Parse the response output to determine if Elasticsearch consumed it correctly
-  std::string responseBody = handler->GetResponseBody();
-  if (responseBody.find("\"failed\" : 0") == std::string::npos)
+  const auto response = handler->GetResponse();
+  std::string failure_reason;
+  if (!detail::IsBulkResponseSuccessful(response.status_code, response.body, records.size(),
+                                        failure_reason))
   {
-    OTEL_INTERNAL_LOG_ERROR(
-        "[ES Log Exporter] Logs were not written to Elasticsearch correctly, response body: "
-        << responseBody);
+    OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Logs were not written to Elasticsearch correctly, "
+                            << failure_reason << ", response body: " << response.body);
     // TODO: Retry logic
     return sdk::common::ExportResult::kFailure;
   }
diff --git a/exporters/elasticsearch/test/es_log_record_exporter_test.cc b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
index a65c0b4c1c..685015bb70 100644
--- a/exporters/elasticsearch/test/es_log_record_exporter_test.cc
+++ b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
@@ -3,12 +3,17 @@
 
 #include "opentelemetry/exporters/elasticsearch/es_log_record_exporter.h"
 #include "opentelemetry/common/timestamp.h"
+#include "opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h"
 #include "opentelemetry/exporters/elasticsearch/es_log_recordable.h"
+#include "opentelemetry/ext/http/client/http_client.h"
 #include "opentelemetry/logs/severity.h"
+#include "opentelemetry/nostd/function_ref.h"
+#include "opentelemetry/nostd/shared_ptr.h"
 #include "opentelemetry/nostd/span.h"
 #include "opentelemetry/nostd/string_view.h"
 #include "opentelemetry/nostd/utility.h"
 #include "opentelemetry/sdk/common/exporter_utils.h"
+#include "opentelemetry/sdk/common/global_log_handler.h"
 #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h"
 #include "opentelemetry/sdk/logs/exporter.h"
 #include "opentelemetry/sdk/logs/recordable.h"
@@ -19,8 +24,10 @@
 #include <chrono>
 #include <cstddef>
 #include <cstdint>
+#include <functional>
 #include <string>
 #include <utility>
+#include <vector>
 #include "nlohmann/json.hpp"
 
 namespace sdklogs       = opentelemetry::sdk::logs;
@@ -142,3 +149,497 @@ TEST(ElasticsearchLogRecordableTests, BasicTests)
 
   EXPECT_EQ(actual, expected);
 }
+
+// The batch outcome is the top level "errors" field. Two success bodies that differ only in
+// whitespace, so a check that depends on formatting cannot answer the same for both.
+namespace
+{
+constexpr const char *kPrettySuccess =
+    R"({"took":30,"errors":false,"items":[{"index":{"_index":"logs","_id":"1",)"
+    R"("_shards":{"total":2,"successful":1,"failed" : 0},"status":201}}]})";
+constexpr const char *kCompactSuccess =
+    R"({"took":30,"errors":false,"items":[{"index":{"_index":"logs","_id":"1",)"
+    R"("_shards":{"total":2,"successful":1,"failed":0},"status":201}}]})";
+constexpr const char *kOneItemRejected =
+    R"({"took":30,"errors":true,"items":[{"index":{"_index":"logs","_id":"1",)"
+    R"("_shards":{"failed" : 0},"status":201}},{"index":{"_index":"logs","_id":"2",)"
+    R"("status":400,"error":{"type":"mapper_parsing_exception","reason":"bad field"}}}]})";
+}  // namespace
+
+TEST(ElasticsearchBulkResponseTests, ReportsSuccessWhenErrorsIsFalse)
+{
+  std::string reason;
+  EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(200, kPrettySuccess, 1, reason));
+  EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(200, kCompactSuccess, 1, reason));
+}
+
+// A successful check must not leave a stale failure reason from a previous call behind.
+TEST(ElasticsearchBulkResponseTests, ClearsFailureReasonOnSuccess)
+{
+  std::string reason = "stale";
+  EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(200, kCompactSuccess, 1, reason));
+  EXPECT_TRUE(reason.empty());
+}
+
+TEST(ElasticsearchBulkResponseTests, ReportsFailureWhenAnyItemWasRejected)
+{
+  std::string reason;
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(200, kOneItemRejected, 2, reason));
+  EXPECT_NE(reason.find("mapper_parsing_exception"), std::string::npos);
+}
+
+TEST(ElasticsearchBulkResponseTests, ReportsFailureOnUnusableBody)
+{
+  std::string reason;
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(200, "not json at all", 1, reason));
+  EXPECT_FALSE(reason.empty());
+
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(200, "", 1, reason));
+  EXPECT_FALSE(reason.empty());
+
+  // An array rather than the expected object.
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(200, "[1,2,3]", 1, reason));
+  EXPECT_FALSE(reason.empty());
+}
+
+TEST(ElasticsearchBulkResponseTests, ReportsFailureWhenErrorsFieldIsMissingOrNotBoolean)
+{
+  std::string reason;
+  EXPECT_FALSE(
+      logs_exporter::detail::IsBulkResponseSuccessful(200, R"({"took":30,"items":[]})", 1, reason));
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(
+      200, R"({"errors":"false","items":[]})", 0, reason));
+}
+
+// A non-2xx status is a failure even when the body reports errors:false. This is the invariant
+// the substring check and the body-only check both missed, on both the sync and async paths.
+TEST(ElasticsearchBulkResponseTests, ReportsFailureOnNon2xxStatus)
+{
+  std::string reason;
+  const char *ok_body = R"({"errors":false,"items":[]})";
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(500, ok_body, 0, reason));
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(429, ok_body, 0, reason));
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(400, ok_body, 0, reason));
+  EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(200, ok_body, 0, reason));
+}
+
+// errors:true with no extractable item error still fails, with the generic reason.
+TEST(ElasticsearchBulkResponseTests, ReportsFailureWhenErrorsTrueWithoutItemError)
+{
+  std::string reason;
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(200, R"({"errors":true,"items":[]})",
+                                                               0, reason));
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(
+      200, R"({"errors":true,"items":[null]})", 1, reason));
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(
+      200, R"({"errors":true,"items":[{"index":42}]})", 1, reason));
+}
+
+// The 2xx range is the success band; the codes just outside it are failures. This pins the boundary
+// so a later change to the status check cannot silently widen or narrow it.
+TEST(ElasticsearchBulkResponseTests, TreatsThe2xxRangeAsTheSuccessBand)
+{
+  std::string reason;
+  const char *ok_body = R"({"errors":false,"items":[]})";
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(199, ok_body, 0, reason));
+  EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(200, ok_body, 0, reason));
+  EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(299, ok_body, 0, reason));
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(300, ok_body, 0, reason));
+}
+
+// The exporter posts an unfiltered /_bulk, so Elasticsearch answers every submitted operation in
+// "items". A body that does not is not an answer to that request, and treating it as success would
+// hand the caller a write acknowledgement nobody made.
+TEST(ElasticsearchBulkResponseTests, RequiresAnAcknowledgementForEverySubmittedOperation)
+{
+  std::string reason;
+  constexpr const char *kNoItems   = R"({"errors":false})";
+  constexpr const char *kNullItems = R"({"errors":false,"items":null})";
+  constexpr const char *kOneItemBody =
+      R"({"errors":false,"items":[{"index":{"_shards":{"failed":0},"status":201}}]})";
+  constexpr const char *kTwoItemBody =
+      R"({"errors":false,"items":[{"index":{"status":201}},{"index":{"status":201}}]})";
+
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(200, kNoItems, 1, reason));
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(200, kNullItems, 1, reason));
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(200, kOneItemBody, 2, reason))
+      << "too few acknowledgements";
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(200, kTwoItemBody, 1, reason))
+      << "more acknowledgements than operations";
+  EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(200, kOneItemBody, 1, reason));
+  EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(200, kTwoItemBody, 2, reason));
+}
+
+// The right count of the wrong entries is still the wrong answer, and the verdict cannot depend on
+// "errors", which the same responder controls.
+TEST(ElasticsearchBulkResponseTests, RejectsItemsEntriesThatDoNotAcknowledgeAnIndexOperation)
+{
+  std::string reason;
+  const auto rejected = [&reason](const char *body, std::size_t expected) {
+    return !logs_exporter::detail::IsBulkResponseSuccessful(200, body, expected, reason);
+  };
+
+  EXPECT_TRUE(rejected(R"({"errors":false,"items":[null,null]})", 2));
+  EXPECT_FALSE(reason.empty());
+  EXPECT_TRUE(rejected(R"({"errors":false,"items":[1,2]})", 2));
+  EXPECT_TRUE(rejected(R"({"errors":false,"items":[[]]})", 1));
+  EXPECT_TRUE(rejected(R"({"errors":false,"items":[{}]})", 1)) << "no operation at all";
+  EXPECT_TRUE(rejected(R"({"errors":false,"items":[{"unknown":{"status":201}}]})", 1))
+      << "an operation the exporter never submitted";
+  EXPECT_TRUE(rejected(R"({"errors":false,"items":[{"index":42}]})", 1))
+      << "the index key holds no result object";
+  EXPECT_TRUE(rejected(R"({"errors":false,"items":[{"index":{},"delete":{}}]})", 1))
+      << "one entry cannot answer two operations";
+  EXPECT_TRUE(rejected(R"({"errors":false,"items":[{"index":{"status":201},"delete":{}}]})", 1))
+      << "a valid acknowledgement does not license a second member";
+  EXPECT_TRUE(rejected(R"({"errors":false,"items":[{"index":{"_index":"logs"}}]})", 1))
+      << "an index result carrying no status";
+  EXPECT_TRUE(rejected(R"({"errors":true,"items":[null,null]})", 2))
+      << "the same shape has to fail whichever way errors reads";
+
+  // The shape a real acknowledgement has.
+  EXPECT_FALSE(rejected(R"({"errors":false,"items":[{"index":{"status":201}}]})", 1));
+
+  // Three shapes, three reasons: they are different things to go and look at.
+  const auto reason_for = [&reason](const char *body) {
+    logs_exporter::detail::IsBulkResponseSuccessful(200, body, 1, reason);
+    return reason;
+  };
+  const std::string padded  = reason_for(R"({"errors":false,"items":[{"index":{},"delete":{}}]})");
+  const std::string other   = reason_for(R"({"errors":false,"items":[{"unknown":{"status":1}}]})");
+  const std::string no_stat = reason_for(R"({"errors":false,"items":[{"index":{"_index":"l"}}]})");
+  EXPECT_NE(padded, other) << padded;
+  EXPECT_NE(other, no_stat) << other;
+  EXPECT_NE(padded, no_stat) << no_stat;
+}
+
+// The status says whether the operation applied, so the band has to hold for the number the server
+// sent rather than for what it becomes on the way into an int.
+TEST(ElasticsearchBulkResponseTests, RejectsAcknowledgementsOutsideTheSuccessBand)
+{
+  std::string reason;
+  const auto applied = [&reason](const char *status) {
+    const std::string body =
+        std::string(R"({"errors":false,"items":[{"index":{"status":)") + status + R"(}}]})";
+    return logs_exporter::detail::IsBulkResponseSuccessful(200, body, 1, reason);
+  };
+
+  EXPECT_TRUE(applied("200"));
+  EXPECT_TRUE(applied("201"));
+  EXPECT_TRUE(applied("299"));
+
+  EXPECT_FALSE(applied("0")) << "zero is a status, not the absence of one";
+  EXPECT_FALSE(reason.empty());
+  EXPECT_FALSE(applied("-0"));
+  EXPECT_FALSE(applied("-1"));
+  EXPECT_FALSE(applied("199"));
+  EXPECT_FALSE(applied("300"));
+  EXPECT_FALSE(applied("4294967496")) << "2^32 + 200, which lands on 200 in a 32 bit int";
+  EXPECT_FALSE(applied("18446744073709551615")) << "no signed type holds it";
+  EXPECT_FALSE(applied("200.5")) << "not an integer";
+  EXPECT_FALSE(applied(R"("200")")) << "a string is not a status";
+  EXPECT_FALSE(applied("null"));
+
+  // The value reaches the log as it arrived, so a reader sees what the server said.
+  applied("4294967496");
+  EXPECT_NE(reason.find("4294967496"), std::string::npos) << reason;
+}
+
+// A false flag claims every operation was applied. No conforming server sends it alongside a
+// rejection, so holding the items to the claim rejects nothing the flag alone would have taken.
+TEST(ElasticsearchBulkResponseTests, RejectsAnAcknowledgedFailureUnderErrorsFalse)
+{
+  std::string reason;
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(
+      200, R"({"errors":false,"items":[{"index":{"status":400}}]})", 1, reason));
+  EXPECT_NE(reason.find("400"), std::string::npos) << "the reason names the status: " << reason;
+
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(
+      200, R"({"errors":false,"items":[{"index":{"status":201}},{"index":{"status":429}}]})", 2,
+      reason))
+      << "one rejected operation among several is still a rejection";
+
+  // The whole 2xx band is a success, the same band the response's own HTTP status uses.
+  EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(
+      200, R"({"errors":false,"items":[{"index":{"status":200}},{"index":{"status":299}}]})", 2,
+      reason));
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(
+      200, R"({"errors":false,"items":[{"index":{"status":300}}]})", 1, reason));
+}
+// ---------------------------------------------------------------------------
+// The response travelling from the HTTP callback to the verdict.
+//
+// The cases above call IsBulkResponseSuccessful() directly, so they cannot show
+// that the status and the body actually reach it. A fake HTTP client drives the
+// callbacks from inside SendRequest(), which runs before Export() reaches the
+// wait, and the assertions are on the ExportResult.
+//
+// This fixture is the same one #4331 adds to this file. Whichever lands first,
+// the other drops the duplicate when it rebases.
+// ---------------------------------------------------------------------------
+namespace
+{
+namespace http_client = opentelemetry::ext::http::client;
+
+class FakeResponse : public http_client::Response
+{
+public:
+  FakeResponse(http_client::StatusCode status, const std::string &body)
+      : status_(status), body_(body.begin(), body.end())
+  {}
+  const http_client::Body &GetBody() const noexcept override { return body_; }
+  bool ForEachHeader(
+      nostd::function_ref<bool(nostd::string_view, nostd::string_view)>) const noexcept override
+  {
+    return true;
+  }
+  bool ForEachHeader(
+      const nostd::string_view &,
+      nostd::function_ref<bool(nostd::string_view, nostd::string_view)>) const noexcept override
+  {
+    return true;
+  }
+  http_client::StatusCode GetStatusCode() const noexcept override { return status_; }
+
+private:
+  http_client::StatusCode status_;
+  http_client::Body body_;
+};
+
+class FakeRequest : public http_client::Request
+{
+public:
+  void SetMethod(http_client::Method) noexcept override {}
+  void SetUri(nostd::string_view) noexcept override {}
+  void SetSslOptions(const http_client::HttpSslOptions &) noexcept override {}
+  void SetBody(http_client::Body &) noexcept override {}
+  void AddHeader(nostd::string_view, nostd::string_view) noexcept override {}
+  void ReplaceHeader(nostd::string_view, nostd::string_view) noexcept override {}
+  void SetTimeoutMs(std::chrono::milliseconds) noexcept override {}
+  void SetCompression(const http_client::Compression &) noexcept override {}
+  void EnableLogging(bool) noexcept override {}
+  void SetRetryPolicy(const http_client::RetryPolicy &) noexcept override {}
+};
+
+using EventScript = std::function<void(http_client::EventHandler &)>;
+
+class FakeSession : public http_client::Session
+{
+public:
+  explicit FakeSession(EventScript script) : script_(std::move(script)) {}
+  std::shared_ptr<http_client::Request> CreateRequest() noexcept override
+  {
+    return std::make_shared<FakeRequest>();
+  }
+  void SendRequest(std::shared_ptr<http_client::EventHandler> handler) noexcept override
+  {
+    script_(*handler);
+  }
+  bool IsSessionActive() noexcept override { return false; }
+  bool CancelSession() noexcept override { return true; }
+  bool FinishSession() noexcept override { return true; }
+
+private:
+  EventScript script_;
+};
+
+class FakeHttpClient : public http_client::HttpClient
+{
+public:
+  explicit FakeHttpClient(EventScript script) : script_(std::move(script)) {}
+  std::shared_ptr<http_client::Session> CreateSession(nostd::string_view) noexcept override
+  {
+    return std::make_shared<FakeSession>(script_);
+  }
+  bool CancelAllSessions() noexcept override { return true; }
+  bool FinishAllSessions() noexcept override { return true; }
+  void SetMaxSessionsPerConnection(std::size_t) noexcept override {}
+
+private:
+  EventScript script_;
+};
+
+// The response has to answer one operation per record, so a case using a body with N items has to
+// export N records or it would be rejected on the count before reaching what it means to test.
+opentelemetry::sdk::common::ExportResult ExportWith(EventScript script, std::size_t records = 1)
+{
+  auto client = std::make_shared<FakeHttpClient>(std::move(script));
+  logs_exporter::ElasticsearchExporterOptions options;
+  logs_exporter::ElasticsearchLogRecordExporter exporter(options, client);
+
+  std::vector<std::unique_ptr<sdklogs::Recordable>> batch;
+  batch.reserve(records);
+  for (std::size_t i = 0; i < records; ++i)
+  {
+    batch.push_back(exporter.MakeRecordable());
+  }
+  return exporter.Export(
+      nostd::span<std::unique_ptr<sdklogs::Recordable>>(batch.data(), batch.size()));
+}
+}  // namespace
+
+// The synchronous wait exists only when the exporter is built without async export, so these cases
+// skip rather than compile out: gtest_add_tests reads the source, and a case that disappeared from
+// the binary would still be registered with CTest. The skip goes in SetUp rather than at the top of
+// each body, because GTEST_SKIP returns and leaves the rest of the body unreachable, which MSVC
+// reports as C4702 and the maintainer mode jobs turn into an error.
+namespace
+{
+class ElasticsearchLogsExporterWiringTests : public ::testing::Test
+{
+protected:
+  void SetUp() override
+  {
+#ifdef ENABLE_ASYNC_EXPORT
+    GTEST_SKIP() << "Export() returns without waiting when async export is enabled";
+#endif
+  }
+};
+}  // namespace
+
+// A body the parser accepts, through the whole path rather than through the helper alone.
+TEST_F(ElasticsearchLogsExporterWiringTests, AcceptedBulkResponseIsASuccessfulExport)
+{
+  const auto result = ExportWith([](http_client::EventHandler &handler) {
+    FakeResponse response(200, kPrettySuccess);
+    handler.OnResponse(response);
+  });
+  EXPECT_EQ(result, opentelemetry::sdk::common::ExportResult::kSuccess);
+}
+
+// The first response decides the outcome, so it has to be the one the verdict is read from. A
+// client that answers twice would otherwise leave the decision with the first and the evidence
+// for it with the second.
+TEST_F(ElasticsearchLogsExporterWiringTests, ASecondResponseDoesNotReplaceTheOneThatDecided)
+{
+  const auto result = ExportWith([](http_client::EventHandler &handler) {
+    FakeResponse rejected(500, kPrettySuccess);
+    handler.OnResponse(rejected);
+    FakeResponse accepted(200, kPrettySuccess);
+    handler.OnResponse(accepted);
+  });
+  EXPECT_EQ(result, opentelemetry::sdk::common::ExportResult::kFailure)
+      << "the 500 that decided the outcome was read back as the 200 that followed it";
+}
+
+// The case #4295 reports: a 200 whose rejected item still leaves a shard counter reading
+// "failed" : 0, so the body looks successful to anything that reads it as text.
+TEST_F(ElasticsearchLogsExporterWiringTests, RejectedItemIsAFailedExport)
+{
+  // Two records, because kOneItemRejected answers two operations. Exporting one would be rejected
+  // on the acknowledgement count and the case would stop testing what it is named for.
+  const auto result = ExportWith(
+      [](http_client::EventHandler &handler) {
+        FakeResponse response(200, kOneItemRejected);
+        handler.OnResponse(response);
+      },
+      2);
+  EXPECT_EQ(result, opentelemetry::sdk::common::ExportResult::kFailure);
+}
+
+// The status has to reach the parser, not just the body. A handler that stored a fixed status,
+// or an Export() that never asked for it, would still pass every case above.
+TEST_F(ElasticsearchLogsExporterWiringTests, ServerErrorIsAFailedExportEvenWithAnAcceptedBody)
+{
+  const auto result = ExportWith([](http_client::EventHandler &handler) {
+    FakeResponse response(500, kPrettySuccess);
+    handler.OnResponse(response);
+  });
+  EXPECT_EQ(result, opentelemetry::sdk::common::ExportResult::kFailure);
+}
+
+// ---------------------------------------------------------------------------
+// Asynchronous path.
+//
+// Export() hands the body to AsyncResponseHandler and returns before any
+// response arrives, so the parsed result decides nothing the caller can see.
+// It decides which internal log line is written, which is what these cases
+// read. The fake delivers the response from inside SendRequest(), so the line
+// is already written by the time Export() returns.
+// ---------------------------------------------------------------------------
+namespace
+{
+struct CapturingLogHandler : public opentelemetry::sdk::common::internal_log::LogHandler
+{
+  void Handle(opentelemetry::sdk::common::internal_log::LogLevel level,
+              const char * /* file */,
+              int /* line */,
+              const char *msg,
+              const opentelemetry::sdk::common::AttributeMap & /* attributes */) noexcept override
+  {
+    messages.emplace_back(level, std::string{msg});
+  }
+
+  std::vector<std::pair<opentelemetry::sdk::common::internal_log::LogLevel, std::string>> messages;
+};
+
+class ElasticsearchLogsExporterAsyncTests : public ::testing::Test
+{
+protected:
+  void SetUp() override
+  {
+#ifndef ENABLE_ASYNC_EXPORT
+    GTEST_SKIP() << "Export() takes the synchronous path when async export is disabled";
+#endif
+    handler_ = nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler>(
+        new CapturingLogHandler());
+    opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogHandler(handler_);
+  }
+
+  void TearDown() override
+  {
+    opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogHandler(
+        nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler>(
+            new opentelemetry::sdk::common::internal_log::DefaultLogHandler()));
+  }
+
+  bool LoggedAnExportFailure() const
+  {
+    const auto &messages = static_cast<CapturingLogHandler *>(handler_.get())->messages;
+    for (const auto &message : messages)
+    {
+      if (message.second.find("Logs were not written to Elasticsearch correctly") !=
+          std::string::npos)
+      {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler> handler_;
+};
+}  // namespace
+
+TEST_F(ElasticsearchLogsExporterAsyncTests, AnAcceptedResponseIsNotReportedAsAFailure)
+{
+  EXPECT_EQ(ExportWith([](http_client::EventHandler &handler) {
+              FakeResponse response(200, kPrettySuccess);
+              handler.OnResponse(response);
+            }),
+            opentelemetry::sdk::common::ExportResult::kSuccess);
+
+#if OTEL_INTERNAL_LOG_LEVEL >= OTEL_INTERNAL_LOG_LEVEL_ERROR
+  EXPECT_FALSE(LoggedAnExportFailure());
+#endif
+}
+
+// The count reaches the parser on this path through the handler rather than through Export(), so
+// a handler that dropped it would still satisfy every synchronous case.
+TEST_F(ElasticsearchLogsExporterAsyncTests, ARejectedResponseIsReportedAsAFailure)
+{
+  // Export() reports success on this path whatever the response says, which is why the outcome has
+  // to be read from the log below. Asserting it here also keeps this case from having no assertion
+  // at all where the internal log level excludes the one that follows.
+  EXPECT_EQ(ExportWith(
+                [](http_client::EventHandler &handler) {
+                  FakeResponse response(200, kOneItemRejected);
+                  handler.OnResponse(response);
+                },
+                2),
+            opentelemetry::sdk::common::ExportResult::kSuccess);
+
+#if OTEL_INTERNAL_LOG_LEVEL >= OTEL_INTERNAL_LOG_LEVEL_ERROR
+  EXPECT_TRUE(LoggedAnExportFailure());
+#endif
+}

From 51a5bcc405513806c78d7165007d5baa484e6e24 Mon Sep 17 00:00:00 2001
From: thc1006 <84045975+thc1006@users.noreply.github.com>
Date: Wed, 5 Aug 2026 23:26:37 +0800
Subject: [PATCH 02/12] Read an item error whichever way the errors flag reads

The errors:true path already treats an item's error member as proof the
operation did not apply. The errors:false path ignored it, so
{"errors":false,"items":[{"index":{"status":201,"error":{...}}}]}
was accepted while the same body with errors:true was rejected. Same
evidence, opposite verdict, decided by a flag a broken responder also
controls, which is the shape this helper exists to close.

Both signals are now read in the single pass that already validates the
items, which also drops the second walk the errors:true branch needed.
---
 .../elasticsearch/detail/es_bulk_response.h   | 34 ++++++++++++-------
 .../test/es_log_record_exporter_test.cc       | 27 +++++++++++++++
 2 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
index 39a9a242f2..2e704c62c5 100644
--- a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
+++ b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
@@ -49,7 +49,7 @@ inline bool IsAcknowledgedStatus(const nlohmann::json &status) noexcept
  * @param expected_items the number of index operations the request submitted
  * @param failure_reason a best-effort explanation when this returns false
  * @return true only when the status is 2xx, "errors" is false, and "items" holds exactly
- *         expected_items index results that each acknowledge a 2xx status
+ *         expected_items index results that each acknowledge a 2xx status and carry no error
  */
 inline bool IsBulkResponseSuccessful(int status_code,
                                      const std::string &body,
@@ -100,7 +100,11 @@ inline bool IsBulkResponseSuccessful(int status_code,
 
     // Each entry is keyed by the action it answers, and every record goes out as an index
     // operation. Read before "errors", which a body that is not this answer does not get to decide.
-    const nlohmann::json *rejected = nullptr;
+    // An operation that was not applied says so twice, through its status and through an "error"
+    // member. Both are read here so that neither reading depends on the "errors" flag, which a
+    // response that contradicts itself also controls.
+    const nlohmann::json *rejected   = nullptr;
+    const nlohmann::json *item_error = nullptr;
     for (const auto &item : *items)
     {
       if (!item.is_object() || item.size() != 1)
@@ -127,6 +131,12 @@ inline bool IsBulkResponseSuccessful(int status_code,
       {
         rejected = &(*status);
       }
+
+      const auto error = operation->find("error");
+      if (item_error == nullptr && error != operation->end())
+      {
+        item_error = &(*error);
+      }
     }
 
     if (!errors->get<bool>())
@@ -138,20 +148,20 @@ inline bool IsBulkResponseSuccessful(int status_code,
             "the response reports no errors but acknowledges operation status " + rejected->dump();
         return false;
       }
+      if (item_error != nullptr)
+      {
+        failure_reason =
+            "the response reports no errors but an item carries error " + item_error->dump();
+        return false;
+      }
       return true;
     }
 
-    // Name the first item error rather than only saying that something failed. Every entry is one
-    // index result by now, so its single member is the result to look in.
-    for (const auto &item : *items)
+    // Name the first item error rather than only saying that something failed.
+    if (item_error != nullptr)
     {
-      const auto &result = *item.begin();
-      const auto error   = result.find("error");
-      if (error != result.end())
-      {
-        failure_reason = "at least one item failed, first error: " + error->dump();
-        return false;
-      }
+      failure_reason = "at least one item failed, first error: " + item_error->dump();
+      return false;
     }
 
     failure_reason = "the response reports errors";
diff --git a/exporters/elasticsearch/test/es_log_record_exporter_test.cc b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
index 685015bb70..d2f9e23f0b 100644
--- a/exporters/elasticsearch/test/es_log_record_exporter_test.cc
+++ b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
@@ -366,6 +366,33 @@ TEST(ElasticsearchBulkResponseTests, RejectsAnAcknowledgedFailureUnderErrorsFals
   EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(
       200, R"({"errors":false,"items":[{"index":{"status":300}}]})", 1, reason));
 }
+
+// An operation that did not apply says so through its status and through an "error" member. The
+// errors:true path already reads the second one, so reading it here as well is what stops the same
+// body from being accepted or rejected according to a flag the responder also controls.
+TEST(ElasticsearchBulkResponseTests, RejectsAnItemErrorUnderErrorsFalse)
+{
+  std::string reason;
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(
+      200,
+      R"({"errors":false,"items":[{"index":{"_index":"logs","status":201,)"
+      R"("error":{"type":"mapper_parsing_exception","reason":"rejected"}}}]})",
+      1, reason));
+  EXPECT_NE(reason.find("mapper_parsing_exception"), std::string::npos)
+      << "the reason names what the server said: " << reason;
+
+  // A conforming server sends the flag and the member together, and that reads the same way.
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(
+      200,
+      R"({"errors":true,"items":[{"index":{"_index":"logs","status":400,)"
+      R"("error":{"type":"mapper_parsing_exception"}}}]})",
+      1, reason));
+  EXPECT_NE(reason.find("mapper_parsing_exception"), std::string::npos) << reason;
+
+  // Only the member decides. An acknowledgement without one stays a success.
+  EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(
+      200, R"({"errors":false,"items":[{"index":{"_index":"logs","status":201}}]})", 1, reason));
+}
 // ---------------------------------------------------------------------------
 // The response travelling from the HTTP callback to the verdict.
 //

From 4f594e638be07906799e5371e0c3ae42800737c5 Mon Sep 17 00:00:00 2001
From: thc1006 <84045975+thc1006@users.noreply.github.com>
Date: Wed, 5 Aug 2026 23:32:34 +0800
Subject: [PATCH 03/12] Let a null error member stay a success

find() reports a key that is present with a null value, so the first
version rejected {"error":null}. A serialiser that writes absent
optionals as null is saying the operation applied, which is what the
flag says too, and rejecting it would fail exports that are fine while
catching nothing. Only a cause contradicts the flag.
---
 .../exporters/elasticsearch/detail/es_bulk_response.h       | 4 +++-
 exporters/elasticsearch/test/es_log_record_exporter_test.cc | 6 +++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
index 2e704c62c5..daefa2d39c 100644
--- a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
+++ b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
@@ -132,8 +132,10 @@ inline bool IsBulkResponseSuccessful(int status_code,
         rejected = &(*status);
       }
 
+      // A null holds no cause, and a serialiser that writes absent optionals as null is saying
+      // the operation applied, which is what the flag says too. Only a cause contradicts it.
       const auto error = operation->find("error");
-      if (item_error == nullptr && error != operation->end())
+      if (item_error == nullptr && error != operation->end() && !error->is_null())
       {
         item_error = &(*error);
       }
diff --git a/exporters/elasticsearch/test/es_log_record_exporter_test.cc b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
index d2f9e23f0b..94583a20fa 100644
--- a/exporters/elasticsearch/test/es_log_record_exporter_test.cc
+++ b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
@@ -389,9 +389,13 @@ TEST(ElasticsearchBulkResponseTests, RejectsAnItemErrorUnderErrorsFalse)
       1, reason));
   EXPECT_NE(reason.find("mapper_parsing_exception"), std::string::npos) << reason;
 
-  // Only the member decides. An acknowledgement without one stays a success.
+  // Only a cause decides. An acknowledgement without the member stays a success, and so does one
+  // whose member is null, which is what a serialiser writing absent optionals produces.
   EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(
       200, R"({"errors":false,"items":[{"index":{"_index":"logs","status":201}}]})", 1, reason));
+  EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(
+      200, R"({"errors":false,"items":[{"index":{"_index":"logs","status":201,"error":null}}]})", 1,
+      reason));
 }
 // ---------------------------------------------------------------------------
 // The response travelling from the HTTP callback to the verdict.

From 074028b7ed347ae2a8177c4282c1fc7b1d577e05 Mon Sep 17 00:00:00 2001
From: thc1006 <84045975+thc1006@users.noreply.github.com>
Date: Fri, 14 Aug 2026 00:14:36 +0000
Subject: [PATCH 04/12] [TEST] Hold the status and the batch size on the way to
 the parser

Two things the helper is given were never held on the way in.

Every asynchronous case answered HTTP 200, so the status never had to
travel. Measured: passing a constant 200 instead of the response status
left all 17 cases green, and an async path that ignores the status is the
defect this change is about.

Every successful case carried one record, so the count never had to
travel either. The only two item case is a rejection, which fails whatever
count it is compared against. Measured: fixing the expected count at one
also left all 17 green, on both paths.

Three cases close it. An async server error under an accepted body, and a
batch of two acknowledged records on each path.

Measured both ways, each in the build where its path is live. With the
status pinned at 200 the async server error case fails alone. With the
count pinned at one the async batch case fails alone, and in a build
without async export the synchronous batch case fails alone. Clean: 19
pass with async export, 20 without, 3 of 3 each.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
---
 .../test/es_log_record_exporter_test.cc       | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/exporters/elasticsearch/test/es_log_record_exporter_test.cc b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
index 94583a20fa..9864f1b513 100644
--- a/exporters/elasticsearch/test/es_log_record_exporter_test.cc
+++ b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
@@ -157,6 +157,12 @@ namespace
 constexpr const char *kPrettySuccess =
     R"({"took":30,"errors":false,"items":[{"index":{"_index":"logs","_id":"1",)"
     R"("_shards":{"total":2,"successful":1,"failed" : 0},"status":201}}]})";
+// Two operations acknowledged, so a batch of two has something to match. 201 for a new document
+// and 200 for an overwrite are both what Elasticsearch answers for an index operation.
+constexpr const char *kTwoItemCompactSuccess = R"({"took":30,"errors":false,"items":[)"
+                                               R"({"index":{"_index":"logs","status":201}},)"
+                                               R"({"index":{"_index":"logs","status":200}}]})";
+
 constexpr const char *kCompactSuccess =
     R"({"took":30,"errors":false,"items":[{"index":{"_index":"logs","_id":"1",)"
     R"("_shards":{"total":2,"successful":1,"failed":0},"status":201}}]})";
@@ -570,6 +576,21 @@ TEST_F(ElasticsearchLogsExporterWiringTests, RejectedItemIsAFailedExport)
 
 // The status has to reach the parser, not just the body. A handler that stored a fixed status,
 // or an Export() that never asked for it, would still pass every case above.
+// The count has to travel. Every other successful case here sends one record, so an expected
+// count fixed at one would satisfy them all, and the only two item case is a rejection that fails
+// whatever the count is.
+TEST_F(ElasticsearchLogsExporterWiringTests, TwoAcceptedRecordsUseTheActualBatchSize)
+{
+  const auto result = ExportWith(
+      [](http_client::EventHandler &handler) {
+        FakeResponse response(200, kTwoItemCompactSuccess);
+        handler.OnResponse(response);
+      },
+      2);
+
+  EXPECT_EQ(result, opentelemetry::sdk::common::ExportResult::kSuccess);
+}
+
 TEST_F(ElasticsearchLogsExporterWiringTests, ServerErrorIsAFailedExportEvenWithAnAcceptedBody)
 {
   const auto result = ExportWith([](http_client::EventHandler &handler) {
@@ -657,6 +678,33 @@ TEST_F(ElasticsearchLogsExporterAsyncTests, AnAcceptedResponseIsNotReportedAsAFa
 
 // The count reaches the parser on this path through the handler rather than through Export(), so
 // a handler that dropped it would still satisfy every synchronous case.
+// The status has to travel on this path too. Every other case here answers 200, so a status fixed
+// at 200 would satisfy them all, and the original defect was this path ignoring the status.
+TEST_F(ElasticsearchLogsExporterAsyncTests, AServerErrorIsReportedEvenWithAnAcceptedBody)
+{
+  EXPECT_EQ(ExportWith([](http_client::EventHandler &handler) {
+              FakeResponse response(500, kCompactSuccess);
+              handler.OnResponse(response);
+            }),
+            opentelemetry::sdk::common::ExportResult::kSuccess);
+
+  EXPECT_TRUE(LoggedAnExportFailure()) << "a server error was not reported on the async path";
+}
+
+// And the count, for the same reason it is held on the synchronous path.
+TEST_F(ElasticsearchLogsExporterAsyncTests, TwoAcceptedRecordsAreNotReportedAsAFailure)
+{
+  EXPECT_EQ(ExportWith(
+                [](http_client::EventHandler &handler) {
+                  FakeResponse response(200, kTwoItemCompactSuccess);
+                  handler.OnResponse(response);
+                },
+                2),
+            opentelemetry::sdk::common::ExportResult::kSuccess);
+
+  EXPECT_FALSE(LoggedAnExportFailure()) << "two acknowledged records were reported as a failure";
+}
+
 TEST_F(ElasticsearchLogsExporterAsyncTests, ARejectedResponseIsReportedAsAFailure)
 {
   // Export() reports success on this path whatever the response says, which is why the outcome has

From 05cd10727adb1720ab0779ea83533aa4da11d967 Mon Sep 17 00:00:00 2001
From: thc1006 <84045975+thc1006@users.noreply.github.com>
Date: Fri, 14 Aug 2026 06:00:15 +0000
Subject: [PATCH 05/12] [TEST] Skip the cases that read the log when the log is
 compiled out

Two of the four assertions that look for the export failure in the
captured log were guarded against a build with error logging compiled
out, and the two added last were not. Below error level
OTEL_INTERNAL_LOG_ERROR expands to nothing, so an absent log looks the
same as a log that was never written: the server error case fails on
correct code, and the two record case stops noticing a hard coded
expected count because the failure it looks for could not be written
either way.

The fixture says it once instead. One skip point, because GTEST_SKIP
returns and a second below it leaves the rest of the body unreachable,
which MSVC reports under maintainer mode. The four assertions are plain
now.

Measured with -DOTEL_INTERNAL_LOG_LEVEL=0. Without the skip two cases
fail on correct code, 2 of 2. With it the four skip and the file passes,
15 of 15. The ordinary builds are unchanged: 19 with async export, 20
without.

The fixture also puts back the handler it found rather than installing a
fresh default, which would displace one another case in the same binary
had installed.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
---
 .../test/es_log_record_exporter_test.cc       | 28 ++++++++++++-------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/exporters/elasticsearch/test/es_log_record_exporter_test.cc b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
index 9864f1b513..424b913f2d 100644
--- a/exporters/elasticsearch/test/es_log_record_exporter_test.cc
+++ b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
@@ -630,19 +630,30 @@ class ElasticsearchLogsExporterAsyncTests : public ::testing::Test
 protected:
   void SetUp() override
   {
-#ifndef ENABLE_ASYNC_EXPORT
+    // One skip point: GTEST_SKIP returns, and a second below it would leave the rest of this body
+    // unreachable, which MSVC reports as C4702 under maintainer mode.
+#if !defined(ENABLE_ASYNC_EXPORT)
     GTEST_SKIP() << "Export() takes the synchronous path when async export is disabled";
-#endif
-    handler_ = nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler>(
+#elif OTEL_INTERNAL_LOG_LEVEL < OTEL_INTERNAL_LOG_LEVEL_ERROR
+    // These cases read the failure out of the log. Below error level it is not written at all, so
+    // a wrong result and a right one look the same and the cases would stop discriminating.
+    GTEST_SKIP() << "the failure these observe is compiled out below error level";
+#else
+    previous_handler_ = opentelemetry::sdk::common::internal_log::GlobalLogHandler::GetLogHandler();
+    handler_          = nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler>(
         new CapturingLogHandler());
     opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogHandler(handler_);
+#endif
   }
 
   void TearDown() override
   {
-    opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogHandler(
-        nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler>(
-            new opentelemetry::sdk::common::internal_log::DefaultLogHandler()));
+    // Put back what was found, rather than a fresh default that would displace a handler another
+    // case in this binary had installed.
+    if (handler_)
+    {
+      opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogHandler(previous_handler_);
+    }
   }
 
   bool LoggedAnExportFailure() const
@@ -660,6 +671,7 @@ class ElasticsearchLogsExporterAsyncTests : public ::testing::Test
   }
 
   nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler> handler_;
+  nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler> previous_handler_;
 };
 }  // namespace
 
@@ -671,9 +683,7 @@ TEST_F(ElasticsearchLogsExporterAsyncTests, AnAcceptedResponseIsNotReportedAsAFa
             }),
             opentelemetry::sdk::common::ExportResult::kSuccess);
 
-#if OTEL_INTERNAL_LOG_LEVEL >= OTEL_INTERNAL_LOG_LEVEL_ERROR
   EXPECT_FALSE(LoggedAnExportFailure());
-#endif
 }
 
 // The count reaches the parser on this path through the handler rather than through Export(), so
@@ -718,7 +728,5 @@ TEST_F(ElasticsearchLogsExporterAsyncTests, ARejectedResponseIsReportedAsAFailur
                 2),
             opentelemetry::sdk::common::ExportResult::kSuccess);
 
-#if OTEL_INTERNAL_LOG_LEVEL >= OTEL_INTERNAL_LOG_LEVEL_ERROR
   EXPECT_TRUE(LoggedAnExportFailure());
-#endif
 }

From 6924b9c9f1fab287ec76a86550481515ce787406 Mon Sep 17 00:00:00 2001
From: thc1006 <84045975+thc1006@users.noreply.github.com>
Date: Fri, 14 Aug 2026 21:53:53 +0000
Subject: [PATCH 06/12] [BUG] Hold both statuses to one band, and require an
 answer to name its index

Three things, one of which changes what is accepted.

The success band was written twice. The response status was compared as a
plain int against 200 and 299, and each operation status went through a
separate predicate that compares in the type nlohmann parsed it as. Same band,
two spellings, and only one of them safe against a status the server can
choose. Both go through one pair of overloads now, documented once, and the
response status reaches the signed one through a cast an int cannot lose
anything to.

An index result names the index it wrote to, and this did not read it. Every
Elasticsearch index result carries _index whether the operation applied or not,
so a result without one is not an answer to an index operation, and this now
says so. Presence and type only, deliberately: the name reported is the index
the write resolved to, and an alias or a date math index makes that different
from the one submitted, so comparing them would fail a normal setup. That is
also why the exporter's own index name is not passed down here.

This is a tightening. A backend that answers a bulk request without _index in
its index results was accepted before and is a failed export now.

And the cases had no bound. The asynchronous ones wait on a response that a
regression can stop delivering, and a case that hangs stops the job rather than
reporting, which is a worse failure than a red test. The 24 enabled cases carry a
120 second CTest timeout now, and so do the three the suite disables, read back
from ctest --show-only=json-v1 rather than assumed from having written it.

Against the same header with the target index check taken out, the case that
holds it fails on all three of its new shapes: no _index at all, an _index that
is a number, and one that is null. All 24 cases pass through ctest, and the
binary reports 19 passed with 5 skipped where the internal log is compiled out.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
---
 CHANGELOG.md                                  |  3 +-
 exporters/elasticsearch/CMakeLists.txt        |  4 ++
 .../elasticsearch/detail/es_bulk_response.h   | 46 ++++++++++++++-----
 .../test/es_log_record_exporter_test.cc       | 38 ++++++++++-----
 4 files changed, 67 insertions(+), 24 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e7282c8052..76e0698fa5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -151,7 +151,8 @@ Increment the:
   [#4298](https://github.com/open-telemetry/opentelemetry-cpp/pull/4298)
 
 * [BUG] Decide Elasticsearch bulk export success from the HTTP status, the
-  errors flag, and one acknowledgement per record
+  errors flag, and one acknowledgement per record that names its target index
+  and carries a 2xx status
   [#4297](https://github.com/open-telemetry/opentelemetry-cpp/pull/4297)
 
 * [BUG] Make SocketAddr string parsing safe and reject malformed addresses
diff --git a/exporters/elasticsearch/CMakeLists.txt b/exporters/elasticsearch/CMakeLists.txt
index 290691f4d9..0a3581d0ea 100644
--- a/exporters/elasticsearch/CMakeLists.txt
+++ b/exporters/elasticsearch/CMakeLists.txt
@@ -61,4 +61,8 @@ if(BUILD_TESTING)
     TARGET es_log_record_exporter_test
     TEST_PREFIX exporter.
     TEST_LIST es_log_record_exporter_test)
+
+  # The asynchronous cases wait on a response that a regression can stop delivering, and a case
+  # that hangs stops the job rather than reporting. A bound turns that back into a red test.
+  set_tests_properties(${es_log_record_exporter_test} PROPERTIES TIMEOUT 120)
 endif() # BUILD_TESTING
diff --git a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
index daefa2d39c..14e431d31d 100644
--- a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
+++ b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
@@ -18,23 +18,33 @@ namespace detail
 {
 
 /**
- * Whether an acknowledged operation status is one that applied the operation. Elasticsearch
- * answers an index operation with 200 or 201, so 2xx is the whole band.
+ * The band a status has to fall in to say the thing it answers was applied. Elasticsearch
+ * answers the bulk request with 200 and an index operation with 200 or 201, so 2xx is the whole
+ * of it on both, and this says so once for both rather than twice with two spellings.
  *
- * Compared in the type the number was parsed as. Narrowing to int first is not safe here: the
- * value comes from the server, is_number_integer() is true for unsigned as well, and 2^32 + 200
- * narrows back into the band on a 32 bit int.
+ * Two overloads rather than one signed parameter, because an operation status arrives through
+ * nlohmann::json and is compared in the type it was parsed as: is_number_integer() is true for
+ * unsigned as well, and 2^32 + 200 narrows back into the band on a 32 bit int.
  */
+inline bool IsSuccessStatus(nlohmann::json::number_unsigned_t value) noexcept
+{
+  return value >= 200U && value <= 299U;
+}
+
+inline bool IsSuccessStatus(nlohmann::json::number_integer_t value) noexcept
+{
+  return value >= 200 && value <= 299;
+}
+
+/** Whether an acknowledged operation status is one that applied the operation. */
 inline bool IsAcknowledgedStatus(const nlohmann::json &status) noexcept
 {
   if (status.is_number_unsigned())
   {
-    const auto value = status.get<nlohmann::json::number_unsigned_t>();
-    return value >= 200U && value <= 299U;
+    return IsSuccessStatus(status.get<nlohmann::json::number_unsigned_t>());
   }
 
-  const auto value = status.get<nlohmann::json::number_integer_t>();
-  return value >= 200 && value <= 299;
+  return IsSuccessStatus(status.get<nlohmann::json::number_integer_t>());
 }
 
 /**
@@ -49,7 +59,8 @@ inline bool IsAcknowledgedStatus(const nlohmann::json &status) noexcept
  * @param expected_items the number of index operations the request submitted
  * @param failure_reason a best-effort explanation when this returns false
  * @return true only when the status is 2xx, "errors" is false, and "items" holds exactly
- *         expected_items index results that each acknowledge a 2xx status and carry no error
+ *         expected_items index results that each name a target index, acknowledge a 2xx status
+ *         and carry no error
  */
 inline bool IsBulkResponseSuccessful(int status_code,
                                      const std::string &body,
@@ -62,7 +73,9 @@ inline bool IsBulkResponseSuccessful(int status_code,
   {
 #endif
     // Inside the try: building a reason allocates.
-    if (status_code < 200 || status_code > 299)
+    // The same band as an operation status, through the same predicate. The cast picks the
+    // signed overload, which an int reaches without losing anything.
+    if (!IsSuccessStatus(static_cast<nlohmann::json::number_integer_t>(status_code)))
     {
       failure_reason = "unexpected HTTP status " + std::to_string(status_code);
       return false;
@@ -120,6 +133,17 @@ inline bool IsBulkResponseSuccessful(int status_code,
         return false;
       }
 
+      // Elasticsearch names the index it wrote to in every index result, whether the operation
+      // applied or not. Presence and type only: the name it reports is the index the write
+      // resolved to, which an alias or a date math index makes different from the one that was
+      // submitted, so it is not something to compare against.
+      const auto target = operation->find("_index");
+      if (target == operation->end() || !target->is_string())
+      {
+        failure_reason = "the response acknowledges an index operation with no target index";
+        return false;
+      }
+
       const auto status = operation->find("status");
       if (status == operation->end() || !status->is_number_integer())
       {
diff --git a/exporters/elasticsearch/test/es_log_record_exporter_test.cc b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
index 424b913f2d..4b43a9ba7c 100644
--- a/exporters/elasticsearch/test/es_log_record_exporter_test.cc
+++ b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
@@ -262,9 +262,9 @@ TEST(ElasticsearchBulkResponseTests, RequiresAnAcknowledgementForEverySubmittedO
   constexpr const char *kNoItems   = R"({"errors":false})";
   constexpr const char *kNullItems = R"({"errors":false,"items":null})";
   constexpr const char *kOneItemBody =
-      R"({"errors":false,"items":[{"index":{"_shards":{"failed":0},"status":201}}]})";
+      R"({"errors":false,"items":[{"index":{"_index":"logs","_shards":{"failed":0},"status":201}}]})";
   constexpr const char *kTwoItemBody =
-      R"({"errors":false,"items":[{"index":{"status":201}},{"index":{"status":201}}]})";
+      R"({"errors":false,"items":[{"index":{"_index":"logs","status":201}},{"index":{"_index":"logs","status":201}}]})";
 
   EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(200, kNoItems, 1, reason));
   EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(200, kNullItems, 1, reason));
@@ -296,15 +296,24 @@ TEST(ElasticsearchBulkResponseTests, RejectsItemsEntriesThatDoNotAcknowledgeAnIn
       << "the index key holds no result object";
   EXPECT_TRUE(rejected(R"({"errors":false,"items":[{"index":{},"delete":{}}]})", 1))
       << "one entry cannot answer two operations";
-  EXPECT_TRUE(rejected(R"({"errors":false,"items":[{"index":{"status":201},"delete":{}}]})", 1))
+  EXPECT_TRUE(rejected(
+      R"({"errors":false,"items":[{"index":{"_index":"logs","status":201},"delete":{}}]})", 1))
       << "a valid acknowledgement does not license a second member";
   EXPECT_TRUE(rejected(R"({"errors":false,"items":[{"index":{"_index":"logs"}}]})", 1))
       << "an index result carrying no status";
+  EXPECT_TRUE(rejected(R"({"errors":false,"items":[{"index":{"status":201}}]})", 1))
+      << "an index result naming no target index";
+  EXPECT_TRUE(rejected(R"({"errors":false,"items":[{"index":{"_index":7,"status":201}}]})", 1))
+      << "a target index that is not a name";
+  EXPECT_TRUE(rejected(R"({"errors":false,"items":[{"index":{"_index":null,"status":201}}]})", 1))
+      << "a target index that is not there under another spelling";
   EXPECT_TRUE(rejected(R"({"errors":true,"items":[null,null]})", 2))
       << "the same shape has to fail whichever way errors reads";
 
-  // The shape a real acknowledgement has.
-  EXPECT_FALSE(rejected(R"({"errors":false,"items":[{"index":{"status":201}}]})", 1));
+  // The shape a real acknowledgement has: one index result, naming the index it wrote to and
+  // carrying the status it wrote with.
+  EXPECT_FALSE(
+      rejected(R"({"errors":false,"items":[{"index":{"_index":"logs","status":201}}]})", 1));
 
   // Three shapes, three reasons: they are different things to go and look at.
   const auto reason_for = [&reason](const char *body) {
@@ -314,9 +323,11 @@ TEST(ElasticsearchBulkResponseTests, RejectsItemsEntriesThatDoNotAcknowledgeAnIn
   const std::string padded  = reason_for(R"({"errors":false,"items":[{"index":{},"delete":{}}]})");
   const std::string other   = reason_for(R"({"errors":false,"items":[{"unknown":{"status":1}}]})");
   const std::string no_stat = reason_for(R"({"errors":false,"items":[{"index":{"_index":"l"}}]})");
+  const std::string no_idx  = reason_for(R"({"errors":false,"items":[{"index":{"status":201}}]})");
   EXPECT_NE(padded, other) << padded;
   EXPECT_NE(other, no_stat) << other;
   EXPECT_NE(padded, no_stat) << no_stat;
+  EXPECT_NE(no_stat, no_idx) << no_idx;
 }
 
 // The status says whether the operation applied, so the band has to hold for the number the server
@@ -326,7 +337,8 @@ TEST(ElasticsearchBulkResponseTests, RejectsAcknowledgementsOutsideTheSuccessBan
   std::string reason;
   const auto applied = [&reason](const char *status) {
     const std::string body =
-        std::string(R"({"errors":false,"items":[{"index":{"status":)") + status + R"(}}]})";
+        std::string(R"({"errors":false,"items":[{"index":{"_index":"logs","status":)") + status +
+        R"(}}]})";
     return logs_exporter::detail::IsBulkResponseSuccessful(200, body, 1, reason);
   };
 
@@ -357,20 +369,22 @@ TEST(ElasticsearchBulkResponseTests, RejectsAnAcknowledgedFailureUnderErrorsFals
 {
   std::string reason;
   EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(
-      200, R"({"errors":false,"items":[{"index":{"status":400}}]})", 1, reason));
+      200, R"({"errors":false,"items":[{"index":{"_index":"logs","status":400}}]})", 1, reason));
   EXPECT_NE(reason.find("400"), std::string::npos) << "the reason names the status: " << reason;
 
   EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(
-      200, R"({"errors":false,"items":[{"index":{"status":201}},{"index":{"status":429}}]})", 2,
-      reason))
+      200,
+      R"({"errors":false,"items":[{"index":{"_index":"logs","status":201}},{"index":{"_index":"logs","status":429}}]})",
+      2, reason))
       << "one rejected operation among several is still a rejection";
 
   // The whole 2xx band is a success, the same band the response's own HTTP status uses.
   EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(
-      200, R"({"errors":false,"items":[{"index":{"status":200}},{"index":{"status":299}}]})", 2,
-      reason));
+      200,
+      R"({"errors":false,"items":[{"index":{"_index":"logs","status":200}},{"index":{"_index":"logs","status":299}}]})",
+      2, reason));
   EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(
-      200, R"({"errors":false,"items":[{"index":{"status":300}}]})", 1, reason));
+      200, R"({"errors":false,"items":[{"index":{"_index":"logs","status":300}}]})", 1, reason));
 }
 
 // An operation that did not apply says so through its status and through an "error" member. The

From 04152bf90cd8fd6d2f9c959cfab36e07528eb709 Mon Sep 17 00:00:00 2001
From: thc1006 <84045975+thc1006@users.noreply.github.com>
Date: Sat, 15 Aug 2026 07:39:52 +0000
Subject: [PATCH 07/12] Wrap the comment the way cmake-format does

The Format job runs cmake-format in place and diffs, and it wraps a comment narrower than --check accepts, so --check with the same config reported clean on a file the job then rewrote. This is what it produces, applied as it produced it, and the file is stable under a second pass.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
---
 exporters/elasticsearch/CMakeLists.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/exporters/elasticsearch/CMakeLists.txt b/exporters/elasticsearch/CMakeLists.txt
index 0a3581d0ea..d38c897ec5 100644
--- a/exporters/elasticsearch/CMakeLists.txt
+++ b/exporters/elasticsearch/CMakeLists.txt
@@ -62,7 +62,8 @@ if(BUILD_TESTING)
     TEST_PREFIX exporter.
     TEST_LIST es_log_record_exporter_test)
 
-  # The asynchronous cases wait on a response that a regression can stop delivering, and a case
-  # that hangs stops the job rather than reporting. A bound turns that back into a red test.
+  # The asynchronous cases wait on a response that a regression can stop
+  # delivering, and a case that hangs stops the job rather than reporting. A
+  # bound turns that back into a red test.
   set_tests_properties(${es_log_record_exporter_test} PROPERTIES TIMEOUT 120)
 endif() # BUILD_TESTING

From 02a2045ca2f5f855ff7cfff87c1fd78c9300d110 Mon Sep 17 00:00:00 2001
From: thc1006 <84045975+thc1006@users.noreply.github.com>
Date: Sat, 15 Aug 2026 20:07:25 +0000
Subject: [PATCH 08/12] [BUG] Accept the statuses that say the batch was
 written, not the whole 2xx band

One predicate answered for both the bulk request and each index operation, and it took
anything from 200 to 299. The comment above it already said what the two actually
answer with, 200 for the request and 200 or 201 for an operation, and then widened past
both.

202 is why that matters. It says the request was accepted and the processing is not
finished, and may never finish. Reading it as success is not a lenient reading of a
vague answer, it is the opposite of what the answer says, and success here is what lets
the caller drop the records it just handed over. A response of 202 carrying items that
each say 202 was a success under the old band, with nothing else in the body to catch
it.

So the request is held to 200 and an operation to 200 or 201, through two predicates
that each say what they are for. The two overloads stay, because an operation status
still arrives through nlohmann::json and 2^32 + 200 still narrows back to 200 on a 32
bit int.

One case carried the old rule in its body rather than in the boundary case: it paired a
200 operation with a 299 one and expected success. It now pairs 200 with 201, which is
the pair a real response produces.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
---
 .../elasticsearch/detail/es_bulk_response.h   | 38 ++++++++++++-------
 .../test/es_log_record_exporter_test.cc       | 26 ++++++++-----
 2 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
index 14e431d31d..ba4797a950 100644
--- a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
+++ b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
@@ -18,22 +18,33 @@ namespace detail
 {
 
 /**
- * The band a status has to fall in to say the thing it answers was applied. Elasticsearch
- * answers the bulk request with 200 and an index operation with 200 or 201, so 2xx is the whole
- * of it on both, and this says so once for both rather than twice with two spellings.
+ * What the bulk request itself has to answer with. Elasticsearch answers a bulk request with 200,
+ * and the rest of the 2xx band does not say the batch was written. 202 is the one that matters:
+ * it says the request was accepted and the processing is not finished, and may never be. Reading
+ * that as success is not a conservative reading of a vague answer, it is the opposite of what the
+ * answer says, and a success here lets the caller drop the records it just handed over.
+ */
+inline bool IsSuccessfulBulkStatus(int status_code) noexcept
+{
+  return 200 == status_code;
+}
+
+/**
+ * What an acknowledged index operation has to carry. Elasticsearch answers one with 201 when it
+ * created the document and 200 when it replaced an existing one.
  *
  * Two overloads rather than one signed parameter, because an operation status arrives through
  * nlohmann::json and is compared in the type it was parsed as: is_number_integer() is true for
- * unsigned as well, and 2^32 + 200 narrows back into the band on a 32 bit int.
+ * unsigned as well, and 2^32 + 200 narrows back to 200 on a 32 bit int.
  */
-inline bool IsSuccessStatus(nlohmann::json::number_unsigned_t value) noexcept
+inline bool IsAppliedStatus(nlohmann::json::number_unsigned_t value) noexcept
 {
-  return value >= 200U && value <= 299U;
+  return 200U == value || 201U == value;
 }
 
-inline bool IsSuccessStatus(nlohmann::json::number_integer_t value) noexcept
+inline bool IsAppliedStatus(nlohmann::json::number_integer_t value) noexcept
 {
-  return value >= 200 && value <= 299;
+  return 200 == value || 201 == value;
 }
 
 /** Whether an acknowledged operation status is one that applied the operation. */
@@ -41,10 +52,10 @@ inline bool IsAcknowledgedStatus(const nlohmann::json &status) noexcept
 {
   if (status.is_number_unsigned())
   {
-    return IsSuccessStatus(status.get<nlohmann::json::number_unsigned_t>());
+    return IsAppliedStatus(status.get<nlohmann::json::number_unsigned_t>());
   }
 
-  return IsSuccessStatus(status.get<nlohmann::json::number_integer_t>());
+  return IsAppliedStatus(status.get<nlohmann::json::number_integer_t>());
 }
 
 /**
@@ -73,9 +84,10 @@ inline bool IsBulkResponseSuccessful(int status_code,
   {
 #endif
     // Inside the try: building a reason allocates.
-    // The same band as an operation status, through the same predicate. The cast picks the
-    // signed overload, which an int reaches without losing anything.
-    if (!IsSuccessStatus(static_cast<nlohmann::json::number_integer_t>(status_code)))
+    // Not the band an operation status is held to. This one answers for the request, and an
+    // operation that applied answers 201 for a document it created, which the request itself
+    // never says.
+    if (!IsSuccessfulBulkStatus(status_code))
     {
       failure_reason = "unexpected HTTP status " + std::to_string(status_code);
       return false;
diff --git a/exporters/elasticsearch/test/es_log_record_exporter_test.cc b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
index 4b43a9ba7c..3701d7faff 100644
--- a/exporters/elasticsearch/test/es_log_record_exporter_test.cc
+++ b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
@@ -241,15 +241,20 @@ TEST(ElasticsearchBulkResponseTests, ReportsFailureWhenErrorsTrueWithoutItemErro
       200, R"({"errors":true,"items":[{"index":42}]})", 1, reason));
 }
 
-// The 2xx range is the success band; the codes just outside it are failures. This pins the boundary
-// so a later change to the status check cannot silently widen or narrow it.
-TEST(ElasticsearchBulkResponseTests, TreatsThe2xxRangeAsTheSuccessBand)
+// 200 is what Elasticsearch answers a bulk request with, and it is the only status that says the
+// batch was written. 202 is the one worth naming: it says the request was accepted and the
+// processing is not finished, so reading it as success would let the caller drop records the
+// server has not promised to keep. This pins the boundary on both sides.
+TEST(ElasticsearchBulkResponseTests, AcceptsOnlyTheStatusThatSaysTheBatchWasWritten)
 {
   std::string reason;
   const char *ok_body = R"({"errors":false,"items":[]})";
   EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(199, ok_body, 0, reason));
   EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(200, ok_body, 0, reason));
-  EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(299, ok_body, 0, reason));
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(201, ok_body, 0, reason));
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(202, ok_body, 0, reason))
+      << "202 says the processing is not finished, which is not a write acknowledgement";
+  EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(299, ok_body, 0, reason));
   EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(300, ok_body, 0, reason));
 }
 
@@ -342,9 +347,12 @@ TEST(ElasticsearchBulkResponseTests, RejectsAcknowledgementsOutsideTheSuccessBan
     return logs_exporter::detail::IsBulkResponseSuccessful(200, body, 1, reason);
   };
 
-  EXPECT_TRUE(applied("200"));
-  EXPECT_TRUE(applied("201"));
-  EXPECT_TRUE(applied("299"));
+  EXPECT_FALSE(applied("199"));
+  EXPECT_TRUE(applied("200")) << "an index operation that replaced a document";
+  EXPECT_TRUE(applied("201")) << "an index operation that created one";
+  EXPECT_FALSE(applied("202")) << "accepted is not applied";
+  EXPECT_FALSE(applied("299"));
+  EXPECT_FALSE(applied("300"));
 
   EXPECT_FALSE(applied("0")) << "zero is a status, not the absence of one";
   EXPECT_FALSE(reason.empty());
@@ -378,10 +386,10 @@ TEST(ElasticsearchBulkResponseTests, RejectsAnAcknowledgedFailureUnderErrorsFals
       2, reason))
       << "one rejected operation among several is still a rejection";
 
-  // The whole 2xx band is a success, the same band the response's own HTTP status uses.
+  // Both of the statuses an index operation answers with when it applied, in one response.
   EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(
       200,
-      R"({"errors":false,"items":[{"index":{"_index":"logs","status":200}},{"index":{"_index":"logs","status":299}}]})",
+      R"({"errors":false,"items":[{"index":{"_index":"logs","status":200}},{"index":{"_index":"logs","status":201}}]})",
       2, reason));
   EXPECT_FALSE(logs_exporter::detail::IsBulkResponseSuccessful(
       200, R"({"errors":false,"items":[{"index":{"_index":"logs","status":300}}]})", 1, reason));

From 6d9f73cf1bc122f03845d9ce0861f19420775428 Mon Sep 17 00:00:00 2001
From: thc1006 <84045975+thc1006@users.noreply.github.com>
Date: Sat, 15 Aug 2026 20:13:04 +0000
Subject: [PATCH 09/12] [BUG] Describe only the response the caller was given,
 and describe it outside the lock

Two things in the same few lines.

The debug line was written before the outcome was decided, so a second response for the
same request described itself as well, even though its status and body had just been
dropped. Two lines then disagree about what the export returned, and the one the caller
was not given reads exactly like the one it was.

It also ran while this thread held mutex_. The log handler is replaceable application
code, so a handler that reaches back into this exporter would have been doing it from
inside the lock its own call needs. The message is now built under the lock, where the
body still exists, and written after it is released.

The case is in a fixture of its own because the line is a debug line: below debug level
the call does not exist, and the compiled in level is only half of it, since what it
dispatches is filtered again at runtime and the default there is Warning. Without
setting both the handler is installed and never told anything, which is how the case
first passed for the wrong reason.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
---
 .../src/es_log_record_exporter.cc             | 23 +++--
 .../test/es_log_record_exporter_test.cc       | 85 ++++++++++++++++++-
 2 files changed, 101 insertions(+), 7 deletions(-)

diff --git a/exporters/elasticsearch/src/es_log_record_exporter.cc b/exporters/elasticsearch/src/es_log_record_exporter.cc
index 2cae327dcc..4d1ff77cf1 100644
--- a/exporters/elasticsearch/src/es_log_record_exporter.cc
+++ b/exporters/elasticsearch/src/es_log_record_exporter.cc
@@ -76,18 +76,15 @@ class ResponseHandler : public http_client::EventHandler
    */
   void OnResponse(http_client::Response &response) noexcept override
   {
+    std::string described;
+    bool decided = false;
+
     // Lock the private members so they can't be read while being modified
     {
       std::unique_lock<std::mutex> lk(mutex_);
 
       std::string body(response.GetBody().begin(), response.GetBody().end());
 
-      if (console_debug_)
-      {
-        OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Got response from Elasticsearch, "
-                                << BuildResponseLogMessage(response, body));
-      }
-
       // Kept with the outcome it decides, and only by the call that decides it. Export() folds the
       // status into the ExportResult, since storing the body alone let a non-2xx response be
       // reported as a success, and a status from one response beside a body from another is not
@@ -95,12 +92,26 @@ class ResponseHandler : public http_client::EventHandler
       // non-2xx response is not logged a second time here with the full body.
       if (completion_ == CompletionState::Pending)
       {
+        if (console_debug_)
+        {
+          described = BuildResponseLogMessage(response, body);
+        }
         status_code_ = response.GetStatusCode();
         body_        = std::move(body);
         recordCompletionLocked(CompletionState::Success);
+        decided = true;
       }
     }
     cv_.notify_all();
+
+    // Outside the lock and only for the response that decided the outcome. The log handler is
+    // replaceable application code, so one that reaches back into this exporter would otherwise do
+    // it while this thread holds mutex_, and a line for a response whose status and body were
+    // discarded describes an answer the caller was never given.
+    if (decided && console_debug_)
+    {
+      OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Got response from Elasticsearch, " << described);
+    }
   }
 
   /**
diff --git a/exporters/elasticsearch/test/es_log_record_exporter_test.cc b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
index 3701d7faff..0a48e3d361 100644
--- a/exporters/elasticsearch/test/es_log_record_exporter_test.cc
+++ b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
@@ -520,10 +520,13 @@ class FakeHttpClient : public http_client::HttpClient
 
 // The response has to answer one operation per record, so a case using a body with N items has to
 // export N records or it would be rejected on the count before reaching what it means to test.
-opentelemetry::sdk::common::ExportResult ExportWith(EventScript script, std::size_t records = 1)
+opentelemetry::sdk::common::ExportResult ExportWith(EventScript script,
+                                                    std::size_t records = 1,
+                                                    bool console_debug  = false)
 {
   auto client = std::make_shared<FakeHttpClient>(std::move(script));
   logs_exporter::ElasticsearchExporterOptions options;
+  options.console_debug_ = console_debug;
   logs_exporter::ElasticsearchLogRecordExporter exporter(options, client);
 
   std::vector<std::unique_ptr<sdklogs::Recordable>> batch;
@@ -695,8 +698,88 @@ class ElasticsearchLogsExporterAsyncTests : public ::testing::Test
   nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler> handler_;
   nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler> previous_handler_;
 };
+
+class ElasticsearchLogsExporterDebugLoggingTests : public ::testing::Test
+{
+protected:
+  void SetUp() override
+  {
+    // One skip point: GTEST_SKIP returns, and a second below it would leave the rest of this body
+    // unreachable, which MSVC reports as C4702 under maintainer mode.
+#ifdef ENABLE_ASYNC_EXPORT
+    GTEST_SKIP() << "Export() returns without waiting when async export is enabled";
+#elif OTEL_INTERNAL_LOG_LEVEL < OTEL_INTERNAL_LOG_LEVEL_DEBUG
+    // The line these read is not filtered below debug level, it is not written, so a response
+    // that described itself and one that stayed quiet would look the same here.
+    GTEST_SKIP() << "the line these observe is compiled out below debug level";
+#else
+    previous_handler_ = opentelemetry::sdk::common::internal_log::GlobalLogHandler::GetLogHandler();
+    handler_          = nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler>(
+        new CapturingLogHandler());
+    opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogHandler(handler_);
+
+    // The compiled in level only decides whether the call exists. What it dispatches is filtered
+    // again at runtime, and the default there is Warning, so without this the handler is installed
+    // and never told anything.
+    previous_level_ = opentelemetry::sdk::common::internal_log::GlobalLogHandler::GetLogLevel();
+    opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogLevel(
+        opentelemetry::sdk::common::internal_log::LogLevel::Debug);
+    restore_level_ = true;
+#endif
+  }
+
+  void TearDown() override
+  {
+    if (restore_level_)
+    {
+      opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogLevel(previous_level_);
+    }
+    if (handler_)
+    {
+      opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogHandler(previous_handler_);
+    }
+  }
+
+  std::size_t ResponsesDescribed() const
+  {
+    std::size_t described = 0;
+    for (const auto &message : static_cast<CapturingLogHandler *>(handler_.get())->messages)
+    {
+      if (message.second.find("Got response from Elasticsearch") != std::string::npos)
+      {
+        ++described;
+      }
+    }
+    return described;
+  }
+
+  nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler> handler_;
+  nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler> previous_handler_;
+  opentelemetry::sdk::common::internal_log::LogLevel previous_level_ =
+      opentelemetry::sdk::common::internal_log::LogLevel::Warning;
+  bool restore_level_ = false;
+};
 }  // namespace
 
+// Only the response the caller was given describes itself. A second one arriving for the same
+// request has its status and body dropped, so a line for it would describe an answer nobody was
+// handed, and the two lines together would disagree about what the export returned.
+TEST_F(ElasticsearchLogsExporterDebugLoggingTests, OnlyTheResponseThatDecidedTheOutcomeIsDescribed)
+{
+  const auto result = ExportWith(
+      [](http_client::EventHandler &handler) {
+        FakeResponse first(200, kCompactSuccess);
+        handler.OnResponse(first);
+        FakeResponse second(500, kCompactSuccess);
+        handler.OnResponse(second);
+      },
+      1, true);
+
+  EXPECT_EQ(result, opentelemetry::sdk::common::ExportResult::kSuccess);
+  EXPECT_EQ(ResponsesDescribed(), static_cast<std::size_t>(1))
+      << "the losing response described itself as well";
+}
+
 TEST_F(ElasticsearchLogsExporterAsyncTests, AnAcceptedResponseIsNotReportedAsAFailure)
 {
   EXPECT_EQ(ExportWith([](http_client::EventHandler &handler) {

From 2a4b7d37d1e1cb8ca145f9054ae384c178fdea35 Mon Sep 17 00:00:00 2001
From: thc1006 <84045975+thc1006@users.noreply.github.com>
Date: Sat, 15 Aug 2026 20:16:33 +0000
Subject: [PATCH 10/12] [CHORE] Name the path the test timeout is there for,
 and say what success means now

The bound guards the synchronous wiring cases, which wait on a completion. The
asynchronous fake calls back from inside SendRequest() and Export() returns without
waiting, so it is not what a stranded completion would hang.

The changelog said an acknowledgement carries a 2xx status. It carries one that says the
operation applied, which is a narrower thing, and the batch status it sits under is
narrower still.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
---
 CHANGELOG.md                           | 5 ++++-
 exporters/elasticsearch/CMakeLists.txt | 7 ++++---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 76e0698fa5..f578920c2e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -152,7 +152,10 @@ Increment the:
 
 * [BUG] Decide Elasticsearch bulk export success from the HTTP status, the
   errors flag, and one acknowledgement per record that names its target index
-  and carries a 2xx status
+  and says the operation applied
+  [#4297](https://github.com/open-telemetry/opentelemetry-cpp/pull/4297)
+* [BUG] Stop reading an Elasticsearch bulk response that only says the request
+  was accepted as a batch that was written
   [#4297](https://github.com/open-telemetry/opentelemetry-cpp/pull/4297)
 
 * [BUG] Make SocketAddr string parsing safe and reject malformed addresses
diff --git a/exporters/elasticsearch/CMakeLists.txt b/exporters/elasticsearch/CMakeLists.txt
index d38c897ec5..999b89cdef 100644
--- a/exporters/elasticsearch/CMakeLists.txt
+++ b/exporters/elasticsearch/CMakeLists.txt
@@ -62,8 +62,9 @@ if(BUILD_TESTING)
     TEST_PREFIX exporter.
     TEST_LIST es_log_record_exporter_test)
 
-  # The asynchronous cases wait on a response that a regression can stop
-  # delivering, and a case that hangs stops the job rather than reporting. A
-  # bound turns that back into a red test.
+  # The synchronous wiring cases wait on a completion, and a regression that
+  # stops delivering one hangs the job rather than reporting. A bound turns that
+  # back into a red test. Not the asynchronous ones: their fake calls back from
+  # inside SendRequest() and Export() returns without waiting.
   set_tests_properties(${es_log_record_exporter_test} PROPERTIES TIMEOUT 120)
 endif() # BUILD_TESTING

From c8a6715ec7eb5af1fe43ba7e86a8e57a6a0e26af Mon Sep 17 00:00:00 2001
From: thc1006 <84045975+thc1006@users.noreply.github.com>
Date: Sat, 15 Aug 2026 20:24:23 +0000
Subject: [PATCH 11/12] [CHORE] Say in the contract what the statuses now have
 to be

The @return still described the 2xx band on both the request and the operation, which is
what the code stopped accepting.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
---
 .../exporters/elasticsearch/detail/es_bulk_response.h       | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
index ba4797a950..ad11df28db 100644
--- a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
+++ b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
@@ -69,9 +69,9 @@ inline bool IsAcknowledgedStatus(const nlohmann::json &status) noexcept
  * @param body the raw response body
  * @param expected_items the number of index operations the request submitted
  * @param failure_reason a best-effort explanation when this returns false
- * @return true only when the status is 2xx, "errors" is false, and "items" holds exactly
- *         expected_items index results that each name a target index, acknowledge a 2xx status
- *         and carry no error
+ * @return true only when the status is 200, "errors" is false, and "items" holds exactly
+ *         expected_items index results that each name a target index, acknowledge a status the
+ *         operation applied under, and carry no error
  */
 inline bool IsBulkResponseSuccessful(int status_code,
                                      const std::string &body,

From 4a4b935aedc855c84a80e5d0d90d5c2427c393b8 Mon Sep 17 00:00:00 2001
From: thc1006 <84045975+thc1006@users.noreply.github.com>
Date: Sat, 15 Aug 2026 20:43:57 +0000
Subject: [PATCH 12/12] [CHORE] Hold the status helper to its own precondition,
 and take the log level with the handler

Three small ones.

IsAcknowledgedStatus() is noexcept and reached straight for get(), leaving the type check
to whoever called it. The one caller does check, so nothing is wrong today, but the two
ways a later one could get it wrong are not symmetrical: a string throws, and out of a
noexcept function that is std::terminate rather than a wrong answer, while a float does
not throw at all, it truncates, so 200.5 would have answered for 200. The check is now
where the assumption is.

The asynchronous cases saved and restored the log handler but not the log level. The
level is process wide, another case in the same binary can lower it, and a failure the
runtime filtered out looks exactly like one the parser never reported, so they would have
stopped discriminating without saying so. This is not hypothetical in this file: the
debug case added earlier passed for that reason until the level went in beside the
handler.

The header now says what it does not do. A repeated key is folded before the document
reaches this, so a body carrying both "errors": true and "errors": false arrives as
whichever one the parser kept, and nothing here can tell the other was sent. Deciding an
outcome from the documented fields is what this is for; standing up to a responder that
is trying to be believed is not.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
---
 .../elasticsearch/detail/es_bulk_response.h   | 19 ++++++++++++++++++-
 .../test/es_log_record_exporter_test.cc       | 15 +++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
index ad11df28db..56d0041675 100644
--- a/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
+++ b/exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h
@@ -47,9 +47,21 @@ inline bool IsAppliedStatus(nlohmann::json::number_integer_t value) noexcept
   return 200 == value || 201 == value;
 }
 
-/** Whether an acknowledged operation status is one that applied the operation. */
+/**
+ * Whether an acknowledged operation status is one that applied the operation.
+ *
+ * The type check is here rather than left to the caller, although the one caller does it too.
+ * This is noexcept, and get() on a value that is not a number throws, so a caller that forgot
+ * would not get a wrong answer, it would get std::terminate. A float does not throw: it
+ * truncates, so 200.5 would answer for 200.
+ */
 inline bool IsAcknowledgedStatus(const nlohmann::json &status) noexcept
 {
+  if (!status.is_number_integer())
+  {
+    return false;
+  }
+
   if (status.is_number_unsigned())
   {
     return IsAppliedStatus(status.get<nlohmann::json::number_unsigned_t>());
@@ -64,6 +76,11 @@ inline bool IsAcknowledgedStatus(const nlohmann::json &status) noexcept
  * Callers include noexcept response handlers, so nothing may escape. Anything that stops the body
  * being inspected counts as a failed export.
  *
+ * This reads the fields the bulk response documents to decide an outcome. It is not a check
+ * against a responder that is trying to be believed: a repeated key, for one, is folded before
+ * this sees the document, so a body carrying both "errors": true and "errors": false arrives as
+ * whichever one the parser kept, and nothing here can tell that the other was ever sent.
+ *
  * @param status_code the response status, which the caller passes rather than this reading the
  *        body alone: "errors" describes item outcomes and cannot override a transport error
  * @param body the raw response body
diff --git a/exporters/elasticsearch/test/es_log_record_exporter_test.cc b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
index 0a48e3d361..21a0593991 100644
--- a/exporters/elasticsearch/test/es_log_record_exporter_test.cc
+++ b/exporters/elasticsearch/test/es_log_record_exporter_test.cc
@@ -668,6 +668,14 @@ class ElasticsearchLogsExporterAsyncTests : public ::testing::Test
     handler_          = nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler>(
         new CapturingLogHandler());
     opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogHandler(handler_);
+
+    // The level as well as the handler. It is process wide and another case in this binary can
+    // lower it, and a failure filtered out at runtime looks exactly like one the parser never
+    // reported, so these cases would stop discriminating without saying so.
+    previous_level_ = opentelemetry::sdk::common::internal_log::GlobalLogHandler::GetLogLevel();
+    opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogLevel(
+        opentelemetry::sdk::common::internal_log::LogLevel::Error);
+    restore_level_ = true;
 #endif
   }
 
@@ -675,6 +683,10 @@ class ElasticsearchLogsExporterAsyncTests : public ::testing::Test
   {
     // Put back what was found, rather than a fresh default that would displace a handler another
     // case in this binary had installed.
+    if (restore_level_)
+    {
+      opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogLevel(previous_level_);
+    }
     if (handler_)
     {
       opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogHandler(previous_handler_);
@@ -697,6 +709,9 @@ class ElasticsearchLogsExporterAsyncTests : public ::testing::Test
 
   nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler> handler_;
   nostd::shared_ptr<opentelemetry::sdk::common::internal_log::LogHandler> previous_handler_;
+  opentelemetry::sdk::common::internal_log::LogLevel previous_level_ =
+      opentelemetry::sdk::common::internal_log::LogLevel::Warning;
+  bool restore_level_ = false;
 };
 
 class ElasticsearchLogsExporterDebugLoggingTests : public ::testing::Test