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.

Sign up for GitHub

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

Jump to bottom

Fix #724 #725

Merged
merged 1 commit into from
May 11, 2021
Merged

Fix #724 #725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions exporters/elasticsearch/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ include_directories(${CMAKE_SOURCE_DIR}/ext/include)

add_library(opentelemetry_exporter_elasticsearch_logs src/es_log_exporter.cc)

target_link_libraries(opentelemetry_exporter_elasticsearch_logs
Copy link
Contributor

@ThomsonTan ThomsonTan May 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Seems this change is only for MinGW, is it needed for other toolchains?

Copy link
Contributor

@maxgolov maxgolov May 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

It builds with msvc already, so I assume it's unique to MinGW?

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems this change is only for MinGW, is it needed for other toolchains?

Yes. Some test targets use target_link_libraries to link additional dependencies which are also depended by exporters. I think the depended targets should be added automatically by linking the exporters, and users should not concern these targets unless they use these targets directly.

These PR mainly remove the linking targets from test targets and add these targets into the exporters.

Copy link
Member Author

Choose a reason for hiding this comment

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

It builds with msvc already, so I assume it's unique to MinGW?

I create a script toolset to build opentelemetry-cpp and here is the output when using MinGW64 https://github.com/atframework/cmake-toolset/runs/2492797467 .I'm trying to build all the dependencies myself or use my vcpkg instead of opentelemetry-cpp/tools/vcpkg to build opentelemetry-cpp .

PUBLIC opentelemetry_trace http_client_curl)

if(BUILD_TESTING)
add_executable(es_log_exporter_test test/es_log_exporter_test.cc)

Expand Down
14 changes: 5 additions & 9 deletions exporters/otlp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ set_target_properties(opentelemetry_exporter_otprotocol

target_link_libraries(
opentelemetry_exporter_otprotocol
PUBLIC opentelemetry_trace opentelemetry_resources opentelemetry_proto)
PUBLIC opentelemetry_trace opentelemetry_resources opentelemetry_proto
protobuf::libprotobuf gRPC::grpc++)

install(
TARGETS opentelemetry_exporter_otprotocol
Expand All @@ -29,7 +30,7 @@ if(BUILD_TESTING)
add_executable(recordable_test test/recordable_test.cc)
target_link_libraries(
recordable_test ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}
opentelemetry_exporter_otprotocol protobuf::libprotobuf gRPC::grpc++)
opentelemetry_exporter_otprotocol)
gtest_add_tests(
TARGET recordable_test
TEST_PREFIX exporter.otlp.
Expand All @@ -50,13 +51,8 @@ if(BUILD_TESTING)
endif()
add_executable(otlp_exporter_test test/otlp_exporter_test.cc)
target_link_libraries(
otlp_exporter_test
${GTEST_BOTH_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT}
${GMOCK_LIB}
opentelemetry_exporter_otprotocol
protobuf::libprotobuf
gRPC::grpc++)
otlp_exporter_test ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}
${GMOCK_LIB} opentelemetry_exporter_otprotocol)
gtest_add_tests(
TARGET otlp_exporter_test
TEST_PREFIX exporter.otlp.
Expand Down
6 changes: 4 additions & 2 deletions exporters/zipkin/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ endif()
add_library(opentelemetry_exporter_zipkin_trace src/zipkin_exporter.cc
src/recordable.cc)

target_link_libraries(opentelemetry_exporter_zipkin_trace
PUBLIC opentelemetry_trace http_client_curl)

install(
TARGETS opentelemetry_exporter_zipkin_trace
EXPORT "${PROJECT_NAME}-target"
Expand All @@ -38,8 +41,7 @@ if(BUILD_TESTING)

target_link_libraries(
zipkin_recordable_test ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}
opentelemetry_exporter_zipkin_trace opentelemetry_resources
http_client_curl)
opentelemetry_exporter_zipkin_trace opentelemetry_resources)

gtest_add_tests(
TARGET zipkin_recordable_test
Expand Down
13 changes: 12 additions & 1 deletion ext/src/http/client/curl/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
find_package(CURL)
if(CURL_FOUND)
add_library(http_client_curl http_client_factory_curl http_client_curl)
target_link_libraries(http_client_curl CURL::libcurl)

set_target_properties(http_client_curl PROPERTIES EXPORT_NAME
http_client_curl)

target_link_libraries(http_client_curl PUBLIC CURL::libcurl)

install(
TARGETS http_client_curl
EXPORT "${PROJECT_NAME}-target"
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})
endif()
1 change: 1 addition & 0 deletions opentelemetry-cpp-config.cmake.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
# opentelemetry-cpp::ostream_span_exporter - Imported target of opentelemetry-cpp::ostream_span_exporter
# opentelemetry-cpp::prometheus_exporter - Imported target of opentelemetry-cpp::prometheus_exporter
# opentelemetry-cpp::zpages - Imported target of opentelemetry-cpp::zpages
# opentelemetry-cpp::http_client_curl - Imported target of opentelemetry-cpp::http_client_curl
#

# =============================================================================
Expand Down
13 changes: 13 additions & 0 deletions sdk/src/common/core.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,19 @@
#include "opentelemetry/version.h"

#if defined(HAVE_ABSEIL)

# if defined(__GNUC__) || defined(__GNUG__)
Copy link
Contributor

@maxgolov maxgolov May 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I am sorry for adding void __cdecl ThrowBadVariantAccess(). What you are doing here seems reasonable.

# ifndef __cdecl
// see https://gcc.gnu.org/onlinedocs/gcc/x86-Function-Attributes.html
// Intel x86 architecture specific calling conventions
# ifdef _M_IX86
# define __cdecl __attribute__((__cdecl__))
# else
# define __cdecl
# endif
# endif
# endif

namespace absl
{
namespace variant_internal
Expand Down