Skip to content

Conversation

@Harshit28j
Copy link
Collaborator

@Harshit28j Harshit28j commented Feb 10, 2026

Relevant issues

LIT-1899

Pre-Submission checklist

  • I have Added testing in the tests/litellm/ directory, Adding at least 1 test is a hard requirement
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem

CI (LiteLLM team)

  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Type

🆕 New Feature
✅ Test

Changes

image
  • Add s3_log_prompts_only behavior to store only prompt messages (no response/metadata) in S3 logs.
  • Add regression tests for prompts-only payloads in both s3_v2 and legacy s3 loggers.

@vercel
Copy link

vercel bot commented Feb 10, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Feb 10, 2026 4:34pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 10, 2026

Greptile Overview

Greptile Summary

This PR adds an s3_log_prompts_only option to both the legacy litellm/integrations/s3.py logger and the newer litellm/integrations/s3_v2.py batch logger. When enabled, the payload written to S3 is reduced to just { "messages": [...] }, omitting the response and other metadata.

The feature is exercised by new/updated tests: a new legacy S3 test that asserts only prompt messages are uploaded, and a new s3_v2 test that asserts create_s3_batch_logging_element() produces the prompts-only payload.

Main issue to address before merge: in the legacy S3Logger, the new flag is only read from litellm.s3_callback_params and is forced to False otherwise, so passing s3_log_prompts_only=True directly to the constructor has no effect (unlike s3_v2, which supports the constructor arg).

Confidence Score: 4/5

  • Mostly safe to merge, but fix the legacy S3Logger flag wiring first.
  • Changes are localized and covered by tests for both s3 implementations. The main correctness gap is that the legacy S3Logger ignores s3_log_prompts_only when passed as a constructor argument, which will surprise direct instantiations and makes behavior inconsistent across s3 vs s3_v2.
  • litellm/integrations/s3.py

Important Files Changed

Filename Overview
litellm/integrations/s3.py Adds s3_log_prompts_only flag and strips S3 payload down to messages when enabled; however the flag is only read from litellm.s3_callback_params and is ignored for direct constructor usage.
litellm/integrations/s3_v2.py Plumbs s3_log_prompts_only through init params and create_s3_batch_logging_element() to log only messages to S3; also refactors s3_verify/s3_use_ssl param handling and minor formatting.
tests/test_litellm/integrations/test_s3.py New unit test verifies legacy s3 logger writes only {messages: ...} to S3 when s3_log_prompts_only is enabled via litellm.s3_callback_params.
tests/test_litellm/integrations/test_s3_v2.py Adds prompts-only payload test for s3_v2 logger and various formatting tweaks; existing tests still rely on patching asyncio.create_task to avoid background flush tasks.

Sequence Diagram

sequenceDiagram
    participant App as LiteLLM
    participant S3 as S3Logger (legacy)
    participant S3v2 as S3Logger (v2)
    participant AWS as S3-compatible endpoint

    App->>S3: log_event(kwargs, start_time, end_time)
    S3->>S3: read litellm.s3_callback_params
    alt s3_log_prompts_only enabled
        S3->>S3: payload_dict = {messages}
    else disabled
        S3->>S3: payload_dict = full StandardLoggingPayload
    end
    S3->>AWS: put_object(Body=safe_dumps(payload_dict))

    App->>S3v2: async_log_success_event(kwargs, start_time, end_time)
    S3v2->>S3v2: _init_s3_params() (reads callback params + ctor args)
    S3v2->>S3v2: create_s3_batch_logging_element()
    alt s3_log_prompts_only enabled
        S3v2->>S3v2: element.payload = {messages}
    else disabled
        S3v2->>S3v2: element.payload = full StandardLoggingPayload
    end
    S3v2->>S3v2: queue element for batch flush
    S3v2->>AWS: PUT signed request (async_upload_data_to_s3)
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 10, 2026

Additional Comments (1)

litellm/integrations/s3.py
Flag ignored for direct init

S3Logger.__init__() only reads s3_log_prompts_only from litellm.s3_callback_params and forces it to False otherwise, so passing S3Logger(s3_log_prompts_only=True) has no effect. This makes the new behavior unusable for callers who construct the logger directly (and is inconsistent with s3_v2.S3Logger, which supports the constructor arg). Consider wiring the kwarg through (or at least not overriding it to False) so the flag works outside global config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant