HyukjinKwon commented on code in PR #48556:
URL: https://github.com/apache/arrow/pull/48556#discussion_r2667045766
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -3726,220 +3689,87 @@ endif()
# ----------------------------------------------------------------------
# OpenTelemetry C++
-macro(build_opentelemetry)
- message(STATUS "Building OpenTelemetry from source")
+function(build_opentelemetry)
+ list(APPEND CMAKE_MESSAGE_INDENT "OpenTelemetry: ")
+ message(STATUS "Building OpenTelemetry from source using FetchContent")
+
if(Protobuf_VERSION VERSION_GREATER_EQUAL 3.22)
message(FATAL_ERROR "GH-36013: Can't use bundled OpenTelemetry with
Protobuf 3.22 or later. "
"Protobuf is version ${Protobuf_VERSION}")
endif()
- 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_VENDORED
+ TRUE
+ PARENT_SCOPE)
+
+ fetchcontent_declare(opentelemetry_proto
+ ${FC_DECLARE_COMMON_OPTIONS}
+ URL ${OPENTELEMETRY_PROTO_SOURCE_URL}
+ URL_HASH
"SHA256=${ARROW_OPENTELEMETRY_PROTO_BUILD_SHA256_CHECKSUM}"
)
- set(_OPENTELEMETRY_APIS api ext sdk)
- set(_OPENTELEMETRY_LIBS
- common
- http_client_curl
- logs
- ostream_log_record_exporter
- ostream_span_exporter
- otlp_http_client
- otlp_http_log_record_exporter
- otlp_http_exporter
- otlp_recordable
- proto
- resources
- trace
- version)
- set(OPENTELEMETRY_BUILD_BYPRODUCTS)
- set(OPENTELEMETRY_LIBRARIES)
-
- foreach(_OPENTELEMETRY_LIB ${_OPENTELEMETRY_APIS})
- add_library(opentelemetry-cpp::${_OPENTELEMETRY_LIB} INTERFACE IMPORTED)
- target_include_directories(opentelemetry-cpp::${_OPENTELEMETRY_LIB} BEFORE
- INTERFACE "${OPENTELEMETRY_INCLUDE_DIR}")
- endforeach()
- foreach(_OPENTELEMETRY_LIB ${_OPENTELEMETRY_LIBS})
- # N.B. OTel targets and libraries don't follow any consistent naming scheme
- if(_OPENTELEMETRY_LIB STREQUAL "http_client_curl")
- set(_OPENTELEMETRY_STATIC_LIBRARY
-
"${OPENTELEMETRY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}opentelemetry_${_OPENTELEMETRY_LIB}${CMAKE_STATIC_LIBRARY_SUFFIX}"
- )
- elseif(_OPENTELEMETRY_LIB STREQUAL "ostream_span_exporter")
- set(_OPENTELEMETRY_STATIC_LIBRARY
-
"${OPENTELEMETRY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}opentelemetry_exporter_ostream_span${CMAKE_STATIC_LIBRARY_SUFFIX}"
- )
- elseif(_OPENTELEMETRY_LIB STREQUAL "otlp_http_client")
- set(_OPENTELEMETRY_STATIC_LIBRARY
-
"${OPENTELEMETRY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}opentelemetry_exporter_otlp_http_client${CMAKE_STATIC_LIBRARY_SUFFIX}"
- )
- elseif(_OPENTELEMETRY_LIB STREQUAL "otlp_http_exporter")
- set(_OPENTELEMETRY_STATIC_LIBRARY
-
"${OPENTELEMETRY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}opentelemetry_exporter_otlp_http${CMAKE_STATIC_LIBRARY_SUFFIX}"
- )
- elseif(_OPENTELEMETRY_LIB STREQUAL "otlp_http_log_record_exporter")
- set(_OPENTELEMETRY_STATIC_LIBRARY
-
"${OPENTELEMETRY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}opentelemetry_exporter_otlp_http_log${CMAKE_STATIC_LIBRARY_SUFFIX}"
- )
- elseif(_OPENTELEMETRY_LIB STREQUAL "ostream_log_record_exporter")
- set(_OPENTELEMETRY_STATIC_LIBRARY
-
"${OPENTELEMETRY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}opentelemetry_exporter_ostream_logs${CMAKE_STATIC_LIBRARY_SUFFIX}"
- )
- else()
- set(_OPENTELEMETRY_STATIC_LIBRARY
-
"${OPENTELEMETRY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}opentelemetry_${_OPENTELEMETRY_LIB}${CMAKE_STATIC_LIBRARY_SUFFIX}"
- )
- endif()
- 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_CMAKE_ARGS} "-DCMAKE_INSTALL_PREFIX=${OPENTELEMETRY_PREFIX}"
- -DWITH_EXAMPLES=OFF)
+ # Use FetchContent_Populate instead of MakeAvailable because
opentelemetry-proto
+ # has no CMakeLists.txt.
+ cmake_policy(PUSH)
+ if(POLICY CMP0169)
+ cmake_policy(SET CMP0169 OLD)
+ endif()
+ fetchcontent_populate(opentelemetry_proto)
+ cmake_policy(POP)
- set(OPENTELEMETRY_PREFIX_PATH_LIST)
- # Don't specify the DEPENDS unless we actually have dependencies, else
- # Ninja/other build systems may consider this target to always be dirty
- set(_OPENTELEMETRY_DEPENDENCIES)
- add_custom_target(opentelemetry_dependencies)
+ fetchcontent_declare(opentelemetry_cpp
+ ${FC_DECLARE_COMMON_OPTIONS}
+ URL ${OPENTELEMETRY_SOURCE_URL}
+ URL_HASH
"SHA256=${ARROW_OPENTELEMETRY_BUILD_SHA256_CHECKSUM}")
- set(_OPENTELEMETRY_DEPENDENCIES "opentelemetry_dependencies")
- list(APPEND ARROW_BUNDLED_STATIC_LIBS ${OPENTELEMETRY_LIBRARIES})
- list(APPEND OPENTELEMETRY_PREFIX_PATH_LIST ${NLOHMANN_JSON_PREFIX})
+ prepare_fetchcontent()
- get_target_property(OPENTELEMETRY_PROTOBUF_INCLUDE_DIR
${ARROW_PROTOBUF_LIBPROTOBUF}
- INTERFACE_INCLUDE_DIRECTORIES)
- set(OPENTELEMETRY_PROTOBUF_LIBRARY
"$<TARGET_FILE:${ARROW_PROTOBUF_LIBPROTOBUF}>")
- set(OPENTELEMETRY_PROTOC_EXECUTABLE
"$<TARGET_FILE:${ARROW_PROTOBUF_PROTOC}>")
- list(APPEND
- OPENTELEMETRY_CMAKE_ARGS
- -DWITH_OTLP_HTTP=ON
- -DWITH_OTLP_GRPC=OFF
- # Disabled because it seemed to cause linking errors. May be worth a
closer look.
- -DWITH_FUNC_TESTS=OFF
- # These options are slated for removal in v1.14 and their features are
deemed stable
- # as of v1.13. However, setting their corresponding ENABLE_* macros in
headers seems
- # finicky - resulting in build failures or ABI-related runtime errors
during HTTP
- # client initialization. There may still be a solution, but we disable
them for now.
- -DWITH_OTLP_HTTP_SSL_PREVIEW=OFF
- -DWITH_OTLP_HTTP_SSL_TLS_PREVIEW=OFF
- "-DProtobuf_INCLUDE_DIR=${OPENTELEMETRY_PROTOBUF_INCLUDE_DIR}"
- "-DProtobuf_LIBRARY=${OPENTELEMETRY_PROTOBUF_LIBRARY}"
- "-DProtobuf_PROTOC_EXECUTABLE=${OPENTELEMETRY_PROTOC_EXECUTABLE}")
-
- # OpenTelemetry with OTLP enabled requires Protobuf definitions from a
- # submodule. This submodule path is hardcoded into their CMake definitions,
- # and submodules are not included in their releases. Add a custom build step
- # to download and extract the Protobufs.
-
- # Adding such a step is rather complicated, so instead: create a separate
- # ExternalProject that just fetches the Protobufs, then add a custom step
- # to the main build to copy the Protobufs.
- externalproject_add(opentelemetry_proto_ep
- ${EP_COMMON_OPTIONS}
- URL_HASH
"SHA256=${ARROW_OPENTELEMETRY_PROTO_BUILD_SHA256_CHECKSUM}"
- URL ${OPENTELEMETRY_PROTO_SOURCE_URL}
- BUILD_COMMAND ""
- CONFIGURE_COMMAND ""
- INSTALL_COMMAND ""
- EXCLUDE_FROM_ALL OFF)
- if(NLOHMANN_JSON_VENDORED)
- add_dependencies(opentelemetry_dependencies nlohmann_json_fc)
- else()
- add_dependencies(opentelemetry_dependencies nlohmann_json::nlohmann_json)
- endif()
+ set(OTELCPP_PROTO_PATH "${opentelemetry_proto_SOURCE_DIR}")
+ set(WITH_EXAMPLES OFF)
+ set(WITH_OTLP_HTTP ON)
+ set(WITH_OTLP_GRPC OFF)
+ set(WITH_FUNC_TESTS OFF)
+ # These options are slated for removal in v1.14 and their features are
deemed stable
+ # as of v1.13. However, setting their corresponding ENABLE_* macros in
headers seems
+ # finicky - resulting in build failures or ABI-related runtime errors during
HTTP
+ # client initialization. There may still be a solution, but we disable them
for now.
+ set(WITH_OTLP_HTTP_SSL_PREVIEW OFF)
+ set(WITH_OTLP_HTTP_SSL_TLS_PREVIEW OFF)
- add_dependencies(opentelemetry_dependencies opentelemetry_proto_ep
- ${ARROW_PROTOBUF_LIBPROTOBUF})
+ fetchcontent_makeavailable(opentelemetry_cpp)
Review Comment:
Hm, seems like this actually breaks the complete docs build job
https://github.com/apache/arrow/actions/runs/20356061612/job/58491733937
--
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]
