gevent could not work with ddtrace in pytest #8281
Assignees
Labels
Comments
|
not suitable with 2.5.0: |
|
deadlock would not happen if not imported gevent.monkey.patch_all() in pytest. |
|
even without gevent.patch_all(), use pytest to run would return error: output: if we do not import ddtrace.auto and gevent.monkey, fined. |
|
when only use gevent.patch and do not import ddtrace.auto. pytest would deadlock. |
|
@zdyj3170101136 many thanks for your investigation on this. It looks like our support for gevent in pytest is incomplete. We will look into it. cc @romainkomorndatadog for (CI) visibility. |
Closed
|
I'm not sure this is exactly a I can reproduce the deadlock with a call to the CLI that disables the eg: In fact, not only can I still reproduce the issue if I comment out every line in our plugins:
but I can still reproduce it if I remove our and I still get the hang: And just to basically confirm, with our
Out of further curiosity, I tried using
I assume the latter doesn't hang because, with |
|
For further testing, I tried simply removing the profile piece, with a really simple script: and ran the same type of tests. As-is, it fails with: If I comment out either the so I think this really points to a bad interaction between |
|
And finally, I gave |
|
And going back to the "real" original state, in an environment with an unmodified So, in the end, I don't think, from the |
|
Here is some added info. Through our DD support channel, Datadog suggested I convert from using I modified my sample project's tox.ini file changing this: which results in this error: to this: which completes successfully. I'm still looking for a solution that works without us having to update every project in the company. |
|
@ptmcg , I was the "far end" of that support conversation. :) I agree our As an extra detail, are you importing from |
|
@ptmcg , another question for you: are you seeing the same kind of deadlocks that this issue originally brought up? I can't tell from the |
|
At the moment we have code like this at the top of our So it is rather mysterious to me just how And the |
|
If I change that code to (to force the import of I get the deadlock condition, and have to ^C to exit. |
|
@ptmcg , one more question, just to double check: are you looking to use our CI Visibility product (ie: running With regards to why I suspect you can validate that by running which should make your error go away as that stops our plugin modules from being imported by This is more of a note for us at Datadog, but it looks like this comes down to an order of operations issue in the way things run when That said, I can't think of any significant changes in the way our |
|
Another note mostly for Datadog folks: this also looks unrelated to our patching of It looks like the main issue is, plainly put: Using Perhaps more relevant, I can introduce the issue by having a test import a file that uses: or But I can't repro the issue using a test that imports a file with: It made me hope for a quick win by using at the top of plugin.py, but that does not resolve the issue. |
At this time, we simply run
Yes, this runs successfully. |
|
I put together #8999 to defer The way We're still investigating why and how to deal with this. |
17 tasks
romainkomorndatadog
added a commit
that referenced
this issue
Apr 19, 2024
…is enabled (#8999) This defers importing ddtrace in the `pytest`, `pytest-bdd`, and `pytest-benchmark` plugins until after it's been determined that the plugin is enabled with (eg: by running `pytest --ddtrace`). The fixtures that use `ddtrace` defer importing until the feature is actually requested due to the fact that fixtures imported later (eg: after plugins have loaded) are not usable. Even though this doesn't actually address the issues reported in #8281 or #4845 (due to the side effects of `ddtrace/__init__.py`), this change is still helpful in other ways (in particular since I'll be introducing a new version of the `pytest` plugin shortly). Fixtures were confirmed to still be working by: * using `--ddtrace-patch-all` * adding tags to spans using the `ddspan` fixture * seeing that the `ddtracer` fixture returns the tracer object. ## 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] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [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] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has 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)
brettlangdon
added a commit
that referenced
this issue
May 1, 2024
17 tasks
brettlangdon
added a commit
that referenced
this issue
May 6, 2024
Fixes #8281 This fix resolves an issue when using ``pytest`` + ``gevent`` where the telemetry writer was eager initialized by ``pytest`` entrypoints loading of our plugin causing a potential dead lock. ## Reproduction The specific case that we targeted solving with this PR: ```python # test.py import ddtrace import gevent.monkey gevent.monkey.patch_all() def test_thing(): pass ``` ```shell $ pytest test.py $ pytest -p ddtrace -p ddtrace.pytest_bdd -p ddtrace.pytest_benchmark test.py $ pytest -p no:ddtrace -p no:ddtrace.pytest_bdd -p no:ddtrace.pytest_benchmark test.py ``` Previously would dead-lock, but now will execute without problem. ## Trade offs We no longer get telemetry enabled and sending data just from `import ddtrace`. Moving telemetry writer starting to `ddtrace.bootstrap.preload` will mean that any unhandled exceptions that occur before the telemetry writer can be started will not get reported. ## 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] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [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] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has 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)
github-actions bot
pushed a commit
that referenced
this issue
May 6, 2024
Fixes #8281 This fix resolves an issue when using ``pytest`` + ``gevent`` where the telemetry writer was eager initialized by ``pytest`` entrypoints loading of our plugin causing a potential dead lock. ## Reproduction The specific case that we targeted solving with this PR: ```python # test.py import ddtrace import gevent.monkey gevent.monkey.patch_all() def test_thing(): pass ``` ```shell $ pytest test.py $ pytest -p ddtrace -p ddtrace.pytest_bdd -p ddtrace.pytest_benchmark test.py $ pytest -p no:ddtrace -p no:ddtrace.pytest_bdd -p no:ddtrace.pytest_benchmark test.py ``` Previously would dead-lock, but now will execute without problem. ## Trade offs We no longer get telemetry enabled and sending data just from `import ddtrace`. Moving telemetry writer starting to `ddtrace.bootstrap.preload` will mean that any unhandled exceptions that occur before the telemetry writer can be started will not get reported. ## 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] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [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] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has 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) (cherry picked from commit 9e862f9)
17 tasks
brettlangdon
added a commit
that referenced
this issue
May 6, 2024
Fixes #8281 This fix resolves an issue when using ``pytest`` + ``gevent`` where the telemetry writer was eager initialized by ``pytest`` entrypoints loading of our plugin causing a potential dead lock. The specific case that we targeted solving with this PR: ```python import ddtrace import gevent.monkey gevent.monkey.patch_all() def test_thing(): pass ``` ```shell $ pytest test.py $ pytest -p ddtrace -p ddtrace.pytest_bdd -p ddtrace.pytest_benchmark test.py $ pytest -p no:ddtrace -p no:ddtrace.pytest_bdd -p no:ddtrace.pytest_benchmark test.py ``` Previously would dead-lock, but now will execute without problem. We no longer get telemetry enabled and sending data just from `import ddtrace`. Moving telemetry writer starting to `ddtrace.bootstrap.preload` will mean that any unhandled exceptions that occur before the telemetry writer can be started will not get reported. - [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] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [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] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has 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) (cherry picked from commit 9e862f9)
17 tasks
brettlangdon
added a commit
that referenced
this issue
May 6, 2024
Fixes #8281 This fix resolves an issue when using ``pytest`` + ``gevent`` where the telemetry writer was eager initialized by ``pytest`` entrypoints loading of our plugin causing a potential dead lock. ## Reproduction The specific case that we targeted solving with this PR: ```python # test.py import ddtrace import gevent.monkey gevent.monkey.patch_all() def test_thing(): pass ``` ```shell $ pytest test.py $ pytest -p ddtrace -p ddtrace.pytest_bdd -p ddtrace.pytest_benchmark test.py $ pytest -p no:ddtrace -p no:ddtrace.pytest_bdd -p no:ddtrace.pytest_benchmark test.py ``` Previously would dead-lock, but now will execute without problem. ## Trade offs We no longer get telemetry enabled and sending data just from `import ddtrace`. Moving telemetry writer starting to `ddtrace.bootstrap.preload` will mean that any unhandled exceptions that occur before the telemetry writer can be started will not get reported. ## 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] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [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] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has 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) (cherry picked from commit 9e862f9)
|
We have a change from #9141 which appears to fix this, we are trying to get the fix released in v2.8.4 and v2.9.1 (v2.9.0 release is already in progress so needs to be a bug fix release). |
brettlangdon
added a commit
that referenced
this issue
May 7, 2024
Backport 9e862f9 from #9141 to 2.8. Fixes #8281 This fix resolves an issue when using ``pytest`` + ``gevent`` where the telemetry writer was eager initialized by ``pytest`` entrypoints loading of our plugin causing a potential dead lock. The specific case that we targeted solving with this PR: ```python import ddtrace import gevent.monkey gevent.monkey.patch_all() def test_thing(): pass ``` ```shell $ pytest test.py $ pytest -p ddtrace -p ddtrace.pytest_bdd -p ddtrace.pytest_benchmark test.py $ pytest -p no:ddtrace -p no:ddtrace.pytest_bdd -p no:ddtrace.pytest_benchmark test.py ``` Previously would dead-lock, but now will execute without problem. We no longer get telemetry enabled and sending data just from `import ddtrace`. Moving telemetry writer starting to `ddtrace.bootstrap.preload` will mean that any unhandled exceptions that occur before the telemetry writer can be started will not get reported. - [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] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [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] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has 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)
brettlangdon
added a commit
that referenced
this issue
May 7, 2024
Backport 9e862f9 from #9141 to 2.9. Fixes #8281 This fix resolves an issue when using ``pytest`` + ``gevent`` where the telemetry writer was eager initialized by ``pytest`` entrypoints loading of our plugin causing a potential dead lock. ## Reproduction The specific case that we targeted solving with this PR: ```python # test.py import ddtrace import gevent.monkey gevent.monkey.patch_all() def test_thing(): pass ``` ```shell $ pytest test.py $ pytest -p ddtrace -p ddtrace.pytest_bdd -p ddtrace.pytest_benchmark test.py $ pytest -p no:ddtrace -p no:ddtrace.pytest_bdd -p no:ddtrace.pytest_benchmark test.py ``` Previously would dead-lock, but now will execute without problem. ## Trade offs We no longer get telemetry enabled and sending data just from `import ddtrace`. Moving telemetry writer starting to `ddtrace.bootstrap.preload` will mean that any unhandled exceptions that occur before the telemetry writer can be started will not get reported. ## 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] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [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] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has 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: Brett Langdon <brett.langdon@datadoghq.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
code
work great when use pprof:
deadlock when use pytest:
The text was updated successfully, but these errors were encountered: