Skip to content

Commit c02a068

Browse files
authored
fix(di): interaction with external wrapping contexts (#13586)
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 636ffa1 commit c02a068

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")
@@ -560,7 +561,10 @@ def wrap(self) -> None:
560561
# Mark the function as wrapped by a wrapping context
561562
t.cast(ContextWrappedFunction, f).__dd_context_wrapped__ = self
562563

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

566570
def unwrap(self) -> None:
@@ -686,7 +690,10 @@ def wrap(self) -> None:
686690
# Mark the function as wrapped by a wrapping context
687691
t.cast(ContextWrappedFunction, f).__dd_context_wrapped__ = self
688692

689-
# Replace the function code with the wrapped code
693+
# Replace the function code with the wrapped code. We also link
694+
# the function to its original code object so that we can retrieve
695+
# it later if required.
696+
link_function_to_code(f.__code__, f)
690697
f.__code__ = bc.to_code()
691698

692699
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 = "tests.submod.stuff" in sys.modules
1213
if was_loaded:
1314
del sys.modules["tests.submod.stuff"]

tests/debugging/function/test_discovery.py

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

118118

119-
def test_discovery_after_external_wrapping(stuff):
119+
@pytest.mark.subprocess
120+
def test_discovery_after_external_wrapping():
120121
import wrapt
121122

123+
from ddtrace.debugging._debugger import DebuggerModuleWatchdog
124+
from ddtrace.debugging._function.discovery import FunctionDiscovery
125+
126+
DebuggerModuleWatchdog.install()
127+
128+
import tests.submod.stuff as stuff
129+
122130
def wrapper(wrapped, inst, args, kwargs):
123131
pass
124132

125-
original_function = stuff.Stuff.instancestuff
126-
127133
wrapt.wrap_function_wrapper(stuff, "Stuff.instancestuff", wrapper)
128134
assert isinstance(stuff.Stuff.instancestuff, (wrapt.BoundFunctionWrapper, wrapt.FunctionWrapper))
129135

130136
code = stuff.Stuff.instancestuff.__code__
131-
f, *_ = FunctionDiscovery(stuff).at_line(36)
137+
f, *_ = FunctionDiscovery.from_module(stuff).at_line(36)
132138

133-
assert f is original_function or isinstance(f, (wrapt.BoundFunctionWrapper, wrapt.FunctionWrapper)), f
139+
assert f is stuff.Stuff.instancestuff.__wrapped__, (f, stuff.Stuff.instancestuff.__wrapped__)
134140
assert f.__code__ is code
135141

136142

@@ -156,3 +162,24 @@ def transform(self, code, module):
156162

157163
finally:
158164
DiscoveryModuleWatchdog.uninstall()
165+
166+
167+
@pytest.mark.subprocess
168+
def test_discovery_after_external_wrapping_context():
169+
from ddtrace.debugging._debugger import DebuggerModuleWatchdog
170+
from ddtrace.debugging._function.discovery import FunctionDiscovery
171+
from ddtrace.internal.module import origin
172+
from ddtrace.internal.wrapping.context import WrappingContext
173+
174+
DebuggerModuleWatchdog.install()
175+
176+
import tests.submod.stuff as stuff
177+
178+
f = stuff.modulestuff
179+
180+
def hook(module):
181+
assert FunctionDiscovery.from_module(module).at_line(f.__code__.co_firstlineno + 1)
182+
183+
WrappingContext(f).wrap() # type: ignore
184+
185+
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)