mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-29 06:33:28 +00:00
fix(proxy): build proxy-only failure logs from an allowlist and ignore request-time GCS credentials
Backport of #26 to rc/1.105.0. Cherry-picked from 9c2877c4be (main).
This commit is contained in:
parent
ea7ca16dbc
commit
dd8d12dfb8
5 changed files with 58 additions and 127 deletions
|
|
@ -18,7 +18,7 @@ from litellm.llms.custom_httpx.http_handler import (
|
|||
httpxSpecialProvider,
|
||||
)
|
||||
from litellm.types.integrations.gcs_bucket import *
|
||||
from litellm.types.utils import StandardCallbackDynamicParams, StandardLoggingPayload
|
||||
from litellm.types.utils import StandardLoggingPayload
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from litellm.llms.vertex_ai.vertex_llm_base import VertexBase
|
||||
|
|
@ -137,49 +137,16 @@ class GCSBucketBase(CustomBatchLogger):
|
|||
|
||||
async def get_gcs_logging_config(self, kwargs: dict[str, Any] | None = {}) -> GCSLoggingConfig:
|
||||
"""
|
||||
This function is used to get the GCS logging config for the GCS Bucket Logger.
|
||||
It checks if the dynamic parameters are provided in the kwargs and uses them to get the GCS logging config.
|
||||
If no dynamic parameters are provided, it uses the default values.
|
||||
Returns the bucket and service account this logger was configured with.
|
||||
"""
|
||||
if kwargs is None:
|
||||
kwargs = {}
|
||||
|
||||
standard_callback_dynamic_params: Final[StandardCallbackDynamicParams | None] = kwargs.get(
|
||||
"standard_callback_dynamic_params", None
|
||||
)
|
||||
|
||||
bucket_name: str
|
||||
path_service_account: str | None
|
||||
if standard_callback_dynamic_params is not None:
|
||||
verbose_logger.debug("Using dynamic GCS logging")
|
||||
verbose_logger.debug("standard_callback_dynamic_params: %s", standard_callback_dynamic_params)
|
||||
|
||||
_bucket_name: str | None = standard_callback_dynamic_params.get("gcs_bucket_name", None) or self.BUCKET_NAME
|
||||
_path_service_account: Final[str | None] = (
|
||||
standard_callback_dynamic_params.get("gcs_path_service_account", None) or self.path_service_account_json
|
||||
if self.BUCKET_NAME is None:
|
||||
raise ValueError(
|
||||
"GCS_BUCKET_NAME is not set in the environment, but GCS Bucket is being used as a logging callback. Please set 'GCS_BUCKET_NAME' in the environment."
|
||||
)
|
||||
|
||||
if _bucket_name is None:
|
||||
raise ValueError(
|
||||
"GCS_BUCKET_NAME is not set in the environment, but GCS Bucket is being used as a logging callback. Please set 'GCS_BUCKET_NAME' in the environment."
|
||||
)
|
||||
bucket_name = _bucket_name
|
||||
path_service_account = _path_service_account
|
||||
vertex_instance = await self.get_or_create_vertex_instance(credentials=path_service_account)
|
||||
else:
|
||||
# If no dynamic parameters, use the default instance
|
||||
if self.BUCKET_NAME is None:
|
||||
raise ValueError(
|
||||
"GCS_BUCKET_NAME is not set in the environment, but GCS Bucket is being used as a logging callback. Please set 'GCS_BUCKET_NAME' in the environment."
|
||||
)
|
||||
bucket_name = self.BUCKET_NAME
|
||||
path_service_account = self.path_service_account_json
|
||||
vertex_instance = await self.get_or_create_vertex_instance(credentials=path_service_account)
|
||||
|
||||
return GCSLoggingConfig(
|
||||
bucket_name=bucket_name,
|
||||
vertex_instance=vertex_instance,
|
||||
path_service_account=path_service_account,
|
||||
bucket_name=self.BUCKET_NAME,
|
||||
vertex_instance=await self.get_or_create_vertex_instance(credentials=self.path_service_account_json),
|
||||
path_service_account=self.path_service_account_json,
|
||||
)
|
||||
|
||||
async def get_or_create_vertex_instance(self, credentials: str | None) -> VertexBase:
|
||||
|
|
|
|||
|
|
@ -1328,6 +1328,13 @@ def _call_type_for_route(route: str | None) -> str | None:
|
|||
|
||||
_PROXY_ONLY_LLM_API_ERRORS: Final = (HTTPException, ProxyException, GuardrailRaisedException)
|
||||
_LOG_DB_METRICS_CALL_TYPES: Final = frozenset(("get_data", "insert_data", "update_data", "delete_data"))
|
||||
_PROXY_ONLY_ERROR_LOGGED_FIELDS: Final = frozenset(
|
||||
{"model", "messages", "input", "prompt", "user", "stream", "metadata", "litellm_metadata", "litellm_call_id"}
|
||||
)
|
||||
|
||||
|
||||
def _proxy_only_error_log_fields(request_data: Mapping[str, _T]) -> Mapping[str, _T]:
|
||||
return MappingProxyType({k: v for k, v in request_data.items() if k in _PROXY_ONLY_ERROR_LOGGED_FIELDS})
|
||||
|
||||
|
||||
_EXCEPTION_CHAIN_MAX_DEPTH: Final = 16
|
||||
|
|
@ -3714,7 +3721,7 @@ class ProxyLogging:
|
|||
original_function=route or "IGNORE_THIS",
|
||||
rules_obj=litellm.utils.Rules(),
|
||||
start_time=datetime.now(),
|
||||
**request_data,
|
||||
**_proxy_only_error_log_fields(request_data),
|
||||
)
|
||||
request_data["litellm_logging_obj"] = litellm_logging_obj # rebind-ok: lifted then popped by the caller
|
||||
if "metadata" not in request_data:
|
||||
|
|
@ -3727,7 +3734,7 @@ class ProxyLogging:
|
|||
_litellm_params: Final = {}
|
||||
|
||||
litellm_param_keys: Final = LoggedLiteLLMParams.__annotations__.keys()
|
||||
for k, v in request_data.items():
|
||||
for k, v in _proxy_only_error_log_fields(request_data).items():
|
||||
if k in litellm_param_keys:
|
||||
_litellm_params[k] = v
|
||||
elif k not in ("model", "user", "litellm_logging_obj"):
|
||||
|
|
|
|||
|
|
@ -91,6 +91,24 @@ class TestGCSBucketBase:
|
|||
custom_llm_provider="vertex_ai",
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_gcs_logging_config_ignores_request_time_bucket_and_credentials(self, monkeypatch):
|
||||
monkeypatch.setenv("GCS_PATH_SERVICE_ACCOUNT", "/admin/sa.json")
|
||||
handler = GCSBucketBase(bucket_name="admin-bucket")
|
||||
|
||||
with patch.object(handler, "get_or_create_vertex_instance", new_callable=AsyncMock) as mock_vertex:
|
||||
config = await handler.get_gcs_logging_config(
|
||||
{
|
||||
"standard_callback_dynamic_params": {
|
||||
"gcs_bucket_name": "caller-bucket",
|
||||
"gcs_path_service_account": "/caller/sa.json",
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
assert (config["bucket_name"], config["path_service_account"]) == ("admin-bucket", "/admin/sa.json")
|
||||
mock_vertex.assert_awaited_once_with(credentials="/admin/sa.json")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_log_json_data_on_gcs_url_encodes_object_name(self):
|
||||
handler = GCSBucketBase(bucket_name="test-bucket")
|
||||
|
|
|
|||
|
|
@ -260,87 +260,3 @@ class TestVertexAIFilesHandler:
|
|||
)
|
||||
|
||||
assert service_account == '{"type": "service_account", "project_id": "p"}'
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_afile_content_honors_per_model_bucket_over_env(self, monkeypatch):
|
||||
"""
|
||||
Regression for #32640: a batch output written to a per-model gcs_bucket_name must be
|
||||
readable even when the global GCS_BUCKET_NAME points at a different bucket. Before the
|
||||
fix the read path resolved the bucket from env only and raised
|
||||
"file_id bucket does not match the configured storage bucket".
|
||||
"""
|
||||
monkeypatch.setenv("GCS_BUCKET_NAME", "env-default-bucket")
|
||||
monkeypatch.delenv("GCS_PATH_SERVICE_ACCOUNT", raising=False)
|
||||
|
||||
file_id = "gs%3A%2F%2Fmy-model-bucket%2Flitellm-vertex-files%2Fuploads%2Fabc-batch-output.jsonl"
|
||||
file_content_request = FileContentRequest(file_id=file_id, extra_headers=None, extra_body=None)
|
||||
|
||||
with (
|
||||
patch.object(self.handler, "download_gcs_object", new_callable=AsyncMock) as mock_download,
|
||||
patch.object(
|
||||
self.handler,
|
||||
"get_or_create_vertex_instance",
|
||||
new_callable=AsyncMock,
|
||||
return_value=object(),
|
||||
),
|
||||
):
|
||||
mock_download.return_value = b"batch output"
|
||||
|
||||
result = await self.handler.afile_content(
|
||||
file_content_request=file_content_request,
|
||||
vertex_credentials="/model/sa.json",
|
||||
vertex_project="test-project",
|
||||
vertex_location="us-central1",
|
||||
timeout=60.0,
|
||||
max_retries=0,
|
||||
litellm_params={
|
||||
"gcs_bucket_name": "my-model-bucket",
|
||||
"vertex_credentials": "/model/sa.json",
|
||||
},
|
||||
)
|
||||
|
||||
assert isinstance(result, HttpxBinaryResponseContent)
|
||||
assert result.response.content == b"batch output"
|
||||
|
||||
dynamic_params = mock_download.call_args.kwargs["standard_callback_dynamic_params"]
|
||||
assert dynamic_params["gcs_bucket_name"] == "my-model-bucket"
|
||||
assert dynamic_params["gcs_path_service_account"] == "/model/sa.json"
|
||||
assert mock_download.call_args.kwargs["object_name"] == "litellm-vertex-files/uploads/abc-batch-output.jsonl"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_afile_content_reads_without_global_env_bucket(self, monkeypatch):
|
||||
"""
|
||||
Regression for #32640: with no global GCS_BUCKET_NAME set, a model-group-level
|
||||
deployment (per-model gcs_bucket_name) must still be readable. Before the fix the read
|
||||
path raised "GCS_BUCKET_NAME is not set in the environment".
|
||||
"""
|
||||
monkeypatch.delenv("GCS_BUCKET_NAME", raising=False)
|
||||
monkeypatch.delenv("GCS_PATH_SERVICE_ACCOUNT", raising=False)
|
||||
|
||||
file_id = "gs%3A%2F%2Fmy-model-bucket%2Flitellm-vertex-files%2Fuploads%2Fabc-batch-output.jsonl"
|
||||
file_content_request = FileContentRequest(file_id=file_id, extra_headers=None, extra_body=None)
|
||||
|
||||
with (
|
||||
patch.object(self.handler, "download_gcs_object", new_callable=AsyncMock) as mock_download,
|
||||
patch.object(
|
||||
self.handler,
|
||||
"get_or_create_vertex_instance",
|
||||
new_callable=AsyncMock,
|
||||
return_value=object(),
|
||||
),
|
||||
):
|
||||
mock_download.return_value = b"batch output"
|
||||
|
||||
result = await self.handler.afile_content(
|
||||
file_content_request=file_content_request,
|
||||
vertex_credentials="/model/sa.json",
|
||||
vertex_project="test-project",
|
||||
vertex_location="us-central1",
|
||||
timeout=60.0,
|
||||
max_retries=0,
|
||||
litellm_params={"gcs_bucket_name": "my-model-bucket"},
|
||||
)
|
||||
|
||||
assert isinstance(result, HttpxBinaryResponseContent)
|
||||
dynamic_params = mock_download.call_args.kwargs["standard_callback_dynamic_params"]
|
||||
assert dynamic_params["gcs_bucket_name"] == "my-model-bucket"
|
||||
|
|
|
|||
|
|
@ -163,6 +163,29 @@ async def test_proxy_only_error_log_keeps_litellm_metadata_in_litellm_params():
|
|||
assert "litellm_metadata" not in captured["optional_params"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_proxy_only_error_log_drops_caller_logging_fields_and_keeps_key_details():
|
||||
request_data: Final[dict[str, object]] = {
|
||||
"model": "gpt-4o",
|
||||
"messages": [{"role": "user", "content": "hi"}],
|
||||
"litellm_metadata": {"tags": ["t"]},
|
||||
"langsmith_base_url": "https://caller.invalid",
|
||||
"standard_callback_dynamic_params": {"gcs_path_service_account": "/caller/sa.json"},
|
||||
}
|
||||
|
||||
await ProxyLogging(user_api_key_cache=DualCache())._handle_logging_proxy_only_error(
|
||||
request_data=request_data,
|
||||
user_api_key_dict=UserAPIKeyAuth(api_key="sk-rejected", team_id="team-a", request_route="/v1/chat/completions"),
|
||||
route="/v1/chat/completions",
|
||||
original_exception=Exception("bad key"),
|
||||
)
|
||||
|
||||
details: Final = request_data["litellm_logging_obj"].model_call_details
|
||||
assert "caller" not in str(details["standard_callback_dynamic_params"])
|
||||
assert "langsmith_base_url" not in details
|
||||
assert details["litellm_params"]["metadata"]["user_api_key_team_id"] == "team-a"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_proxy_only_error_log_keeps_the_request_litellm_call_id(monkeypatch: pytest.MonkeyPatch):
|
||||
"""LIT-7836: a route that already stamped the caller's litellm_call_id must
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue