westonpace commented on a change in pull request #10260:
URL: https://github.com/apache/arrow/pull/10260#discussion_r749742583



##########
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 think at some point we may end up with a new "query engine" module 
which includes things like the substrait consumer and data holder (these 
components need to reference datasets and parquet and so can't be placed in the 
compute module).  The "query engine" module would then include OpenTelemetry 
not just as a development module but in production too.  I see the ability to 
generate statistics on running queries to be an essential part of a query 
engine and not just a development profiling feature.
   
   Maybe then we could enable OT only if "flight" or "query engine" were 
enabled?  Either way, I agree this is a good discussion for the ML.




-- 
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]


Reply via email to