Skip to content

Commit f2c33a6

Browse files
fix(dynamic-instrumentation): rebind FastAPI Depends() sub-dependencies (#785)
## Description A **function-level** Dynamic Instrumentation breakpoint set on a function that is used as a FastAPI `Depends()` **sub-dependency** silently never fires. The instrumentation status stays `READY` (no `ERROR`) and **zero snapshots** are produced, even though the dependency callable executes on every request. ## Root cause When DI instruments a module-level function it replaces it with a wrapper and then calls `_patch_framework_references` → `_patch_single_fastapi_app` to fix up framework-held references. That patcher rebinds: - `route.endpoint`, and - the top-level `route.dependant.call` …but the per-request dependency solver invokes sub-dependency callables via `route.dependant.dependencies[*].call` (recursively). Those references were **never** rebound, so FastAPI kept calling the original, unwrapped function object and the DI wrapper never ran. This is distinct from the inherited-method / nested-class silent-failure cases: here the code-object identity is exactly correct (`route.dependant.dependencies[0].call is original_func`); the only problem is that the patcher didn't reach the sub-dependency reference. ```python async def square_dependency(value: int = 7) -> int: # used only via Depends() return value * value @app.get("/dep") async def decorators_dependency(dep: int = Depends(square_dependency)): return {"result": dep} ``` A breakpoint on `square_dependency` (function-level) produced 0 snapshots; the same function with a **line-level** breakpoint fired correctly (the line engine binds the code object directly), which isolates the defect to the function-level FastAPI-patch path. ## Fix Walk the whole `Dependant` tree (`endpoint` + `dependant` + nested `dependencies`) and rebind only the references that match the target function. Restricting rebinding to matches — rather than the previous unconditional `route.endpoint = new_func` — prevents clobbering an unrelated endpoint on a route that matched solely via one of its sub-dependencies. The same recursive walk is applied in both the identity-match pass and the name+module fallback, so the fallback now also reaches sub-dependencies. ## Testing - Added two regression tests in `test_function_wrapper_fastapi.py`: - `test_patch_fastapi_routes_sub_dependency` — a `Depends()` sub-dependency is rebound on `dependant.dependencies[*].call`, the original is no longer referenced, and the route's own endpoint is not clobbered. - `test_patch_fastapi_routes_only_target_sub_dependency_rebound` — with two `Depends()` on one route, only the targeted sub-dependency is rebound; the other is left intact. - Full debugger test suite passes locally: **547 passed, 43 skipped** (the FastAPI file: 11 passed, 9 pre-existing + 2 new). - Verified end-to-end on the live Application Signals pipeline (FastAPI app → CloudWatch Agent → CloudWatch Logs): the function-level breakpoint on the `Depends()` sub-dependency now reaches status `ACTIVE` ("breakpoint is being hit") and produces a snapshot capturing the argument (`value=7`) and return (`49`); a sibling route-handler breakpoint passed as a same-app control, and the endpoint response was unchanged. ## By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 0d1bda1 commit f2c33a6

2 files changed

Lines changed: 147 additions & 31 deletions

File tree

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

Lines changed: 67 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,29 +1170,58 @@ def _patch_single_fastapi_app( # pylint: disable=too-many-locals
11701170
):
11711171
"""Patch route handler references in a single FastAPI app instance.
11721172
1173-
Rebinds both ``route.endpoint`` and ``route.dependant.call`` (the attribute the
1174-
dispatcher actually invokes). The pre-built Dependant remains valid because the
1175-
DI wrapper preserves the handler signature via functools.wraps, so no rebuild is
1176-
needed. ``app.include_router`` flattens sub-routers into ``app.router.routes``,
1177-
so iterating that list covers all routes.
1173+
Rebinds ``route.endpoint``, ``route.dependant.call``, and — recursively —
1174+
every matching ``route.dependant.dependencies[*].call`` (the attributes the
1175+
dispatcher actually invokes). The last one matters for functions used as
1176+
``Depends(...)`` sub-dependencies: the per-request solver calls those
1177+
directly, so rebinding only the top-level handler would leave a
1178+
sub-dependency breakpoint silently never firing. The pre-built Dependant
1179+
remains valid because the DI wrapper preserves the handler signature via
1180+
functools.wraps, so no rebuild is needed. ``app.include_router`` flattens
1181+
sub-routers into ``app.router.routes``, so iterating that list covers all
1182+
routes.
11781183
"""
11791184
router = getattr(fastapi_app, "router", None)
11801185
routes = getattr(router, "routes", None)
11811186
if not isinstance(routes, list):
11821187
logger.debug("FastAPI app '%s' has no router.routes list", app_name)
11831188
return
11841189

1185-
def _rebind(route) -> None:
1186-
route.endpoint = new_func
1187-
dependant = getattr(route, "dependant", None)
1188-
if dependant is not None and hasattr(dependant, "call"):
1190+
def _rebind_dependant(dependant, matches) -> int:
1191+
"""Replace every matching ``call`` in a Dependant tree with new_func.
1192+
1193+
FastAPI stores ``Depends(...)`` sub-dependency callables on
1194+
``dependant.dependencies[*].call`` (recursively). The per-request
1195+
solver invokes those directly, so a function used only as a
1196+
sub-dependency is never reached by rebinding the top-level
1197+
``dependant.call`` alone. Walks the whole tree and rebinds only the
1198+
references that match the target, so a route matched via one
1199+
sub-dependency does not have its unrelated calls clobbered.
1200+
Returns the number of references rebound.
1201+
"""
1202+
if dependant is None:
1203+
return 0
1204+
count = 0
1205+
if matches(getattr(dependant, "call", None)):
11891206
dependant.call = new_func
1190-
1191-
def _is_identity_match(route) -> bool:
1192-
if getattr(route, "endpoint", None) is original_func:
1193-
return True
1194-
dependant = getattr(route, "dependant", None)
1195-
return dependant is not None and getattr(dependant, "call", None) is original_func
1207+
count += 1
1208+
for sub in getattr(dependant, "dependencies", None) or []:
1209+
count += _rebind_dependant(sub, matches)
1210+
return count
1211+
1212+
def _rebind_route(route, matches) -> int:
1213+
"""Rebind matching references on a route: its ``endpoint`` and every
1214+
matching ``call`` in its Dependant tree. Only references that match
1215+
are touched. Returns the number of references rebound."""
1216+
count = 0
1217+
if matches(getattr(route, "endpoint", None)):
1218+
route.endpoint = new_func
1219+
count += 1
1220+
count += _rebind_dependant(getattr(route, "dependant", None), matches)
1221+
return count
1222+
1223+
def _is_identity(func) -> bool:
1224+
return func is original_func
11961225

11971226
# Identity match first. Guard each route independently so one bad route
11981227
# cannot abort patching of the others.
@@ -1201,17 +1230,17 @@ def _is_identity_match(route) -> bool:
12011230
try:
12021231
if getattr(route, "endpoint", None) is None:
12031232
continue # not an APIRoute (e.g. Mount/WebSocketRoute)
1204-
if _is_identity_match(route):
1205-
_rebind(route)
1206-
patched_count += 1
1233+
rebound = _rebind_route(route, _is_identity)
1234+
if rebound:
1235+
patched_count += rebound
12071236
logger.debug("Patched FastAPI route '%s' to use DI wrapper", getattr(route, "path", "?"))
12081237
except Exception as exc:
12091238
logger.warning("Error patching FastAPI route '%s': %s", getattr(route, "path", "?"), exc)
12101239
continue
12111240

12121241
if patched_count > 0:
12131242
logger.debug(
1214-
"Patched %d FastAPI route(s) for function '%s' on app '%s'",
1243+
"Patched %d FastAPI reference(s) for function '%s' on app '%s'",
12151244
patched_count,
12161245
func_name,
12171246
app_name,
@@ -1223,29 +1252,36 @@ def _is_identity_match(route) -> bool:
12231252
# so requiring both name and module avoids patching a same-named handler from a
12241253
# different module. If the original has no __module__, fall back to name-only.
12251254
def _matches(endpoint):
1255+
if endpoint is None:
1256+
return False
12261257
if getattr(endpoint, "__name__", None) != func_name:
12271258
return False
12281259
if func_module is None:
12291260
return True
12301261
return getattr(endpoint, "__module__", None) == func_module
12311262

1232-
matching_routes = [r for r in routes if getattr(r, "endpoint", None) is not None and _matches(r.endpoint)]
1233-
if matching_routes:
1263+
fallback_count = 0
1264+
for route in routes:
1265+
try:
1266+
if getattr(route, "endpoint", None) is None:
1267+
continue
1268+
rebound = _rebind_route(route, _matches)
1269+
if rebound:
1270+
fallback_count += rebound
1271+
logger.debug("Patched FastAPI route '%s' by name+module match", getattr(route, "path", "?"))
1272+
except Exception as exc:
1273+
logger.warning("Error patching FastAPI route '%s': %s", getattr(route, "path", "?"), exc)
1274+
continue
1275+
1276+
if fallback_count > 0:
12341277
logger.debug(
1235-
"FastAPI app '%s' has %d route(s) with name '%s' (module '%s') but identity "
1236-
"mismatch. Patching by name+module.",
1278+
"FastAPI app '%s' had identity mismatch; patched %d reference(s) for '%s' (module '%s') "
1279+
"by name+module match.",
12371280
app_name,
1238-
len(matching_routes),
1281+
fallback_count,
12391282
func_name,
12401283
func_module,
12411284
)
1242-
for route in matching_routes:
1243-
try:
1244-
_rebind(route)
1245-
logger.debug("Patched FastAPI route '%s' by name+module match", getattr(route, "path", "?"))
1246-
except Exception as exc:
1247-
logger.warning("Error patching FastAPI route '%s': %s", getattr(route, "path", "?"), exc)
1248-
continue
12491285

12501286
@staticmethod
12511287
def _get_qualified_name(original_func: Callable) -> str:

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/debugger/test_function_wrapper_fastapi.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,86 @@ def other_view():
164164
self.assertIs(route.endpoint, other_endpoint)
165165
self.assertIs(route.dependant.call, other_call)
166166

167+
# ------------------------------------------------------------------
168+
# Depends() sub-dependency patching (regression: sub-dependency
169+
# breakpoints silently never fired because only the top-level
170+
# dependant.call was rebound, never dependant.dependencies[*].call)
171+
# ------------------------------------------------------------------
172+
173+
def test_patch_fastapi_routes_sub_dependency(self):
174+
"""A function used as a Depends() sub-dependency is rebound on dependant.dependencies[*].call.
175+
176+
The per-request solver invokes the sub-dependency via
177+
route.dependant.dependencies[i].call, not the top-level endpoint, so
178+
rebinding only endpoint/dependant.call would leave it pointing at the
179+
original (the breakpoint would silently never fire).
180+
"""
181+
try:
182+
from fastapi import Depends, FastAPI
183+
except ImportError:
184+
self.skipTest("FastAPI not installed")
185+
186+
def square_dependency(value: int = 7) -> int:
187+
return value * value
188+
189+
app = FastAPI()
190+
191+
@app.get("/dep")
192+
def decorators_dependency(dep: int = Depends(square_dependency)):
193+
return {"result": dep}
194+
195+
route = _route_for(app, "/dep")
196+
sub_calls = [d.call for d in route.dependant.dependencies]
197+
# Precondition: the solver currently points at the ORIGINAL sub-dependency.
198+
self.assertIn(square_dependency, sub_calls)
199+
200+
wrapper = lambda value=7: value * value # noqa: E731
201+
wrapper.__name__ = square_dependency.__name__
202+
wrapper.__module__ = square_dependency.__module__
203+
204+
mod = _make_module(self.module_name, app=app, square_dependency=square_dependency)
205+
206+
FunctionWrapper._patch_fastapi_routes(mod, square_dependency, wrapper)
207+
208+
rebound_calls = [d.call for d in route.dependant.dependencies]
209+
self.assertIn(wrapper, rebound_calls)
210+
self.assertNotIn(square_dependency, rebound_calls)
211+
# The route's own endpoint (a different function) must not be clobbered.
212+
self.assertIsNot(route.endpoint, wrapper)
213+
214+
def test_patch_fastapi_routes_only_target_sub_dependency_rebound(self):
215+
"""With two Depends() on a route, only the targeted sub-dependency is rebound."""
216+
try:
217+
from fastapi import Depends, FastAPI
218+
except ImportError:
219+
self.skipTest("FastAPI not installed")
220+
221+
def square_dependency(value: int = 7) -> int:
222+
return value * value
223+
224+
def inner_dep() -> int:
225+
return 1
226+
227+
app = FastAPI()
228+
229+
@app.get("/nested")
230+
def nested_dependency(dep: int = Depends(square_dependency), x: int = Depends(inner_dep)):
231+
return {"result": dep + x}
232+
233+
route = _route_for(app, "/nested")
234+
wrapper = lambda value=7: value * value # noqa: E731
235+
wrapper.__name__ = square_dependency.__name__
236+
wrapper.__module__ = square_dependency.__module__
237+
238+
mod = _make_module(self.module_name, app=app, square_dependency=square_dependency)
239+
240+
FunctionWrapper._patch_fastapi_routes(mod, square_dependency, wrapper)
241+
242+
rebound_calls = [d.call for d in route.dependant.dependencies]
243+
self.assertIn(wrapper, rebound_calls) # target rebound
244+
self.assertIn(inner_dep, rebound_calls) # the other dep left intact
245+
self.assertNotIn(square_dependency, rebound_calls)
246+
167247
# ------------------------------------------------------------------
168248
# Integration: _replace_function_in_module with FastAPI patching
169249
# ------------------------------------------------------------------

0 commit comments

Comments
 (0)