Skip to content

Commit 189419b

Browse files
committed
Cover the path the response takes to the verdict
The helper tests call IsBulkResponseSuccessful() directly, so they cannot show that the status and the body reach it. A handler that stored a fixed status, or an Export() that never asked for one, passes all of them. Three cases through the exporter with a fake HTTP client: an accepted bulk response, the rejected item from #4295 whose shard counter still reads "failed" : 0, and a 500 carrying a body the parser would otherwise accept. The fake client is the same one #4331 adds to this file. Whichever lands first, the other drops the duplicate when it rebases.
1 parent f71fdf7 commit 189419b

1 file changed

Lines changed: 152 additions & 0 deletions

File tree

‎exporters/elasticsearch/test/es_log_record_exporter_test.cc‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
#include "opentelemetry/common/timestamp.h"
66
#include "opentelemetry/exporters/elasticsearch/detail/es_bulk_response.h"
77
#include "opentelemetry/exporters/elasticsearch/es_log_recordable.h"
8+
#include "opentelemetry/ext/http/client/http_client.h"
89
#include "opentelemetry/logs/severity.h"
10+
#include "opentelemetry/nostd/function_ref.h"
911
#include "opentelemetry/nostd/span.h"
1012
#include "opentelemetry/nostd/string_view.h"
1113
#include "opentelemetry/nostd/utility.h"
@@ -20,6 +22,8 @@
2022
#include <chrono>
2123
#include <cstddef>
2224
#include <cstdint>
25+
#include <functional>
26+
#include <memory>
2327
#include <string>
2428
#include <utility>
2529
#include "nlohmann/json.hpp"
@@ -251,3 +255,151 @@ TEST(ElasticsearchBulkResponseTests, DoesNotRequireItemsForSuccess)
251255
std::string reason;
252256
EXPECT_TRUE(logs_exporter::detail::IsBulkResponseSuccessful(200, R"({"errors":false})", reason));
253257
}
258+
// ---------------------------------------------------------------------------
259+
// The response travelling from the HTTP callback to the verdict.
260+
//
261+
// The cases above call IsBulkResponseSuccessful() directly, so they cannot show
262+
// that the status and the body actually reach it. A fake HTTP client drives the
263+
// callbacks from inside SendRequest(), which runs before Export() reaches the
264+
// wait, and the assertions are on the ExportResult.
265+
//
266+
// This fixture is the same one #4331 adds to this file. Whichever lands first,
267+
// the other drops the duplicate when it rebases.
268+
// ---------------------------------------------------------------------------
269+
namespace
270+
{
271+
namespace http_client = opentelemetry::ext::http::client;
272+
273+
class FakeResponse : public http_client::Response
274+
{
275+
public:
276+
FakeResponse(http_client::StatusCode status, const std::string &body)
277+
: status_(status), body_(body.begin(), body.end())
278+
{}
279+
const http_client::Body &GetBody() const noexcept override { return body_; }
280+
bool ForEachHeader(
281+
nostd::function_ref<bool(nostd::string_view, nostd::string_view)>) const noexcept override
282+
{
283+
return true;
284+
}
285+
bool ForEachHeader(
286+
const nostd::string_view &,
287+
nostd::function_ref<bool(nostd::string_view, nostd::string_view)>) const noexcept override
288+
{
289+
return true;
290+
}
291+
http_client::StatusCode GetStatusCode() const noexcept override { return status_; }
292+
293+
private:
294+
http_client::StatusCode status_;
295+
http_client::Body body_;
296+
};
297+
298+
class FakeRequest : public http_client::Request
299+
{
300+
public:
301+
void SetMethod(http_client::Method) noexcept override {}
302+
void SetUri(nostd::string_view) noexcept override {}
303+
void SetSslOptions(const http_client::HttpSslOptions &) noexcept override {}
304+
void SetBody(http_client::Body &) noexcept override {}
305+
void AddHeader(nostd::string_view, nostd::string_view) noexcept override {}
306+
void ReplaceHeader(nostd::string_view, nostd::string_view) noexcept override {}
307+
void SetTimeoutMs(std::chrono::milliseconds) noexcept override {}
308+
void SetCompression(const http_client::Compression &) noexcept override {}
309+
void EnableLogging(bool) noexcept override {}
310+
void SetRetryPolicy(const http_client::RetryPolicy &) noexcept override {}
311+
};
312+
313+
using EventScript = std::function<void(http_client::EventHandler &)>;
314+
315+
class FakeSession : public http_client::Session
316+
{
317+
public:
318+
explicit FakeSession(EventScript script) : script_(std::move(script)) {}
319+
std::shared_ptr<http_client::Request> CreateRequest() noexcept override
320+
{
321+
return std::make_shared<FakeRequest>();
322+
}
323+
void SendRequest(std::shared_ptr<http_client::EventHandler> handler) noexcept override
324+
{
325+
script_(*handler);
326+
}
327+
bool IsSessionActive() noexcept override { return false; }
328+
bool CancelSession() noexcept override { return true; }
329+
bool FinishSession() noexcept override { return true; }
330+
331+
private:
332+
EventScript script_;
333+
};
334+
335+
class FakeHttpClient : public http_client::HttpClient
336+
{
337+
public:
338+
explicit FakeHttpClient(EventScript script) : script_(std::move(script)) {}
339+
std::shared_ptr<http_client::Session> CreateSession(nostd::string_view) noexcept override
340+
{
341+
return std::make_shared<FakeSession>(script_);
342+
}
343+
bool CancelAllSessions() noexcept override { return true; }
344+
bool FinishAllSessions() noexcept override { return true; }
345+
void SetMaxSessionsPerConnection(std::size_t) noexcept override {}
346+
347+
private:
348+
EventScript script_;
349+
};
350+
351+
opentelemetry::sdk::common::ExportResult ExportWith(EventScript script)
352+
{
353+
auto client = std::make_shared<FakeHttpClient>(std::move(script));
354+
logs_exporter::ElasticsearchExporterOptions options;
355+
logs_exporter::ElasticsearchLogRecordExporter exporter(options, client);
356+
auto record = exporter.MakeRecordable();
357+
return exporter.Export(nostd::span<std::unique_ptr<sdklogs::Recordable>>(&record, 1));
358+
}
359+
} // namespace
360+
361+
// The synchronous wait exists only when the exporter is built without async export, so these cases
362+
// skip rather than compile out: gtest_add_tests reads the source, and a case that disappeared from
363+
// the binary would still be registered with CTest.
364+
#ifdef ENABLE_ASYNC_EXPORT
365+
# define SKIP_WITHOUT_SYNC_EXPORT() \
366+
GTEST_SKIP() << "Export() returns without waiting when async export is enabled"
367+
#else
368+
# define SKIP_WITHOUT_SYNC_EXPORT() (void)0
369+
#endif
370+
371+
// A body the parser accepts, through the whole path rather than through the helper alone.
372+
TEST(ElasticsearchLogsExporterWiringTests, AcceptedBulkResponseIsASuccessfulExport)
373+
{
374+
SKIP_WITHOUT_SYNC_EXPORT();
375+
const auto result = ExportWith([](http_client::EventHandler &handler) {
376+
FakeResponse response(200, kPrettySuccess);
377+
handler.OnResponse(response);
378+
});
379+
EXPECT_EQ(result, opentelemetry::sdk::common::ExportResult::kSuccess);
380+
}
381+
382+
// The case #4295 reports: 200 with a rejected item. The shard counter in the body still reads
383+
// "failed" : 0, which is what the old substring check matched on, so this fails before the fix
384+
// and passes after it.
385+
TEST(ElasticsearchLogsExporterWiringTests, RejectedItemIsAFailedExport)
386+
{
387+
SKIP_WITHOUT_SYNC_EXPORT();
388+
const auto result = ExportWith([](http_client::EventHandler &handler) {
389+
FakeResponse response(200, kOneItemRejected);
390+
handler.OnResponse(response);
391+
});
392+
EXPECT_EQ(result, opentelemetry::sdk::common::ExportResult::kFailure);
393+
}
394+
395+
// The status has to reach the parser, not just the body. A handler that stored a fixed status,
396+
// or an Export() that never asked for it, would still pass every case above.
397+
TEST(ElasticsearchLogsExporterWiringTests, ServerErrorIsAFailedExportEvenWithAnAcceptedBody)
398+
{
399+
SKIP_WITHOUT_SYNC_EXPORT();
400+
const auto result = ExportWith([](http_client::EventHandler &handler) {
401+
FakeResponse response(500, kPrettySuccess);
402+
handler.OnResponse(response);
403+
});
404+
EXPECT_EQ(result, opentelemetry::sdk::common::ExportResult::kFailure);
405+
}

0 commit comments

Comments
 (0)