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

[BUILD] Transitive dependency issue with the otlp http exporter #2154

Merged
merged 2 commits into from
May 26, 2023
Merged
Changes from all commits
Commits
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
31 changes: 21 additions & 10 deletions exporters/otlp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,9 @@ if(WITH_OTLP_HTTP)

target_link_libraries(
opentelemetry_exporter_otlp_http_client
PUBLIC opentelemetry_sdk opentelemetry_proto opentelemetry_http_client_curl
Copy link
Member

@owent owent May 24, 2023

Choose a reason for hiding this comment

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

opentelemetry_exporter_otlp_http_client use the headers of opentelemetry_sdk and opentelemetry_ext.I believe it would be better to keep public link opentelemetry_sdk and opentelemetry_ext and private link opentelemetry_proto, opentelemetry_http_client_curl and nlohmann_json.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes sense - I think I lazily just made everything private instead of looking more carefully. Thanks for the review.

nlohmann_json::nlohmann_json)
PUBLIC opentelemetry_sdk opentelemetry_ext
PRIVATE opentelemetry_proto opentelemetry_http_client_curl
nlohmann_json::nlohmann_json)
if(nlohmann_json_clone)
add_dependencies(opentelemetry_exporter_otlp_http_client
nlohmann_json::nlohmann_json)
Expand Down Expand Up @@ -197,8 +198,9 @@ endif()

if(BUILD_TESTING)
add_executable(otlp_recordable_test test/otlp_recordable_test.cc)
target_link_libraries(otlp_recordable_test ${GTEST_BOTH_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT} opentelemetry_otlp_recordable)
target_link_libraries(
otlp_recordable_test ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}
opentelemetry_otlp_recordable protobuf::libprotobuf)
gtest_add_tests(
TARGET otlp_recordable_test
TEST_PREFIX exporter.otlp.
Expand All @@ -217,8 +219,10 @@ if(BUILD_TESTING)

add_executable(otlp_metrics_serialization_test
test/otlp_metrics_serialization_test.cc)
target_link_libraries(otlp_metrics_serialization_test ${GTEST_BOTH_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT} opentelemetry_otlp_recordable)
target_link_libraries(
otlp_metrics_serialization_test ${GTEST_BOTH_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT} opentelemetry_otlp_recordable
protobuf::libprotobuf)
gtest_add_tests(
TARGET otlp_metrics_serialization_test
TEST_PREFIX exporter.otlp.
Expand Down Expand Up @@ -307,9 +311,14 @@ if(BUILD_TESTING)
if(WITH_OTLP_HTTP)
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
opentelemetry_http_client_nosend)
otlp_http_exporter_test
${GTEST_BOTH_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT}
${GMOCK_LIB}
opentelemetry_exporter_otlp_http
opentelemetry_http_client_nosend
nlohmann_json::nlohmann_json
protobuf::libprotobuf)
gtest_add_tests(
TARGET otlp_http_exporter_test
TEST_PREFIX exporter.otlp.
Expand Down Expand Up @@ -362,7 +371,9 @@ if(BUILD_TESTING)
${GMOCK_LIB}
opentelemetry_exporter_otlp_http_metric
opentelemetry_metrics
opentelemetry_http_client_nosend)
opentelemetry_http_client_nosend
nlohmann_json::nlohmann_json
protobuf::libprotobuf)
gtest_add_tests(
TARGET otlp_http_metric_exporter_test
TEST_PREFIX exporter.otlp.
Expand Down