Skip to content

Commit 75d5b36

Browse files
fix(profiling): platform-specific files should have platform-specific filenames [backport 2.13] (#10853)
Backport 0dd7362 from #10840 to 2.13. libdd_wrapper.so and crashtracker_exe were being over-written in contexts (autoinject) where platform needs to be inferred cleanly. This should fix it. Setting a broad backport requirement since this breaks crashtracker in SSI ## 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) Co-authored-by: David Sanchez <[email protected]>
1 parent 5b937e9 commit 75d5b36

File tree

9 files changed

+89
-8
lines changed

9 files changed

+89
-8
lines changed

.gitlab/package.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ build_base_venvs:
1515
paths:
1616
- .riot/venv_*
1717
- ddtrace/**/*.so*
18-
- ddtrace/internal/datadog/profiling/crashtracker/crashtracker_exe
18+
- ddtrace/internal/datadog/profiling/crashtracker/crashtracker_exe*
1919

2020
download_ddtrace_artifacts:
2121
image: registry.ddbuild.io/github-cli:v27480869-eafb11d-2.43.0

.gitlab/prepare-oci-package.sh

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,16 @@ BUILD_DIR=sources
2525

2626
echo -n "$PYTHON_PACKAGE_VERSION" > sources/version
2727

28+
echo "Cleaning up binaries for ${ARCH}"
29+
if [ "${ARCH}" == "arm64" ]; then
30+
echo "Removing x86_64 binaries"
31+
find ../pywheels-dep/ -type f -name '*x86_64*' -exec rm -f {} \;
32+
elif [ "${ARCH}" == "amd64" ]; then
33+
echo "Removing aarch64 binaries"
34+
find ../pywheels-dep/ -type f -name '*aarch64*' -exec rm -f {} \;
35+
else
36+
echo "No ARCH set, not removing any binaries"
37+
fi
2838
cp -r ../pywheels-dep/site-packages* sources/ddtrace_pkgs
2939

3040
cp ../lib-injection/sitecustomize.py sources/

ddtrace/internal/datadog/profiling/crashtracker/CMakeLists.txt

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,16 @@ target_include_directories(crashtracker_exe PRIVATE
102102
..
103103
${Datadog_INCLUDE_DIRS}
104104
)
105-
set_target_properties(crashtracker_exe PROPERTIES INSTALL_RPATH "$ORIGIN/..")
105+
106+
# The CRASHTRACKER_EXE_TARGET_NAME should have been set by dd_wrapper
107+
if (NOT CRASHTRACKER_EXE_TARGET_NAME)
108+
message(FATAL_ERROR "CRASHTRACKER_EXE_TARGET_NAME not set")
109+
endif()
110+
111+
set_target_properties(crashtracker_exe PROPERTIES
112+
INSTALL_RPATH "$ORIGIN/.."
113+
OUTPUT_NAME ${CRASHTRACKER_EXE_TARGET_NAME}
114+
)
106115
target_link_libraries(crashtracker_exe PRIVATE
107116
dd_wrapper
108117
)

ddtrace/internal/datadog/profiling/crashtracker/_crashtracker.pyx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ cdef extern from "<string_view>" namespace "std" nogil:
1717
# For now, the crashtracker code is bundled in the libdatadog Profiling FFI.
1818
# This is primarily to reduce binary size.
1919
cdef extern from "crashtracker_interface.hpp":
20+
const char *crashtracker_get_exe_name()
2021
void crashtracker_set_url(string_view url)
2122
void crashtracker_set_service(string_view service)
2223
void crashtracker_set_env(string_view env)
@@ -150,9 +151,10 @@ def set_tag(key: StringType, value: StringType) -> None:
150151

151152

152153
def start() -> bool:
153-
# The file is "crashtracker_exe" in the same directory as the libdd_wrapper.so
154+
crashtracker_filename_raw = crashtracker_get_exe_name()
155+
crashtracker_filename = crashtracker_filename_raw.decode("utf-8")
154156
exe_dir = os.path.dirname(__file__)
155-
crashtracker_path = os.path.join(exe_dir, "crashtracker_exe")
157+
crashtracker_path = os.path.join(exe_dir, crashtracker_filename)
156158
crashtracker_path_bytes = ensure_binary_or_empty(crashtracker_path)
157159
bin_exists = crashtracker_set_receiver_binary_path(
158160
string_view(<const char*>crashtracker_path_bytes, len(crashtracker_path_bytes))

ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ include(AnalysisFunc)
1818
include(FindClangtidy)
1919
include(FindCppcheck)
2020
include(FindInfer)
21+
include(CheckSymbolExists)
2122

2223
# Library sources
2324
add_library(dd_wrapper SHARED
@@ -42,10 +43,53 @@ target_include_directories(dd_wrapper PRIVATE
4243
include
4344
${Datadog_INCLUDE_DIRS}
4445
)
46+
4547
target_link_libraries(dd_wrapper PRIVATE
4648
${Datadog_LIBRARIES}
4749
)
48-
set_target_properties(dd_wrapper PROPERTIES POSITION_INDEPENDENT_CODE ON)
50+
51+
# Figure out the suffix. Try to approximate the cpython way of doing things.
52+
## C library
53+
check_symbol_exists(__GLIBC__ "features.h" HAVE_GLIBC)
54+
check_symbol_exists(__MUSL__ "features.h" HAVE_MUSL)
55+
56+
set(PLATFORM_LIBC "unknown")
57+
if (HAVE_GLIBC)
58+
set(PLATFORM_LIBC "glibc")
59+
elseif (HAVE_MUSL)
60+
set(PLATFORM_LIBC "musl")
61+
endif()
62+
63+
## Processor
64+
set(PLATFORM_PROCESSOR "${CMAKE_SYSTEM_PROCESSOR}")
65+
66+
## Put the suffix together
67+
set(PLATFORM_SUFFIX "${PLATFORM_LIBC}-${PLATFORM_PROCESSOR}")
68+
string(TOLOWER ${PLATFORM_SUFFIX} PLATFORM_SUFFIX)
69+
70+
71+
# target output name should combine the system name and the processor
72+
# this won't necessarily match the cpython way 1-1, but as long as it encodes the major moving parts, it's fine
73+
set(DD_WRAPPER_TARGET_NAME "dd_wrapper-${PLATFORM_SUFFIX}")
74+
75+
set_target_properties(dd_wrapper PROPERTIES
76+
POSITION_INDEPENDENT_CODE ON
77+
OUTPUT_NAME "${DD_WRAPPER_TARGET_NAME}"
78+
)
79+
80+
# The name of the crashtracker executable has to be propagated to a a few different targets, and it needs to be
81+
# inferred at runtime, so we set it here. Any component which used dd_wrapper will have access to a uniform name.
82+
set(CRASHTRACKER_EXE_BASE_NAME "crashtracker_exe")
83+
set(CRASHTRACKER_EXE_TARGET_NAME ${CRASHTRACKER_EXE_BASE_NAME}-${PLATFORM_SUFFIX})
84+
85+
target_compile_definitions(dd_wrapper PRIVATE
86+
CRASHTRACKER_EXE_TARGET_NAME="${CRASHTRACKER_EXE_TARGET_NAME}"
87+
)
88+
89+
# Also propagate the target name to the parent scope, since it can be used for non-code purposes
90+
# (e.g., filenames)
91+
set(CRASHTRACKER_EXE_TARGET_NAME ${CRASHTRACKER_EXE_TARGET_NAME} PARENT_SCOPE)
92+
4993

5094
# For a regular build, the LIB_INSTALL_DIR represents the final location of the library, so nothing special is needed.
5195
# However, for an inplace build, setup.py will pass a temporary path as the extension output directory, so while it

ddtrace/internal/datadog/profiling/dd_wrapper/include/crashtracker_interface.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
extern "C"
99
{
1010
#endif
11+
const char* crashtracker_get_exe_name();
1112
void crashtracker_set_url(std::string_view url);
1213
void crashtracker_set_service(std::string_view service);
1314
void crashtracker_set_env(std::string_view env);

ddtrace/internal/datadog/profiling/dd_wrapper/src/crashtracker_interface.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,21 @@
66
#include <signal.h>
77
#include <unistd.h>
88

9+
// If the crashtracker exe target name is not set, then fail
10+
#ifndef CRASHTRACKER_EXE_TARGET_NAME
11+
#error "CRASHTRACKER_EXE_TARGET_NAME must be defined"
12+
#endif
13+
914
// A global instance of the crashtracker is created here.
1015
Datadog::Crashtracker crashtracker;
1116
bool crashtracker_initialized = false;
1217

18+
const char*
19+
crashtracker_get_exe_name() // cppcheck-suppress unusedFunction
20+
{
21+
return CRASHTRACKER_EXE_TARGET_NAME;
22+
}
23+
1324
void
1425
crashtracker_postfork_child()
1526
{
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
fixes:
2+
- |
3+
Profiling: all files with platform-dependent code have had their filenames updated to reflect the platform they are
4+
for. This fixes issues where the wrong file would be used on a given platform.

setup.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ def build_extension_cmake(self, ext):
367367

368368
# If this is an inplace build, propagate this fact to CMake in case it's helpful
369369
# In particular, this is needed for build products which are not otherwise managed
370-
# by setuptools/distutils, such libdd_wrapper.so
370+
# by setuptools/distutils
371371
if IS_EDITABLE:
372372
# the INPLACE_LIB_INSTALL_DIR should be the source dir of the extension
373373
cmake_args.append("-DINPLACE_LIB_INSTALL_DIR={}".format(ext.source_dir))
@@ -580,8 +580,8 @@ def get_exts_for(name):
580580
"ddtrace.appsec": ["rules.json"],
581581
"ddtrace.appsec._ddwaf": ["libddwaf/*/lib/libddwaf.*"],
582582
"ddtrace.appsec._iast._taint_tracking": ["CMakeLists.txt"],
583-
"ddtrace.internal.datadog.profiling": ["libdd_wrapper.*"],
584-
"ddtrace.internal.datadog.profiling.crashtracker": ["crashtracker_exe"],
583+
"ddtrace.internal.datadog.profiling": ["libdd_wrapper*.*"],
584+
"ddtrace.internal.datadog.profiling.crashtracker": ["crashtracker_exe*"],
585585
},
586586
zip_safe=False,
587587
# enum34 is an enum backport for earlier versions of python

0 commit comments

Comments
 (0)