fix(profiling): better autoinject detection by sanchda · Pull Request #9904 · DataDog/dd-trace-py

sanchda · 2024-07-23T03:54:40Z

Previously, Profiling was injecting its own configuration during lib-injection handling. This was actually a little brittle.

Instead, we move to a model where the profiler detects (in its normal configuration) whether the library was injected. Hopefully this is more durable and correct.

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

sanchda · 2024-07-23T03:54:57Z

fix(profiling): better autoinject detection #9904 👈
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @sanchda and the rest of your teammates on Graphite

github-actions · 2024-07-23T03:55:07Z

CODEOWNERS have been resolved as:

releasenotes/notes/profiling-fix-autoinject-detection-fa5035fb0c4719cc.yaml  @DataDog/apm-python
tests/lib-injection/dd-lib-python-init-test-protobuf-old/Dockerfile     @DataDog/apm-core-python
tests/lib-injection/dd-lib-python-init-test-protobuf-old/Readme.md      @DataDog/apm-core-python
tests/lib-injection/dd-lib-python-init-test-protobuf-old/addressbook_pb2.py  @DataDog/apm-core-python
tests/lib-injection/dd-lib-python-init-test-protobuf-old/requirements.txt  @DataDog/apm-core-python
tests/lib-injection/dd-lib-python-init-test-protobuf-old/test.py        @DataDog/apm-core-python
tests/lib-injection/dd-lib-python-init-test-protobuf-old/validate_telemetry.py  @DataDog/apm-core-python
.github/workflows/lib-inject-prune.yml                                  @DataDog/python-guild @DataDog/apm-core-python
.gitlab-ci.yml                                                          @DataDog/apm-core-python
ddtrace/settings/profiling.py                                           @DataDog/profiling-python
lib-injection/sitecustomize.py                                          @DataDog/apm-core-python
pyproject.toml                                                          @DataDog/python-guild
tests/.suitespec.json                                                   @DataDog/python-guild @DataDog/apm-core-python
tests/lib-injection/dd-lib-python-init-test-django-unsupported-package-force/Dockerfile  @DataDog/apm-core-python
tests/lib-injection/dd-lib-python-init-test-django/Dockerfile           @DataDog/apm-core-python

datadog-dd-trace-py-rkomorn · 2024-07-23T04:16:44Z

Datadog Report

Branch report: 07-23-fix_profiling_better_autoinject_detection
Commit report: d896677
Test service: dd-trace-py

✅ 0 Failed, 122120 Passed, 56300 Skipped, 3h 55m 46.05s Total duration (5h 20m 19.43s time saved)

Kyle-Verhoog

just one thought, otherwise good to me!

pr-commenter · 2024-07-25T13:08:22Z

Benchmarks

Benchmark execution time: 2024-08-07 13:32:43

Comparing candidate commit 920c8e7 in PR branch 07-23-fix_profiling_better_autoinject_detection with baseline commit ab8588c in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 214 metrics, 2 unstable metrics.

…hub.com:DataDog/dd-trace-py into 07-23-fix_profiling_better_autoinject_detection

codecov-commenter · 2024-08-01T22:26:05Z

Codecov Report

Attention: Patch coverage is 0% with 19 lines in your changes missing coverage. Please review.

Project coverage is 10.55%. Comparing base (120ba6d) to head (d3934f4).
Report is 4 commits behind head on main.

Files	Patch %	Lines
ddtrace/settings/profiling.py	0.00%	19 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #9904       +/-   ##
===========================================
- Coverage   73.57%   10.55%   -63.03%     
===========================================
  Files        1421     1381       -40     
  Lines      131945   129203     -2742     
===========================================
- Hits        97078    13635    -83443     
- Misses      34867   115568    +80701

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This reverts commit 2570706.

This reverts commit b03ab71.

github-actions · 2024-08-07T13:41:32Z

The backport to 2.9 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.9 2.9
# Navigate to the new working tree
cd .worktrees/backport-2.9
# Create a new branch
git switch --create backport-9904-to-2.9
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d638a7b92d1ef01b78fad6cb1d44c27543c47059
# Push it to GitHub
git push --set-upstream origin backport-9904-to-2.9
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.9

Then, create a pull request where the base branch is 2.9 and the compare/head branch is backport-9904-to-2.9.

github-actions · 2024-08-07T13:41:32Z

The backport to 2.10 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.10 2.10
# Navigate to the new working tree
cd .worktrees/backport-2.10
# Create a new branch
git switch --create backport-9904-to-2.10
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d638a7b92d1ef01b78fad6cb1d44c27543c47059
# Push it to GitHub
git push --set-upstream origin backport-9904-to-2.10
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.10

Then, create a pull request where the base branch is 2.10 and the compare/head branch is backport-9904-to-2.10.

Previously, Profiling was injecting its own configuration during lib-injection handling. This was actually a little brittle. Instead, we move to a model where the profiler detects (in its normal configuration) whether the library was injected. Hopefully this is more durable and correct. ## 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 d638a7b)

Backport d638a7b from #9904 to 2.11. [PROF-10280] Previously, Profiling was injecting its own configuration during lib-injection handling. This was actually a little brittle. Instead, we move to a model where the profiler detects (in its normal configuration) whether the library was injected. Hopefully this is more durable and correct. ## 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-10280]: https://datadoghq.atlassian.net/browse/PROF-10280?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: David Sanchez <[email protected]>

sanchda marked this pull request as ready for review July 23, 2024 03:56

sanchda requested review from a team as code owners July 23, 2024 03:56

sanchda requested a review from brettlangdon July 23, 2024 03:56

sanchda force-pushed the 07-23-fix_profiling_better_autoinject_detection branch from 7441ae3 to 61a2f15 Compare July 23, 2024 04:00

sanchda requested a review from a team as a code owner July 23, 2024 04:00

sanchda requested a review from Yun-Kim July 23, 2024 04:00

sanchda force-pushed the 07-23-fix_profiling_better_autoinject_detection branch 3 times, most recently from de372ba to 00b1a64 Compare July 23, 2024 14:37

sanchda requested a review from Kyle-Verhoog July 23, 2024 14:38

fix(profiling): better autoinject detection

5a48fd7

sanchda force-pushed the 07-23-fix_profiling_better_autoinject_detection branch from 00b1a64 to 5a48fd7 Compare July 23, 2024 14:41

Kyle-Verhoog reviewed Jul 25, 2024

View reviewed changes

Comment thread ddtrace/settings/profiling.py Outdated

sanchda force-pushed the 07-23-fix_profiling_better_autoinject_detection branch from 5a48fd7 to cf467b4 Compare July 25, 2024 12:19

sanchda enabled auto-merge (squash) July 25, 2024 12:20

sanchda force-pushed the 07-23-fix_profiling_better_autoinject_detection branch from f8700dc to 778da61 Compare July 25, 2024 12:29

fix(profiling): better autoinject detection

1ac1a6c

sanchda force-pushed the 07-23-fix_profiling_better_autoinject_detection branch from 778da61 to 1ac1a6c Compare July 25, 2024 12:31

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

5903fdf

sanchda added backport 2.9 labels Jul 25, 2024

Kyle-Verhoog approved these changes Jul 25, 2024

View reviewed changes

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

ef96779

taegyunkim self-requested a review July 25, 2024 20:00

Merge branch '07-23-fix_profiling_better_autoinject_detection' of git…

99d7def

…hub.com:DataDog/dd-trace-py into 07-23-fix_profiling_better_autoinject_detection

taegyunkim added 3 commits August 1, 2024 16:47

fix failing test

c06f92f

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

eb4f36a

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

d896677

taegyunkim approved these changes Aug 1, 2024

View reviewed changes

sanchda added 14 commits August 1, 2024 20:20

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

d9811fb

Remove horrible test from suitespec

2570706

Revert more things

b03ab71

Revert "Remove horrible test from suitespec"

4aaff12

This reverts commit 2570706.

Revert "Revert more things"

18ae6aa

This reverts commit b03ab71.

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

db7624b

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

71f9b34

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

a235467

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

598e519

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

01ab131

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

4331c56

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

d3934f4

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

c9c2ae3

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

61a7d09

brettlangdon approved these changes Aug 6, 2024

View reviewed changes

sanchda added the backport 2.11 label Aug 7, 2024

Merge branch 'main' into 07-23-fix_profiling_better_autoinject_detection

920c8e7

sanchda merged commit d638a7b into main Aug 7, 2024

sanchda deleted the 07-23-fix_profiling_better_autoinject_detection branch August 7, 2024 13:41

github-actions Bot mentioned this pull request Aug 7, 2024

fix(profiling): better autoinject detection [backport 2.11] #10119

Merged

2 tasks

szegedi mentioned this pull request Sep 9, 2024

PROF-10509: Add profiling scenarios to Python onboarding tests DataDog/system-tests#3002

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(profiling): better autoinject detection#9904

fix(profiling): better autoinject detection#9904
sanchda merged 41 commits into
mainfrom
07-23-fix_profiling_better_autoinject_detection

sanchda commented Jul 23, 2024 •

edited by taegyunkim

Loading

Uh oh!

sanchda commented Jul 23, 2024

Uh oh!

github-actions Bot commented Jul 23, 2024 •

edited

Loading

Uh oh!

datadog-dd-trace-py-rkomorn Bot commented Jul 23, 2024 •

edited

Loading

Uh oh!

Kyle-Verhoog left a comment

Uh oh!

Uh oh!

pr-commenter Bot commented Jul 25, 2024 •

edited

Loading

Uh oh!

codecov-commenter commented Aug 1, 2024 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 7, 2024

Uh oh!

github-actions Bot commented Aug 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

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

Uh oh!

Checklist

Reviewer Checklist

Uh oh!

sanchda commented Jul 23, 2024

Uh oh!

github-actions Bot commented Jul 23, 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 Jul 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Datadog Report

Uh oh!

Kyle-Verhoog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pr-commenter Bot commented Jul 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Uh oh!

codecov-commenter commented Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Aug 7, 2024

Uh oh!

github-actions Bot commented Aug 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sanchda commented Jul 23, 2024 •

edited by taegyunkim

Loading

github-actions Bot commented Jul 23, 2024 •

edited

Loading

datadog-dd-trace-py-rkomorn Bot commented Jul 23, 2024 •

edited

Loading

pr-commenter Bot commented Jul 25, 2024 •

edited

Loading

codecov-commenter commented Aug 1, 2024 •

edited

Loading