Skip to content

fix(profiler): version gate python 3.11 changes to stack collector#4448

Merged
Kyle-Verhoog merged 1 commit into
DataDog:1.xfrom
Yun-Kim:yunkim/profiler-311-revert
Nov 3, 2022
Merged

fix(profiler): version gate python 3.11 changes to stack collector#4448
Kyle-Verhoog merged 1 commit into
DataDog:1.xfrom
Yun-Kim:yunkim/profiler-311-revert

Conversation

@Yun-Kim

@Yun-Kim Yun-Kim commented Nov 2, 2022

Copy link
Copy Markdown
Contributor

Description

From #4125, two sections in the stack collector were modified to reflect Python 3.11 changes. However, those changes are not the exact same logic as the former code, including version gating and exception representation. I'm specifically applying these changes only if Python 3.11 or higher are being run. Otherwise, it is being reverted to what it was prior to #4125.

While I'm unsure what is exactly causing the issue, we saw an issue (only when the profiler was activated) when running dogweb CI where tests involving sqlalchemy failed on setup with sessions becoming immediately inactive before any queries were run. Furthermore, we saw failures in asyncpg framework tests and major memory/CPU usage spikes when #4125 was merged into 1.x, both of which went away with this fix.

Checklist

Motivation

Design

Testing strategy

Relevant issue(s)

Testing strategy

Reviewer Checklist

  • Title is accurate.
  • Description motivates each change.
  • No unnecessary changes were introduced in this PR.
  • Avoid breaking API changes unless absolutely necessary.
  • Tests provided or description of manual testing performed is included in the code or PR.
  • Release note has been added for fixes and features, or else changelog/no-changelog label added.
  • All relevant GitHub issues are correctly linked.
  • Backports are identified and tagged with Mergifyio.

@Yun-Kim Yun-Kim added the changelog/no-changelog A changelog entry is not required for this PR. label Nov 2, 2022
@Yun-Kim
Yun-Kim marked this pull request as ready for review November 2, 2022 22:05
@Yun-Kim
Yun-Kim requested a review from a team as a code owner November 2, 2022 22:06
Comment thread ddtrace/profiling/collector/stack.pyx

@Kyle-Verhoog Kyle-Verhoog left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we add to the PR description why we had to make this change (the issues we saw occur)

Comment thread ddtrace/profiling/collector/stack.pyx
@Kyle-Verhoog
Kyle-Verhoog merged commit 6bc6ff1 into DataDog:1.x Nov 3, 2022
@Yun-Kim
Yun-Kim deleted the yunkim/profiler-311-revert branch November 3, 2022 14:02
@Yun-Kim

Yun-Kim commented Nov 3, 2022

Copy link
Copy Markdown
Contributor Author

@Mergifyio backport 1.6

@mergify

mergify Bot commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

backport 1.6

✅ Backports have been created

Details

mergify Bot pushed a commit that referenced this pull request Nov 3, 2022
…4448)

From #4125, two sections in the stack collector were modified to reflect Python 3.11 changes. However, those changes are not the exact same logic as the former code, including version gating and exception representation. I'm specifically applying these changes only if Python 3.11 or higher are being run. Otherwise, it is being reverted to what it was prior to #4125.

While I'm unsure what is exactly causing the issue, we saw an issue (only when the profiler was activated) when running dogweb CI where tests involving sqlalchemy failed on setup with sessions becoming immediately inactive before any queries were run. Furthermore, we saw failures in asyncpg framework tests and major memory/CPU usage spikes when #4125 was merged into 1.x, both of which went away with this fix.

(cherry picked from commit 6bc6ff1)
Kyle-Verhoog added a commit that referenced this pull request Nov 3, 2022
…4448) (#4452)

From #4125, two sections in the stack collector were modified to reflect Python 3.11 changes. However, those changes are not the exact same logic as the former code, including version gating and exception representation. I'm specifically applying these changes only if Python 3.11 or higher are being run. Otherwise, it is being reverted to what it was prior to #4125.

While I'm unsure what is exactly causing the issue, we saw an issue (only when the profiler was activated) when running dogweb CI where tests involving sqlalchemy failed on setup with sessions becoming immediately inactive before any queries were run. Furthermore, we saw failures in asyncpg framework tests and major memory/CPU usage spikes when #4125 was merged into 1.x, both of which went away with this fix.

(cherry picked from commit 6bc6ff1)

Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Kyle Verhoog <[email protected]>
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.

6 participants