Fix long S3 logging object keys

This commit is contained in:
Raashish Aggarwal 2026-03-26 21:04:49 +05:30
parent bdf4acc472
commit e534cb33d4
2 changed files with 223 additions and 88 deletions

View file

@ -1,6 +1,7 @@
#### What this does ####
# On success + failure, log events to Supabase
import hashlib
from datetime import datetime
from typing import Optional, cast
@ -8,6 +9,8 @@ import litellm
from litellm._logging import print_verbose, verbose_logger
from litellm.types.utils import StandardLoggingPayload
MAX_S3_FILENAME_COMPONENT_LENGTH = 250
class S3Logger:
# Class variables or attributes
@ -185,6 +188,11 @@ def get_s3_object_key(
start_time: datetime,
s3_file_name: str,
) -> str:
if len(s3_file_name) > MAX_S3_FILENAME_COMPONENT_LENGTH:
digest = hashlib.sha256(s3_file_name.encode("utf-8")).hexdigest()[:16]
prefix_length = MAX_S3_FILENAME_COMPONENT_LENGTH - len(digest) - 1
s3_file_name = f"{s3_file_name[:prefix_length]}_{digest}"
s3_object_key = (
(s3_path.rstrip("/") + "/" if s3_path else "")
+ prefix

View file

@ -4,6 +4,7 @@ from unittest.mock import MagicMock, patch
import pytest
from litellm.integrations.s3 import get_s3_object_key
from litellm.integrations.s3_v2 import S3Logger
from litellm.types.utils import StandardLoggingPayload
@ -25,8 +26,8 @@ class TestS3V2UnitTests:
"json.dumps(" not in source_code
), "S3 v2 should not use json.dumps directly"
@patch('asyncio.create_task')
@patch('litellm.integrations.s3_v2.CustomBatchLogger.periodic_flush')
@patch("asyncio.create_task")
@patch("litellm.integrations.s3_v2.CustomBatchLogger.periodic_flush")
def test_s3_v2_endpoint_url(self, mock_periodic_flush, mock_create_task):
"""testing s3 endpoint url"""
from unittest.mock import AsyncMock, MagicMock
@ -46,7 +47,7 @@ class TestS3V2UnitTests:
test_element = s3BatchLoggingElement(
s3_object_key="2025-09-14/test-key.json",
payload={"test": "data"},
s3_object_download_filename="test-file.json"
s3_object_download_filename="test-file.json",
)
# Test 1: Custom endpoint URL with bucket name
@ -55,7 +56,7 @@ class TestS3V2UnitTests:
s3_endpoint_url="https://s3.amazonaws.com",
s3_aws_access_key_id="test-key",
s3_aws_secret_access_key="test-secret",
s3_region_name="us-east-1"
s3_region_name="us-east-1",
)
s3_logger.async_httpx_client = AsyncMock()
@ -75,7 +76,7 @@ class TestS3V2UnitTests:
s3_endpoint_url="https://minio.example.com:9000",
s3_aws_access_key_id="minio-key",
s3_aws_secret_access_key="minio-secret",
s3_region_name="us-east-1"
s3_region_name="us-east-1",
)
s3_logger_minio.async_httpx_client = AsyncMock()
@ -86,15 +87,19 @@ class TestS3V2UnitTests:
call_args_minio = s3_logger_minio.async_httpx_client.put.call_args
assert call_args_minio is not None
url_minio = call_args_minio[0][0]
expected_minio_url = "https://minio.example.com:9000/litellm-logs/2025-09-14/test-key.json"
assert url_minio == expected_minio_url, f"Expected MinIO URL {expected_minio_url}, got {url_minio}"
expected_minio_url = (
"https://minio.example.com:9000/litellm-logs/2025-09-14/test-key.json"
)
assert (
url_minio == expected_minio_url
), f"Expected MinIO URL {expected_minio_url}, got {url_minio}"
# Test 3: Custom endpoint without bucket name (should fall back to default)
s3_logger_no_bucket = S3Logger(
s3_endpoint_url="https://s3.amazonaws.com",
s3_aws_access_key_id="test-key",
s3_aws_secret_access_key="test-secret",
s3_region_name="us-east-1"
s3_region_name="us-east-1",
)
s3_logger_no_bucket.async_httpx_client = AsyncMock()
@ -117,20 +122,27 @@ class TestS3V2UnitTests:
s3_endpoint_url="https://custom.s3.endpoint.com",
s3_aws_access_key_id="sync-key",
s3_aws_secret_access_key="sync-secret",
s3_region_name="us-east-1"
s3_region_name="us-east-1",
)
mock_sync_client = MagicMock()
mock_sync_client.put.return_value = mock_response
with patch('litellm.integrations.s3_v2._get_httpx_client', return_value=mock_sync_client):
with patch(
"litellm.integrations.s3_v2._get_httpx_client",
return_value=mock_sync_client,
):
s3_logger_sync.upload_data_to_s3(test_element)
call_args_sync = mock_sync_client.put.call_args
assert call_args_sync is not None
url_sync = call_args_sync[0][0]
expected_sync_url = "https://custom.s3.endpoint.com/sync-bucket/2025-09-14/test-key.json"
assert url_sync == expected_sync_url, f"Expected sync URL {expected_sync_url}, got {url_sync}"
expected_sync_url = (
"https://custom.s3.endpoint.com/sync-bucket/2025-09-14/test-key.json"
)
assert (
url_sync == expected_sync_url
), f"Expected sync URL {expected_sync_url}, got {url_sync}"
# Test 5: Download method with custom endpoint
s3_logger_download = S3Logger(
@ -138,7 +150,7 @@ class TestS3V2UnitTests:
s3_endpoint_url="https://download.s3.endpoint.com",
s3_aws_access_key_id="download-key",
s3_aws_secret_access_key="download-secret",
s3_region_name="us-east-1"
s3_region_name="us-east-1",
)
mock_download_response = MagicMock()
@ -147,18 +159,24 @@ class TestS3V2UnitTests:
s3_logger_download.async_httpx_client = AsyncMock()
s3_logger_download.async_httpx_client.get.return_value = mock_download_response
result = asyncio.run(s3_logger_download._download_object_from_s3("2025-09-14/download-test-key.json"))
result = asyncio.run(
s3_logger_download._download_object_from_s3(
"2025-09-14/download-test-key.json"
)
)
call_args_download = s3_logger_download.async_httpx_client.get.call_args
assert call_args_download is not None
url_download = call_args_download[0][0]
expected_download_url = "https://download.s3.endpoint.com/download-bucket/2025-09-14/download-test-key.json"
assert url_download == expected_download_url, f"Expected download URL {expected_download_url}, got {url_download}"
assert (
url_download == expected_download_url
), f"Expected download URL {expected_download_url}, got {url_download}"
assert result == {"downloaded": "data"}
@patch('asyncio.create_task')
@patch('litellm.integrations.s3_v2.CustomBatchLogger.periodic_flush')
@patch("asyncio.create_task")
@patch("litellm.integrations.s3_v2.CustomBatchLogger.periodic_flush")
def test_s3_v2_virtual_hosted_style(self, mock_periodic_flush, mock_create_task):
"""Test s3_use_virtual_hosted_style parameter for virtual-hosted-style URLs"""
from unittest.mock import AsyncMock, MagicMock
@ -178,7 +196,7 @@ class TestS3V2UnitTests:
test_element = s3BatchLoggingElement(
s3_object_key="2025-09-14/test-key.json",
payload={"test": "data"},
s3_object_download_filename="test-file.json"
s3_object_download_filename="test-file.json",
)
# Test 1: Virtual-hosted-style with custom endpoint
@ -188,7 +206,7 @@ class TestS3V2UnitTests:
s3_aws_access_key_id="test-key",
s3_aws_secret_access_key="test-secret",
s3_region_name="us-east-1",
s3_use_virtual_hosted_style=True
s3_use_virtual_hosted_style=True,
)
s3_logger_virtual.async_httpx_client = AsyncMock()
@ -199,8 +217,12 @@ class TestS3V2UnitTests:
call_args = s3_logger_virtual.async_httpx_client.put.call_args
assert call_args is not None
url = call_args[0][0]
expected_url = "https://test-bucket.s3.custom-endpoint.com/2025-09-14/test-key.json"
assert url == expected_url, f"Expected virtual-hosted-style URL {expected_url}, got {url}"
expected_url = (
"https://test-bucket.s3.custom-endpoint.com/2025-09-14/test-key.json"
)
assert (
url == expected_url
), f"Expected virtual-hosted-style URL {expected_url}, got {url}"
# Test 2: Path-style (default behavior with s3_use_virtual_hosted_style=False)
s3_logger_path = S3Logger(
@ -209,7 +231,7 @@ class TestS3V2UnitTests:
s3_aws_access_key_id="test-key",
s3_aws_secret_access_key="test-secret",
s3_region_name="us-east-1",
s3_use_virtual_hosted_style=False
s3_use_virtual_hosted_style=False,
)
s3_logger_path.async_httpx_client = AsyncMock()
@ -220,8 +242,12 @@ class TestS3V2UnitTests:
call_args_path = s3_logger_path.async_httpx_client.put.call_args
assert call_args_path is not None
url_path = call_args_path[0][0]
expected_path_url = "https://s3.custom-endpoint.com/test-bucket/2025-09-14/test-key.json"
assert url_path == expected_path_url, f"Expected path-style URL {expected_path_url}, got {url_path}"
expected_path_url = (
"https://s3.custom-endpoint.com/test-bucket/2025-09-14/test-key.json"
)
assert (
url_path == expected_path_url
), f"Expected path-style URL {expected_path_url}, got {url_path}"
# Test 3: Virtual-hosted-style with http protocol
s3_logger_http = S3Logger(
@ -230,7 +256,7 @@ class TestS3V2UnitTests:
s3_aws_access_key_id="minio-key",
s3_aws_secret_access_key="minio-secret",
s3_region_name="us-east-1",
s3_use_virtual_hosted_style=True
s3_use_virtual_hosted_style=True,
)
s3_logger_http.async_httpx_client = AsyncMock()
@ -241,8 +267,12 @@ class TestS3V2UnitTests:
call_args_http = s3_logger_http.async_httpx_client.put.call_args
assert call_args_http is not None
url_http = call_args_http[0][0]
expected_http_url = "http://http-bucket.minio.local:9000/2025-09-14/test-key.json"
assert url_http == expected_http_url, f"Expected virtual-hosted-style URL with http {expected_http_url}, got {url_http}"
expected_http_url = (
"http://http-bucket.minio.local:9000/2025-09-14/test-key.json"
)
assert (
url_http == expected_http_url
), f"Expected virtual-hosted-style URL with http {expected_http_url}, got {url_http}"
# Test 4: Sync upload method with virtual-hosted-style
s3_logger_sync_virtual = S3Logger(
@ -251,20 +281,27 @@ class TestS3V2UnitTests:
s3_aws_access_key_id="sync-key",
s3_aws_secret_access_key="sync-secret",
s3_region_name="us-east-1",
s3_use_virtual_hosted_style=True
s3_use_virtual_hosted_style=True,
)
mock_sync_client = MagicMock()
mock_sync_client.put.return_value = mock_response
with patch('litellm.integrations.s3_v2._get_httpx_client', return_value=mock_sync_client):
with patch(
"litellm.integrations.s3_v2._get_httpx_client",
return_value=mock_sync_client,
):
s3_logger_sync_virtual.upload_data_to_s3(test_element)
call_args_sync = mock_sync_client.put.call_args
assert call_args_sync is not None
url_sync = call_args_sync[0][0]
expected_sync_url = "https://sync-bucket.storage.example.com/2025-09-14/test-key.json"
assert url_sync == expected_sync_url, f"Expected virtual-hosted-style sync URL {expected_sync_url}, got {url_sync}"
expected_sync_url = (
"https://sync-bucket.storage.example.com/2025-09-14/test-key.json"
)
assert (
url_sync == expected_sync_url
), f"Expected virtual-hosted-style sync URL {expected_sync_url}, got {url_sync}"
# Test 5: Download method with virtual-hosted-style
s3_logger_download_virtual = S3Logger(
@ -273,25 +310,34 @@ class TestS3V2UnitTests:
s3_aws_access_key_id="download-key",
s3_aws_secret_access_key="download-secret",
s3_region_name="us-east-1",
s3_use_virtual_hosted_style=True
s3_use_virtual_hosted_style=True,
)
mock_download_response = MagicMock()
mock_download_response.status_code = 200
mock_download_response.json = MagicMock(return_value={"downloaded": "data"})
s3_logger_download_virtual.async_httpx_client = AsyncMock()
s3_logger_download_virtual.async_httpx_client.get.return_value = mock_download_response
s3_logger_download_virtual.async_httpx_client.get.return_value = (
mock_download_response
)
result = asyncio.run(s3_logger_download_virtual._download_object_from_s3("2025-09-14/download-test-key.json"))
result = asyncio.run(
s3_logger_download_virtual._download_object_from_s3(
"2025-09-14/download-test-key.json"
)
)
call_args_download = s3_logger_download_virtual.async_httpx_client.get.call_args
assert call_args_download is not None
url_download = call_args_download[0][0]
expected_download_url = "https://download-bucket.download.endpoint.com/2025-09-14/download-test-key.json"
assert url_download == expected_download_url, f"Expected virtual-hosted-style download URL {expected_download_url}, got {url_download}"
assert (
url_download == expected_download_url
), f"Expected virtual-hosted-style download URL {expected_download_url}, got {url_download}"
assert result == {"downloaded": "data"}
@pytest.mark.asyncio
async def test_async_log_event_skips_when_standard_logging_object_missing():
"""
@ -334,7 +380,9 @@ async def test_async_log_event_skips_when_standard_logging_object_missing():
# Nothing should have been queued (catches the case where code falls
# through without returning and appends None to the queue)
assert len(logger.log_queue) == 0, "log_queue should be empty when standard_logging_object is missing"
assert (
len(logger.log_queue) == 0
), "log_queue should be empty when standard_logging_object is missing"
@pytest.mark.asyncio
@ -347,15 +395,24 @@ async def test_strip_base64_removes_file_and_nontext_entries():
"role": "user",
"content": [
{"type": "text", "text": "Hello world"},
{"type": "image", "file": {"file_data": "data:image/png;base64,AAAA"}},
{"type": "file", "file": {"file_data": "data:application/pdf;base64,BBBB"}},
{
"type": "image",
"file": {"file_data": "data:image/png;base64,AAAA"},
},
{
"type": "file",
"file": {"file_data": "data:application/pdf;base64,BBBB"},
},
],
},
{
"role": "assistant",
"content": [
{"type": "text", "text": "Response"},
{"type": "audio", "file": {"file_data": "data:audio/wav;base64,CCCC"}},
{
"type": "audio",
"file": {"file_data": "data:audio/wav;base64,CCCC"},
},
],
},
]
@ -451,7 +508,7 @@ async def test_strip_base64_mixed_nested_objects():
async def test_s3_verify_false_handling():
"""
Test that s3_verify=False is properly handled and not treated as None.
This is a regression test for the bug where s3_verify=False was being
ignored because 'False or s3_verify' would evaluate to s3_verify (None).
"""
@ -469,25 +526,35 @@ async def test_s3_verify_false_handling():
"s3_verify": False, # This should NOT be ignored
"s3_use_ssl": False, # This should also NOT be ignored
}
with patch('asyncio.create_task'):
with patch('litellm.integrations.s3_v2.get_async_httpx_client') as mock_get_client:
with patch("asyncio.create_task"):
with patch(
"litellm.integrations.s3_v2.get_async_httpx_client"
) as mock_get_client:
mock_client = AsyncMock()
mock_get_client.return_value = mock_client
# Create logger
logger = S3Logger()
# Verify s3_verify is False, not None
assert logger.s3_verify is False, f"Expected s3_verify=False, got {logger.s3_verify}"
assert logger.s3_use_ssl is False, f"Expected s3_use_ssl=False, got {logger.s3_use_ssl}"
assert (
logger.s3_verify is False
), f"Expected s3_verify=False, got {logger.s3_verify}"
assert (
logger.s3_use_ssl is False
), f"Expected s3_use_ssl=False, got {logger.s3_use_ssl}"
# Verify that get_async_httpx_client was called with ssl_verify=False
mock_get_client.assert_called_once()
call_kwargs = mock_get_client.call_args.kwargs
assert 'params' in call_kwargs, "params should be passed to get_async_httpx_client"
assert call_kwargs['params'] == {'ssl_verify': False}, f"Expected ssl_verify=False in params, got {call_kwargs.get('params')}"
assert (
"params" in call_kwargs
), "params should be passed to get_async_httpx_client"
assert call_kwargs["params"] == {
"ssl_verify": False
}, f"Expected ssl_verify=False in params, got {call_kwargs.get('params')}"
# Clean up
litellm.s3_callback_params = None
@ -508,27 +575,31 @@ async def test_s3_verify_none_handling():
"s3_aws_secret_access_key": "test-secret",
"s3_region_name": "us-east-1",
}
with patch('asyncio.create_task'):
with patch('litellm.integrations.s3_v2.get_async_httpx_client') as mock_get_client:
with patch("asyncio.create_task"):
with patch(
"litellm.integrations.s3_v2.get_async_httpx_client"
) as mock_get_client:
mock_client = AsyncMock()
mock_get_client.return_value = mock_client
# Create logger without explicit s3_verify
logger = S3Logger()
# Verify s3_verify is None (default)
assert logger.s3_verify is None, f"Expected s3_verify=None, got {logger.s3_verify}"
assert (
logger.s3_verify is None
), f"Expected s3_verify=None, got {logger.s3_verify}"
# Verify that get_async_httpx_client was called
mock_get_client.assert_called_once()
call_kwargs = mock_get_client.call_args.kwargs
# When s3_verify is None, params={'ssl_verify': None} which is fine - uses default behavior
# The important thing is it's not False
if 'params' in call_kwargs and call_kwargs['params'] is not None:
assert call_kwargs['params'].get('ssl_verify') is None
if "params" in call_kwargs and call_kwargs["params"] is not None:
assert call_kwargs["params"].get("ssl_verify") is None
# Either params is None or params={'ssl_verify': None} is acceptable
# Clean up
litellm.s3_callback_params = None
@ -537,7 +608,7 @@ async def test_s3_verify_none_handling():
async def test_s3_verify_false_creates_httpx_client_with_verify_false():
"""
Test that when s3_verify=False, the actual httpx client has verify=False.
This validates that ssl_verify=False flows through to the httpx.AsyncClient.
"""
from unittest.mock import patch
@ -553,22 +624,24 @@ async def test_s3_verify_false_creates_httpx_client_with_verify_false():
"s3_region_name": "us-east-1",
"s3_verify": False,
}
with patch('asyncio.create_task'):
with patch("asyncio.create_task"):
# Create logger - this creates the httpx client
logger = S3Logger()
# Verify the logger has s3_verify=False
assert logger.s3_verify is False
# Check the actual httpx client has verify=False
# The async_httpx_client.client is the actual httpx.AsyncClient
if hasattr(logger.async_httpx_client, 'client'):
if hasattr(logger.async_httpx_client, "client"):
httpx_client = logger.async_httpx_client.client
# Check the _verify attribute (httpx internal)
if hasattr(httpx_client, '_verify'):
assert httpx_client._verify is False, f"Expected httpx client _verify=False, got {httpx_client._verify}"
if hasattr(httpx_client, "_verify"):
assert (
httpx_client._verify is False
), f"Expected httpx client _verify=False, got {httpx_client._verify}"
# Clean up
litellm.s3_callback_params = None
@ -592,38 +665,40 @@ async def test_s3_verify_false_async_client():
"s3_region_name": "us-east-1",
"s3_verify": False,
}
with patch('asyncio.create_task'):
with patch("asyncio.create_task"):
logger = S3Logger()
# Verify s3_verify is False
assert logger.s3_verify is False
# Create test element
test_element = s3BatchLoggingElement(
s3_object_key="2025-11-03/test-key.json",
payload={"test": "data"},
s3_object_download_filename="test-file.json"
s3_object_download_filename="test-file.json",
)
# Mock the async httpx client's put method
mock_response = MagicMock()
mock_response.status_code = 200
mock_response.raise_for_status = MagicMock()
logger.async_httpx_client.put = AsyncMock(return_value=mock_response)
# Call async upload
await logger.async_upload_data_to_s3(test_element)
# Verify put was called
assert logger.async_httpx_client.put.called
# Check that the async httpx client was created with verify=False
if hasattr(logger.async_httpx_client, 'client'):
if hasattr(logger.async_httpx_client, "client"):
httpx_client = logger.async_httpx_client.client
if hasattr(httpx_client, '_verify'):
assert httpx_client._verify is False, f"Expected async httpx client _verify=False, got {httpx_client._verify}"
if hasattr(httpx_client, "_verify"):
assert (
httpx_client._verify is False
), f"Expected async httpx client _verify=False, got {httpx_client._verify}"
# Clean up
litellm.s3_callback_params = None
@ -636,8 +711,14 @@ async def test_strip_base64_recursive_redaction():
{
"content": [
{"type": "text", "text": "normal text"},
{"type": "text", "text": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUg"},
{"type": "text", "text": "Nested: {'data': 'data:application/pdf;base64,AAA...'}"},
{
"type": "text",
"text": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUg",
},
{
"type": "text",
"text": "Nested: {'data': 'data:application/pdf;base64,AAA...'}",
},
{"file": {"file_data": "data:application/pdf;base64,AAAA"}},
{"metadata": {"preview": "data:audio/mp3;base64,AAAAA=="}},
]
@ -653,6 +734,7 @@ async def test_strip_base64_recursive_redaction():
# Base64 redacted globally
import json
for c in content:
if isinstance(c, dict):
s = json.dumps(c).lower()
@ -660,14 +742,19 @@ async def test_strip_base64_recursive_redaction():
assert "base64," not in s, f"Found real base64 blob in: {s}"
# --------------------------------------------------------------
# Shared fixture that silences asyncio.create_task during tests
# --------------------------------------------------------------
@pytest.fixture(autouse=True)
def patch_asyncio_create_task():
"""Prevent 'no running event loop' errors when S3Logger calls asyncio.create_task()."""
with patch("asyncio.create_task"):
def _discard_task(coro):
if hasattr(coro, "close"):
coro.close()
return None
with patch("asyncio.create_task", side_effect=_discard_task):
yield
@ -687,7 +774,7 @@ def patch_asyncio_create_task():
],
)
def test_s3_object_key_prefix_combinations(
use_team_prefix, use_key_prefix, team_alias, key_alias, expected_prefix
use_team_prefix, use_key_prefix, team_alias, key_alias, expected_prefix
):
"""
Validate correct S3 prefix composition for team alias + key alias combinations.
@ -796,3 +883,43 @@ async def test_combined_prefix_reflects_in_s3_object_key():
result = logger.create_s3_batch_logging_element(datetime.utcnow(), payload)
key = result.s3_object_key
assert "myteam/apikey/" in key, f"Expected both prefixes in key: {key}"
def test_get_s3_object_key_truncates_long_filename_components():
start_time = datetime(2026, 3, 26, 12, 51, 27, 995047)
long_response_id = "resp_" + ("a" * 400)
original_file_name = f"time-{start_time.strftime('%H-%M-%S-%f')}_{long_response_id}"
key = get_s3_object_key(
s3_path="",
prefix="",
start_time=start_time,
s3_file_name=original_file_name,
)
filename = key.split("/")[-1]
assert len(f"{original_file_name}.json") > 255
assert len(filename) <= 255
assert filename.endswith(".json")
assert filename.startswith(f"time-{start_time.strftime('%H-%M-%S-%f')}_resp_")
def test_create_s3_batch_logging_element_limits_long_ids():
start_time = datetime(2026, 3, 26, 12, 51, 27, 995047)
long_response_id = "resp_" + ("a" * 400)
payload = StandardLoggingPayload(
id=long_response_id,
metadata={},
messages=[],
)
with patch(
"litellm.integrations.s3_v2.CustomBatchLogger.periodic_flush",
return_value=None,
):
logger = S3Logger()
result = logger.create_s3_batch_logging_element(start_time, payload)
assert result is not None
assert len(result.s3_object_key.split("/")[-1]) <= 255
assert result.s3_object_key.endswith(".json")