From 73b79ab3c2408065f5eacaa5bdd4bee459ae9ceb Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 14 Dec 2020 13:48:21 +0530 Subject: [PATCH 01/10] add client factory --- ext/test/http/CMakeLists.txt | 1 + ext/test/http/curl_http_test.cc | 20 ++++++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/ext/test/http/CMakeLists.txt b/ext/test/http/CMakeLists.txt index ab558d2ca7..ee80b61c9e 100644 --- a/ext/test/http/CMakeLists.txt +++ b/ext/test/http/CMakeLists.txt @@ -2,6 +2,7 @@ find_package(CURL) if(CURL_FOUND) set(CURL_LIBRARY "-lcurl") set(FILENAME curl_http_test) + add_compile_definitions(WITH_CURL) add_executable(${FILENAME} ${FILENAME}.cc) include_directories(${CURL_INCLUDE_DIR}) target_link_libraries(${FILENAME} ${GTEST_BOTH_LIBRARIES} diff --git a/ext/test/http/curl_http_test.cc b/ext/test/http/curl_http_test.cc index 1c7245ed47..56051e8377 100644 --- a/ext/test/http/curl_http_test.cc +++ b/ext/test/http/curl_http_test.cc @@ -1,4 +1,5 @@ #include "opentelemetry/ext//http/client/curl//http_client_curl.h" +#include "opentelemetry/ext/http/client/http_client_factory.h" #include "opentelemetry/ext/http/server/http_server.h" #include @@ -182,9 +183,10 @@ TEST_F(BasicCurlHttpTests, HttpResponse) TEST_F(BasicCurlHttpTests, SendGetRequest) { received_requests_.clear(); - curl::SessionManager session_manager; + auto session_manager = http_client::HttpClientFactory::Create(); + EXPECT_TRUE(session_manager != nullptr); - auto session = session_manager.CreateSession("127.0.0.1", HTTP_PORT); + auto session = session_manager->CreateSession("127.0.0.1", HTTP_PORT); auto request = session->CreateRequest(); request->SetUri("get/"); GetEventHandler *handler = new GetEventHandler(); @@ -198,9 +200,10 @@ TEST_F(BasicCurlHttpTests, SendGetRequest) TEST_F(BasicCurlHttpTests, SendPostRequest) { received_requests_.clear(); - curl::SessionManager session_manager; + auto session_manager = http_client::HttpClientFactory::Create(); + EXPECT_TRUE(session_manager != nullptr); - auto session = session_manager.CreateSession("127.0.0.1", HTTP_PORT); + auto session = session_manager->CreateSession("127.0.0.1", HTTP_PORT); auto request = session->CreateRequest(); request->SetUri("post/"); request->SetMethod(http_client::Method::Post); @@ -215,8 +218,8 @@ TEST_F(BasicCurlHttpTests, SendPostRequest) session->FinishSession(); ASSERT_TRUE(handler->is_called_); - session_manager.CancelAllSessions(); - session_manager.FinishAllSessions(); + session_manager->CancelAllSessions(); + session_manager->FinishAllSessions(); delete handler; } @@ -224,9 +227,10 @@ TEST_F(BasicCurlHttpTests, SendPostRequest) TEST_F(BasicCurlHttpTests, RequestTimeout) { received_requests_.clear(); - curl::SessionManager session_manager; + auto session_manager = http_client::HttpClientFactory::Create(); + EXPECT_TRUE(session_manager != nullptr); - auto session = session_manager.CreateSession("127.0.0.10", HTTP_PORT); // Non Existing address + auto session = session_manager->CreateSession("127.0.0.10", HTTP_PORT); // Non Existing address auto request = session->CreateRequest(); request->SetUri("get/"); GetEventHandler *handler = new GetEventHandler(); From ea73ca49df9f1c00d401da7a51e9ac88cc6d9f54 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 14 Dec 2020 13:50:21 +0530 Subject: [PATCH 02/10] missing file --- .../ext/http/client/http_client_factory.h | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 ext/include/opentelemetry/ext/http/client/http_client_factory.h diff --git a/ext/include/opentelemetry/ext/http/client/http_client_factory.h b/ext/include/opentelemetry/ext/http/client/http_client_factory.h new file mode 100644 index 0000000000..7b89f1cde9 --- /dev/null +++ b/ext/include/opentelemetry/ext/http/client/http_client_factory.h @@ -0,0 +1,32 @@ +#pragma once +#include "opentelemetry/ext/http/client/curl/http_client_curl.h" +#include "opentelemetry/ext/http/client/http_client.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace ext +{ +namespace http +{ +namespace client +{ + +class HttpClientFactory +{ +public: +#ifdef WITH_CURL + static std::shared_ptr Create() + { + + return std::make_shared(); + } +#else + static std::shared_ptr Create() + { + return std::shared_ptr(nullptr); + } +#endif +}; +} // namespace client +} // namespace http +} // namespace ext +OPENTELEMETRY_END_NAMESPACE \ No newline at end of file From fd946348cf42a67a5bcf24f51927131def17f522 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 14 Dec 2020 13:54:18 +0530 Subject: [PATCH 03/10] conditional include on curl header --- .../opentelemetry/ext/http/client/http_client_factory.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ext/include/opentelemetry/ext/http/client/http_client_factory.h b/ext/include/opentelemetry/ext/http/client/http_client_factory.h index 7b89f1cde9..c7429cfa26 100644 --- a/ext/include/opentelemetry/ext/http/client/http_client_factory.h +++ b/ext/include/opentelemetry/ext/http/client/http_client_factory.h @@ -1,6 +1,8 @@ #pragma once -#include "opentelemetry/ext/http/client/curl/http_client_curl.h" #include "opentelemetry/ext/http/client/http_client.h" +#ifdef WITH_CURL +# include "opentelemetry/ext/http/client/curl/http_client_curl.h" +#endif OPENTELEMETRY_BEGIN_NAMESPACE namespace ext @@ -29,4 +31,4 @@ class HttpClientFactory } // namespace client } // namespace http } // namespace ext -OPENTELEMETRY_END_NAMESPACE \ No newline at end of file +OPENTELEMETRY_END_NAMESPACE From 81ef00230fa63158ec4134933ad6afd89f66ad0e Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 14 Dec 2020 13:57:19 +0530 Subject: [PATCH 04/10] fix conflict --- ext/test/http/curl_http_test.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/test/http/curl_http_test.cc b/ext/test/http/curl_http_test.cc index 56051e8377..065b4d5578 100644 --- a/ext/test/http/curl_http_test.cc +++ b/ext/test/http/curl_http_test.cc @@ -230,7 +230,8 @@ TEST_F(BasicCurlHttpTests, RequestTimeout) auto session_manager = http_client::HttpClientFactory::Create(); EXPECT_TRUE(session_manager != nullptr); - auto session = session_manager->CreateSession("127.0.0.10", HTTP_PORT); // Non Existing address + auto session = + session_manager->CreateSession("222.222.222.200", HTTP_PORT); // Non Existing address auto request = session->CreateRequest(); request->SetUri("get/"); GetEventHandler *handler = new GetEventHandler(); From 5651b3274d225c557f9a2d9ded3a9285b811e4dd Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 14 Dec 2020 14:06:32 +0530 Subject: [PATCH 05/10] remove newline --- ext/include/opentelemetry/ext/http/client/http_client_factory.h | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/include/opentelemetry/ext/http/client/http_client_factory.h b/ext/include/opentelemetry/ext/http/client/http_client_factory.h index c7429cfa26..b49ecfe1ab 100644 --- a/ext/include/opentelemetry/ext/http/client/http_client_factory.h +++ b/ext/include/opentelemetry/ext/http/client/http_client_factory.h @@ -18,7 +18,6 @@ class HttpClientFactory #ifdef WITH_CURL static std::shared_ptr Create() { - return std::make_shared(); } #else From b76d66dc50b1e42ee01b2f6435cd3636b94b3c02 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 14 Dec 2020 15:05:12 +0530 Subject: [PATCH 06/10] fix bazel build --- ext/test/http/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/test/http/BUILD b/ext/test/http/BUILD index 678b2e5da7..99c3c0427a 100644 --- a/ext/test/http/BUILD +++ b/ext/test/http/BUILD @@ -6,6 +6,7 @@ cc_test( # TODO: Move copts/linkopts for static CURL usage into shared bzl file. copts = [ "-DCURL_STATICLIB", + "-DWITH_CURL", ], linkopts = select({ "//bazel:windows": [ From 3de0387836e56342c191962b2db2e0105373fb6f Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Mon, 14 Dec 2020 21:16:28 +0530 Subject: [PATCH 07/10] make compile fail if client lib missing --- .../opentelemetry/ext/http/client/http_client_factory.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/ext/include/opentelemetry/ext/http/client/http_client_factory.h b/ext/include/opentelemetry/ext/http/client/http_client_factory.h index b49ecfe1ab..78128f1f9a 100644 --- a/ext/include/opentelemetry/ext/http/client/http_client_factory.h +++ b/ext/include/opentelemetry/ext/http/client/http_client_factory.h @@ -11,7 +11,6 @@ namespace http { namespace client { - class HttpClientFactory { public: @@ -20,11 +19,6 @@ class HttpClientFactory { return std::make_shared(); } -#else - static std::shared_ptr Create() - { - return std::shared_ptr(nullptr); - } #endif }; } // namespace client From 2f248fec6f6b5f2bb624c34e0756129fb88e6542 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 8 Jan 2021 16:22:45 +0530 Subject: [PATCH 08/10] review comments --- ext/CMakeLists.txt | 2 +- .../ext/http/client/http_client_factory.h | 10 +--------- ext/src/CMakeLists.txt | 1 + ext/src/http/client/curl/BUILD | 14 ++++++++++++++ ext/src/http/client/curl/CMakeLists.txt | 4 ++++ .../http/client/curl/http_client_factory_curl.cc | 9 +++++++++ ext/test/http/BUILD | 1 + ext/test/http/CMakeLists.txt | 5 +++-- 8 files changed, 34 insertions(+), 12 deletions(-) create mode 100644 ext/src/http/client/curl/BUILD create mode 100644 ext/src/http/client/curl/CMakeLists.txt create mode 100644 ext/src/http/client/curl/http_client_factory_curl.cc diff --git a/ext/CMakeLists.txt b/ext/CMakeLists.txt index fa8f1e9498..5f3316827e 100644 --- a/ext/CMakeLists.txt +++ b/ext/CMakeLists.txt @@ -1,5 +1,5 @@ -add_subdirectory(src) include_directories(include) +add_subdirectory(src) if(BUILD_TESTING) add_subdirectory(test) diff --git a/ext/include/opentelemetry/ext/http/client/http_client_factory.h b/ext/include/opentelemetry/ext/http/client/http_client_factory.h index 78128f1f9a..3093b1a149 100644 --- a/ext/include/opentelemetry/ext/http/client/http_client_factory.h +++ b/ext/include/opentelemetry/ext/http/client/http_client_factory.h @@ -1,8 +1,5 @@ #pragma once #include "opentelemetry/ext/http/client/http_client.h" -#ifdef WITH_CURL -# include "opentelemetry/ext/http/client/curl/http_client_curl.h" -#endif OPENTELEMETRY_BEGIN_NAMESPACE namespace ext @@ -14,12 +11,7 @@ namespace client class HttpClientFactory { public: -#ifdef WITH_CURL - static std::shared_ptr Create() - { - return std::make_shared(); - } -#endif + static std::shared_ptr Create(); }; } // namespace client } // namespace http diff --git a/ext/src/CMakeLists.txt b/ext/src/CMakeLists.txt index 189a03f69c..d449d06b06 100644 --- a/ext/src/CMakeLists.txt +++ b/ext/src/CMakeLists.txt @@ -1 +1,2 @@ add_subdirectory(zpages) +add_subdirectory(http/client/curl) diff --git a/ext/src/http/client/curl/BUILD b/ext/src/http/client/curl/BUILD new file mode 100644 index 0000000000..4d90b4d28b --- /dev/null +++ b/ext/src/http/client/curl/BUILD @@ -0,0 +1,14 @@ +package(default_visibility = ["//visibility:public"]) + +cc_library( + name = "http_client_curl", + srcs = glob(["**/*.cc"]), + hdrs = glob(["**/*.h"]), + include_prefix = "src/http/client/curl", + deps = [ + "//api", + "//ext:headers", + "//sdk:headers", + "//sdk/src/common:random", + ], +) diff --git a/ext/src/http/client/curl/CMakeLists.txt b/ext/src/http/client/curl/CMakeLists.txt new file mode 100644 index 0000000000..2026dbaf28 --- /dev/null +++ b/ext/src/http/client/curl/CMakeLists.txt @@ -0,0 +1,4 @@ +find_package(CURL) +if(CURL_FOUND) + add_library(opentelemetry_curl_factory http_client_factory_curl) +endif() diff --git a/ext/src/http/client/curl/http_client_factory_curl.cc b/ext/src/http/client/curl/http_client_factory_curl.cc new file mode 100644 index 0000000000..f75885aa43 --- /dev/null +++ b/ext/src/http/client/curl/http_client_factory_curl.cc @@ -0,0 +1,9 @@ +#include "opentelemetry/ext/http/client/curl/http_client_curl.h" +#include "opentelemetry/ext/http/client/http_client.h" +#include "opentelemetry/ext/http/client/http_client_factory.h" + +std::shared_ptr +opentelemetry::ext::http::client::HttpClientFactory::Create() +{ + return std::make_shared(); +} \ No newline at end of file diff --git a/ext/test/http/BUILD b/ext/test/http/BUILD index 99c3c0427a..7251c0fde6 100644 --- a/ext/test/http/BUILD +++ b/ext/test/http/BUILD @@ -18,6 +18,7 @@ cc_test( }), deps = [ "//ext:headers", + "//ext/src/http/client/curl:http_client_curl", "//sdk/src/trace", "@com_google_googletest//:gtest_main", "@curl", diff --git a/ext/test/http/CMakeLists.txt b/ext/test/http/CMakeLists.txt index bc00b0f638..15d0f7196a 100644 --- a/ext/test/http/CMakeLists.txt +++ b/ext/test/http/CMakeLists.txt @@ -5,8 +5,9 @@ if(CURL_FOUND) add_compile_definitions(WITH_CURL) add_executable(${FILENAME} ${FILENAME}.cc) include_directories(${CURL_INCLUDE_DIR}) - target_link_libraries(${FILENAME} ${GTEST_BOTH_LIBRARIES} - ${CMAKE_THREAD_LIBS_INIT} ${CURL_LIBRARIES}) + target_link_libraries( + ${FILENAME} ${GTEST_BOTH_LIBRARIES} opentelemetry_curl_factory + ${CMAKE_THREAD_LIBS_INIT} ${CURL_LIBRARIES}) gtest_add_tests( TARGET ${FILENAME} TEST_PREFIX ext.http.curl. From 9d7f6123e8197a71baaac549351ae5e9cd7df785 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 8 Jan 2021 19:05:09 +0530 Subject: [PATCH 09/10] fix cmake --- ext/CMakeLists.txt | 2 ++ ext/test/http/CMakeLists.txt | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/ext/CMakeLists.txt b/ext/CMakeLists.txt index a2baa47a32..0062e6c671 100644 --- a/ext/CMakeLists.txt +++ b/ext/CMakeLists.txt @@ -1,3 +1,4 @@ +include_directories(sdk) add_library(opentelemetry_ext INTERFACE) target_include_directories( opentelemetry_ext @@ -19,6 +20,7 @@ install( FILES_MATCHING PATTERN "*.h") +include_directories(include) add_subdirectory(src) if(BUILD_TESTING) diff --git a/ext/test/http/CMakeLists.txt b/ext/test/http/CMakeLists.txt index ba279b047b..0fcaa24c48 100644 --- a/ext/test/http/CMakeLists.txt +++ b/ext/test/http/CMakeLists.txt @@ -10,7 +10,8 @@ if(CURL_FOUND) target_link_libraries(${FILENAME} CURL::libcurl opentelemetry_curl_factory) else() include_directories(${CURL_INCLUDE_DIRS}) - target_link_libraries(${FILENAME} ${CURL_LIBRARIES} opentelemetry_curl_factory) + target_link_libraries(${FILENAME} ${CURL_LIBRARIES} + opentelemetry_curl_factory) endif() gtest_add_tests( TARGET ${FILENAME} From 34213e323b2f2c580f85be649c33625a6a447221 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 8 Jan 2021 19:33:06 +0530 Subject: [PATCH 10/10] fix curl for bazel --- ext/src/http/client/curl/BUILD | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/src/http/client/curl/BUILD b/ext/src/http/client/curl/BUILD index 4d90b4d28b..8360692601 100644 --- a/ext/src/http/client/curl/BUILD +++ b/ext/src/http/client/curl/BUILD @@ -2,13 +2,13 @@ package(default_visibility = ["//visibility:public"]) cc_library( name = "http_client_curl", - srcs = glob(["**/*.cc"]), - hdrs = glob(["**/*.h"]), + srcs = ["http_client_factory_curl.cc"], include_prefix = "src/http/client/curl", deps = [ "//api", "//ext:headers", "//sdk:headers", "//sdk/src/common:random", + "@curl", ], )