Skip to content

chore(gevent): disable gevent hub reinit #4252

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

Merged
merged 3 commits into from
Oct 3, 2022

Conversation

mabdinur
Copy link
Contributor

@mabdinur mabdinur commented Oct 3, 2022

Description

Re-initialize the gevent hub after-in-child fork hooks when DD_TRACE_GEVENT_HUB_PATCHED is set to true. This mitigates a performance regression introduced by: #4070.

Note: This PR simply mitigates the performance regression introduced by #4070. Reinitializing gevent patching when a process forks is required to use remote config, the profiler and debugger. This issue will be fixed in a follow up change.

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.
  • PR cannot be broken up into smaller PRs.
  • 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.
  • Add to milestone.

@mabdinur mabdinur added the changelog/no-changelog A changelog entry is not required for this PR. label Oct 3, 2022
@mabdinur mabdinur marked this pull request as ready for review October 3, 2022 19:05
@mabdinur mabdinur requested a review from a team as a code owner October 3, 2022 19:05
@mabdinur
Copy link
Contributor Author

mabdinur commented Oct 3, 2022

@Mergifyio backport 1.5

@mergify
Copy link
Contributor

mergify bot commented Oct 3, 2022

backport 1.5

✅ Backports have been created

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.

unrelated, but I'm not a big fan of the register being done at import time of this module. It's implicit when this code runs and could cause conflicts when importing modules

@Kyle-Verhoog Kyle-Verhoog merged commit 57ea1f8 into 1.x Oct 3, 2022
@Kyle-Verhoog Kyle-Verhoog deleted the munir/ddtrace-disable-gevent-hub-reinit-default branch October 3, 2022 21:41
@github-actions github-actions bot added this to the v1.6.0 milestone Oct 3, 2022
mergify bot pushed a commit that referenced this pull request Oct 3, 2022
Re-initialize the gevent hub after-in-child fork hooks when DD_TRACE_GEVENT_HUB_PATCHED is set to true. This mitigates a performance regression introduced by: #4070.

(cherry picked from commit 57ea1f8)
Kyle-Verhoog pushed a commit that referenced this pull request Oct 3, 2022
Adds release note for the following change: #4252
mergify bot pushed a commit that referenced this pull request Oct 3, 2022
Adds release note for the following change: #4252

(cherry picked from commit 60391dc)
mergify bot added a commit that referenced this pull request Oct 6, 2022
This is an automatic backport of pull request #4252 done by [Mergify](https://mergify.com).


---


<details>
<summary>Mergify commands and options</summary>

<br />

More conditions and actions can be found in the [documentation](https://docs.mergify.com/).

You can also trigger Mergify actions by commenting on this pull request:

- `@Mergifyio refresh` will re-evaluate the rules
- `@Mergifyio rebase` will rebase this PR on its base branch
- `@Mergifyio update` will merge the base branch into this PR
- `@Mergifyio backport <destination>` will backport this PR on `<destination>` branch

Additionally, on Mergify [dashboard](https://dashboard.mergify.com/) you can:

- look at your merge queues
- generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com
</details>
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.

3 participants