Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: unvendor psutil and wrapt #6651

Merged
merged 11 commits into from
Sep 13, 2023
Merged

Conversation

majorgreys
Copy link
Contributor

@majorgreys majorgreys commented Aug 11, 2023

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

@majorgreys majorgreys added the changelog/no-changelog A changelog entry is not required for this PR. label Aug 11, 2023
@majorgreys majorgreys marked this pull request as ready for review August 11, 2023 16:40
@majorgreys majorgreys requested review from a team as code owners August 11, 2023 16:40
@majorgreys majorgreys requested review from juanjux, emmettbutler, Yun-Kim and jbertran and removed request for a team August 11, 2023 16:40
@majorgreys majorgreys force-pushed the majorgreys/2.x-unvendor-wrapt-psutil branch from c991789 to 0488d07 Compare August 11, 2023 16:41
emmettbutler
emmettbutler previously approved these changes Aug 11, 2023
@pr-commenter
Copy link

pr-commenter bot commented Aug 11, 2023

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

Yun-Kim
Yun-Kim previously approved these changes Aug 24, 2023
@majorgreys majorgreys requested a review from a team as a August 24, 2023 19:12
@majorgreys majorgreys force-pushed the 2.x-dev branch 3 times, most recently from 49a3ea6 to 3cd8cd2 Compare August 25, 2023 16:34
@majorgreys majorgreys changed the base branch from 2.x-dev to 2.x August 29, 2023 14:34
@majorgreys majorgreys dismissed stale reviews from Yun-Kim and emmettbutler August 29, 2023 14:34

The base branch was changed.

@majorgreys majorgreys force-pushed the majorgreys/2.x-unvendor-wrapt-psutil branch from 0488d07 to 3074dac Compare September 7, 2023 20:09
@emmettbutler emmettbutler self-assigned this Sep 13, 2023
@emmettbutler emmettbutler self-requested a review September 13, 2023 15:56
emmettbutler
emmettbutler previously approved these changes Sep 13, 2023
@emmettbutler emmettbutler self-requested a review September 13, 2023 16:41
@emmettbutler emmettbutler requested review from shatzi and removed request for a team September 13, 2023 16:46
Copy link
Collaborator

@duncanista duncanista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@majorgreys majorgreys merged commit 53ac961 into 2.x Sep 13, 2023
@majorgreys majorgreys deleted the majorgreys/2.x-unvendor-wrapt-psutil branch September 13, 2023 17:39
majorgreys added a commit that referenced this pull request Sep 15, 2023
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]>
majorgreys added a commit that referenced this pull request Sep 18, 2023
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]>
majorgreys added a commit that referenced this pull request Oct 26, 2023
@majorgreys majorgreys mentioned this pull request Oct 26, 2023
18 tasks
brettlangdon added a commit that referenced this pull request Oct 31, 2023
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]>
majorgreys added a commit that referenced this pull request Oct 31, 2023
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)
majorgreys added a commit that referenced this pull request Oct 31, 2023
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)
ZStriker19 pushed a commit that referenced this pull request Nov 2, 2023
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.
mabdinur pushed a commit that referenced this pull request Nov 2, 2023
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/no-changelog A changelog entry is not required for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants