Skip to content

Commit d8b10f5

Browse files
fix(di): dynamic function discovery fallback [backport 3.10] (#13999)
Backport b55f08f from #13947 to 3.10. We implement a dynamic function discovery fallback when the function-from-code resolution via the GC fails. This can happen if the target application has interacted with the GC, e.g. by freezing it at a time that will prevent the current discovery from being able to resolve the function from the referenced code object. ## Testing Strategy The original issue was reproducible with a local deployment of [synapse](https://github.com/element-hq/synapse). The investigation led to the conclusion that the issue was caused by the way the application interacts with the GC https://github.com/element-hq/synapse/blob/1dc29563c1504a2523e467aa7bef6a7ac05cc60c/synapse/app/_base.py#L623C1-L629C35. Commenting out these lines makes the issue disappear. We have tested the fix against the unmodified application to verify that the proposed fix works. Refs: [DYNIS-28](https://datadoghq.atlassian.net/browse/DYNIS-28) ## 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) [DYNIS-28]: https://datadoghq.atlassian.net/browse/DYNIS-28?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: Gabriele N. Tornetta <[email protected]>
1 parent 5b4ad29 commit d8b10f5

File tree

2 files changed

+41
-3
lines changed

2 files changed

+41
-3
lines changed

ddtrace/debugging/_function/discovery.py

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -326,11 +326,43 @@ def at_line(self, line: int) -> List[FullyNamedFunction]:
326326

327327
return []
328328

329+
def _resolve_pair(self, pair: _FunctionCodePair, fullname: str) -> FullyNamedFunction:
330+
try:
331+
return pair.resolve()
332+
except ValueError as e:
333+
# We failed to lookup the function by its code object via the GC.
334+
# This should not happen, unless the target application is doing
335+
# something unusual with the GC. In this case we try our best to
336+
# look up the function dynamically from the module. If the function
337+
# is decorated, there are chances this might fail.
338+
target = (module := self._module)
339+
part = None
340+
for part in fullname.replace(f"{module.__name__}.", "", 1).split("."):
341+
try:
342+
target = getattr(target, part)
343+
except AttributeError:
344+
raise e
345+
346+
if target is module:
347+
# We didn't move from the module so we don't have a function.
348+
raise e
349+
350+
code = pair.code
351+
assert code is not None # nosec
352+
f = undecorated(cast(FunctionType, target), cast(str, part), Path(code.co_filename).resolve())
353+
if not (isinstance(f, FunctionType) and f.__code__ is code):
354+
raise e
355+
356+
# Cache the lookup result
357+
pair.function = f
358+
359+
return cast(FullyNamedFunction, f)
360+
329361
def by_name(self, qualname: str) -> FullyNamedFunction:
330362
"""Get the function by its qualified name."""
331363
fullname = f"{self._module.__name__}.{qualname}"
332364
try:
333-
return self._fullname_index[fullname].resolve()
365+
return self._resolve_pair(self._fullname_index[fullname], fullname)
334366
except ValueError:
335367
pass
336368
except KeyError:
@@ -342,11 +374,11 @@ def by_name(self, qualname: str) -> FullyNamedFunction:
342374
if qualname == name or qualname.endswith(f".{name}"):
343375
for fcp in list(fcps):
344376
try:
345-
f = fcp.resolve()
377+
f = self._resolve_pair(fcp, fullname)
346378

347379
# We have resolved the function so we can now
348380
# get its full name
349-
self._fullname_index[f"{self._module.__name__}.{f.__qualname__}"] = fcp
381+
self._fullname_index[fullname] = fcp
350382

351383
# We can remove the entry from the name index
352384
fcps.pop(0)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
fixes:
3+
- |
4+
dynamic instrumentation: improve support for function probes with frameworks
5+
and applications that interact with the Python garbage collector (e.g.
6+
synapse).

0 commit comments

Comments
 (0)