kou commented on a change in pull request #10260:
URL: https://github.com/apache/arrow/pull/10260#discussion_r749684986
##########
File path: cpp/cmake_modules/ThirdpartyToolchain.cmake
##########
@@ -162,6 +163,8 @@ macro(build_dependency DEPENDENCY_NAME)
build_lz4()
elseif("${DEPENDENCY_NAME}" STREQUAL "ORC")
build_orc()
+ elseif("${DEPENDENCY_NAME}" STREQUAL "opentelemetry-cpp")
+ build_opentelemetry()
Review comment:
```suggestion
elseif("${DEPENDENCY_NAME}" STREQUAL "opentelemetry-cpp")
build_opentelemetry()
elseif("${DEPENDENCY_NAME}" STREQUAL "ORC")
build_orc()
```
##########
File path: cpp/cmake_modules/ThirdpartyToolchain.cmake
##########
@@ -3661,17 +3711,7 @@ macro(build_google_cloud_cpp_storage)
add_dependencies(google_cloud_cpp_dependencies nlohmann_json_ep)
# Typically the steps to build the AWKSSDK provide `CURL::libcurl`, but if
that is
# disabled we need to provide our own.
- if(NOT TARGET CURL::libcurl)
- find_package(CURL REQUIRED)
- if(NOT TARGET CURL::libcurl)
- # For CMake 3.11 or older
- add_library(CURL::libcurl UNKNOWN IMPORTED)
- set_target_properties(CURL::libcurl
- PROPERTIES INTERFACE_INCLUDE_DIRECTORIES
- "${CURL_INCLUDE_DIRS}" IMPORTED_LOCATION
-
"${CURL_LIBRARIES}")
- endif()
- endif()
+ find_curl()
Review comment:
There is one more `find_package(CURL)` in
`build_google_cloud_cpp_storage`.
##########
File path: cpp/cmake_modules/ThirdpartyToolchain.cmake
##########
@@ -3839,6 +3851,115 @@ if(ARROW_ORC)
message(STATUS "Found ORC headers: ${ORC_INCLUDE_DIR}")
endif()
+# ----------------------------------------------------------------------
+# OpenTelemetry C++
+
+macro(build_opentelemetry)
+ message("Building OpenTelemetry from source")
+ set(OPENTELEMETRY_PREFIX
"${CMAKE_CURRENT_BINARY_DIR}/opentelemetry_ep-install")
+ set(OPENTELEMETRY_INCLUDE_DIR "${OPENTELEMETRY_PREFIX}/include")
+ set(OPENTELEMETRY_STATIC_LIB
+
"${OPENTELEMETRY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}opentelemetry${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ )
+ set(_OPENTELEMETRY_APIS api sdk)
+ set(_OPENTELEMETRY_LIBS common resources trace)
+ set(OPENTELEMETRY_BUILD_BYPRODUCTS)
+ set(OPENTELEMETRY_LIBRARIES)
+
+ foreach(_OPENTELEMETRY_LIB ${_OPENTELEMETRY_APIS})
+ add_library(opentelemetry-cpp::${_OPENTELEMETRY_LIB} INTERFACE IMPORTED)
+ set_target_properties(opentelemetry-cpp::${_OPENTELEMETRY_LIB}
+ PROPERTIES INTERFACE_INCLUDE_DIRECTORIES
+ "${OPENTELEMETRY_INCLUDE_DIR}")
+ endforeach()
+ foreach(_OPENTELEMETRY_LIB ${_OPENTELEMETRY_LIBS})
+ set(_OPENTELEMETRY_STATIC_LIBRARY
+
"${OPENTELEMETRY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}opentelemetry_${_OPENTELEMETRY_LIB}${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ )
+ add_library(opentelemetry-cpp::${_OPENTELEMETRY_LIB} STATIC IMPORTED)
+ set_target_properties(opentelemetry-cpp::${_OPENTELEMETRY_LIB}
+ PROPERTIES IMPORTED_LOCATION
${_OPENTELEMETRY_STATIC_LIBRARY})
+ list(APPEND OPENTELEMETRY_BUILD_BYPRODUCTS
${_OPENTELEMETRY_STATIC_LIBRARY})
+ list(APPEND OPENTELEMETRY_LIBRARIES
opentelemetry-cpp::${_OPENTELEMETRY_LIB})
+ endforeach()
+
+ set(OPENTELEMETRY_CMAKE_ARGS
+ ${EP_COMMON_TOOLCHAIN}
+ "-DCMAKE_INSTALL_PREFIX=${OPENTELEMETRY_PREFIX}"
+ "-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}"
+ -DCMAKE_INSTALL_LIBDIR=lib
+ "-DCMAKE_CXX_FLAGS=${EP_CXX_FLAGS}"
+ -DBUILD_TESTING=OFF
+ -DWITH_EXAMPLES=OFF)
+ if(ARROW_WITH_OPENTELEMETRY)
+ list(APPEND ARROW_BUNDLED_STATIC_LIBS ${OPENTELEMETRY_LIBRARIES})
+ else()
+ set(OPENTELEMETRY_CMAKE_ARGS ${OPENTELEMETRY_CMAKE_ARGS}
"-DWITH_API_ONLY=ON")
+ if(WIN32)
+ # WITH_ETW does not respect WITH_API_ONLY
+ set(OPENTELEMETRY_CMAKE_ARGS ${OPENTELEMETRY_CMAKE_ARGS} -DWITH_ETW=OFF)
+ endif()
+ endif()
+ if(CMAKE_SYSTEM_PROCESSOR STREQUAL "s390x")
+ externalproject_add(opentelemetry_ep
+ ${EP_LOG_OPTIONS}
+ URL_HASH
"SHA256=${ARROW_OPENTELEMETRY_BUILD_SHA256_CHECKSUM}"
+ # OpenTelemetry tries to determine the processor arch
for vcpkg,
+ # which fails on s390x, even though it doesn't use
vcpkg there
+ CONFIGURE_COMMAND ${CMAKE_COMMAND} -E env ARCH=s390x
+ ${CMAKE_COMMAND} -G
${CMAKE_GENERATOR}
+ "<SOURCE_DIR><SOURCE_SUBDIR>"
+ ${OPENTELEMETRY_CMAKE_ARGS}
+ BUILD_COMMAND ${CMAKE_COMMAND} --build "<BINARY_DIR>"
--target all
+ INSTALL_COMMAND ${CMAKE_COMMAND} --build
"<BINARY_DIR>" --target
+ install
+ # CMAKE_ARGS ${OPENTELEMETRY_CMAKE_ARGS}
+ URL ${OPENTELEMETRY_SOURCE_URL}
+ BUILD_BYPRODUCTS ${OPENTELEMETRY_BUILD_BYPRODUCTS}
+ EXCLUDE_FROM_ALL NOT
+ ${ARROW_WITH_OPENTELEMETRY})
+ else()
+ externalproject_add(opentelemetry_ep
+ ${EP_LOG_OPTIONS}
+ URL_HASH
"SHA256=${ARROW_OPENTELEMETRY_BUILD_SHA256_CHECKSUM}"
+ CMAKE_ARGS ${OPENTELEMETRY_CMAKE_ARGS}
+ URL ${OPENTELEMETRY_SOURCE_URL}
+ BUILD_BYPRODUCTS ${OPENTELEMETRY_BUILD_BYPRODUCTS}
+ EXCLUDE_FROM_ALL NOT
+ ${ARROW_WITH_OPENTELEMETRY})
+ endif()
+ add_dependencies(toolchain opentelemetry_ep)
+ add_dependencies(toolchain-tests opentelemetry_ep)
+
+ set(OPENTELEMETRY_VENDORED 1)
+
+ set_target_properties(opentelemetry-cpp::common
+ PROPERTIES INTERFACE_LINK_LIBRARIES
+
"opentelemetry-cpp::api;opentelemetry-cpp::sdk;Threads::Threads"
+ )
+ set_target_properties(opentelemetry-cpp::resources
+ PROPERTIES INTERFACE_LINK_LIBRARIES
"opentelemetry-cpp::common")
+ set_target_properties(opentelemetry-cpp::trace
+ PROPERTIES INTERFACE_LINK_LIBRARIES
+
"opentelemetry-cpp::common;opentelemetry-cpp::resources"
+ )
+
+ foreach(_OPENTELEMETRY_LIB ${_OPENTELEMETRY_LIBS})
+ add_dependencies(opentelemetry-cpp::${_OPENTELEMETRY_LIB} opentelemetry_ep)
+ endforeach()
+endmacro()
+
+# For now OpenTelemetry is always bundled from upstream
+if(1)
Review comment:
I'm not sure why do we need OpenTelemetry even when we don't enable
Flight...
##########
File path: cpp/src/arrow/CMakeLists.txt
##########
@@ -575,6 +576,12 @@ if(ARROW_WITH_BACKTRACE)
endforeach()
endif()
+if(ARROW_WITH_OPENTELEMETRY)
+ foreach(LIB_TARGET ${ARROW_LIBRARIES})
+ target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_WITH_OPENTELEMETRY)
Review comment:
Could you use `cpp/src/arrow/util/config.h.cmake` instead?
##########
File path: cpp/cmake_modules/ThirdpartyToolchain.cmake
##########
@@ -225,7 +228,7 @@ macro(resolve_dependency DEPENDENCY_NAME)
list(APPEND FIND_PACKAGE_ARGUMENTS CONFIG)
endif()
if(${DEPENDENCY_NAME}_SOURCE STREQUAL "AUTO")
- find_package(${FIND_PACKAGE_ARGUMENTS})
+ find_package(${FIND_PACKAGE_ARGUMENTS} QUIET)
Review comment:
Why do you want to add `QUIET` here?
I think that `find_package()` output is useful for debugging, user support
and so on.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
