From 79cd78dbc4aa5b0c512402c07a1d9d8e2d0c23bc Mon Sep 17 00:00:00 2001 From: Taegyun Kim Date: Fri, 11 Oct 2024 11:34:36 +0200 Subject: [PATCH] fix(profiling): explicitly link with thread library (#10994) python:3.11-bookworm image for aarch64 has `pthread_atfork` in static libpthread.a not in libpthread.so so need to explicitly link for those cases. Otherwise, we get undefined symbol `pthread_atfork`. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [ ] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --- .../profiling/dd_wrapper/CMakeLists.txt | 9 ++++++++- .../datadog/profiling/stack_v2/CMakeLists.txt | 19 +++++++++++++------ ...ling-pthread-aarch64-d4548fd1842d0665.yaml | 6 ++++++ 3 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/profiling-pthread-aarch64-d4548fd1842d0665.yaml diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt b/ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt index 46977361e1f..5dce448919f 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt @@ -21,6 +21,13 @@ include(FindCppcheck) include(FindInfer) include(CheckSymbolExists) +set(THREADS_PREFER_PTHREAD_FLAG ON) +find_package(Threads REQUIRED) + +if(NOT Threads_FOUND OR NOT CMAKE_USE_PTHREADS_INIT) + message(FATAL_ERROR "pthread compatible library not found") +endif() + # Library sources add_library( dd_wrapper SHARED @@ -44,7 +51,7 @@ target_compile_features(dd_wrapper PUBLIC cxx_std_17) target_include_directories(dd_wrapper PRIVATE include ${Datadog_INCLUDE_DIRS}) -target_link_libraries(dd_wrapper PRIVATE ${Datadog_LIBRARIES}) +target_link_libraries(dd_wrapper PRIVATE ${Datadog_LIBRARIES} Threads::Threads) # Figure out the suffix. Try to approximate the cpython way of doing things. C library check_symbol_exists(__GLIBC__ "features.h" HAVE_GLIBC) diff --git a/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt b/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt index 610d08ca473..6c330c9c970 100644 --- a/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt @@ -22,11 +22,19 @@ include(FindCppcheck) add_subdirectory(../dd_wrapper ${CMAKE_CURRENT_BINARY_DIR}/../dd_wrapper_build) find_package(Python3 COMPONENTS Interpreter Development) + # Make sure we have necessary Python variables if(NOT Python3_INCLUDE_DIRS) message(FATAL_ERROR "Python3_INCLUDE_DIRS not found") endif() +set(THREADS_PREFER_PTHREAD_FLAG ON) +find_package(Threads REQUIRED) + +if(NOT Threads_FOUND OR NOT CMAKE_USE_PTHREADS_INIT) + message(FATAL_ERROR "pthread compatible library not found") +endif() + # Add echion set(ECHION_COMMIT "9d5bcc5867d7aefff73c837adcba4ef46eecebc6" @@ -70,9 +78,9 @@ target_compile_definitions(${EXTENSION_NAME} PRIVATE UNWIND_NATIVE_DISABLE) # warning(push, 0 then pop for the same effect. target_include_directories( ${EXTENSION_NAME} PRIVATE .. # include dd_wrapper from the root in order to make its paths transparent in the code - include) + include) target_include_directories(${EXTENSION_NAME} SYSTEM PRIVATE ${echion_SOURCE_DIR} ${Python3_INCLUDE_DIRS} - include/vendored include/util) + include/vendored include/util) # Echion sources need to be given the current platform if(WIN32) @@ -92,11 +100,10 @@ set_target_properties(${EXTENSION_NAME} PROPERTIES SUFFIX "") # typical. set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "$ORIGIN/..") +target_link_libraries(${EXTENSION_NAME} PRIVATE dd_wrapper Threads::Threads) + if(Python3_LIBRARIES) - target_link_libraries(${EXTENSION_NAME} PRIVATE dd_wrapper ${Python3_LIBRARIES}) -else() - # for manylinux builds - target_link_libraries(${EXTENSION_NAME} PRIVATE dd_wrapper) + target_link_libraries(${EXTENSION_NAME} PRIVATE ${Python3_LIBRARIES}) endif() # Extensions are built as dynamic libraries, so PIC is required. diff --git a/releasenotes/notes/profiling-pthread-aarch64-d4548fd1842d0665.yaml b/releasenotes/notes/profiling-pthread-aarch64-d4548fd1842d0665.yaml new file mode 100644 index 00000000000..cd7eeb08900 --- /dev/null +++ b/releasenotes/notes/profiling-pthread-aarch64-d4548fd1842d0665.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + profiling: fixes an issue where stack v2 couldn't be enabled as pthread + was not properly linked on some debian based images for aarch64 architecture. +