Skip to content

Commit ef12149

Browse files
authored
fix(di): interaction with external wrapping contexts [backport #13586 to 2.21] (#13636)
Backport c02a068 from #13586 to 2.21. We fix an incompatibility within the interaction between Dynamic Instrumentation and external wrapping context (e.g. Code Origin). This was due to a loss of link between a function and its original code object due to a wrapping context being wrapped on a function not retrieved via the function discovery machinery. We make sure that any wrapping via wrapping context preserves the link between the function and its original code object. With the universal wrapping context mechanism, we don't have to worry about potential multiple wrapping, as bytecode is modified only once. ## 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)
1 parent 7fd3c13 commit ef12149

File tree

7 files changed

+108
-14
lines changed

7 files changed

+108
-14
lines changed

ddtrace/debugging/_function/discovery.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,10 @@ def __init__(self, code: Optional[CodeType] = None, function: Optional[FunctionT
142142
self.code = function.__code__ if function is not None else code
143143

144144
def resolve(self) -> FullyNamedFunction:
145+
if self.code is None:
146+
msg = "Cannot resolve pair with no code object"
147+
raise ValueError(msg)
148+
145149
if self.function is not None:
146150
return cast(FullyNamedFunction, self.function)
147151

@@ -159,6 +163,17 @@ def resolve(self) -> FullyNamedFunction:
159163
raise ValueError(msg)
160164

161165
self.function = _f = functions[0]
166+
try:
167+
# We try to update the code object to the one currently in use with
168+
# the function. This is not necessarily the code object that was
169+
# stored in the pair, which generally comes from the code objects
170+
# generated by module compilation. The benefit of doing this is that
171+
# we can relinquish a reference to the original code object so that
172+
# it can be garbage collected if needed.
173+
self.code = _f.__code__
174+
except AttributeError:
175+
pass
176+
162177
f = cast(FullyNamedFunction, _f)
163178
f.__fullname__ = f"{f.__module__}.{f.__qualname__}"
164179

@@ -245,13 +260,13 @@ def __init__(self, module: ModuleType) -> None:
245260
return
246261

247262
self._module = module
248-
self._fullname_index = _collect_functions(module)
249263
if PYTHON_VERSION_INFO < (3, 11):
250264
self._name_index: Dict[str, List[_FunctionCodePair]] = defaultdict(list)
251265
self._cached: Dict[int, List[FullyNamedFunction]] = {}
252266

253267
# Create the line to function mapping
254268
if hasattr(module, "__dd_code__"):
269+
self._fullname_index = {}
255270
for code in module.__dd_code__:
256271
fcp = _FunctionCodePair(code=code)
257272

@@ -266,6 +281,7 @@ def __init__(self, module: ModuleType) -> None:
266281
for lineno in linenos(code):
267282
self[lineno].append(fcp)
268283
else:
284+
self._fullname_index = _collect_functions(module)
269285
# If the module was already loaded we don't have its code object
270286
seen_functions = set()
271287
for _, fcp in self._fullname_index.items():
@@ -351,7 +367,7 @@ def from_module(cls, module: ModuleType) -> "FunctionDiscovery":
351367
information on the module object itself. Subsequent calls will
352368
return the cached information.
353369
"""
354-
# Cache the function tree on the module
370+
# Cache the function discovery on the module
355371
try:
356372
return module.__function_discovery__
357373
except AttributeError:

ddtrace/internal/utils/inspection.py

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from types import FunctionType
99
from typing import Iterator
1010
from typing import List
11+
from typing import MutableMapping
1112
from typing import Set
1213
from typing import cast
1314

@@ -126,8 +127,45 @@ def collect_code_objects(code: CodeType) -> Iterator[CodeType]:
126127
q.append(new_code)
127128

128129

130+
_CODE_TO_ORIGINAL_FUNCTION_MAPPING: MutableMapping[CodeType, FunctionType] = dict()
131+
132+
133+
def link_function_to_code(code: CodeType, function: FunctionType) -> None:
134+
"""
135+
Link a function to a code object. This is used to speed up the search for
136+
the original function from a code object.
137+
"""
138+
global _CODE_TO_ORIGINAL_FUNCTION_MAPPING
139+
140+
_CODE_TO_ORIGINAL_FUNCTION_MAPPING[code] = function
141+
142+
129143
@lru_cache(maxsize=(1 << 14)) # 16k entries
130-
def functions_for_code(code: CodeType) -> List[FunctionType]:
144+
def _functions_for_code_gc(code: CodeType) -> List[FunctionType]:
131145
import gc
132146

133147
return [_ for _ in gc.get_referrers(code) if isinstance(_, FunctionType) and _.__code__ is code]
148+
149+
150+
def functions_for_code(code: CodeType) -> List[FunctionType]:
151+
global _CODE_TO_ORIGINAL_FUNCTION_MAPPING
152+
153+
try:
154+
# Try to get the function from the original code-to-function mapping
155+
return [_CODE_TO_ORIGINAL_FUNCTION_MAPPING[code]]
156+
except KeyError:
157+
# If the code is not in the mapping, we fall back to the garbage
158+
# collector
159+
return _functions_for_code_gc(code)
160+
161+
162+
def clear():
163+
"""Clear the inspection state.
164+
165+
This should be called when modules are reloaded to ensure that the mappings
166+
stay relevant.
167+
"""
168+
global _CODE_TO_ORIGINAL_FUNCTION_MAPPING
169+
170+
_functions_for_code_gc.cache_clear()
171+
_CODE_TO_ORIGINAL_FUNCTION_MAPPING.clear()

ddtrace/internal/wrapping/context.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from bytecode import Bytecode
1818

1919
from ddtrace.internal.assembly import Assembly
20+
from ddtrace.internal.utils.inspection import link_function_to_code
2021

2122

2223
T = t.TypeVar("T")
@@ -562,7 +563,10 @@ def wrap(self) -> None:
562563
# Mark the function as wrapped by a wrapping context
563564
t.cast(ContextWrappedFunction, f).__dd_context_wrapped__ = self
564565

565-
# Replace the function code with the wrapped code
566+
# Replace the function code with the wrapped code. We also link
567+
# the function to its original code object so that we can retrieve
568+
# it later if required.
569+
link_function_to_code(f.__code__, f)
566570
f.__code__ = bc.to_code()
567571

568572
def unwrap(self) -> None:
@@ -688,7 +692,10 @@ def wrap(self) -> None:
688692
# Mark the function as wrapped by a wrapping context
689693
t.cast(ContextWrappedFunction, f).__dd_context_wrapped__ = self
690694

691-
# Replace the function code with the wrapped code
695+
# Replace the function code with the wrapped code. We also link
696+
# the function to its original code object so that we can retrieve
697+
# it later if required.
698+
link_function_to_code(f.__code__, f)
692699
f.__code__ = bc.to_code()
693700

694701
def unwrap(self) -> None:
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
dynamic instrumentation: fixed an incompatibility issue with code origin
5+
that caused line probes on the entry point functions to fail to instrument.

tests/debugging/conftest.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@
22

33
import pytest
44

5-
from ddtrace.internal.utils.inspection import functions_for_code
5+
from ddtrace.internal.utils import inspection
66

77

88
@pytest.fixture
99
def stuff():
10-
functions_for_code.cache_clear()
10+
inspection.clear()
11+
1112
was_loaded = False
1213
if "tests.submod.stuff" in sys.modules:
1314
was_loaded = True

tests/debugging/function/test_discovery.py

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,21 +101,27 @@ def test_function_mangled(stuff_discovery):
101101
assert original_method is original_func
102102

103103

104-
def test_discovery_after_external_wrapping(stuff):
104+
@pytest.mark.subprocess
105+
def test_discovery_after_external_wrapping():
105106
import wrapt
106107

108+
from ddtrace.debugging._debugger import DebuggerModuleWatchdog
109+
from ddtrace.debugging._function.discovery import FunctionDiscovery
110+
111+
DebuggerModuleWatchdog.install()
112+
113+
import tests.submod.stuff as stuff
114+
107115
def wrapper(wrapped, inst, args, kwargs):
108116
pass
109117

110-
original_function = stuff.Stuff.instancestuff
111-
112118
wrapt.wrap_function_wrapper(stuff, "Stuff.instancestuff", wrapper)
113119
assert isinstance(stuff.Stuff.instancestuff, (wrapt.BoundFunctionWrapper, wrapt.FunctionWrapper))
114120

115121
code = stuff.Stuff.instancestuff.__code__
116-
f, *_ = FunctionDiscovery(stuff).at_line(36)
122+
f, *_ = FunctionDiscovery.from_module(stuff).at_line(36)
117123

118-
assert f is original_function or isinstance(f, (wrapt.BoundFunctionWrapper, wrapt.FunctionWrapper)), f
124+
assert f is stuff.Stuff.instancestuff.__wrapped__, (f, stuff.Stuff.instancestuff.__wrapped__)
119125
assert f.__code__ is code
120126

121127

@@ -139,3 +145,24 @@ def transform(self, code, module):
139145
assert home.__qualname__ == "home"
140146

141147
DiscoveryModuleWatchdog.uninstall()
148+
149+
150+
@pytest.mark.subprocess
151+
def test_discovery_after_external_wrapping_context():
152+
from ddtrace.debugging._debugger import DebuggerModuleWatchdog
153+
from ddtrace.debugging._function.discovery import FunctionDiscovery
154+
from ddtrace.internal.module import origin
155+
from ddtrace.internal.wrapping.context import WrappingContext
156+
157+
DebuggerModuleWatchdog.install()
158+
159+
import tests.submod.stuff as stuff
160+
161+
f = stuff.modulestuff
162+
163+
def hook(module):
164+
assert FunctionDiscovery.from_module(module).at_line(f.__code__.co_firstlineno + 1)
165+
166+
WrappingContext(f).wrap() # type: ignore
167+
168+
DebuggerModuleWatchdog.register_origin_hook(origin(stuff), hook) # type: ignore

tests/debugging/test_debugger_span_decoration.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from ddtrace.debugging._probe.model import SpanDecorationTag
88
from ddtrace.debugging._probe.model import SpanDecorationTargetSpan
99
from ddtrace.debugging._signal.model import EvaluationError
10-
from ddtrace.internal.utils.inspection import functions_for_code
10+
from ddtrace.internal.utils import inspection
1111
from tests.debugging.mocking import debugger
1212
from tests.debugging.utils import create_span_decoration_function_probe
1313
from tests.debugging.utils import create_span_decoration_line_probe
@@ -22,7 +22,7 @@ def setUp(self):
2222

2323
import tests.submod.traced_stuff as ts
2424

25-
functions_for_code.cache_clear()
25+
inspection.clear()
2626

2727
self.traced_stuff = ts
2828
self.backup_tracer = ddtrace.tracer

0 commit comments

Comments
 (0)