Skip to content

Commit

Permalink
fix(profiling): explicitly link with thread library [backport-2.12] (#…
Browse files Browse the repository at this point in the history
…11004)

Manual backport of #10994 to 2.12

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
- [x] 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)
  • Loading branch information
taegyunkim authored Oct 11, 2024
1 parent f4be357 commit 262b778
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 6 deletions.
11 changes: 8 additions & 3 deletions ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,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
src/uploader_builder.cpp
Expand All @@ -46,9 +53,7 @@ target_include_directories(dd_wrapper PRIVATE
${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
Expand Down
17 changes: 14 additions & 3 deletions ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,19 @@ include(FindCppcheck)
# but whether or not we keep that design is unknown. Hack it for now.
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 "b2f2d49f2f5d5c4dd78d1d9b83280db8ac2949c0" CACHE STRING "Commit hash of echion to use")
FetchContent_Declare(
Expand Down Expand Up @@ -90,9 +98,12 @@ set_target_properties(${EXTENSION_NAME} PROPERTIES SUFFIX "")
# RPATH is needed for sofile discovery at runtime, since Python packages are not
# installed in the system path. This is typical.
set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "$ORIGIN/..")
target_link_libraries(${EXTENSION_NAME} PRIVATE
dd_wrapper
)

target_link_libraries(${EXTENSION_NAME} PRIVATE dd_wrapper Threads::Threads)

if(Python3_LIBRARIES)
target_link_libraries(${EXTENSION_NAME} PRIVATE ${Python3_LIBRARIES})
endif()

# Extensions are built as dynamic libraries, so PIC is required.
set_target_properties(${EXTENSION_NAME} PROPERTIES POSITION_INDEPENDENT_CODE ON)
Expand Down
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit 262b778

Please sign in to comment.