Skip to content

Commit b192a5b

Browse files
fix: remove potential circular import psycopg [backport 3.12] (#14340)
Backport bea1bd2 from #14329 to 3.12. Signed-off-by: Juanjo Alvarez <[email protected]>## 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) Signed-off-by: Juanjo Alvarez <[email protected]>Signed-off-by: Juanjo Alvarez <[email protected]>Signed-off-by: Juanjo Alvarez <[email protected]>Signed-off-by: Juanjo Alvarez <[email protected]> Signed-off-by: Juanjo Alvarez <[email protected]> Co-authored-by: Juanjo Alvarez Martinez <[email protected]>
1 parent e9116b8 commit b192a5b

File tree

6 files changed

+80
-13
lines changed

6 files changed

+80
-13
lines changed

ddtrace/contrib/internal/psycopg/patch.py

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,22 @@
2626
from ddtrace.trace import Pin
2727

2828

29-
try:
30-
psycopg_import = import_module("psycopg")
29+
# These will be initialized lazily to avoid circular imports
30+
_original_connect = None
31+
_original_async_connect = None
3132

32-
# must get the original connect class method from the class __dict__ to use later in unpatch
33-
# Python 3.11 and wrapt result in the class method being rebinded as an instance method when
34-
# using unwrap
35-
_original_connect = psycopg_import.Connection.__dict__["connect"]
36-
_original_async_connect = psycopg_import.AsyncConnection.__dict__["connect"]
37-
# AttributeError can happen due to circular imports under certain integration methods
38-
except (ImportError, AttributeError):
39-
pass
33+
34+
def _get_psycopg3_original_methods():
35+
"""Get psycopg3 original method references, avoiding top-level evaluation to avoid circular imports"""
36+
global _original_connect, _original_async_connect
37+
if _original_connect is None or _original_async_connect is None:
38+
try:
39+
psycopg_import = import_module("psycopg")
40+
_original_connect = psycopg_import.Connection.__dict__["connect"]
41+
_original_async_connect = psycopg_import.AsyncConnection.__dict__["connect"]
42+
# AttributeError can happen due to circular imports under certain integration methods
43+
except (ImportError, AttributeError):
44+
pass
4045

4146

4247
def _psycopg_sql_injector(dbm_comment, sql_statement):
@@ -131,6 +136,8 @@ def _patch(psycopg_module):
131136

132137
config.psycopg["_patched_modules"].add(psycopg_module)
133138
else:
139+
_get_psycopg3_original_methods()
140+
134141
_w(psycopg_module, "connect", patched_connect_factory(psycopg_module))
135142
_w(psycopg_module, "Cursor", init_cursor_from_connection_factory(psycopg_module))
136143
_w(psycopg_module, "AsyncCursor", init_cursor_from_connection_factory(psycopg_module))
@@ -162,8 +169,10 @@ def _unpatch(psycopg_module):
162169

163170
# _u throws an attribute error for Python 3.11, no __get__ on the BoundFunctionWrapper
164171
# unlike Python Class Methods which implement __get__
165-
psycopg_module.Connection.connect = _original_connect
166-
psycopg_module.AsyncConnection.connect = _original_async_connect
172+
if _original_connect is not None:
173+
psycopg_module.Connection.connect = _original_connect
174+
if _original_async_connect is not None:
175+
psycopg_module.AsyncConnection.connect = _original_async_connect
167176

168177
pin = Pin.get_from(psycopg_module)
169178
if pin:
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
psycopg: This fix resolves a potential circular import with the psycopg3 contrib.

tests/contrib/psycopg/test_psycopg_patch.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
# script. If you want to make changes to it, you should make sure that you have
33
# removed the ``_generated`` suffix from the file name, to prevent the content
44
# from being overwritten by future re-generations.
5-
65
from ddtrace.contrib.internal.psycopg.patch import get_version
76
from ddtrace.contrib.internal.psycopg.patch import get_versions
87
from ddtrace.contrib.internal.psycopg.patch import patch

tests/contrib/psycopg2/fixtures/__init__.py

Whitespace-only changes.
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#!/usr/bin/env python3
2+
"""
3+
Reproduction script for psycopg circular import issue.
4+
Run with: ddtrace-run python reproduce_psycopg_cyclic_import_error.py
5+
"""
6+
7+
import sys
8+
9+
10+
def clear_psycopg_modules():
11+
"""Clear all psycopg-related modules from sys.modules"""
12+
modules_to_remove = [name for name in list(sys.modules.keys()) if ("psycopg" in name and "ddtrace" not in name)]
13+
for module in modules_to_remove:
14+
del sys.modules[module]
15+
16+
17+
def main():
18+
clear_psycopg_modules()
19+
import psycopg2 # noqa:F401
20+
21+
22+
if __name__ == "__main__":
23+
main()

tests/contrib/psycopg2/test_psycopg_patch.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
# script. If you want to make changes to it, you should make sure that you have
33
# removed the ``_generated`` suffix from the file name, to prevent the content
44
# from being overwritten by future re-generations.
5+
import os
6+
import subprocess
7+
import sys
58

69
from ddtrace.contrib.internal.psycopg.patch import get_version
710
from ddtrace.contrib.internal.psycopg.patch import get_versions
@@ -40,3 +43,32 @@ def test_and_emit_get_version(self):
4043
assert versions.get("psycopg2")
4144
emit_integration_and_version_to_test_agent("psycopg", versions["psycopg2"], "psycopg2")
4245
unpatch()
46+
47+
def test_psycopg_circular_import_fix(self):
48+
fixture_path = os.path.join(os.path.dirname(__file__), "fixtures", "reproduce_psycopg_cyclic_import_error.py")
49+
50+
env = os.environ.copy()
51+
codebase_root = os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(__file__))))
52+
env["PYTHONPATH"] = os.pathsep.join([codebase_root, env.get("PYTHONPATH", "")])
53+
54+
# Run the reproduction script with ddtrace-run
55+
# Note: tried with @run_in_subprocess but that fails to reproduce the error in the affected version
56+
# (v3.10.2) for some reason while running the separate script works.
57+
result = subprocess.run(
58+
[sys.executable, "-m", "ddtrace.commands.ddtrace_run", "python", fixture_path],
59+
capture_output=True,
60+
text=True,
61+
env=env,
62+
timeout=30,
63+
)
64+
65+
full_output = result.stdout + result.stderr
66+
if "failed to enable ddtrace support for psycopg: partially initialized module" in full_output:
67+
self.fail("Circular import issue detected: the original bug is present and should be fixed")
68+
elif "psycopg2" in full_output and "ImportError" in full_output:
69+
self.skipTest("psycopg2 not available for testing")
70+
elif result.returncode != 0:
71+
self.fail(
72+
f"Reproduction script failed unexpectedly: "
73+
f"returncode={result.returncode}, stdout={result.stdout}, stderr={result.stderr}"
74+
)

0 commit comments

Comments
 (0)