fix(profiling): platform-specific files should have platform-specific filenames (#10840)

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)

Author: sanchda

.gitlab/package.yml

@@ -15,7 +15,7 @@ build_base_venvs:
  paths:
  - .riot/venv_*
  - ddtrace/**/*.so*
- - ddtrace/internal/datadog/profiling/crashtracker/crashtracker_exe
+ - ddtrace/internal/datadog/profiling/crashtracker/crashtracker_exe*
 
 download_ddtrace_artifacts:
  image: registry.ddbuild.io/github-cli:v27480869-eafb11d-2.43.0

.gitlab/prepare-oci-package.sh

@@ -25,6 +25,16 @@ BUILD_DIR=sources
 
 echo -n "$PYTHON_PACKAGE_VERSION" > sources/version
 
+echo "Cleaning up binaries for ${ARCH}"
+if [ "${ARCH}" == "arm64" ]; then
+ echo "Removing x86_64 binaries"
+ find ../pywheels-dep/ -type f -name '*x86_64*' -exec rm -f {} \;
+elif [ "${ARCH}" == "amd64" ]; then
+ echo "Removing aarch64 binaries"
+ find ../pywheels-dep/ -type f -name '*aarch64*' -exec rm -f {} \;
+else
+ echo "No ARCH set, not removing any binaries"
+fi
 cp -r ../pywheels-dep/site-packages* sources/ddtrace_pkgs
 
 cp ../lib-injection/sources/* sources/

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

@@ -102,7 +102,16 @@ target_include_directories(crashtracker_exe PRIVATE
  ..
  ${Datadog_INCLUDE_DIRS}
 )
-set_target_properties(crashtracker_exe PROPERTIES INSTALL_RPATH "$ORIGIN/..")
+
+# The CRASHTRACKER_EXE_TARGET_NAME should have been set by dd_wrapper
+if (NOT CRASHTRACKER_EXE_TARGET_NAME)
+ message(FATAL_ERROR "CRASHTRACKER_EXE_TARGET_NAME not set")
+endif()
+
+set_target_properties(crashtracker_exe PROPERTIES
+ INSTALL_RPATH "$ORIGIN/.."
+ OUTPUT_NAME ${CRASHTRACKER_EXE_TARGET_NAME}
+)
 target_link_libraries(crashtracker_exe PRIVATE
  dd_wrapper
 )

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

@@ -17,6 +17,7 @@ cdef extern from "<string_view>" namespace "std" nogil:
 # For now, the crashtracker code is bundled in the libdatadog Profiling FFI.
 # This is primarily to reduce binary size.
 cdef extern from "crashtracker_interface.hpp":
+ const char *crashtracker_get_exe_name()
  void crashtracker_set_url(string_view url)
  void crashtracker_set_service(string_view service)
  void crashtracker_set_env(string_view env)
@@ -150,9 +151,10 @@ def set_tag(key: StringType, value: StringType) -> None:
 
 
 def start() -> bool:
- # The file is "crashtracker_exe" in the same directory as the libdd_wrapper.so
+ crashtracker_filename_raw = crashtracker_get_exe_name()
+ crashtracker_filename = crashtracker_filename_raw.decode("utf-8")
  exe_dir = os.path.dirname(__file__)
- crashtracker_path = os.path.join(exe_dir, "crashtracker_exe")
+ crashtracker_path = os.path.join(exe_dir, crashtracker_filename)
  crashtracker_path_bytes = ensure_binary_or_empty(crashtracker_path)
  bin_exists = crashtracker_set_receiver_binary_path(
  string_view(<const char*>crashtracker_path_bytes, len(crashtracker_path_bytes))

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

@@ -18,6 +18,7 @@ include(AnalysisFunc)
 include(FindClangtidy)
 include(FindCppcheck)
 include(FindInfer)
+include(CheckSymbolExists)
 
 # Library sources
 add_library(dd_wrapper SHARED
@@ -42,10 +43,53 @@ target_include_directories(dd_wrapper PRIVATE
  include
  ${Datadog_INCLUDE_DIRS}
 )
+
 target_link_libraries(dd_wrapper PRIVATE
  ${Datadog_LIBRARIES}
 )
-set_target_properties(dd_wrapper PROPERTIES POSITION_INDEPENDENT_CODE ON)
+
+# Figure out the suffix. Try to approximate the cpython way of doing things.
+## C library
+check_symbol_exists(__GLIBC__ "features.h" HAVE_GLIBC)
+check_symbol_exists(__MUSL__ "features.h" HAVE_MUSL)
+
+set(PLATFORM_LIBC "unknown")
+if (HAVE_GLIBC)
+ set(PLATFORM_LIBC "glibc")
+elseif (HAVE_MUSL)
+ set(PLATFORM_LIBC "musl")
+endif()
+
+## Processor
+set(PLATFORM_PROCESSOR "${CMAKE_SYSTEM_PROCESSOR}")
+
+## Put the suffix together
+set(PLATFORM_SUFFIX "${PLATFORM_LIBC}-${PLATFORM_PROCESSOR}")
+string(TOLOWER ${PLATFORM_SUFFIX} PLATFORM_SUFFIX)
+
+
+# target output name should combine the system name and the processor
+# this won't necessarily match the cpython way 1-1, but as long as it encodes the major moving parts, it's fine
+set(DD_WRAPPER_TARGET_NAME "dd_wrapper-${PLATFORM_SUFFIX}")
+
+set_target_properties(dd_wrapper PROPERTIES
+ POSITION_INDEPENDENT_CODE ON
+ OUTPUT_NAME "${DD_WRAPPER_TARGET_NAME}"
+)
+
+# The name of the crashtracker executable has to be propagated to a a few different targets, and it needs to be
+# inferred at runtime, so we set it here. Any component which used dd_wrapper will have access to a uniform name.
+set(CRASHTRACKER_EXE_BASE_NAME "crashtracker_exe")
+set(CRASHTRACKER_EXE_TARGET_NAME ${CRASHTRACKER_EXE_BASE_NAME}-${PLATFORM_SUFFIX})
+
+target_compile_definitions(dd_wrapper PRIVATE
+ CRASHTRACKER_EXE_TARGET_NAME="${CRASHTRACKER_EXE_TARGET_NAME}"
+)
+
+# Also propagate the target name to the parent scope, since it can be used for non-code purposes
+# (e.g., filenames)
+set(CRASHTRACKER_EXE_TARGET_NAME ${CRASHTRACKER_EXE_TARGET_NAME} PARENT_SCOPE)
+
 
 # For a regular build, the LIB_INSTALL_DIR represents the final location of the library, so nothing special is needed.
 # 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

@@ -8,6 +8,7 @@
 extern "C"
 {
 #endif
+ const char* crashtracker_get_exe_name();
  void crashtracker_set_url(std::string_view url);
  void crashtracker_set_service(std::string_view service);
  void crashtracker_set_env(std::string_view env);

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

@@ -6,10 +6,21 @@
 #include <signal.h>
 #include <unistd.h>
 
+// If the crashtracker exe target name is not set, then fail
+#ifndef CRASHTRACKER_EXE_TARGET_NAME
+#error "CRASHTRACKER_EXE_TARGET_NAME must be defined"
+#endif
+
 // A global instance of the crashtracker is created here.
 Datadog::Crashtracker crashtracker;
 bool crashtracker_initialized = false;
 
+const char*
+crashtracker_get_exe_name() // cppcheck-suppress unusedFunction
+{
+ return CRASHTRACKER_EXE_TARGET_NAME;
+}
+
 void
 crashtracker_postfork_child()
 {

releasenotes/notes/profiling-fix-extension-filenames-ac311c1851a7c467.yaml

@@ -0,0 +1,4 @@
+fixes:
+ - |
+ Profiling: all files with platform-dependent code have had their filenames updated to reflect the platform they are
+ for. This fixes issues where the wrong file would be used on a given platform.

setup.py

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