-
Notifications
You must be signed in to change notification settings - Fork 418
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
Deadlock with pytest
+ddtrace
in py 3.11 that -p no:ddtrace
does not solve
#4845
Comments
Hey @olivierlefloch, Thanks for brining this issue to our attention 😄
I believe
I ran the dd-trace-py gevent tests in circleci with
If you're more comfortable sharing this information privately you can contact our support team here and we can continue this conversation there. |
This worked as expected with Python 3.10.3 and,
I tried downgrading Pytest, but this combination still fails:
It seems |
Hey, Sorry for the late reply. ddtrace v1.3.0 introduced pytest-bdd plugin (feature). This plugin starts the global tracer and can be disabled using Try running pytest with the following command and let us know if this disables the tracer: |
Awesome. Thanks! The plugin no longer loads with,
I still get a "failed to send traces to Datadog Agent" error for tests that cover functions wrapped with |
I too have seen these "failed to send traces to Datadog Agent" for some time. The only thing that fixes it is to have the environment variable ( Related: #4179 |
I can confirm this is still an issue with Python Also an issue in |
This is still an issue with Python The specific error we are seeing: Unable to export profile: ddtrace.profiling.exporter.ExportError: HTTP upload request failed: [Errno 111] Connection refused. Ignoring. |
I work on the @matroscoe , when you say "this is still an issue", you're referring to the deadlock, or to the "failed to send traces" log entries (or both)? |
@romainkomorndatadog I am specifically referring to the issue with the datadog system still being enabled and attempting to send traces with both the |
@matroscoe , since you're not using (or you're disabling) the In the I don't have a repro case yet but I suspect that likely won't resolve the issue if |
@romainkomorndatadog to be clear I think you are right. We have included Something like a I think at least my companies use case would be satisfied if we could be provided with an option to specifically disable the upload attempt. |
Hi @matroscoe could you please clarify whether or not setting If that's the case, that's what we'd currently recommend. We can add your request for a plugin flag (e.g. |
@romainkomorndatadog To chime in here - outside of the logging concerns raised by others - I cannot seem to disable the ddtrace pytest plugin at all using the solutions recommended here. Passing in
This is on Python 3.12.1, ddtrace 2.3.2, and pytest 7.4.3. I'm surprised to see ddtrace within the runtime of my tests by default. It's not a behavior I opted into or was clear to me when I installed ddtrace for application tracing. |
@jlucas91 , I think there are two (maybe three) separate issues here: First, I did notice today that it looks like we have a problem with Python 3.10+ that I just created #8039 to look into. It looks like that's what you're reporting in your stacktrace? Next, just to be clear (and I do realize this is splitting hairs a bit),
shows that the And lastly, the issue that remains is that the plugin file does I think it may be appropriate in this case for us to lazy-load the module (ie: only |
…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)
Given the recent changes made for this and #8281 (comment) , I'm going to mark this as closed as well. Since there have been multiple different (but related) issues (gevent deadlocks, logging, traces being sent, etc) discussed in both this and #8281 , I'd really appreciate it if, if someone is tempted to reopen this issue, they could instead open a new one with a single topic and details. |
…s to try-except blocks (#10196) There are two changes in this commit: 1- remove "sub plugin" strategy introduced in #8999 The sub-plugin strategy was originally implemented to delay the import of `ddtrace`, however that import is (in the current scenario) inevitable because the `pyproject.toml` entry-points that tell `pytest` where to find plugins already import `ddtrace`, and the original motivation of the change in (#8281 and #4845) was instead addressed by #9141 . The motivation to revert this change is that some hooks (eg: `pytest_load_inital_conftests`) added by the class-based plugins do not execute when the plugin is loaded in the `pytest_configure()` hook in the `plugin.py` file itself. All the methods in the plugin classes use the `@staticmethod` decorator and do not benefit from being in a class. This change allows the versioned plugin files to implement hooks by virtue of being imported into the original plugin module. 2- move v2 functions to try-except blocks Since CI Visibility should never affect `pytest`'s execution (or, at the very least, should never prevent it from running to completion), the hooks themselves should never raise exceptions. With this change, some functions simply wrap the entire body in a try/except block, while some others define a separate function that is then called in a try/except block. A decorator was not used for this purpose because some hooks yield, while others don't, and logging relevant errors is also a little simpler. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
Summary of problem
When adding Python 3.11 support to an existing codebase that uses
ddtrace
, we encounter deadlocks at the end of the test suite (afterpytest
reports its results) in CI. This does not occur on previous versions of Python.Furthermore, disabling the
ddtrace
plugin using-p no:ddtrace
does not resolve the issue.Specifying
DD_TRACE_ENABLED=false pytest …
does work around the issue, however.Which version of dd-trace-py are you using?
1.6.3
Which version of pip are you using?
22.3.1
Which libraries and their versions are you using?
How can we reproduce your problem?
I have not (yet?) put together a minimal reproduction case as there seem to potentially be several issues at play here ; happy to help drill down as appropriate.
What is the result that you get?
Github Actions times out after our self imposted timeout.
What is the result that you expected?
ddtrace
plugin enabled, on Python 3.11,-p no:ddtrace
fully disables the plugin, including starting a global tracer.Related issues
The text was updated successfully, but these errors were encountered: