Skip to content

Conversation

@sanchda
Copy link
Contributor

@sanchda sanchda commented Sep 26, 2024

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
Copy link
Contributor

github-actions bot commented Sep 26, 2024

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 
@sanchda sanchda enabled auto-merge (squash) September 26, 2024 19:10
@datadog-dd-trace-py-rkomorn
Copy link

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

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)

@brettlangdon brettlangdon requested a review from a team as a code owner September 26, 2024 19:19
@pr-commenter
Copy link

pr-commenter bot commented Sep 26, 2024

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%]
@sanchda sanchda merged commit 0dd7362 into main Sep 27, 2024
529 checks passed
@sanchda sanchda deleted the sanchda/fix_dd_wrapper_filename branch September 27, 2024 19:22
github-actions bot pushed a commit that referenced this pull request Sep 27, 2024
… 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)
github-actions bot pushed a commit that referenced this pull request Sep 27, 2024
… 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)
github-actions bot pushed a commit that referenced this pull request Sep 27, 2024
… 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)
brettlangdon pushed a commit that referenced this pull request Oct 1, 2024
… 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 <838104+sanchda@users.noreply.github.com>
brettlangdon added a commit that referenced this pull request Oct 1, 2024
… 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 <838104+sanchda@users.noreply.github.com> Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>
brettlangdon pushed a commit that referenced this pull request Oct 1, 2024
… 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 <838104+sanchda@users.noreply.github.com>
taegyunkim added a commit that referenced this pull request Oct 1, 2024
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment