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

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

Merged
merged 1 commit into from
Nov 3, 2022

Conversation

Yun-Kim
Copy link
Contributor

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

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
Copy link
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

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)

ddtrace/profiling/collector/stack.pyx Show resolved Hide resolved
@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
Copy link
Contributor Author

Yun-Kim commented Nov 3, 2022

@Mergifyio backport 1.6

@mergify
Copy link
Contributor

mergify bot commented Nov 3, 2022

backport 1.6

✅ Backports have been created

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