From 5c2985292170d3249eefbab793613e90eeb4f92b Mon Sep 17 00:00:00 2001 From: ethandmd Date: Wed, 14 May 2025 14:18:21 -0700 Subject: [PATCH 1/6] support unix sockets in grpc client --- exporters/otlp/src/otlp_grpc_client.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/exporters/otlp/src/otlp_grpc_client.cc b/exporters/otlp/src/otlp_grpc_client.cc index 4d2fb6d38e..a810287562 100644 --- a/exporters/otlp/src/otlp_grpc_client.cc +++ b/exporters/otlp/src/otlp_grpc_client.cc @@ -333,7 +333,13 @@ std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientO } std::shared_ptr channel; - std::string grpc_target = url.host_ + ":" + std::to_string(static_cast(url.port_)); + std::string grpc_target; + + if (url.scheme_ == "unix") { + grpc_target = "unix:" + url.path_; + } else { + grpc_target = url.host_ + ":" + std::to_string(static_cast(url.port_)); + } grpc::ChannelArguments grpc_arguments; grpc_arguments.SetUserAgentPrefix(options.user_agent); From 65ff244915ebbc0580a5ae670f3f08d7cce17b40 Mon Sep 17 00:00:00 2001 From: ethandmd Date: Wed, 14 May 2025 14:34:43 -0700 Subject: [PATCH 2/6] ci format --- exporters/otlp/src/otlp_grpc_client.cc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/exporters/otlp/src/otlp_grpc_client.cc b/exporters/otlp/src/otlp_grpc_client.cc index a810287562..2f5230cde6 100644 --- a/exporters/otlp/src/otlp_grpc_client.cc +++ b/exporters/otlp/src/otlp_grpc_client.cc @@ -335,10 +335,13 @@ std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientO std::shared_ptr channel; std::string grpc_target; - if (url.scheme_ == "unix") { - grpc_target = "unix:" + url.path_; - } else { - grpc_target = url.host_ + ":" + std::to_string(static_cast(url.port_)); + if (url.scheme_ == "unix") + { + grpc_target = "unix:" + url.path_; + } + else + { + grpc_target = url.host_ + ":" + std::to_string(static_cast(url.port_)); } grpc::ChannelArguments grpc_arguments; grpc_arguments.SetUserAgentPrefix(options.user_agent); From e9c8f91c5adb0c769edbe1d4200d7f0f53816912 Mon Sep 17 00:00:00 2001 From: ethandmd Date: Tue, 27 May 2025 23:20:39 -0700 Subject: [PATCH 3/6] refactor grpc target to helper function --- exporters/otlp/CMakeLists.txt | 10 +++++ .../exporters/otlp/otlp_grpc_client.h | 2 + exporters/otlp/src/otlp_grpc_client.cc | 44 ++++++++++++------- exporters/otlp/test/otlp_grpc_target_test.cc | 37 ++++++++++++++++ 4 files changed, 77 insertions(+), 16 deletions(-) create mode 100644 exporters/otlp/test/otlp_grpc_target_test.cc diff --git a/exporters/otlp/CMakeLists.txt b/exporters/otlp/CMakeLists.txt index 08303fe359..b5c95c5718 100644 --- a/exporters/otlp/CMakeLists.txt +++ b/exporters/otlp/CMakeLists.txt @@ -377,6 +377,16 @@ if(BUILD_TESTING) TEST_PREFIX exporter.otlp. TEST_LIST otlp_grpc_exporter_test) + add_executable(otlp_grpc_target_test + test/otlp_grpc_target_test.cc) + target_link_libraries( + otlp_grpc_target_test ${GTEST_BOTH_LIBRARIES} + ${CMAKE_THREAD_LIBS_INIT} ${GMOCK_LIB} opentelemetry_exporter_otlp_grpc) + gtest_add_tests( + TARGET otlp_grpc_target_test + TEST_PREFIX exporter.otlp. + TEST_LIST otlp_grpc_target_test) + add_executable(otlp_grpc_exporter_factory_test test/otlp_grpc_exporter_factory_test.cc) target_link_libraries( diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client.h index 3ded3a16eb..00e6625c3c 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client.h @@ -62,6 +62,8 @@ class OtlpGrpcClient ~OtlpGrpcClient(); + static std::string GetGrpcTarget(const std::string endpoint); + /** * Create gRPC channel from the exporter options. */ diff --git a/exporters/otlp/src/otlp_grpc_client.cc b/exporters/otlp/src/otlp_grpc_client.cc index 2f5230cde6..6ba467f35d 100644 --- a/exporters/otlp/src/otlp_grpc_client.cc +++ b/exporters/otlp/src/otlp_grpc_client.cc @@ -22,10 +22,11 @@ #include #include "opentelemetry/common/timestamp.h" -#include "opentelemetry/ext/http/common/url_parser.h" #include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/common/global_log_handler.h" +#include "opentelemetry/ext/http/common/url_parser.h" +#include "opentelemetry/sdk/common/global_log_handler.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace exporter @@ -309,32 +310,21 @@ OtlpGrpcClient::~OtlpGrpcClient() #endif } -std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientOptions &options) +std::string OtlpGrpcClient::GetGrpcTarget(const std::string endpoint) { - - if (options.endpoint.empty()) - { - OTEL_INTERNAL_LOG_ERROR("[OTLP GRPC Client] empty endpoint"); - - return nullptr; - } // // Scheme is allowed in OTLP endpoint definition, but is not allowed for creating gRPC // channel. Passing URI with scheme to grpc::CreateChannel could resolve the endpoint to some // unexpected address. // - - ext::http::common::UrlParser url(options.endpoint); + ext::http::common::UrlParser url(endpoint); if (!url.success_) { - OTEL_INTERNAL_LOG_ERROR("[OTLP GRPC Client] invalid endpoint: " << options.endpoint); - - return nullptr; + OTEL_INTERNAL_LOG_ERROR("[OTLP GRPC Client] invalid endpoint: " << endpoint); + return ""; } - std::shared_ptr channel; std::string grpc_target; - if (url.scheme_ == "unix") { grpc_target = "unix:" + url.path_; @@ -343,6 +333,28 @@ std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientO { grpc_target = url.host_ + ":" + std::to_string(static_cast(url.port_)); } + return grpc_target; +} + +std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientOptions &options) +{ + + if (options.endpoint.empty()) + { + OTEL_INTERNAL_LOG_ERROR("[OTLP GRPC Client] empty endpoint"); + + return nullptr; + } + + std::shared_ptr channel; + std::string grpc_target = GetGrpcTarget(options.endpoint); + + if (grpc_target.empty()) + { + OTEL_INTERNAL_LOG_ERROR("[OTLP GRPC Client] invalid endpoint: " << options.endpoint); + return nullptr; + } + grpc::ChannelArguments grpc_arguments; grpc_arguments.SetUserAgentPrefix(options.user_agent); diff --git a/exporters/otlp/test/otlp_grpc_target_test.cc b/exporters/otlp/test/otlp_grpc_target_test.cc new file mode 100644 index 0000000000..478ad7de8b --- /dev/null +++ b/exporters/otlp/test/otlp_grpc_target_test.cc @@ -0,0 +1,37 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include +#include + +#include "opentelemetry/exporters/otlp/otlp_grpc_client.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace exporter +{ +namespace otlp +{ + +TEST(OtlpGrpcClientEndpointTest, GrpcClientTest) +{ + OtlpGrpcClientOptions opts1; + opts1.endpoint = "unix:///tmp/otel1.sock"; + + OtlpGrpcClientOptions opts2; + opts2.endpoint = "unix:tmp/otel2.sock"; + + OtlpGrpcClientOptions opts3; + opts3.endpoint = "localhost:4317"; + + auto target1 = OtlpGrpcClient::GetGrpcTarget(opts1.endpoint); + auto target2 = OtlpGrpcClient::GetGrpcTarget(opts2.endpoint); + auto target3 = OtlpGrpcClient::GetGrpcTarget(opts3.endpoint); + + EXPECT_EQ(target1, "unix:/tmp/otel1.sock"); + EXPECT_EQ(target2, ""); + EXPECT_EQ(target3, "localhost:4317"); +} + +} // namespace otlp +} // namespace exporter +OPENTELEMETRY_END_NAMESPACE From dad1b621b12dc679e8a80c315a8b4f837c9fe3ac Mon Sep 17 00:00:00 2001 From: ethandmd Date: Tue, 27 May 2025 23:51:40 -0700 Subject: [PATCH 4/6] formatting --- bazel/extra_deps.bzl | 2 +- examples/grpc/BUILD | 6 +++--- exporters/otlp/BUILD | 18 ++++++++++++++++-- exporters/otlp/CMakeLists.txt | 7 +++---- exporters/otlp/src/otlp_grpc_client.cc | 3 +-- exporters/otlp/test/otlp_grpc_target_test.cc | 2 +- 6 files changed, 25 insertions(+), 13 deletions(-) diff --git a/bazel/extra_deps.bzl b/bazel/extra_deps.bzl index 00a32c50bf..f68bc0f19e 100644 --- a/bazel/extra_deps.bzl +++ b/bazel/extra_deps.bzl @@ -3,8 +3,8 @@ # Load prometheus C++ dependencies. -load("@com_github_jupp0r_prometheus_cpp//bazel:repositories.bzl", "prometheus_cpp_repositories") load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace") +load("@com_github_jupp0r_prometheus_cpp//bazel:repositories.bzl", "prometheus_cpp_repositories") load("@rules_foreign_cc//foreign_cc:repositories.bzl", "rules_foreign_cc_dependencies") def opentelemetry_extra_deps(): diff --git a/examples/grpc/BUILD b/examples/grpc/BUILD index b1d04e9fbd..5fe8898ac8 100644 --- a/examples/grpc/BUILD +++ b/examples/grpc/BUILD @@ -1,10 +1,10 @@ # Copyright The OpenTelemetry Authors # SPDX-License-Identifier: Apache-2.0 -package(default_visibility = ["//visibility:public"]) - -load("@rules_proto//proto:defs.bzl", "proto_library") load("@com_github_grpc_grpc//bazel:cc_grpc_library.bzl", "cc_grpc_library") +load("@rules_proto//proto:defs.bzl", "proto_library") + +package(default_visibility = ["//visibility:public"]) proto_library( name = "messages_proto", diff --git a/exporters/otlp/BUILD b/exporters/otlp/BUILD index ba55ba72c7..6198e39fe4 100644 --- a/exporters/otlp/BUILD +++ b/exporters/otlp/BUILD @@ -1,10 +1,10 @@ # Copyright The OpenTelemetry Authors # SPDX-License-Identifier: Apache-2.0 -package(default_visibility = ["//visibility:public"]) - load("//bazel:otel_cc_benchmark.bzl", "otel_cc_benchmark") +package(default_visibility = ["//visibility:public"]) + cc_library( name = "otlp_recordable", srcs = [ @@ -474,6 +474,20 @@ cc_test( ], ) +cc_test( + name = "otlp_grpc_target_test", + srcs = ["test/otlp_grpc_target_test.cc"], + tags = [ + "otlp", + "otlp_grpc", + "test", + ], + deps = [ + ":otlp_grpc_client", + "@com_google_googletest//:gtest_main", + ], +) + cc_test( name = "otlp_grpc_exporter_factory_test", srcs = ["test/otlp_grpc_exporter_factory_test.cc"], diff --git a/exporters/otlp/CMakeLists.txt b/exporters/otlp/CMakeLists.txt index b5c95c5718..3ec20517e0 100644 --- a/exporters/otlp/CMakeLists.txt +++ b/exporters/otlp/CMakeLists.txt @@ -377,11 +377,10 @@ if(BUILD_TESTING) TEST_PREFIX exporter.otlp. TEST_LIST otlp_grpc_exporter_test) - add_executable(otlp_grpc_target_test - test/otlp_grpc_target_test.cc) + add_executable(otlp_grpc_target_test test/otlp_grpc_target_test.cc) target_link_libraries( - otlp_grpc_target_test ${GTEST_BOTH_LIBRARIES} - ${CMAKE_THREAD_LIBS_INIT} ${GMOCK_LIB} opentelemetry_exporter_otlp_grpc) + otlp_grpc_target_test ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} + ${GMOCK_LIB} opentelemetry_exporter_otlp_grpc) gtest_add_tests( TARGET otlp_grpc_target_test TEST_PREFIX exporter.otlp. diff --git a/exporters/otlp/src/otlp_grpc_client.cc b/exporters/otlp/src/otlp_grpc_client.cc index 6ba467f35d..5430951f28 100644 --- a/exporters/otlp/src/otlp_grpc_client.cc +++ b/exporters/otlp/src/otlp_grpc_client.cc @@ -22,11 +22,10 @@ #include #include "opentelemetry/common/timestamp.h" +#include "opentelemetry/ext/http/common/url_parser.h" #include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/common/global_log_handler.h" -#include "opentelemetry/ext/http/common/url_parser.h" -#include "opentelemetry/sdk/common/global_log_handler.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace exporter diff --git a/exporters/otlp/test/otlp_grpc_target_test.cc b/exporters/otlp/test/otlp_grpc_target_test.cc index 478ad7de8b..0073aeedbc 100644 --- a/exporters/otlp/test/otlp_grpc_target_test.cc +++ b/exporters/otlp/test/otlp_grpc_target_test.cc @@ -1,8 +1,8 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#include #include +#include #include "opentelemetry/exporters/otlp/otlp_grpc_client.h" From 7a211f6c1c02916b1d7d0df85accd19e0064ecc9 Mon Sep 17 00:00:00 2001 From: ethandmd Date: Wed, 28 May 2025 08:34:41 -0700 Subject: [PATCH 5/6] performance cppcheck fixes --- exporters/otlp/src/otlp_grpc_client.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/otlp/src/otlp_grpc_client.cc b/exporters/otlp/src/otlp_grpc_client.cc index 5430951f28..b520d1bd06 100644 --- a/exporters/otlp/src/otlp_grpc_client.cc +++ b/exporters/otlp/src/otlp_grpc_client.cc @@ -309,7 +309,7 @@ OtlpGrpcClient::~OtlpGrpcClient() #endif } -std::string OtlpGrpcClient::GetGrpcTarget(const std::string endpoint) +std::string OtlpGrpcClient::GetGrpcTarget(const std::string &endpoint) { // // Scheme is allowed in OTLP endpoint definition, but is not allowed for creating gRPC From 4e252a2fb4a6d5864f46099c587c7f85c1e4562e Mon Sep 17 00:00:00 2001 From: ethandmd Date: Wed, 28 May 2025 09:10:04 -0700 Subject: [PATCH 6/6] update header declaration --- .../include/opentelemetry/exporters/otlp/otlp_grpc_client.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client.h index 00e6625c3c..b83c721281 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client.h @@ -62,7 +62,7 @@ class OtlpGrpcClient ~OtlpGrpcClient(); - static std::string GetGrpcTarget(const std::string endpoint); + static std::string GetGrpcTarget(const std::string &endpoint); /** * Create gRPC channel from the exporter options.