Skip to content

fix(proxy): unregister logging callbacks removed from the stored config - #43429

Merged
yuneng-berri merged 2 commits into
rc/1.103.0from
litellm_rc_1_103_0_callback_delete_sticks
Sep 27, 2026
Merged

yuneng-berri merged 2 commits into
rc/1.103.0from
litellm_rc_1_103_0_callback_delete_sticks

Conversation

@yuneng-berri

@yuneng-berri yuneng-berri commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • On rc/1.103.0, deleting a logging callback reports success but it keeps exporting
  • The deleted callback stays listed in the Admin UI's callback list

How it solves it:

User Flow

Before: an admin deletes a logging callback, gets a success message, and the proxy keeps exporting to it

  1. They add langfuse_otel with POST http://localhost:4000/config/update and traffic starts reaching Langfuse
  2. They delete it with POST http://localhost:4000/config/callback/delete and get Successfully deleted callback: langfuse_otel
  3. GET http://localhost:4000/get/config/callbacks keeps listing langfuse_otel on every worker
  4. New chat completions keep exporting to Langfuse

After: the delete sticks on every worker

  1. They add langfuse_otel with POST http://localhost:4000/config/update and traffic starts reaching Langfuse
  2. They delete it with POST http://localhost:4000/config/callback/delete and get Successfully deleted callback: langfuse_otel
  3. GET http://localhost:4000/get/config/callbacks returns an empty list on every worker within 30 seconds
  4. New chat completions export nothing, and adding the callback back makes exports resume

Relevant issues

Backport of #43428

Affected release

The read-only listing appears since v1.102.0-rc.1 (#38974). The leftover exports are older and also happen on v1.101.0

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

The source and test additions are line for line the same as #43428, both commits included. The only cherry-pick conflict was test file placement: the anchor test on main does not exist on rc, so the new tests sit at the same spot without it. On this branch the new tests fail with the source change reverted (8 failed) and pass with it, and test_proxy_server.py, test_proxy_config.py and test_delete_callbacks_endpoint.py pass (724 tests)

Screenshots / Proof of Fix

Proxy started with --num_workers 2, a local Postgres, STORE_MODEL_IN_DB=True, a mock_response model named mock-gpt, and LANGFUSE_OTEL_HOST pointing at a local OTLP capture server that counts export POSTs. No real provider calls are needed since the bug is in callback registration, not the LLM call. "OTLP exports" counts export batches the capture server received

Before (cc111d1)

  1. curl -s localhost:4000/config/update -H "Authorization: Bearer $LITELLM_MASTER_KEY" -H "Content-Type: application/json" -d '{"litellm_settings":{"success_callback":["langfuse_otel"]}}' returns {"message":"Config updated successfully"}, and 6 chats produce 1 export batch
  2. curl -s localhost:4000/config/callback/delete -H "Authorization: Bearer $LITELLM_MASTER_KEY" -H "Content-Type: application/json" -d '{"callback_name":"langfuse_otel"}' returns {"message":"Successfully deleted callback: langfuse_otel","removed_callback":"langfuse_otel","remaining_callbacks":[],...}
  3. curl -s localhost:4000/get/config/callbacks -H "Authorization: Bearer $LITELLM_MASTER_KEY" still returns [{"name": "langfuse_otel", ..., "type": "success"}]
  4. After waiting 35 seconds for every worker's periodic sync, 10 chats produce 2 export batches, and 4 more /get/config/callbacks calls across workers all return ["langfuse_otel"]

After (8823a69)

  1. Same /config/update call returns {"message":"Config updated successfully"}, and 6 chats produce 1 export batch
  2. Same /config/callback/delete call returns the same success message
  3. /get/config/callbacks returns []
  4. After waiting 35 seconds, 10 chats produce 0 export batches, and 4 more /get/config/callbacks calls across workers all return []
  5. Re-adding with the same /config/update call, then waiting 35 seconds, 10 chats produce 2 export batches

Type

🐛 Bug Fix

Caveats (if any)

Medium

  • A callback listed in both the YAML file and the DB survives a DB delete
    • The YAML registered it first, so the sync never owned it
  • Other workers drop a deleted callback on their next sync, up to 30 seconds later
  • A DB-added callback is now unregistered when the YAML file starts setting the same key
    • YAML wins that key, so a restart already dropped it before this PR
  • fix(proxy): unregister logging callbacks removed from the stored config #43428 has not merged on main yet, so its CI and review results still apply here

Low

  • Non-string entries in the stored callback lists are now ignored instead of registered

POST /config/callback/delete saved the config and resynced, but the resync only
ever added callbacks, so a deleted callback kept exporting and kept showing in
/get/config/callbacks as read-only on every worker.

ProxyConfig now tracks which callback list entries each DB config sync
registered and unregisters them once the stored config stops listing them.
Callbacks it did not register (YAML, code) are never touched, and a failed
config load skips the sync instead of treating the config as empty.
@greptile-apps

greptile-apps Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[Medium risk] Fixes callback cleanup when database config updates.

The PR should not merge until deleting a DB callback is shown to preserve the same callback when it was registered by code.

Findings

  1. P1 Code callback loses ownership ▶

Summary

The PR tracks callbacks added during DB config synchronization and unregisters them when the resolved config drops their keys. It also preserves callbacks when config loading fails and adds removal and ownership tests.

Reviews (1) · Last reviewed commit: "fix(proxy): unregister logging callbacks..."

Comment on lines +7058 to +7062
before_ids: Final = frozenset((list_name, id(entry)) for list_name, entry in before)
return tuple(
(list_name, entry)
for list_name, entry in _registered_callback_entries()
if (list_name, id(entry)) not in before_ids

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Code callback loses ownership

If code already registered "langfuse_otel" as a success callback, registering the same callback from the DB replaces that string with a logger instance. This snapshot records the instance as DB-owned. When an admin deletes the DB entry, the new removal logic deletes the instance, so the code-configured callback stops running too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Valid, reproduced it. Fixed in #43434 for rc and on #43428 for main: the sync records replaced entries and restores them on delete

@yuneng-berri
yuneng-berri merged commit c991f4b into rc/1.103.0 Sep 27, 2026
8 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_rc_1_103_0_callback_delete_sticks branch September 27, 2026 07:50
yuneng-berri added a commit that referenced this pull request Sep 27, 2026
…ig (#43429) (#43432)

* fix(proxy): unregister logging callbacks removed from the stored config

POST /config/callback/delete saved the config and resynced, but the resync only
ever added callbacks, so a deleted callback kept exporting and kept showing in
/get/config/callbacks as read-only on every worker.

ProxyConfig now tracks which callback list entries each DB config sync
registered and unregisters them once the stored config stops listing them.
Callbacks it did not register (YAML, code) are never touched, and a failed
config load skips the sync instead of treating the config as empty.

* refactor(proxy): keep callback sync comprehensions to one for clause

(cherry picked from commit c991f4b)
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