Skip to content

Commit e1ea20d

Browse files
authored
chore(iast): fix iast pytest collection error (#15204)
## What Does This Do? This PR continues the work of the following PRs #15175 #15141 Fixes a bug where IAST's `post_preload()` function drops the `inspect` module from `sys.modules`, causing pytest's test collection phase to fail with signature introspection errors ## Motivation When IAST is enabled and `post_preload()` drops the `inspect` module from memory, pytest encounters errors during test collection: ``` ERROR collecting unexpected object <Signature (self, enabled, clieny, client, mock_client, session_user)> in __signature__ attribute Could not determine arguments of <function test_get_user at XXX>: unexpected object <Signature> in __signature__ attribute ``` ### Root Cause The issue occurs because: 1. **Pytest imports `inspect` early** and creates `Signature` objects during its initialization 2. **IAST's `post_preload()` removes `inspect`** from `sys.modules` to avoid Gevent conflicts 3. **Pytest reimports `inspect`** when it continues to use inspect functions 4. **Type/identity checks fail** because the `Signature` class from the old module doesn't match the new one This is a module identity mismatch problem: `old_inspect.Signature` ≠ `new_inspect.Signature` When pytest tries to validate signatures with `isinstance(sig, inspect.Signature)`, it fails because the signature object's class is from the old (deleted) module, but `inspect.Signature` refers to the class from the new (reimported) module. ## Gevent Compatibility Note While the original implementation dropped `inspect` to prevent Gevent conflicts, this created a worse problem by breaking pytest entirely. **Trade-off Decision:** - Pytest compatibility is **critical** for developers (breaks all testing) - Gevent timeout issues (if they occur with `inspect`) are **rare and workaroundable** - Breaking all pytest collection is **not acceptable** If Gevent issues resurface with the `inspect` module, they should be addressed through: 1. Local imports of `inspect` within IAST functions (as originally intended) 2. Documentation for Gevent users 3. Alternative approaches that don't break pytest
1 parent 7c26d49 commit e1ea20d

File tree

2 files changed

+163
-5
lines changed

2 files changed

+163
-5
lines changed

ddtrace/internal/iast/product.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,21 @@
1212
1313
Root Cause:
1414
-----------
15-
IAST relies on modules like `importlib.metadata`, `importlib`, `subprocess`, and `inspect`
15+
IAST relies on modules like `importlib.metadata`, `importlib`, and `subprocess`
1616
which, when loaded at module level, cannot be properly released from memory. This creates
1717
conflicts between the in-memory versions of these modules and Gevent's monkey patching,
1818
leading to sporadic blocking operations that can cause worker timeouts.
1919
2020
2121
Caveat:
22-
Adding incorrect top-level imports (especially `importlib.metadata`, `inspect`, or
23-
`subprocess`) could reintroduce the flaky gevent timeout errors. Always import these
24-
modules locally within functions when needed.
22+
Adding incorrect top-level imports (especially `importlib.metadata` or `subprocess`)
23+
could reintroduce the flaky gevent timeout errors. Always import these modules locally
24+
within functions when needed.
25+
26+
Note on inspect module:
27+
While the `inspect` module can also cause gevent conflicts, we cannot drop it from
28+
sys.modules as it breaks pytest's test collection phase. Pytest creates inspect.Signature
29+
objects that must remain valid throughout its lifecycle.
2530
"""
2631

2732
import sys
@@ -81,7 +86,9 @@ def post_preload():
8186
# These modules must be cleaned up to prevent memory conflicts that
8287
# lead to sporadic worker timeouts in Gevent-based applications
8388
_drop_safe("importlib.metadata") # Used by native C extensions, conflicts with gevent
84-
_drop_safe("inspect") # Used by taint sinks, must be imported locally
89+
# NOTE: We do NOT drop "inspect" here as it breaks pytest's test collection phase.
90+
# Pytest relies on inspect.signature objects remaining consistent across its lifecycle.
91+
# Dropping inspect causes: "unexpected object <Signature> in __signature__ attribute"
8592

8693

8794
def start():
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
"""
2+
Regression test for inspect module drop causing pytest collection errors.
3+
4+
This test verifies that the IAST product module does NOT drop the inspect module,
5+
as doing so breaks pytest's ability to introspect function signatures during test collection.
6+
7+
Issue: When IAST's post_preload() drops the inspect module, pytest's collection
8+
phase fails with errors like:
9+
"unexpected object <Signature (...)> in __signature__ attribute"
10+
11+
This happens because:
12+
1. pytest imports inspect and creates Signature objects
13+
2. IAST drops inspect from sys.modules
14+
3. pytest tries to use the old Signature objects with a fresh inspect module
15+
4. The mismatch causes type/identity checks to fail
16+
"""
17+
import sys
18+
19+
from tests.utils import override_env
20+
21+
22+
def sample_function_with_many_params(
23+
self,
24+
mfa_enabled,
25+
totp_client_response,
26+
client,
27+
mock_terry_client,
28+
session_user,
29+
):
30+
"""Function similar to the one that failed in the bug report."""
31+
pass
32+
33+
34+
class TestInspectModuleDropRegression:
35+
"""Test that IAST does not drop inspect module."""
36+
37+
def test_simulated_inspect_drop_breaks_pytest_collection(self):
38+
"""
39+
This test demonstrates the bug by manually simulating what happens.
40+
41+
It shows that when inspect is dropped and reimported, old Signature
42+
objects become incompatible with the new inspect module - exactly
43+
what causes pytest collection to fail.
44+
"""
45+
import inspect as original_inspect
46+
47+
# Step 1: pytest creates signatures during collection
48+
original_sig = original_inspect.signature(sample_function_with_many_params)
49+
50+
# Step 2: Manually simulate what _drop_safe("inspect") does
51+
del sys.modules["inspect"]
52+
53+
# Step 3: Force reimport (happens when pytest continues to use inspect)
54+
import inspect as new_inspect
55+
56+
# Step 4: Verify inspect was actually reimported (different module object)
57+
assert new_inspect is not original_inspect, "Test setup error: inspect should be reimported"
58+
59+
# Step 5: The bug - isinstance() check fails with mismatched modules
60+
# This is exactly what breaks pytest's _pytest._code.code module
61+
is_valid_signature = isinstance(original_sig, new_inspect.Signature)
62+
63+
# With the bug present, this would be False, breaking pytest
64+
assert not is_valid_signature, (
65+
f"Expected isinstance check to fail after inspect module drop. "
66+
f"Signature type: {type(original_sig)}, "
67+
f"Expected type: {new_inspect.Signature}"
68+
)
69+
70+
def test_iast_post_preload_does_not_drop_inspect(self):
71+
"""
72+
This is the actual regression test for the fix.
73+
74+
It verifies that when IAST's post_preload() is called,
75+
it does NOT drop the inspect module from sys.modules.
76+
77+
If this test fails, it means _drop_safe("inspect") is being called
78+
in post_preload(), which will break pytest.
79+
"""
80+
import inspect # noqa: F401 - imported to ensure it's in sys.modules before test
81+
82+
# Capture the current state before IAST initialization
83+
inspect_id_before = id(sys.modules.get("inspect"))
84+
inspect_module_before = sys.modules.get("inspect")
85+
86+
# Call post_preload directly (this is what happens in production)
87+
# We need to ensure IAST is enabled for post_preload to run its logic
88+
with override_env({"DD_IAST_ENABLED": "true"}):
89+
# Force reload of asm_config to pick up the environment variable
90+
import importlib
91+
92+
from ddtrace.settings import asm
93+
94+
importlib.reload(asm)
95+
96+
from ddtrace.internal.iast import product
97+
98+
# Call post_preload - this should NOT drop inspect
99+
product.post_preload()
100+
101+
# Verify inspect is still in sys.modules
102+
assert "inspect" in sys.modules, (
103+
"CRITICAL: inspect module was removed from sys.modules! "
104+
"This means _drop_safe('inspect') is being called in post_preload(), "
105+
"which breaks pytest test collection."
106+
)
107+
108+
# Verify it's the SAME inspect module (not dropped and reimported)
109+
inspect_id_after = id(sys.modules.get("inspect"))
110+
assert inspect_id_before == inspect_id_after, (
111+
f"CRITICAL: inspect module was dropped and reimported! "
112+
f"Module ID changed from {inspect_id_before} to {inspect_id_after}. "
113+
f"This means _drop_safe('inspect') is being called, breaking pytest."
114+
)
115+
116+
# Double-check by comparing module objects directly
117+
inspect_module_after = sys.modules.get("inspect")
118+
assert inspect_module_before is inspect_module_after, (
119+
"CRITICAL: inspect module identity changed after post_preload(). "
120+
"This indicates _drop_safe('inspect') is being called."
121+
)
122+
123+
def test_inspect_functionality_after_manual_drop(self):
124+
"""
125+
Additional test showing inspect operations work after manual drop.
126+
127+
This confirms the module can be reimported, but signatures created
128+
with the old module become incompatible.
129+
"""
130+
import inspect as original_inspect
131+
132+
# Create signature before drop
133+
sig_before = original_inspect.signature(sample_function_with_many_params)
134+
135+
# Drop and reimport
136+
if "inspect" in sys.modules:
137+
del sys.modules["inspect"]
138+
139+
import inspect as new_inspect
140+
141+
# Create new signature after reimport
142+
sig_after = new_inspect.signature(sample_function_with_many_params)
143+
144+
# Both signatures work individually
145+
assert len(sig_before.parameters) == 6
146+
assert len(sig_after.parameters) == 6
147+
148+
# But they're incompatible due to different module versions
149+
assert type(sig_before) is not type(sig_after)
150+
assert not isinstance(sig_before, new_inspect.Signature)
151+
assert isinstance(sig_after, new_inspect.Signature)

0 commit comments

Comments
 (0)