From 32325312334374dd88bdd8c23f5d527fe730c991 Mon Sep 17 00:00:00 2001 From: Taegyun Kim Date: Mon, 9 Dec 2024 15:53:14 -0500 Subject: [PATCH] ci(profiling): gh action to run native tests with sanitizers (#11623) 1. Adds a GitHub action running profiling native tests with {thread, safety} sanitizers 2. Adds Thread sanitizer suppressions list 3. Updates echion commit to point to `taegyunkim/algorithm-include` from `taegyunkim/macos-fix`, https://github.com/taegyunkim/echion/compare/taegyunkim/macos-fix...taegyunkim:echion:taegyunkim/algorithm-include This change was necessary to compile native tests on the actions ubuntu runner. ## 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: Emmett Butler <723615+emmettbutler@users.noreply.github.com> --- .github/workflows/profiling-native.yml | 48 +++++++++++++++++++ .../profiling/dd_wrapper/test/CMakeLists.txt | 7 ++- .../profiling/dd_wrapper/test/TSan.supp | 4 ++ .../datadog/profiling/stack_v2/CMakeLists.txt | 2 +- 4 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/profiling-native.yml create mode 100644 ddtrace/internal/datadog/profiling/dd_wrapper/test/TSan.supp diff --git a/.github/workflows/profiling-native.yml b/.github/workflows/profiling-native.yml new file mode 100644 index 00000000000..98722552dbd --- /dev/null +++ b/.github/workflows/profiling-native.yml @@ -0,0 +1,48 @@ +name: Profiling Native Tests with Sanitizers + +on: + push: + branches: + - main + - "mq-working-branch**" + pull_request: + paths: + - ddtrace/internal/datadog/profiling/** + - ddtrace/profiling/** + workflow_dispatch: {} + +jobs: + test: + runs-on: ${{ matrix.os }} + timeout-minutes: 5 + strategy: + fail-fast: false + matrix: + os: [ubuntu-24.04] + python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] + sanitizer: ["safety", "thread"] + + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + fetch-depth: 1 + + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + + - name: Install llvm 19 + run: | + # Ubuntu-24.04 GH actions image has llvm-18, but we use 19 as it's + # the latest one available. + wget https://apt.llvm.org/llvm.sh + chmod +x llvm.sh + sudo ./llvm.sh 19 + + - name: Run tests with sanitizers + run: | + # DEV: We currently have tests in dd_wrapper and stack_v2, setting + # stack_v2 here will also run tests in dd_wrapper. Revisit this when + # that changes. + ./ddtrace/internal/datadog/profiling/build_standalone.sh --${{matrix.sanitizer}} RelWithDebInfo stack_v2_test diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/test/CMakeLists.txt b/ddtrace/internal/datadog/profiling/dd_wrapper/test/CMakeLists.txt index 6a1e7f406f6..0be6098cd2a 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/test/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/test/CMakeLists.txt @@ -21,7 +21,12 @@ function(dd_wrapper_add_test name) target_link_libraries(${name} PRIVATE gmock gtest_main dd_wrapper nlohmann_json::nlohmann_json) add_ddup_config(${name}) - gtest_discover_tests(${name}) + gtest_discover_tests(${name} + PROPERTIES + # We start new threads after fork(), and we want to continue + # running the tests after that instead of dying. + ENVIRONMENT "TSAN_OPTIONS=die_after_fork=0:suppressions=${CMAKE_CURRENT_SOURCE_DIR}/TSan.supp" + ) set_target_properties(${name} PROPERTIES INSTALL_RPATH "$ORIGIN/..") diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/test/TSan.supp b/ddtrace/internal/datadog/profiling/dd_wrapper/test/TSan.supp new file mode 100644 index 00000000000..bc9e9d23668 --- /dev/null +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/test/TSan.supp @@ -0,0 +1,4 @@ +# libdd is not compiled with sanitizers so probably that's why we get these +# data races from ddog_ArrayQueue and Datadog::Sample operations +race:ddog_ArrayQueue +race:Datadog::Sample:: diff --git a/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt b/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt index 9fc78ee8046..50c8d056c79 100644 --- a/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt @@ -37,7 +37,7 @@ endif() # Add echion set(ECHION_COMMIT - "9d5bcc5867d7aefff73c837adcba4ef46eecebc6" + "ed744987f224fae3f93c842b2b5ffb083984ff8b" CACHE STRING "Commit hash of echion to use") FetchContent_Declare( echion