Skip to content

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Aug 7, 2024

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

  • 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

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)
@github-actions github-actions bot requested review from a team as code owners August 7, 2024 13:41
@datadog-dd-trace-py-rkomorn
Copy link

datadog-dd-trace-py-rkomorn bot commented Aug 7, 2024

Datadog Report

Branch report: backport-9904-to-2.11
Commit report: f1630a7
Test service: dd-trace-py

✅ 0 Failed, 118436 Passed, 60014 Skipped, 3h 39m 19.33s Total duration (5h 33m 47.84s time saved)

@pr-commenter
Copy link

pr-commenter bot commented Aug 7, 2024

Benchmarks

Benchmark execution time: 2024-08-07 14:31:00

Comparing candidate commit 3c944c5 in PR branch backport-9904-to-2.11 with baseline commit fea19c5 in branch 2.11.

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

@sanchda sanchda enabled auto-merge (squash) August 7, 2024 14:36
@codecov-commenter
Copy link

Codecov Report

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

Project coverage is 10.55%. Comparing base (9c353ce) to head (3c944c5).
Report is 2 commits behind head on 2.11.

Files Patch % Lines
ddtrace/settings/profiling.py 0.00% 19 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             2.11   #10119       +/-   ##
===========================================
- Coverage   34.18%   10.55%   -23.64%     
===========================================
  Files        1385     1381        -4     
  Lines      129280   129237       -43     
===========================================
- Hits        44191    13636    -30555     
- Misses      85089   115601    +30512     

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

@sanchda sanchda closed this Aug 7, 2024
auto-merge was automatically disabled August 7, 2024 15:17

Pull request was closed

@sanchda sanchda reopened this Aug 7, 2024
@sanchda sanchda enabled auto-merge (squash) August 7, 2024 15:17
@github-actions
Copy link
Contributor Author

github-actions bot commented Aug 7, 2024

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

@sanchda sanchda merged commit d72d15e into 2.11 Aug 14, 2024
@sanchda sanchda deleted the backport-9904-to-2.11 branch August 14, 2024 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants