mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
fix: silent metrics race condition
This commit is contained in:
parent
5a547aa857
commit
70de83a6d8
2 changed files with 41 additions and 7 deletions
|
|
@ -1331,13 +1331,32 @@ class Router:
|
|||
def _get_silent_experiment_kwargs(self, **kwargs) -> dict:
|
||||
"""
|
||||
Prepare kwargs for a silent experiment by ensuring isolation from the primary call.
|
||||
"""
|
||||
# Copy kwargs to ensure isolation (use safe_deep_copy to handle non-serializable objects like OTEL spans)
|
||||
from litellm.litellm_core_utils.core_helpers import safe_deep_copy
|
||||
|
||||
silent_kwargs = safe_deep_copy(kwargs)
|
||||
if "metadata" not in silent_kwargs:
|
||||
silent_kwargs["metadata"] = {}
|
||||
IMPORTANT: We avoid calling safe_deep_copy(kwargs) because it temporarily
|
||||
mutates the original dict (pops litellm_parent_otel_span, replaces with
|
||||
"placeholder", then restores). Since this runs in a background thread while
|
||||
the primary request's async callbacks may still be reading the same dict,
|
||||
that mutation causes a race condition that breaks otel/prometheus callbacks
|
||||
for the primary request.
|
||||
"""
|
||||
import copy
|
||||
|
||||
# Shallow copy top-level kwargs — does NOT mutate the original
|
||||
silent_kwargs = dict(kwargs)
|
||||
|
||||
# Deep-copy metadata so we don't share state with the primary request.
|
||||
# Remove the OTEL span BEFORE deep-copying (it's not picklable and is
|
||||
# thread-bound anyway).
|
||||
original_metadata = kwargs.get("metadata") or {}
|
||||
metadata_copy = {
|
||||
k: v
|
||||
for k, v in original_metadata.items()
|
||||
if k != "litellm_parent_otel_span"
|
||||
}
|
||||
try:
|
||||
silent_kwargs["metadata"] = copy.deepcopy(metadata_copy)
|
||||
except Exception:
|
||||
silent_kwargs["metadata"] = dict(metadata_copy)
|
||||
|
||||
silent_kwargs["metadata"]["is_silent_experiment"] = True
|
||||
|
||||
|
|
|
|||
|
|
@ -19,11 +19,26 @@ def test_get_silent_experiment_kwargs():
|
|||
},
|
||||
]
|
||||
router = Router(model_list=model_list)
|
||||
kwargs = {"metadata": {"foo": "bar"}, "litellm_call_id": "call-123"}
|
||||
mock_span = MagicMock()
|
||||
kwargs = {
|
||||
"metadata": {"foo": "bar", "litellm_parent_otel_span": mock_span},
|
||||
"litellm_call_id": "call-123",
|
||||
"stream": True,
|
||||
"proxy_server_request": {"body": {"model": "test"}},
|
||||
}
|
||||
result = router._get_silent_experiment_kwargs(**kwargs)
|
||||
assert result["metadata"]["is_silent_experiment"] is True
|
||||
assert result["metadata"]["foo"] == "bar"
|
||||
assert "litellm_call_id" not in result
|
||||
# stream must be forced to False so callbacks fire in background
|
||||
assert result["stream"] is False
|
||||
# proxy_server_request must be preserved for spend log metadata
|
||||
assert "proxy_server_request" in result
|
||||
# parent OTEL span must be removed — it's thread-bound and invalid in the
|
||||
# background thread's new event loop
|
||||
assert "litellm_parent_otel_span" not in result["metadata"]
|
||||
# CRITICAL: original kwargs must NOT be mutated (race condition with primary callbacks)
|
||||
assert kwargs["metadata"]["litellm_parent_otel_span"] is mock_span
|
||||
|
||||
|
||||
def test_silent_experiment_completion_direct():
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue