Skip to content

Commit bbc788a

Browse files
liustvesrprash
liustve
and
srprash
authored
fix(debugger): patch Django URLPattern.callback so DI breakpoints fire (#776)
## Problem Dynamic Instrumentation (DI) breakpoints set on Django view functions silently never fire. Django's `path()` / `re_path()` / `include()` constructors store the view function as a **direct object reference** on `URLPattern.callback` at module-import time (`django/urls/resolvers.py`). When DI replaces the function on its defining module via `setattr(views_module, "my_view", wrapper)`, the URL resolver tree still holds the pre-wrap reference. At request time, Django's `URLResolver.resolve()` returns the stale callback and the original (unwrapped) function executes — no snapshot, no instrumentation. This is the same failure mode the existing `_patch_flask_view_functions` patcher solves for Flask's `app.view_functions` dict, just for a different framework's internal cache. ## Fix Add a parallel Django code path mirroring the Flask patcher's structure: - **`_patch_django_url_patterns`** (entry point): import-guarded against missing Django; reaches the active resolver tree via `django.urls.get_resolver(None)` (the authoritative root resolver Django uses at request time), with a belt-and-suspenders module scan for unconfigured / test-only contexts. - **`_patch_single_resolver`**: recursive walker through `URLResolver.url_patterns` to handle `include()`-nested children. `visited: set[int]` of `id(resolver)` for cycle protection. - **`_maybe_patch_pattern`**: atomic mutator. Identity match first, then `__name__` + `__module__` fallback (handles `functools.wraps`-preserving decorators like `@login_required` / `@csrf_exempt` and OTel auto-instrumentation wrappers). Wired into `_patch_framework_references` after the Flask call. A failure in either framework patcher is logged at DEBUG and never blocks the other. ## Why this discovery strategy The common Django layout is **cross-module**: `urlpatterns` lives in a dedicated `urls.py` while views live in `views.py`. The `module` arg the wrapper passes to `_patch_framework_references` is the **views** module, which has no `urlpatterns`. Flask's "scan `dir(module)` only" pattern is insufficient — we have to reach into Django's global resolver via `get_resolver(None)`. The defensive module scan covers configured-but-not-yet-resolved cases. ## Class-based views — no special handling needed Django's `View.dispatch()` re-resolves methods via `getattr(self, method_name)` at request time. The closure returned by `as_view()` captures `cls` once, but method lookup is dynamic on every request. Patching `MyView.get` via the existing class-method `setattr` path is sufficient — the closure's stale `cls` cell is irrelevant. The patcher does NOT mutate `as_view()` closure cells; the closure has `__name__ == "view"` (not the user's view name), so it's naturally excluded from the name+module fallback. ## Tests `tests/.../test_function_wrapper_django.py` (NEW, 15 tests, mirrors Flask test layout): - Identity match - Name+module fallback (handles `@functools.wraps`-style wrapping) - No Django installed (no-op) - No urlpatterns in module - Function not in url patterns - **`include()`-nested resolvers** (recursive descent) - **`get_resolver(None)` cross-module discovery** (the common Django layout) - **Decorator-wrapped view** (`functools.wraps`-preserving) - **CBV unaffected** (locks in the design decision: closure is not mutated) - Resolver-traversal-error swallowed - `get_resolver` failure swallowed (module-scan fallback) - `_replace_function_in_module` integration - `restore_function` integration - Dispatcher delegation - Exception swallowing `tests/.../test_function_wrapper_branches.py` — added `TestPatchDjangoUrlPatternsErrors` (7 tests) for defensive `except` branches: import-guard, `get_resolver` failure, `dir(module)` failure, per-attribute `getattr` failure, malformed resolver, `callback` getter raises, `callback` setter raises. ## Validation | Python | Tests passing | New tests | |---|---|---| | 3.9 | 544 passed, 29 skipped | +22 (15 Django + 7 branches) | | 3.10 | 544 passed, 29 skipped | +22 | | 3.11 | 544 passed, 29 skipped | +22 | | 3.12 | 548 passed, 25 skipped | +22 | - `black` / `isort` / `flake8` / `pylint` (10.00/10) / `codespell`: clean - No changes to existing Flask patcher; existing Flask tests unchanged ## Test plan - [x] Unit tests pass on Python 3.9 / 3.10 / 3.11 / 3.12 - [x] Linters clean (black, isort, flake8, pylint, codespell) - [ ] E2E: configure DI probe on a Django view in the sample app, confirm snapshot lands in CW Logs --------- Co-authored-by: Prashant Srivastava <srprash@amazon.com>
1 parent eab5ee3 commit bbc788a

17 files changed

Lines changed: 1797 additions & 5 deletions

File tree

‎aws-opentelemetry-distro/src/amazon/opentelemetry/distro/debugger/_function_wrapper.py‎

Lines changed: 224 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -796,21 +796,32 @@ def _patch_framework_references(module, original_func: Callable, new_func: Calla
796796
797797
Currently supports:
798798
- Flask: patches app.view_functions entries that reference the original function
799+
- Django: patches URLPattern.callback entries (incl. include()-nested resolvers)
800+
that reference the original function
799801
- FastAPI: patches APIRoute.endpoint / APIRoute.dependant.call on app.router.routes
800802
801803
Args:
802804
module: The module object containing the function
803805
original_func: The original function that was replaced
804806
new_func: The new wrapper function
805807
"""
808+
# Each framework patcher runs in its own try/except so a failure in one
809+
# never prevents the others from running.
806810
try:
807811
# Flask: patch view_functions on any Flask app instances in the module
808812
FunctionWrapper._patch_flask_view_functions(module, original_func, new_func)
813+
except Exception as exc: # pylint: disable=broad-exception-caught
814+
logger.debug("flask reference patching encountered an error: %s", exc)
815+
try:
816+
# Django: patch URLPattern.callback in the resolver tree
817+
FunctionWrapper._patch_django_url_patterns(module, original_func, new_func)
818+
except Exception as exc: # pylint: disable=broad-exception-caught
819+
logger.debug("django reference patching encountered an error: %s", exc)
820+
try:
809821
# FastAPI: patch route table entries on any FastAPI app instances in the module
810822
FunctionWrapper._patch_fastapi_routes(module, original_func, new_func)
811-
except Exception as exc:
812-
# Never let framework patching failures break instrumentation
813-
logger.debug("Framework reference patching encountered an error: %s", exc)
823+
except Exception as exc: # pylint: disable=broad-exception-caught
824+
logger.debug("fastapi reference patching encountered an error: %s", exc)
814825

815826
@staticmethod
816827
def _patch_flask_view_functions(module, original_func: Callable, new_func: Callable) -> None:
@@ -845,11 +856,11 @@ def _patch_flask_view_functions(module, original_func: Callable, new_func: Calla
845856
attr, attr_name, original_func, new_func, func_name, func_module
846857
)
847858
except Exception as exc:
848-
logger.warning("Error checking module attribute '%s' for Flask app: %s", attr_name, exc)
859+
logger.debug("Error checking module attribute '%s' for Flask app: %s", attr_name, exc)
849860
continue
850861

851862
except Exception as exc:
852-
logger.warning("Error patching Flask view_functions: %s", exc)
863+
logger.debug("Error patching Flask view_functions: %s", exc)
853864

854865
@staticmethod
855866
def _patch_single_flask_app(flask_app, app_name, original_func, new_func, func_name, func_module=None):
@@ -903,6 +914,214 @@ def _matches(vf):
903914
view_functions[ep] = new_func
904915
logger.debug("Patched Flask view_functions[%s] by name+module match", ep)
905916

917+
@staticmethod
918+
def _patch_django_url_patterns( # pylint: disable=too-many-branches,too-many-nested-blocks
919+
module, original_func: Callable, new_func: Callable
920+
) -> None:
921+
"""
922+
Patch Django URLPattern.callback entries that reference the original function.
923+
924+
Django's ``path('foo/', views.foo)`` stores ``views.foo`` directly on
925+
``URLPattern.callback`` at module-import time. When DI replaces
926+
``views.foo`` on its module, the URL resolver tree still holds the
927+
pre-wrap reference, so requests bypass the DI wrapper. This method
928+
finds and patches those references.
929+
930+
Discovery walks Django's authoritative resolver via ``get_resolver(None)``
931+
(the same root resolver Django uses at request time). It also scans the
932+
passed-in module for top-level ``urlpatterns`` / ``URLPattern`` /
933+
``URLResolver`` attributes as a defensive belt-and-suspenders pass for
934+
unconfigured / test-only contexts.
935+
936+
Args:
937+
module: The module object that may contain Django URL config
938+
original_func: The original function to find on URLPattern.callback
939+
new_func: The new wrapper function to replace it with
940+
"""
941+
try:
942+
try:
943+
# pylint: disable=import-outside-toplevel
944+
from django.urls import URLPattern, URLResolver, get_resolver
945+
except ImportError:
946+
return # Django not installed, nothing to patch
947+
948+
func_name = getattr(original_func, "__name__", "unknown")
949+
func_module = getattr(original_func, "__module__", None)
950+
# Track resolver and pattern ids separately. The same URLPattern
951+
# can be reached from both the get_resolver(None) walk and the
952+
# module-scan fallback; pattern dedup avoids a no-op double write.
953+
visited: set = set()
954+
visited_patterns: set = set()
955+
956+
# Authoritative source: Django's root resolver (the same instance the
957+
# framework dispatches through at request time).
958+
try:
959+
root_resolver = get_resolver(None)
960+
except Exception as exc: # pylint: disable=broad-exception-caught
961+
logger.debug("Django get_resolver(None) failed (likely unconfigured): %s", exc)
962+
root_resolver = None
963+
if root_resolver is not None:
964+
FunctionWrapper._patch_single_resolver(
965+
root_resolver,
966+
"<root>",
967+
original_func,
968+
new_func,
969+
func_name,
970+
func_module,
971+
URLPattern,
972+
URLResolver,
973+
visited,
974+
visited_patterns,
975+
)
976+
977+
# Belt-and-suspenders: scan the passed-in module for top-level
978+
# patterns/resolvers (covers configured-but-not-yet-resolved cases).
979+
for attr_name in dir(module):
980+
try:
981+
attr = getattr(module, attr_name, None)
982+
except Exception as exc: # pylint: disable=broad-exception-caught
983+
logger.debug("Error reading module attribute '%s' for Django scan: %s", attr_name, exc)
984+
continue
985+
try:
986+
if isinstance(attr, URLPattern):
987+
FunctionWrapper._maybe_patch_pattern(
988+
attr, original_func, new_func, func_name, func_module, visited_patterns
989+
)
990+
elif isinstance(attr, URLResolver):
991+
FunctionWrapper._patch_single_resolver(
992+
attr,
993+
attr_name,
994+
original_func,
995+
new_func,
996+
func_name,
997+
func_module,
998+
URLPattern,
999+
URLResolver,
1000+
visited,
1001+
visited_patterns,
1002+
)
1003+
elif isinstance(attr, (list, tuple)) and attr_name == "urlpatterns":
1004+
for item in attr:
1005+
if isinstance(item, URLPattern):
1006+
FunctionWrapper._maybe_patch_pattern(
1007+
item, original_func, new_func, func_name, func_module, visited_patterns
1008+
)
1009+
elif isinstance(item, URLResolver):
1010+
FunctionWrapper._patch_single_resolver(
1011+
item,
1012+
attr_name,
1013+
original_func,
1014+
new_func,
1015+
func_name,
1016+
func_module,
1017+
URLPattern,
1018+
URLResolver,
1019+
visited,
1020+
visited_patterns,
1021+
)
1022+
except Exception as exc: # pylint: disable=broad-exception-caught
1023+
logger.debug("Error checking module attribute '%s' for Django patterns: %s", attr_name, exc)
1024+
continue
1025+
1026+
except Exception as exc: # pylint: disable=broad-exception-caught
1027+
logger.debug("Error patching Django URLPattern.callback: %s", exc)
1028+
1029+
@staticmethod
1030+
def _patch_single_resolver( # pylint: disable=too-many-arguments
1031+
resolver,
1032+
resolver_label: str,
1033+
original_func: Callable,
1034+
new_func: Callable,
1035+
func_name: str,
1036+
func_module,
1037+
url_pattern_cls,
1038+
url_resolver_cls,
1039+
visited: set,
1040+
visited_patterns: set,
1041+
) -> None:
1042+
"""Recursively walk a URLResolver, patching matching URLPattern.callback
1043+
entries and descending through ``include()``-nested children. ``visited``
1044+
protects against resolver cycles (``id(resolver)``); ``visited_patterns``
1045+
prevents double-patching the same URLPattern when reachable from both
1046+
the get_resolver(None) walk and the module-scan fallback."""
1047+
if id(resolver) in visited:
1048+
return
1049+
visited.add(id(resolver))
1050+
try:
1051+
patterns = resolver.url_patterns # @cached_property; may raise ImproperlyConfigured
1052+
except Exception as exc: # pylint: disable=broad-exception-caught
1053+
logger.debug("Error reading url_patterns on resolver '%s': %s", resolver_label, exc)
1054+
return
1055+
1056+
for item in patterns:
1057+
try:
1058+
if isinstance(item, url_pattern_cls):
1059+
FunctionWrapper._maybe_patch_pattern(
1060+
item, original_func, new_func, func_name, func_module, visited_patterns
1061+
)
1062+
elif isinstance(item, url_resolver_cls):
1063+
FunctionWrapper._patch_single_resolver(
1064+
item,
1065+
resolver_label,
1066+
original_func,
1067+
new_func,
1068+
func_name,
1069+
func_module,
1070+
url_pattern_cls,
1071+
url_resolver_cls,
1072+
visited,
1073+
visited_patterns,
1074+
)
1075+
except Exception as exc: # pylint: disable=broad-exception-caught
1076+
logger.debug("Error walking pattern in resolver '%s': %s", resolver_label, exc)
1077+
continue
1078+
1079+
@staticmethod
1080+
def _maybe_patch_pattern( # pylint: disable=too-many-arguments
1081+
pattern,
1082+
original_func: Callable,
1083+
new_func: Callable,
1084+
func_name: str,
1085+
func_module,
1086+
visited_patterns: set,
1087+
) -> None:
1088+
"""Patch a single URLPattern.callback if it matches by identity, or by
1089+
``__name__`` + ``__module__`` (handles ``functools.wraps``-preserving
1090+
decorators like ``@login_required`` and OTel auto-instrumentation
1091+
wrappers). Skips patterns already visited so multiple discovery roots
1092+
don't double-patch."""
1093+
if id(pattern) in visited_patterns:
1094+
return
1095+
visited_patterns.add(id(pattern))
1096+
try:
1097+
cb = getattr(pattern, "callback", None)
1098+
except Exception as exc: # pylint: disable=broad-exception-caught
1099+
logger.debug("Error reading callback on URLPattern: %s", exc)
1100+
return
1101+
if cb is None:
1102+
return
1103+
1104+
if cb is original_func:
1105+
try:
1106+
pattern.callback = new_func
1107+
logger.debug("Patched Django URLPattern.callback (identity) for %s", func_name)
1108+
except Exception as exc: # pylint: disable=broad-exception-caught
1109+
logger.debug("Error setting URLPattern.callback (identity): %s", exc)
1110+
return
1111+
1112+
# Identity miss — try name+module fallback. CBV closures created by
1113+
# View.as_view() have __name__ == 'view', not the user's view name,
1114+
# so they're naturally excluded from this match.
1115+
if getattr(cb, "__name__", None) != func_name:
1116+
return
1117+
if func_module is not None and getattr(cb, "__module__", None) != func_module:
1118+
return
1119+
try:
1120+
pattern.callback = new_func
1121+
logger.debug("Patched Django URLPattern.callback (name+module) for %s", func_name)
1122+
except Exception as exc: # pylint: disable=broad-exception-caught
1123+
logger.debug("Error setting URLPattern.callback (name+module): %s", exc)
1124+
9061125
@staticmethod
9071126
def _patch_fastapi_routes(module, original_func: Callable, new_func: Callable) -> None:
9081127
"""

0 commit comments

Comments
 (0)