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

sanchda · 2024-09-26T19:02:25Z

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

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
The change includes or references documentation updates if necessary
Backport labels are set (if applicable)

Reviewer Checklist

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 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

github-actions · 2024-09-26T19:03:13Z

CODEOWNERS have been resolved as:

releasenotes/notes/profiling-fix-extension-filenames-ac311c1851a7c467.yaml  @DataDog/apm-python
.gitlab/package.yml                                                     @DataDog/python-guild @DataDog/apm-core-python
.gitlab/prepare-oci-package.sh                                          @DataDog/python-guild @DataDog/apm-core-python
ddtrace/internal/datadog/profiling/crashtracker/CMakeLists.txt          @DataDog/profiling-python
ddtrace/internal/datadog/profiling/crashtracker/_crashtracker.pyx       @DataDog/profiling-python
ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt            @DataDog/profiling-python
ddtrace/internal/datadog/profiling/dd_wrapper/include/crashtracker_interface.hpp  @DataDog/profiling-python
ddtrace/internal/datadog/profiling/dd_wrapper/src/crashtracker_interface.cpp  @DataDog/profiling-python
setup.py                                                                @DataDog/python-guild

…dd-trace-py into sanchda/fix_dd_wrapper_filename

datadog-dd-trace-py-rkomorn · 2024-09-26T19:19:03Z

Datadog Report

Branch report: sanchda/fix_dd_wrapper_filename
Commit report: 7a085c9
Test service: dd-trace-py

✅ 0 Failed, 592 Passed, 694 Skipped, 19m 55.09s Total duration (17m 24.52s time saved)

pr-commenter · 2024-09-26T19:42:42Z

Benchmarks

Benchmark execution time: 2024-09-27 17:07:06

Comparing candidate commit 7a085c9 in PR branch sanchda/fix_dd_wrapper_filename with baseline commit e160b36 in branch main.

Found 1 performance improvements and 1 performance regressions! Performance is the same for 211 metrics, 51 unstable metrics.

scenario:iast_aspects-aspect_iast_do_operator_add_params

🟩 max_rss_usage [-2.750MB; -2.494MB] or [-9.383%; -8.510%]

scenario:iast_aspects-aspect_iast_do_stringio_init_and_read

🟥 max_rss_usage [+2.353MB; +2.713MB] or [+8.819%; +10.171%]

… 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) (cherry picked from commit 0dd7362)

… 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]>

… filenames [backport 2.14] (#10854) Backport 0dd7362 from #10840 to 2.14. 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]> Co-authored-by: Brett Langdon <[email protected]>

… filenames [backport 2.12] (#10852) Backport 0dd7362 from #10840 to 2.12. 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]>

With [fix(profiling): platform-specific files should have platform-specific filenames](#10840), crashtracker binaries have trailing platform specific strings in their names, e.g. `crashtracker_exe-glibc-aarch64` ## 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)

General fixups, add crashtracker

f21100f

sanchda requested review from a team as code owners September 26, 2024 19:02

sanchda requested review from P403n1x87 and romainkomorn-exdatadog September 26, 2024 19:02

sanchda added [DEPRECATED] backport 2.12 [DEPRECATED] backport 2.13 [DEPRECATED] backport 2.14 labels Sep 26, 2024

sanchda added 3 commits September 26, 2024 14:03

Merge branch 'main' into sanchda/fix_dd_wrapper_filename

24b179a

Make sure the right crashtracker exe is propagated by setup

d1928ec

Merge branch 'sanchda/fix_dd_wrapper_filename' of github.com:DataDog/…

0097466

…dd-trace-py into sanchda/fix_dd_wrapper_filename

taegyunkim approved these changes Sep 26, 2024

View reviewed changes

sanchda enabled auto-merge (squash) September 26, 2024 19:10

fix missing job artifact

b2c1cd8

brettlangdon requested a review from a team as a code owner September 26, 2024 19:19

brettlangdon requested a review from emmettbutler September 26, 2024 19:19

brettlangdon approved these changes Sep 26, 2024

View reviewed changes

sanchda and others added 2 commits September 27, 2024 15:59

Drop versions

41feac6

reduce size of arch specific OCI packages

7a085c9

brettlangdon mentioned this pull request Sep 27, 2024

fix: support dynamic libdd_wrapper.so filename DataDog/datadog-lambda-python#516

Merged

11 tasks

sanchda merged commit 0dd7362 into main Sep 27, 2024
529 checks passed

sanchda deleted the sanchda/fix_dd_wrapper_filename branch September 27, 2024 19:22

github-actions bot mentioned this pull request Sep 27, 2024

fix(profiling): platform-specific files should have platform-specific filenames [backport 2.12] #10852

Merged

2 tasks

github-actions bot mentioned this pull request Sep 27, 2024

fix(profiling): platform-specific files should have platform-specific filenames [backport 2.13] #10853

Merged

2 tasks

github-actions bot mentioned this pull request Sep 27, 2024

fix(profiling): platform-specific files should have platform-specific filenames [backport 2.14] #10854

Merged

2 tasks

taegyunkim mentioned this pull request Oct 1, 2024

chore: ignore crashtracker binary in git #10896

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

sanchda commented Sep 26, 2024 •

edited by taegyunkim

Loading

Uh oh!

github-actions bot commented Sep 26, 2024 •

edited

Loading

Uh oh!

datadog-dd-trace-py-rkomorn bot commented Sep 26, 2024 •

edited

Loading

Uh oh!

pr-commenter bot commented Sep 26, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

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

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

Uh oh!

Conversation

sanchda commented Sep 26, 2024 • edited by taegyunkim Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Reviewer Checklist

Uh oh!

github-actions bot commented Sep 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

datadog-dd-trace-py-rkomorn bot commented Sep 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Datadog Report

Uh oh!

pr-commenter bot commented Sep 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

scenario:iast_aspects-aspect_iast_do_operator_add_params

scenario:iast_aspects-aspect_iast_do_stringio_init_and_read

Uh oh!

Uh oh!

Uh oh!

sanchda commented Sep 26, 2024 •

edited by taegyunkim

Loading

github-actions bot commented Sep 26, 2024 •

edited

Loading

datadog-dd-trace-py-rkomorn bot commented Sep 26, 2024 •

edited

Loading

pr-commenter bot commented Sep 26, 2024 •

edited

Loading