chore: unvendor psutil and wrapt by majorgreys · Pull Request #6651 · DataDog/dd-trace-py

majorgreys · 2023-08-11T15:57:05Z

We had vendored psutil in #1160 due to a lack of wheels for all support architectures and OSes. This has since been resolved.

At the same time, we can also unvendor wrapt which was added in #755, which also releases all wheels we need.

We pin the minimum version to the previously vendored version.

Checklist

Change(s) are motivated and described in the PR description.
Testing strategy is described if automated tests are not included in the PR.
Risk is outlined (performance impact, potential for breakage, maintainability, etc).
Change is maintainable (easy to change, telemetry, documentation).
Library release note guidelines are followed. If no release note is required, add label changelog/no-changelog.
Documentation is included (in-code, generated user docs, public corp docs).
Backport labels are set (if applicable)

Reviewer Checklist

Title is accurate.
No unnecessary changes are introduced.
Description motivates each change.
Avoids breaking API changes unless absolutely necessary.
Testing strategy adequately addresses listed risk(s).
Change is maintainable (easy to change, telemetry, documentation).
Release note makes sense to a user of the library.
Reviewer has explicitly 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

pr-commenter · 2023-08-11T17:27:45Z

Benchmarks

Benchmark execution time: 2023-08-11 17:27:42

Comparing candidate commit 0488d07 in PR branch majorgreys/2.x-unvendor-wrapt-psutil with baseline commit 3fbb4eb in branch 2.x-dev.

Found 37 performance improvements and 4 performance regressions! Performance is the same for 53 metrics, 0 unstable metrics.

scenario:flasksimple-appsec-get

🟩 max_rss_usage [-1.676MB; -1.527MB] or [-5.005%; -4.562%]

scenario:flasksimple-appsec-post

🟩 max_rss_usage [-1.467MB; -1.293MB] or [-4.398%; -3.876%]

scenario:flasksimple-appsec-telemetry

🟩 max_rss_usage [-2.351MB; -2.212MB] or [-6.885%; -6.478%]

scenario:flasksimple-baseline

🟩 max_rss_usage [-2.345MB; -2.184MB] or [-6.875%; -6.404%]

scenario:flasksimple-iast-get

🟩 max_rss_usage [-2.406MB; -2.256MB] or [-7.044%; -6.606%]

scenario:flasksimple-profiler

🟩 max_rss_usage [-2.431MB; -2.256MB] or [-7.112%; -6.599%]

scenario:flasksimple-tracer

🟩 max_rss_usage [-2.358MB; -2.172MB] or [-6.911%; -6.366%]

scenario:otelspan-add-metrics

🟥 max_rss_usage [+1.289MB; +1.385MB] or [+3.167%; +3.402%]

scenario:otelspan-add-tags

🟥 max_rss_usage [+1.278MB; +1.396MB] or [+3.138%; +3.427%]

scenario:otelspan-start-finish

🟩 max_rss_usage [-1.622MB; -1.515MB] or [-5.640%; -5.267%]

scenario:otelspan-start-finish-telemetry

🟩 max_rss_usage [-1.657MB; -1.573MB] or [-5.756%; -5.466%]

scenario:samplingrules-average_match

🟩 max_rss_usage [-1.597MB; -1.448MB] or [-5.801%; -5.258%]

scenario:samplingrules-high_match

🟩 max_rss_usage [-1.639MB; -1.468MB] or [-5.950%; -5.329%]

scenario:samplingrules-low_match

🟩 max_rss_usage [-2.232MB; -2.088MB] or [-7.839%; -7.335%]

scenario:samplingrules-very_low_match

🟩 max_rss_usage [-1.650MB; -1.508MB] or [-3.806%; -3.479%]

scenario:sethttpmeta-all-disabled

🟩 max_rss_usage [-1.511MB; -1.363MB] or [-5.401%; -4.872%]

scenario:sethttpmeta-all-enabled

🟩 max_rss_usage [-1.516MB; -1.361MB] or [-5.419%; -4.867%]

scenario:sethttpmeta-collectipvariant_exists

🟩 max_rss_usage [-1.461MB; -1.334MB] or [-5.230%; -4.773%]

scenario:sethttpmeta-no-collectipvariant

🟩 max_rss_usage [-1.532MB; -1.390MB] or [-5.463%; -4.957%]

scenario:sethttpmeta-no-useragentvariant

🟩 max_rss_usage [-1.612MB; -1.458MB] or [-5.759%; -5.211%]

scenario:sethttpmeta-obfuscation-disabled

🟩 max_rss_usage [-2.155MB; -2.007MB] or [-7.511%; -6.997%]

scenario:sethttpmeta-obfuscation-no-query

🟩 max_rss_usage [-1.527MB; -1.379MB] or [-5.457%; -4.927%]

scenario:sethttpmeta-obfuscation-regular-case-explicit-query

🟩 max_rss_usage [-2.225MB; -2.090MB] or [-7.702%; -7.235%]

scenario:sethttpmeta-obfuscation-regular-case-implicit-query

🟩 max_rss_usage [-2.257MB; -2.114MB] or [-7.816%; -7.319%]

scenario:sethttpmeta-obfuscation-send-querystring-disabled

🟩 max_rss_usage [-2.162MB; -2.025MB] or [-7.485%; -7.012%]

scenario:sethttpmeta-obfuscation-worst-case-explicit-query

🟩 max_rss_usage [-2.134MB; -1.980MB] or [-7.398%; -6.865%]

scenario:sethttpmeta-obfuscation-worst-case-implicit-query

🟩 max_rss_usage [-2.239MB; -2.064MB] or [-7.742%; -7.136%]

scenario:sethttpmeta-useragentvariant_exists_1

🟩 max_rss_usage [-1.522MB; -1.386MB] or [-5.443%; -4.957%]

scenario:sethttpmeta-useragentvariant_exists_2

🟩 max_rss_usage [-2.304MB; -2.163MB] or [-8.013%; -7.522%]

scenario:sethttpmeta-useragentvariant_exists_3

🟩 max_rss_usage [-2.343MB; -2.167MB] or [-8.157%; -7.544%]

scenario:sethttpmeta-useragentvariant_not_exists_1

🟩 max_rss_usage [-1.495MB; -1.340MB] or [-5.342%; -4.786%]

scenario:sethttpmeta-useragentvariant_not_exists_2

🟩 max_rss_usage [-1.133MB; -0.984MB] or [-4.115%; -3.574%]

scenario:span-add-metrics

🟥 max_rss_usage [+2.617MB; +2.788MB] or [+4.470%; +4.763%]

scenario:span-add-tags

🟥 max_rss_usage [+1.367MB; +1.494MB] or [+3.336%; +3.644%]

scenario:span-start

🟩 max_rss_usage [-1.205MB; -1.049MB] or [-2.483%; -2.161%]

scenario:span-start-finish

🟩 max_rss_usage [-1.653MB; -1.494MB] or [-5.957%; -5.381%]

scenario:span-start-finish-telemetry

🟩 max_rss_usage [-1.432MB; -1.288MB] or [-5.169%; -4.649%]

scenario:span-start-finish-traceid128

🟩 max_rss_usage [-1.485MB; -1.347MB] or [-5.362%; -4.863%]

scenario:tracer-large

🟩 max_rss_usage [-2.245MB; -2.096MB] or [-7.675%; -7.163%]

scenario:tracer-medium

🟩 max_rss_usage [-1.570MB; -1.415MB] or [-5.677%; -5.115%]

scenario:tracer-small

🟩 max_rss_usage [-2.265MB; -2.081MB] or [-7.974%; -7.325%]

The base branch was changed.

duncanista

LGTM!

We had vendored psutil in #1160 due to a lack of wheels for all support architectures and OSes. This has since been resolved. At the same time, we can also unvendor wrapt which was added in #755, which also releases all wheels we need. We pin the minimum version to the previously vendored version. ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] 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: Emmett Butler <[email protected]>

Reverts #6651 Fixes #7316 Closes #7202 ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] 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) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that. --------- Co-authored-by: Zachary Groves <[email protected]> Co-authored-by: Brett Langdon <[email protected]>

Reverts #6651 Fixes #7316 Closes #7202 - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] 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) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that. --------- Co-authored-by: Zachary Groves <[email protected]> Co-authored-by: Brett Langdon <[email protected]> (cherry picked from commit 9f79fd5)

Reverts #6651 Fixes #7316 Closes #7202 --------- Co-authored-by: Zachary Groves <[email protected]> Co-authored-by: Brett Langdon <[email protected]> (cherry picked from commit 9f79fd5) ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] 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) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that.

Reverts #6651 Fixes #7316 Closes #7202 Co-authored-by: Zachary Groves <[email protected]> Co-authored-by: Brett Langdon <[email protected]> (cherry picked from commit 9f79fd5) ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] 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) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that.

majorgreys added the changelog/no-changelog A changelog entry is not required for this PR. label Aug 11, 2023

majorgreys marked this pull request as ready for review August 11, 2023 16:40

majorgreys requested review from a team as code owners August 11, 2023 16:40

majorgreys requested review from Yun-Kim, emmettbutler, jbertran and juanjux and removed request for a team August 11, 2023 16:40

majorgreys force-pushed the majorgreys/2.x-unvendor-wrapt-psutil branch from c991789 to 0488d07 Compare August 11, 2023 16:41

emmettbutler previously approved these changes Aug 11, 2023

View reviewed changes

Yun-Kim previously approved these changes Aug 24, 2023

View reviewed changes

majorgreys force-pushed the 2.x-dev branch from 3fbb4eb to b59b9a4 Compare August 24, 2023 19:12

majorgreys requested a review from a team as a August 24, 2023 19:12

majorgreys force-pushed the 2.x-dev branch from b59b9a4 to 3052251 Compare August 24, 2023 19:15

Yun-Kim force-pushed the 2.x-dev branch from 3052251 to 7d4ef11 Compare August 24, 2023 20:25

majorgreys force-pushed the 2.x-dev branch 3 times, most recently from 49a3ea6 to 3cd8cd2 Compare August 25, 2023 16:34

majorgreys changed the base branch from 2.x-dev to 2.x August 29, 2023 14:34

majorgreys force-pushed the majorgreys/2.x-unvendor-wrapt-psutil branch from 0488d07 to 3074dac Compare September 7, 2023 20:09

emmettbutler self-assigned this Sep 13, 2023

Merge branch '2.x' into majorgreys/2.x-unvendor-wrapt-psutil

c15039c

emmettbutler self-requested a review September 13, 2023 15:56

emmettbutler previously approved these changes Sep 13, 2023

View reviewed changes

style

0196cec

emmettbutler dismissed their stale review via 0196cec September 13, 2023 16:04

remove vendor imports from some tests

83180f2

emmettbutler self-requested a review September 13, 2023 16:41

emmettbutler approved these changes Sep 13, 2023

View reviewed changes

emmettbutler requested review from shatzi and removed request for a team September 13, 2023 16:46

shatzi approved these changes Sep 13, 2023

View reviewed changes

Merge branch '2.x' into majorgreys/2.x-unvendor-wrapt-psutil

ed85cec

duncanista approved these changes Sep 13, 2023

View reviewed changes

majorgreys merged commit 53ac961 into 2.x Sep 13, 2023

majorgreys deleted the majorgreys/2.x-unvendor-wrapt-psutil branch September 13, 2023 17:39

majorgreys mentioned this pull request Oct 11, 2023

fix(lib-injection): build wheels for dependencies #7202

Closed

18 tasks

majorgreys mentioned this pull request Oct 24, 2023

ddtrace 2 + python 3.12 requires compilation of psutil? #7316

Closed

majorgreys added a commit that referenced this pull request Oct 26, 2023

chore: revert #6651

cb5843a

majorgreys mentioned this pull request Oct 26, 2023

chore: re-vendor psutil and wrapt #7332

Merged

18 tasks

majorgreys mentioned this pull request Oct 31, 2023

chore: re-vendor psutil and wrapt (backport #7332 to 2.0) #7436

Merged

18 tasks

majorgreys mentioned this pull request Oct 31, 2023

chore: re-vendor psutil and wrapt (backport #7332 to 2.1) #7437

Merged

18 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore: unvendor psutil and wrapt#6651

chore: unvendor psutil and wrapt#6651
majorgreys merged 11 commits into
2.xfrom
majorgreys/2.x-unvendor-wrapt-psutil

majorgreys commented Aug 11, 2023 •

edited by mabdinur

Loading

Uh oh!

pr-commenter Bot commented Aug 11, 2023

Uh oh!

duncanista left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

majorgreys commented Aug 11, 2023 • edited by mabdinur Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Reviewer Checklist

Uh oh!

pr-commenter Bot commented Aug 11, 2023

Benchmarks

scenario:flasksimple-appsec-get

scenario:flasksimple-appsec-post

scenario:flasksimple-appsec-telemetry

scenario:flasksimple-baseline

scenario:flasksimple-iast-get

scenario:flasksimple-profiler

scenario:flasksimple-tracer

scenario:otelspan-add-metrics

scenario:otelspan-add-tags

scenario:otelspan-start-finish

scenario:otelspan-start-finish-telemetry

scenario:samplingrules-average_match

scenario:samplingrules-high_match

scenario:samplingrules-low_match

scenario:samplingrules-very_low_match

scenario:sethttpmeta-all-disabled

scenario:sethttpmeta-all-enabled

scenario:sethttpmeta-collectipvariant_exists

scenario:sethttpmeta-no-collectipvariant

scenario:sethttpmeta-no-useragentvariant

scenario:sethttpmeta-obfuscation-disabled

scenario:sethttpmeta-obfuscation-no-query

scenario:sethttpmeta-obfuscation-regular-case-explicit-query

scenario:sethttpmeta-obfuscation-regular-case-implicit-query

scenario:sethttpmeta-obfuscation-send-querystring-disabled

scenario:sethttpmeta-obfuscation-worst-case-explicit-query

scenario:sethttpmeta-obfuscation-worst-case-implicit-query

scenario:sethttpmeta-useragentvariant_exists_1

scenario:sethttpmeta-useragentvariant_exists_2

scenario:sethttpmeta-useragentvariant_exists_3

scenario:sethttpmeta-useragentvariant_not_exists_1

scenario:sethttpmeta-useragentvariant_not_exists_2

scenario:span-add-metrics

scenario:span-add-tags

scenario:span-start

scenario:span-start-finish

scenario:span-start-finish-telemetry

scenario:span-start-finish-traceid128

scenario:tracer-large

scenario:tracer-medium

scenario:tracer-small

Uh oh!

duncanista left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

majorgreys commented Aug 11, 2023 •

edited by mabdinur

Loading