Skip to content

Commit 70cc39e

Browse files
hooliohtaegyunkimP403n1x87
authored
chore(build): consolidate libdatadog symbols in lib_native (#13984)
Both profiling and src/native depend on libdatadog, but slightly different manner prior to this PR. - Profiling consumed statically built libdatadog libraries - src/native Rust crate simply added libdatadog crates as dependencies This PR makes sure that we build libdatadog dependency as part of src/native and export profiling ffi symbols there. Regarding benchmark SLOs: - Mostly increasing `max_rss_usage`. Looks ok and acceptable to @taegyunkim. We're changing the way we build and consume libdatadog dependency. [PROF-12235](https://datadoghq.atlassian.net/browse/PROF-12235?atlOrigin=eyJpIjoiNzM4ODY4ZTI3MjFjNGMwMjgzMGUwNjhkODhlNjA1NjIiLCJwIjoiaiJ9) ## 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) [PROF-12235]: https://datadoghq.atlassian.net/browse/PROF-12235?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Signed-off-by: Taegyun Kim <[email protected]> Co-authored-by: Taegyun Kim <[email protected]> Co-authored-by: Gabriele N. Tornetta <[email protected]>
1 parent 336704e commit 70cc39e

File tree

20 files changed

+2888
-635
lines changed

20 files changed

+2888
-635
lines changed

.github/workflows/build_deploy.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ jobs:
7070

7171
- name: Install build dependencies
7272
# Rust + Cargo are needed for Cryptography
73-
run: apk add git gcc g++ musl-dev libffi-dev openssl-dev bash rust cargo make cmake
73+
run: apk add git gcc g++ musl-dev libffi-dev openssl-dev bash rust cargo make cmake patchelf
7474

7575
- name: Check source package
7676
run: |

.github/workflows/build_python_3.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ jobs:
8484
fi
8585
CIBW_BEFORE_ALL_WINDOWS: rustup target add i686-pc-windows-msvc
8686
CIBW_BEFORE_ALL_MACOS: rustup target add aarch64-apple-darwin
87-
CIBW_ENVIRONMENT_LINUX: PATH=$HOME/.cargo/bin:$PATH CMAKE_BUILD_PARALLEL_LEVEL=24
87+
CIBW_ENVIRONMENT_LINUX: PATH=$HOME/.cargo/bin:$PATH CMAKE_BUILD_PARALLEL_LEVEL=24 CMAKE_ARGS="-DNATIVE_TESTING=OFF"
8888
CIBW_REPAIR_WHEEL_COMMAND_LINUX: |
8989
mkdir ./tempwheelhouse &&
9090
unzip -l {wheel} | grep '\.so' &&
@@ -122,11 +122,11 @@ jobs:
122122
fi
123123
CIBW_BEFORE_ALL_WINDOWS: rustup target add i686-pc-windows-msvc
124124
CIBW_BEFORE_ALL_MACOS: rustup target add aarch64-apple-darwin
125-
CIBW_ENVIRONMENT_LINUX: PATH=$HOME/.cargo/bin:$PATH CMAKE_BUILD_PARALLEL_LEVEL=24
125+
CIBW_ENVIRONMENT_LINUX: PATH=$HOME/.cargo/bin:$PATH CMAKE_BUILD_PARALLEL_LEVEL=24 CMAKE_ARGS="-DNATIVE_TESTING=OFF"
126126
# SYSTEM_VERSION_COMPAT is a workaround for versioning issue, a.k.a.
127127
# `platform.mac_ver()` reports incorrect MacOS version at 11.0
128128
# See: https://stackoverflow.com/a/65402241
129-
CIBW_ENVIRONMENT_MACOS: CMAKE_BUILD_PARALLEL_LEVEL=24 SYSTEM_VERSION_COMPAT=0
129+
CIBW_ENVIRONMENT_MACOS: CMAKE_BUILD_PARALLEL_LEVEL=24 SYSTEM_VERSION_COMPAT=0 CMAKE_ARGS="-DNATIVE_TESTING=OFF"
130130
CIBW_REPAIR_WHEEL_COMMAND_LINUX: |
131131
mkdir ./tempwheelhouse &&
132132
unzip -l {wheel} | grep '\.so' &&

.github/workflows/system-tests.yml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ jobs:
3535
SYSTEM_TESTS_AWS_ACCESS_KEY_ID: ${{ secrets.IDM_AWS_ACCESS_KEY_ID }}
3636
SYSTEM_TESTS_AWS_SECRET_ACCESS_KEY: ${{ secrets.IDM_AWS_SECRET_ACCESS_KEY }}
3737
steps:
38+
- name: Install Dependencies
39+
run: sudo apt-get install -y patchelf
3840

3941
- name: Checkout system tests
4042
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
@@ -271,6 +273,9 @@ jobs:
271273
env:
272274
TEST_LIBRARY: python
273275
steps:
276+
- name: Install Dependencies
277+
run: sudo apt-get install -y patchelf
278+
274279
- name: Checkout system tests
275280
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
276281
with:
@@ -316,4 +321,4 @@ jobs:
316321
run: exit 0
317322
- name: Fails if anything else failed
318323
if: needs.parametric.result != 'success' || needs.system-tests.result != 'success'
319-
run: exit 1
324+
run: exit 1

0 commit comments

Comments
 (0)