mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-09 22:31:41 +00:00
fix(router): keep retry breadcrumbs per request and out of the request snapshot
Retry breadcrumbs were appended to one list owned by the Router and shared by every request, and each breadcrumb copied the whole kwargs including the proxy's snapshot of the inbound request. That snapshot's body aliases the live request metadata, breadcrumbs included, so every new breadcrumb nested all the earlier ones inside itself. Memory stayed small because these are shared references, but under --detailed_debug the repr of that structure expands, so one debug line grew from 10k to 219M characters over 14 failing requests and the proxy stopped answering. Breadcrumbs now accumulate in the metadata of the request that produced them, the request snapshot is excluded from a breadcrumb, and the cap of the last 4 failed attempts applies per request.
This commit is contained in:
parent
993766be0e
commit
7bc2d0b06e
2 changed files with 118 additions and 37 deletions
|
|
@ -586,16 +586,20 @@ set_live_deployment_replay(_replay_live_router_model_cost)
|
||||||
|
|
||||||
|
|
||||||
# Kwargs that carry no signal about the failed attempt, so log_retry drops them from a
|
# Kwargs that carry no signal about the failed attempt, so log_retry drops them from a
|
||||||
# breadcrumb entirely: the request payload and the router-internal walk state. Credentials are
|
# breadcrumb entirely: the request payload, the proxy's snapshot of the inbound request (its body
|
||||||
# handled separately by mask_credentials_in_payload, which scrubs credential-named values from
|
# aliases the live request metadata, earlier breadcrumbs included, so copying it would nest every
|
||||||
# whatever kwargs remain rather than trying to enumerate every credential-bearing key here.
|
# breadcrumb inside the next one), and the router-internal walk state. Credentials are handled
|
||||||
|
# separately by mask_credentials_in_payload, which scrubs credential-named values from whatever
|
||||||
|
# kwargs remain rather than trying to enumerate every credential-bearing key here.
|
||||||
RETRY_BREADCRUMB_EXCLUDED_KWARGS: Final = frozenset(
|
RETRY_BREADCRUMB_EXCLUDED_KWARGS: Final = frozenset(
|
||||||
(
|
(
|
||||||
"messages",
|
"messages",
|
||||||
"original_function",
|
"original_function",
|
||||||
"attempted_targets",
|
"attempted_targets",
|
||||||
|
"proxy_server_request",
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
RETRY_BREADCRUMB_LIMIT: Final = 4
|
||||||
|
|
||||||
|
|
||||||
class Router:
|
class Router:
|
||||||
|
|
@ -954,7 +958,6 @@ class Router:
|
||||||
self.total_calls: defaultdict = defaultdict(int) # dict to store total calls made to each model
|
self.total_calls: defaultdict = defaultdict(int) # dict to store total calls made to each model
|
||||||
self.fail_calls: defaultdict = defaultdict(int) # dict to store fail_calls made to each model
|
self.fail_calls: defaultdict = defaultdict(int) # dict to store fail_calls made to each model
|
||||||
self.success_calls: defaultdict = defaultdict(int) # dict to store success_calls made to each model
|
self.success_calls: defaultdict = defaultdict(int) # dict to store success_calls made to each model
|
||||||
self.previous_models: list = [] # list to store failed calls (passed in as metadata to next call)
|
|
||||||
|
|
||||||
# make Router.chat.completions.create compatible for openai.chat.completions.create
|
# make Router.chat.completions.create compatible for openai.chat.completions.create
|
||||||
default_litellm_params = default_litellm_params or {}
|
default_litellm_params = default_litellm_params or {}
|
||||||
|
|
@ -8048,35 +8051,30 @@ class Router:
|
||||||
"""
|
"""
|
||||||
When a retry or fallback happens, log the details of the just failed model call - similar to Sentry breadcrumbing
|
When a retry or fallback happens, log the details of the just failed model call - similar to Sentry breadcrumbing
|
||||||
"""
|
"""
|
||||||
try:
|
_metadata_var: Final = "litellm_metadata" if "litellm_metadata" in kwargs else "metadata"
|
||||||
_metadata_var: Final = "litellm_metadata" if "litellm_metadata" in kwargs else "metadata"
|
request_metadata: Final[Mapping[str, object]] = kwargs[_metadata_var]
|
||||||
# Log failed model as the previous model
|
attempt_kwargs: Final = MappingProxyType(
|
||||||
previous_model: Final = {
|
{k: v for k, v in kwargs.items() if k != _metadata_var and k not in RETRY_BREADCRUMB_EXCLUDED_KWARGS}
|
||||||
|
)
|
||||||
|
attempt_metadata: Final = MappingProxyType(
|
||||||
|
{k: v for k, v in request_metadata.items() if k != "previous_models"}
|
||||||
|
)
|
||||||
|
previous_model: Final = MappingProxyType(
|
||||||
|
{
|
||||||
"exception_type": type(e).__name__,
|
"exception_type": type(e).__name__,
|
||||||
"exception_string": str(e),
|
"exception_string": str(e),
|
||||||
|
**attempt_kwargs,
|
||||||
|
_metadata_var: attempt_metadata,
|
||||||
}
|
}
|
||||||
for (
|
)
|
||||||
k,
|
earlier_breadcrumbs: Final = request_metadata.get("previous_models")
|
||||||
v,
|
kept_breadcrumbs: Final[tuple[object, ...]] = (
|
||||||
) in kwargs.items(): # log everything in kwargs except the old previous_models value - prevent nesting
|
tuple(earlier_breadcrumbs)[-(RETRY_BREADCRUMB_LIMIT - 1) :]
|
||||||
if k != _metadata_var and k not in RETRY_BREADCRUMB_EXCLUDED_KWARGS:
|
if isinstance(earlier_breadcrumbs, (list, tuple))
|
||||||
previous_model[k] = v
|
else ()
|
||||||
elif k == _metadata_var and isinstance(v, dict):
|
)
|
||||||
previous_model[_metadata_var] = {}
|
kwargs[_metadata_var]["previous_models"] = (*kept_breadcrumbs, mask_credentials_in_payload(previous_model))
|
||||||
for metadata_k, metadata_v in kwargs[_metadata_var].items():
|
return kwargs
|
||||||
if metadata_k != "previous_models":
|
|
||||||
previous_model[k][metadata_k] = metadata_v
|
|
||||||
|
|
||||||
# check current size of self.previous_models, if it's larger than 3, remove the first element
|
|
||||||
if len(self.previous_models) > 3:
|
|
||||||
self.previous_models.pop(0)
|
|
||||||
|
|
||||||
scrubbed_previous_model: Final = mask_credentials_in_payload(previous_model)
|
|
||||||
self.previous_models.append(scrubbed_previous_model)
|
|
||||||
kwargs[_metadata_var]["previous_models"] = self.previous_models
|
|
||||||
return kwargs
|
|
||||||
except Exception as e:
|
|
||||||
raise e
|
|
||||||
|
|
||||||
def _update_usage(self, deployment_id: str, parent_otel_span: Span | None) -> int:
|
def _update_usage(self, deployment_id: str, parent_otel_span: Span | None) -> int:
|
||||||
"""
|
"""
|
||||||
|
|
|
||||||
|
|
@ -9274,9 +9274,11 @@ class _FallbackAttemptRecorder(CustomLogger):
|
||||||
def __init__(self):
|
def __init__(self):
|
||||||
super().__init__()
|
super().__init__()
|
||||||
self.failed_targets = []
|
self.failed_targets = []
|
||||||
|
self.breadcrumbs_per_target = []
|
||||||
|
|
||||||
async def log_failure_fallback_event(self, original_model_group, kwargs, original_exception):
|
async def log_failure_fallback_event(self, original_model_group, kwargs, original_exception):
|
||||||
self.failed_targets.append(kwargs.get("model"))
|
self.failed_targets.append(kwargs.get("model"))
|
||||||
|
self.breadcrumbs_per_target.append(kwargs.get("metadata", {}).get("previous_models", ()))
|
||||||
|
|
||||||
|
|
||||||
def _cyclic_fallback_router(num_retries=0):
|
def _cyclic_fallback_router(num_retries=0):
|
||||||
|
|
@ -9347,14 +9349,16 @@ async def test_retry_breadcrumbs_do_not_carry_the_walk_state():
|
||||||
A retry has to be configured for the walk state to reach log_retry at all."""
|
A retry has to be configured for the walk state to reach log_retry at all."""
|
||||||
router = _cyclic_fallback_router(num_retries=1)
|
router = _cyclic_fallback_router(num_retries=1)
|
||||||
capture = _LogCapture(logging.ERROR)
|
capture = _LogCapture(logging.ERROR)
|
||||||
|
recorder = _FallbackAttemptRecorder()
|
||||||
|
|
||||||
await _drive_cyclic_fallback(router, capture)
|
await _drive_cyclic_fallback(router, capture, recorder)
|
||||||
|
|
||||||
assert router.previous_models, "no retry breadcrumbs were recorded"
|
breadcrumbs = [breadcrumb for hop in recorder.breadcrumbs_per_target for breadcrumb in hop]
|
||||||
|
assert breadcrumbs, "no retry breadcrumbs were recorded"
|
||||||
assert any(
|
assert any(
|
||||||
"fallback_depth" in breadcrumb for breadcrumb in router.previous_models
|
"fallback_depth" in breadcrumb for breadcrumb in breadcrumbs
|
||||||
), "no breadcrumb carried router walk state, so this test cannot see the leak"
|
), "no breadcrumb carried router walk state, so this test cannot see the leak"
|
||||||
for breadcrumb in router.previous_models:
|
for breadcrumb in breadcrumbs:
|
||||||
assert "attempted_targets" not in breadcrumb
|
assert "attempted_targets" not in breadcrumb
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -9392,15 +9396,94 @@ async def test_retry_breadcrumbs_never_carry_a_forwarded_credential(container_ke
|
||||||
container still reaches the breadcrumb, but the raw secret never does, whatever key holds it."""
|
container still reaches the breadcrumb, but the raw secret never does, whatever key holds it."""
|
||||||
router = _cyclic_fallback_router(num_retries=1)
|
router = _cyclic_fallback_router(num_retries=1)
|
||||||
capture = _LogCapture(logging.ERROR)
|
capture = _LogCapture(logging.ERROR)
|
||||||
|
metadata = {}
|
||||||
|
|
||||||
await _drive_cyclic_fallback(router, capture, **request_kwargs)
|
await _drive_cyclic_fallback(router, capture, metadata=metadata, **request_kwargs)
|
||||||
|
|
||||||
assert router.previous_models, "no retry breadcrumbs were recorded"
|
breadcrumbs = metadata["previous_models"]
|
||||||
dumped = json.dumps(router.previous_models, default=str)
|
assert breadcrumbs, "no retry breadcrumbs were recorded"
|
||||||
|
dumped = json.dumps(breadcrumbs, default=str)
|
||||||
assert container_key in dumped, "the credential-bearing kwarg never reached the breadcrumb, so this test cannot see the leak"
|
assert container_key in dumped, "the credential-bearing kwarg never reached the breadcrumb, so this test cannot see the leak"
|
||||||
assert _BREADCRUMB_CREDENTIAL_CANARY not in dumped
|
assert _BREADCRUMB_CREDENTIAL_CANARY not in dumped
|
||||||
|
|
||||||
|
|
||||||
|
def _always_failing_router(num_retries):
|
||||||
|
return litellm.Router(
|
||||||
|
model_list=[
|
||||||
|
{
|
||||||
|
"model_name": "broken-group",
|
||||||
|
"litellm_params": {
|
||||||
|
"model": "openai/gpt-4o-mini",
|
||||||
|
"api_key": "sk-fake",
|
||||||
|
"mock_response": "litellm.InternalServerError",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
],
|
||||||
|
num_retries=num_retries,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def _fail_one_proxy_shaped_request(router, request_marker):
|
||||||
|
"""The proxy hands the router a metadata dict and a proxy_server_request whose body is a
|
||||||
|
shallow copy of the request, so body["metadata"] is the very same dict the router later
|
||||||
|
stamps previous_models onto."""
|
||||||
|
metadata = {"request_marker": request_marker}
|
||||||
|
with pytest.raises(litellm.InternalServerError):
|
||||||
|
await router.acompletion(
|
||||||
|
model="broken-group",
|
||||||
|
messages=[{"role": "user", "content": "hi"}],
|
||||||
|
metadata=metadata,
|
||||||
|
proxy_server_request={
|
||||||
|
"url": "http://localhost:4000/v1/chat/completions",
|
||||||
|
"method": "POST",
|
||||||
|
"headers": {},
|
||||||
|
"body": {"model": "broken-group", "metadata": metadata},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
return metadata["previous_models"]
|
||||||
|
|
||||||
|
|
||||||
|
def _nested_breadcrumb_lists(node):
|
||||||
|
if isinstance(node, dict):
|
||||||
|
return [v for k, v in node.items() if k == "previous_models"] + [
|
||||||
|
found for v in node.values() for found in _nested_breadcrumb_lists(v)
|
||||||
|
]
|
||||||
|
if isinstance(node, (list, tuple)):
|
||||||
|
return [found for item in node for found in _nested_breadcrumb_lists(item)]
|
||||||
|
return []
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_retry_breadcrumbs_stay_per_request_and_flat_across_failing_requests():
|
||||||
|
"""Every failed attempt appends a breadcrumb to metadata["previous_models"], and the proxy's
|
||||||
|
request snapshot aliases that same metadata dict. Kept on the Router and copied wholesale,
|
||||||
|
each breadcrumb embedded every earlier one from every earlier request, so the breadcrumb
|
||||||
|
tree, and with it the debug repr of the kwargs, roughly doubled on each failed attempt until
|
||||||
|
a single-worker proxy spent minutes in the redaction regex and stopped answering."""
|
||||||
|
router = _always_failing_router(num_retries=2)
|
||||||
|
|
||||||
|
breadcrumbs_per_request = [
|
||||||
|
await _fail_one_proxy_shaped_request(router, f"request-{request_number}") for request_number in range(1, 7)
|
||||||
|
]
|
||||||
|
|
||||||
|
for request_number, breadcrumbs in enumerate(breadcrumbs_per_request, start=1):
|
||||||
|
assert len(breadcrumbs) == 3, "one initial attempt plus two retries failed, each leaving one breadcrumb"
|
||||||
|
assert {breadcrumb["metadata"]["request_marker"] for breadcrumb in breadcrumbs} == {f"request-{request_number}"}
|
||||||
|
for breadcrumb in breadcrumbs:
|
||||||
|
assert _nested_breadcrumb_lists(breadcrumb) == []
|
||||||
|
assert len({len(repr(breadcrumbs)) for breadcrumbs in breadcrumbs_per_request}) == 1
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_retry_breadcrumbs_keep_only_the_last_four_attempts():
|
||||||
|
router = _always_failing_router(num_retries=6)
|
||||||
|
|
||||||
|
breadcrumbs = await _fail_one_proxy_shaped_request(router, "request-1")
|
||||||
|
|
||||||
|
assert len(breadcrumbs) == 4
|
||||||
|
assert [breadcrumb["metadata"]["attempted_retries"] for breadcrumb in breadcrumbs] == [3, 4, 5, 6]
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_fallback_traceback_stays_available_at_debug_level():
|
async def test_fallback_traceback_stays_available_at_debug_level():
|
||||||
"""Dropping the stack from the ERROR line is only safe because the fallback path still
|
"""Dropping the stack from the ERROR line is only safe because the fallback path still
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue