Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mock for http exporters #1185

Merged
merged 46 commits into from
Feb 19, 2022
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
8884077
zipkin
esigo Jan 28, 2022
843b2c2
otlp_http
esigo Jan 28, 2022
fcd0816
virtual for test only
esigo Jan 29, 2022
f914d64
otlp_http_log
esigo Jan 29, 2022
aeb88ab
client message matcher
esigo Jan 30, 2022
67f969a
otlp_http fix binary integeratoin
esigo Jan 30, 2022
1da7779
clean up
esigo Jan 30, 2022
a4256db
otlp_http_log_exporter
esigo Jan 30, 2022
230add4
otlp_http_log more checks
esigo Jan 30, 2022
8f4e600
clean up
esigo Jan 30, 2022
3ef1582
zipkin matcher
esigo Jan 30, 2022
a47f576
cmake define
esigo Jan 30, 2022
adcc7d3
bazel test option
esigo Jan 30, 2022
8f06a91
fix bazel CI
esigo Jan 30, 2022
2bf1bd8
bazel mac
esigo Jan 30, 2022
5c7ec0a
fix race
esigo Jan 30, 2022
c6c6b4b
Windows TEST define
esigo Jan 30, 2022
296de51
rename define
esigo Jan 31, 2022
c00a3fd
Merge branch 'mock-http-exporters' of https:/esigo/opente…
esigo Jan 31, 2022
f5e84e7
fix typo
esigo Jan 31, 2022
f9d6d5b
Merge branch 'main' into mock-http-exporters
esigo Jan 31, 2022
03777d5
clean
esigo Jan 31, 2022
a67edc1
Merge branch 'main' into mock-http-exporters
esigo Feb 4, 2022
aaa04f2
no_send client otlp_http_exporter
esigo Feb 5, 2022
19ec997
no_send client otlp_http_log_exporter
esigo Feb 5, 2022
79a1709
Merge branch 'main' into mock-http-exporters
esigo Feb 5, 2022
62cc1dd
revert unnecessary changes
esigo Feb 5, 2022
4e3b68d
revert unnecessary changes
esigo Feb 5, 2022
79341af
Merge branch 'main' into mock-http-exporters
esigo Feb 11, 2022
ceac43a
Merge branch 'main' into mock-http-exporters
esigo Feb 12, 2022
fe5dd6f
http_client_nosend
esigo Feb 12, 2022
f724e1a
cmake
esigo Feb 12, 2022
f500f4d
http_client_factory bazel
esigo Feb 12, 2022
75427be
http_client_factory cmake
esigo Feb 12, 2022
1101015
fix cmake
esigo Feb 13, 2022
7033b30
fix cmake link
esigo Feb 13, 2022
7f3773a
fix cmake Windows
esigo Feb 13, 2022
486fa33
revert client factory
esigo Feb 16, 2022
cbd5d10
Merge branch 'main' into mock-http-exporters
esigo Feb 16, 2022
7b18c4c
fix CI
esigo Feb 16, 2022
525bcc4
Merge branch 'main' into mock-http-exporters
esigo Feb 17, 2022
52650a6
Merge branch 'main' into mock-http-exporters
lalitb Feb 19, 2022
9d82629
Merge branch 'main' into mock-http-exporters
lalitb Feb 19, 2022
d9b7291
Merge branch 'main' into mock-http-exporters
lalitb Feb 19, 2022
6fd0201
remove dup code
esigo Feb 19, 2022
a643d1a
clean code dup
esigo Feb 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ list(APPEND CMAKE_PREFIX_PATH "${CMAKE_BINARY_DIR}")

include(CTest)
if(BUILD_TESTING)
add_definitions(-DENABLE_TEST)
if(EXISTS ${CMAKE_BINARY_DIR}/lib/libgtest.a)
# Prefer GTest from build tree. GTest is not always working with
# CMAKE_PREFIX_PATH
Expand Down Expand Up @@ -426,6 +427,7 @@ include_directories(api/include)
add_subdirectory(api)

if(NOT WITH_API_ONLY)
set(BUILD_TESTING ${BUILD_TESTING})
include_directories(sdk/include)
include_directories(sdk)
include_directories(ext/include)
Expand Down
2 changes: 1 addition & 1 deletion ci/do_ci.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ $VCPKG_DIR="$SRC_DIR\vcpkg"

switch ($action) {
"bazel.build" {
bazel build $BAZEL_OPTIONS -- //...
bazel build --copt=-DENABLE_TEST $BAZEL_OPTIONS -- //...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit - --copt=-DENABLE_TEST is also part of BAZEL_OPTIONS, so can be removed here.

$exit = $LASTEXITCODE
if ($exit -ne 0) {
exit $exit
Expand Down
2 changes: 1 addition & 1 deletion ci/do_ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ mkdir -p "${BUILD_DIR}"
[ -z "${PLUGIN_DIR}" ] && export PLUGIN_DIR=$HOME/plugin
mkdir -p "${PLUGIN_DIR}"

BAZEL_OPTIONS="--copt=-DENABLE_METRICS_PREVIEW --copt=-DENABLE_LOGS_PREVIEW"
BAZEL_OPTIONS="--copt=-DENABLE_METRICS_PREVIEW --copt=-DENABLE_LOGS_PREVIEW --copt=-DENABLE_TEST"
BAZEL_TEST_OPTIONS="$BAZEL_OPTIONS --test_output=errors"

# https:/bazelbuild/bazel/issues/4341
Expand Down
2 changes: 1 addition & 1 deletion examples/http/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ cc_binary(
"//api",
"//exporters/ostream:ostream_span_exporter",
"//ext:headers",
"//ext/src/http/client/curl:http_client_curl",
"//ext/src/http/client:http_client_factory",
"//sdk/src/trace",
],
)
Expand Down
8 changes: 4 additions & 4 deletions examples/http/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ else()
add_executable(http_server server.cc)

target_link_libraries(
http_client ${CMAKE_THREAD_LIBS_INIT} opentelemetry_trace http_client_curl
opentelemetry_exporter_ostream_span ${CURL_LIBRARIES})
http_client ${CMAKE_THREAD_LIBS_INIT} opentelemetry_trace
http_client_factory opentelemetry_exporter_ostream_span ${CURL_LIBRARIES})

target_link_libraries(
http_server ${CMAKE_THREAD_LIBS_INIT} opentelemetry_trace http_client_curl
opentelemetry_exporter_ostream_span)
http_server ${CMAKE_THREAD_LIBS_INIT} opentelemetry_trace
http_client_factory opentelemetry_exporter_ostream_span)
endif()
2 changes: 1 addition & 1 deletion exporters/elasticsearch/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ cc_library(
tags = ["es"],
deps = [
"//ext:headers",
"//ext/src/http/client/curl:http_client_curl",
"//ext/src/http/client:http_client_factory",
"//sdk/src/logs",
"@curl",
"@github_nlohmann_json//:json",
Expand Down
2 changes: 1 addition & 1 deletion exporters/elasticsearch/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ target_include_directories(

target_link_libraries(
opentelemetry_exporter_elasticsearch_logs
PUBLIC opentelemetry_trace opentelemetry_logs http_client_curl)
PUBLIC opentelemetry_trace opentelemetry_logs http_client_factory)

install(
TARGETS opentelemetry_exporter_elasticsearch_logs
Expand Down
2 changes: 1 addition & 1 deletion exporters/jaeger/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ cc_library(
tags = ["jaeger"],
deps = [
":thrift",
"//ext/src/http/client/curl:http_client_curl",
"//ext/src/http/client:http_client_factory",
],
)

Expand Down
2 changes: 1 addition & 1 deletion exporters/jaeger/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ target_include_directories(

target_link_libraries(
opentelemetry_exporter_jaeger_trace
PUBLIC opentelemetry_resources http_client_curl
PUBLIC opentelemetry_resources http_client_factory
PRIVATE thrift::thrift)

if(MSVC)
Expand Down
4 changes: 3 additions & 1 deletion exporters/otlp/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ cc_library(
],
deps = [
"//api",
"//ext/src/http/client/curl:http_client_curl",
"//ext/src/http/client:http_client_factory",
"//sdk:headers",
"@com_github_opentelemetry_proto//:common_proto_cc",
"@github_nlohmann_json//:json",
Expand Down Expand Up @@ -234,6 +234,7 @@ cc_test(
deps = [
":otlp_http_exporter",
"//api",
"//ext/src/http/client/nosend:http_client_nosend",
"@com_google_googletest//:gtest_main",
],
)
Expand All @@ -249,6 +250,7 @@ cc_test(
deps = [
":otlp_http_log_exporter",
"//api",
"//ext/src/http/client/nosend:http_client_nosend",
"@com_google_googletest//:gtest_main",
],
)
Expand Down
14 changes: 9 additions & 5 deletions exporters/otlp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ if(WITH_OTLP_HTTP)
PROPERTIES EXPORT_NAME otlp_http_client)
target_link_libraries(
opentelemetry_exporter_otlp_http_client
PUBLIC opentelemetry_sdk opentelemetry_proto http_client_curl
PUBLIC opentelemetry_sdk opentelemetry_proto http_client_factory
nlohmann_json::nlohmann_json)
if(nlohmann_json_clone)
add_dependencies(opentelemetry_exporter_otlp_http_client
Expand Down Expand Up @@ -170,7 +170,7 @@ if(BUILD_TESTING)
add_executable(otlp_http_exporter_test test/otlp_http_exporter_test.cc)
target_link_libraries(
otlp_http_exporter_test ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}
${GMOCK_LIB} opentelemetry_exporter_otlp_http)
${GMOCK_LIB} opentelemetry_exporter_otlp_http http_client_factory)
gtest_add_tests(
TARGET otlp_http_exporter_test
TEST_PREFIX exporter.otlp.
Expand All @@ -180,9 +180,13 @@ if(BUILD_TESTING)
add_executable(otlp_http_log_exporter_test
test/otlp_http_log_exporter_test.cc)
target_link_libraries(
otlp_http_log_exporter_test ${GTEST_BOTH_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT} ${GMOCK_LIB}
opentelemetry_exporter_otlp_http_log opentelemetry_logs)
otlp_http_log_exporter_test
${GTEST_BOTH_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT}
${GMOCK_LIB}
opentelemetry_exporter_otlp_http_log
opentelemetry_logs
http_client_factory)
gtest_add_tests(
TARGET otlp_http_log_exporter_test
TEST_PREFIX exporter.otlp.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,17 @@ class OtlpHttpClient
std::string http_uri_;
mutable opentelemetry::common::SpinLockMutex lock_;
bool isShutdown() const noexcept;
// For testing
friend class OtlpHttpExporterTestPeer;
friend class OtlpHttpLogExporterTestPeer;
/**
* Create an OtlpHttpClient using the specified http client.
* Only tests can call this constructor directly.
* @param options the Otlp http client options to be used for exporting
* @param http_client the http client to be used for exporting
*/
OtlpHttpClient(OtlpHttpClientOptions &&options,
std::shared_ptr<ext::http::client::HttpClient> http_client);
};
} // namespace otlp
} // namespace exporter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,15 @@ class OtlpHttpExporter final : public opentelemetry::sdk::trace::SpanExporter
const OtlpHttpExporterOptions options_;

// Object that stores the HTTP sessions that have been created
OtlpHttpClient http_client_;
std::unique_ptr<OtlpHttpClient> http_client_;
// For testing
friend class OtlpHttpExporterTestPeer;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated with line 95?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, cleaned.

/**
* Create an OtlpHttpExporter using the specified http client.
* Only tests can call this constructor directly.
* @param http_client the http client to be used for exporting
*/
OtlpHttpExporter(std::unique_ptr<OtlpHttpClient> http_client);
};
} // namespace otlp
} // namespace exporter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,15 @@ class OtlpHttpLogExporter final : public opentelemetry::sdk::logs::LogExporter
const OtlpHttpLogExporterOptions options_;

// Object that stores the HTTP sessions that have been created
OtlpHttpClient http_client_;
std::unique_ptr<OtlpHttpClient> http_client_;
// For testing
friend class OtlpHttpLogExporterTestPeer;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated with line 94.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, cleaned

/**
* Create an OtlpHttpLogExporter using the specified http client.
* Only tests can call this constructor directly.
* @param http_client the http client to be used for exporting
*/
OtlpHttpLogExporter(std::unique_ptr<OtlpHttpClient> http_client);
};
} // namespace otlp
} // namespace exporter
Expand Down
10 changes: 7 additions & 3 deletions exporters/otlp/src/otlp_http_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@
#include "opentelemetry/ext/http/client/http_client_factory.h"
#include "opentelemetry/ext/http/common/url_parser.h"

#include "nlohmann/json.hpp"

#include "opentelemetry/exporters/otlp/protobuf_include_prefix.h"

#include <mutex>
#include "google/protobuf/message.h"
#include "google/protobuf/reflection.h"
#include "google/protobuf/stubs/common.h"
#include "nlohmann/json.hpp"

#if defined(GOOGLE_PROTOBUF_VERSION) && GOOGLE_PROTOBUF_VERSION >= 3007000
# include "google/protobuf/stubs/strutil.h"
Expand Down Expand Up @@ -362,7 +361,7 @@ static void ConvertGenericMessageToJson(nlohmann::json &value,
}
}

static bool SerializeToHttpBody(http_client::Body &output, const google::protobuf::Message &message)
bool SerializeToHttpBody(http_client::Body &output, const google::protobuf::Message &message)
{
auto body_size = message.ByteSizeLong();
if (body_size > 0)
Expand Down Expand Up @@ -566,6 +565,11 @@ OtlpHttpClient::OtlpHttpClient(OtlpHttpClientOptions &&options)
: options_(options), http_client_(http_client::HttpClientFactory::Create())
{}

OtlpHttpClient::OtlpHttpClient(OtlpHttpClientOptions &&options,
std::shared_ptr<ext::http::client::HttpClient> http_client)
: options_(options), http_client_(http_client)
{}

// ----------------------------- HTTP Client methods ------------------------------
opentelemetry::sdk::common::ExportResult OtlpHttpClient::Export(
const google::protobuf::Message &message) noexcept
Expand Down
21 changes: 12 additions & 9 deletions exporters/otlp/src/otlp_http_exporter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,18 @@ OtlpHttpExporter::OtlpHttpExporter() : OtlpHttpExporter(OtlpHttpExporterOptions(

OtlpHttpExporter::OtlpHttpExporter(const OtlpHttpExporterOptions &options)
: options_(options),
http_client_(OtlpHttpClientOptions(options.url,
options.content_type,
options.json_bytes_mapping,
options.use_json_name,
options.console_debug,
options.timeout,
options.http_headers))
http_client_(new OtlpHttpClient(OtlpHttpClientOptions(options.url,
options.content_type,
options.json_bytes_mapping,
options.use_json_name,
options.console_debug,
options.timeout,
options.http_headers)))
{}

OtlpHttpExporter::OtlpHttpExporter(std::unique_ptr<OtlpHttpClient> http_client)
: options_(OtlpHttpExporterOptions()), http_client_(std::move(http_client))
{}
// ----------------------------- Exporter methods ------------------------------

std::unique_ptr<opentelemetry::sdk::trace::Recordable> OtlpHttpExporter::MakeRecordable() noexcept
Expand All @@ -45,12 +48,12 @@ opentelemetry::sdk::common::ExportResult OtlpHttpExporter::Export(
{
proto::collector::trace::v1::ExportTraceServiceRequest service_request;
OtlpRecordableUtils::PopulateRequest(spans, &service_request);
return http_client_.Export(service_request);
return http_client_->Export(service_request);
}

bool OtlpHttpExporter::Shutdown(std::chrono::microseconds timeout) noexcept
{
return http_client_.Shutdown(timeout);
return http_client_->Shutdown(timeout);
}

} // namespace otlp
Expand Down
21 changes: 12 additions & 9 deletions exporters/otlp/src/otlp_http_log_exporter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,18 @@ OtlpHttpLogExporter::OtlpHttpLogExporter() : OtlpHttpLogExporter(OtlpHttpLogExpo

OtlpHttpLogExporter::OtlpHttpLogExporter(const OtlpHttpLogExporterOptions &options)
: options_(options),
http_client_(OtlpHttpClientOptions(options.url,
options.content_type,
options.json_bytes_mapping,
options.use_json_name,
options.console_debug,
options.timeout,
options.http_headers))
http_client_(new OtlpHttpClient(OtlpHttpClientOptions(options.url,
options.content_type,
options.json_bytes_mapping,
options.use_json_name,
options.console_debug,
options.timeout,
options.http_headers)))
{}

OtlpHttpLogExporter::OtlpHttpLogExporter(std::unique_ptr<OtlpHttpClient> http_client)
: options_(OtlpHttpLogExporterOptions()), http_client_(std::move(http_client))
{}
// ----------------------------- Exporter methods ------------------------------

std::unique_ptr<opentelemetry::sdk::logs::Recordable> OtlpHttpLogExporter::MakeRecordable() noexcept
Expand All @@ -47,12 +50,12 @@ opentelemetry::sdk::common::ExportResult OtlpHttpLogExporter::Export(
{
proto::collector::logs::v1::ExportLogsServiceRequest service_request;
OtlpRecordableUtils::PopulateRequest(logs, &service_request);
return http_client_.Export(service_request);
return http_client_->Export(service_request);
}

bool OtlpHttpLogExporter::Shutdown(std::chrono::microseconds timeout) noexcept
{
return http_client_.Shutdown(timeout);
return http_client_->Shutdown(timeout);
}

} // namespace otlp
Expand Down
Loading