Repository navigation
fix(proxy): surface runtime-registered callbacks in UI Logging page - #38974
yucheng-berri merged 16 commits into
Conversation
Greptile SummaryThis PR surfaces active runtime callbacks that are missing from saved dashboard configuration and marks those rows read-only. It also normalizes callback aliases, excludes internal hooks, preserves secret redaction, and hides UI actions for runtime-only rows Confidence Score: 5/5The PR appears safe to merge, with no outstanding correctness, security, or repository-rule findings The latest alias-filtering change uses the canonical names produced by the callback registry, while explicit internal callback exclusions remain intact. All four previous threads were manually resolved without explanatory replies and therefore are not outstanding
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/logging_callback_manager.py | Exposes callback objects and preserves self-assigned OpenTelemetry integration names |
| litellm/proxy/proxy_server.py | Adds normalized runtime callback discovery, deduplication, internal-hook filtering, and read-only response rows |
| tests/test_litellm/proxy/proxy_server/test_routes_config.py | Covers runtime-only callbacks, aliases, dotted paths, internal hooks, callback types, and secret redaction |
| ui/litellm-dashboard/src/components/Settings/LoggingAndAlerts/LoggingCallbacks/LoggingCallbacksTableColumns.tsx | Replaces action controls with a read-only label for runtime-only callbacks |
| ui/litellm-dashboard/src/components/Settings/LoggingAndAlerts/LoggingCallbacks/types.ts | Extends callback rows with the optional read-only state |
Reviews (11): Last reviewed commit: "fix(proxy): keep runtime-only s3 and sqs..." | Re-trigger Greptile
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
Devin Review found 3 potential issues.
3 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| _normalized_runtime = _normalize_callback_alias(_runtime_cb_name) | ||
| # Skip if this callback is in config or already appended. | ||
| if _normalized_runtime not in _added_normalized_names: |
There was a problem hiding this comment.
🟡 Distinct callback modes disappear
When configured and runtime registrations share a name but handle different outcomes, _added_normalized_names drops the runtime registration. The Logging page hides it
Learn more
Deduplicate callback rows by both canonical callback identity and effective event type, rather than callback name alone. Preserve the existing behavior where the same callback can appear once for success and once for failure. Consider how a success_and_failure runtime registration overlaps configured success or failure registrations, and add regression tests covering configured success plus runtime failure and the inverse.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Matching on name plus mode would add a duplicate read-only row whenever config lists one callback under both success_callback and failure_callback, since the runtime reports that as success_and_failure.
| return str(callback_name) | ||
| _alias_map: Final[dict[str, str]] = { | ||
| "opentelemetry": "otel", | ||
| "s3_v2": "s3", |
There was a problem hiding this comment.
The alias map exists so runtime rows land on the names the dashboard knows (otel, s3, generic_api). Two storage sinks configured separately still show as two editable rows.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
2ad0a58 to
fd75b76
Compare
|
@greptileai please review the current head fd75b76 |
|
bugbot run |
|
@greptileai please review the current head 85e41ae |
|
bugbot run |
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
Devin Review found 1 new potential issue.
3 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| return (callback,) | ||
| if callback is None: | ||
| return () | ||
| return tuple(callback) if isinstance(callback, (list, dict)) else () |
There was a problem hiding this comment.
🟡 Dictionary callback deletion fails
With dictionary-shaped settings, normalize_callback marks each key editable. Deleting that row reaches list-only delete_callback and returns an error
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Dict-shaped success_callback rows were already listed as editable on the base branch, and the delete route was list-only before this PR. Unchanged here, out of scope.
|
Dict deletion (Devin comment 3898539019): pre-existing on staging. Base delete_callback is byte-identical; dict-shaped configs show as editable rows in staging too, so the delete failure exists before this PR. Custom callbacks listed twice (Devin comments 3898496017 + 3896630196): fixed in latest commit 79bffe1. Dotted-path callbacks now marked read-only to prevent duplicate display and UI action failures. Configured otel alias hides sibling (Devin 3898496017): inherent to the design; registry maps multiple otel names to one class, so configured one hides its siblings. This is the intentional tradeoff to fix the ArizeLogger duplicate shown in the ticket's bug. Base had the same behavior (all instances of a class hidden as duplicates). Callback modes disappear (Devin 3898495881) + s3_v2/s3 collapse (Devin 3898630391): stale anchors to reworked code. Current behavior: runtime row hidden only when its normalized name matches a configured name. No regression from base. |
|
@greptileai please review the current head 79bffe1 |
|
bugbot run |
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
Devin Review found 1 new potential issue.
4 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| configured_modules: Final = frozenset( | ||
| configured_name.rsplit(".", 1)[0] for configured_name in configured_callback_names if "." in configured_name | ||
| ) | ||
| dotted_instance_names: Final = frozenset( | ||
| _callback_display_name(instance) | ||
| for instance in litellm.logging_callback_manager.get_custom_loggers_for_type(CustomLogger) | ||
| if type(instance).__module__ in configured_modules |
There was a problem hiding this comment.
🟡 Sibling callbacks disappear from inventory
When one dotted callback is configured, configured_modules hides every custom logger from that module. Separately registered siblings disappear from the Logging page
Learn more
Dotted-path deduplication in litellm/proxy/proxy_server.py:_hidden_runtime_callback_names uses only the configured module name. This suppresses every CustomLogger class defined in that module, even when config points to just one exported instance and another instance from the same module was registered independently at runtime. Deduplicate against the exact object resolved by each configured dotted path, or preserve enough callback identity during inventory construction to match only the configured instance. Add a regression test with two different CustomLogger instances from one module, only one present in config.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Matching on the exact configured object would mean importing user modules on every dashboard request. Two independently registered loggers from one module with one in config is rare; module matching is the pragmatic dedup.
|
@greptileai please review the current head acf2934 |
|
bugbot run |
…acks Config-file callbacks fire at runtime but never appear in the UI Logging and Alerts page because /get/config/callbacks only reads the DB-merged config. Append runtime-registered callbacks from LoggingCallbackManager as read-only rows, deduplicated against configured rows via alias normalization. UI hides edit/delete/test actions for read-only rows.
- Filter _PROXY*, ShadowEval, ServiceLogging, SkillsInjection, ResponsesID prefixes - Update test to exclude read_only rows from count assertions - Still allows deployment/guardrail callbacks to surface if configured Note: comprehensive internal-hook filtering deferred, live-pr-risk will observe real behavior on running proxy.
…n tests - Line-concat type error: normalize_callback now returns empty list for non-list types (dict/tuple/set) instead of passing through unchanged; prevents TypeError when config values are non-list - Test quality TQ005: replace manual try/finally save-restore of litellm.callbacks with monkeypatch.setattr in test_get_config_callbacks_appends_runtime_only_callbacks and test_get_config_callbacks_redacts_runtime_only_row_secrets_for_view_only_admin - Ruff format: wrap _internal_callback_prefixes tuple and isinstance check across multiple lines to respect 120-char limit - All three new tests pass
…ntory Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
…ed one arize, weave_otel and langfuse_otel all initialize OpenTelemetry subclasses, so hiding runtime callbacks by configured class made one saved OTel callback swallow its YAML siblings. Match runtime instances by their own callback_name and only fall back to class identity for bare OpenTelemetry Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…entory Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
…ntory _is_litellm_internal_callback checked registry membership with the display alias (s3, sqs), which is not a registry key, so runtime-only S3Logger and SQSLogger instances were classified as internal and dropped from /get/config/callbacks. Check the registered name instead and cover both loggers in the internal-exclusion regression test Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d63e655. Configure here.
00b6318
into
litellm_internal_staging
TLDR
Problem this solves:
How it solves it:
GET /get/config/callbacksappends every active callback missing from the saved configread_only: trueand the UI hides their actionsUser Flow
Before: an admin adds a second callback in the dashboard and the YAML one vanishes from the page even though both keep logging
success_callback: ["langsmith"]in the YAML configlangfuse, so they conclude Langsmith was overwrittenAfter: the page lists both callbacks, and the YAML one is marked read only
success_callback: ["langsmith"]in the YAML configlangfuseandlangsmithwith"read_only": trueRelevant issues
Linear ticket
Resolves LIT-5281
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<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@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Shared setup: two proxies from the same Postgres database, one at the merge base and one at the PR tip, each started with
--num_workers 2. The YAML config hassuccess_callback: ["langsmith"],callbacks: ["arize", "weave_otel", "s3_v2"]andstore_model_in_db: true.langfuse_otelwas added through the dashboard payload (POST /config/update), so it lives in the saved config only. LangSmith EU, Arize, Weave and Langfuse Cloud are real destinations and the model is realopenai/gpt-5.4-mini. Thes3_v2callback points at a bucket this rig has no access to (the upload fails with a 404 in the proxy log), it is there to prove thes3_v2->s3alias row is listed.$PORTis 20381 for Before and 20383 for After. Dashboard screenshots for this run, plus an earlier Langfuse-only run with Langfuse UI screenshots, are in PR commentsBefore (ef3a3c1)
Real request reaches the YAML callbacks
curl -s localhost:$PORT/v1/chat/completions -H "Authorization: Bearer $LITELLM_MASTER_KEY" -H 'Content-Type: application/json' -d '{"model":"gpt-5.4-mini","messages":[{"role":"user","content":"Reply with exactly: ok"}],"metadata":{"tags":["lit5281-1789004664-$PORT"]}}'{"model": "gpt-5.4-mini", "content": "ok"}curl -s -X POST "$LANGSMITH_BASE_URL/api/v1/runs/query" -H "x-api-key: $LANGSMITH_API_KEY" -d '{"session":["<project id>"],"filter":"has(tags, \"lit5281-1789004664-20381\")"}'returns oneLLMRunwithstatus: successwhose output id matches the completion idcurl -s -u "api:$WANDB_API_KEY" -X POST https://trace.wandb.ai/calls/stream_query -d '{"project_id":"$WANDB_PROJECT_ID","limit":400}'contains onelitellm_requestcall carrying that tag, exported by the YAMLweave_otelcallbackCallback inventory
for i in 1 2 3 4 5 6; do curl -s localhost:$PORT/get/config/callbacks -H "Authorization: Bearer $LITELLM_MASTER_KEY"; donelangsmithandlangfuse_otelonly. The YAMLarize,weave_otelands3_v2callbacks that just ran are missingDashboard
Langsmithandlangfuse_otel, each with the actions menuDeleting and re-saving callbacks from the dashboard
curl -s -X POST localhost:$PORT/config/callback/delete -H "Authorization: Bearer $LITELLM_MASTER_KEY" -H 'Content-Type: application/json' -d '{"callback_name":"arize"}'404 {"detail":{"error":"Callback 'arize' not found in active configuration"}}, inventory unchanged"callback_name":"langfuse_otel"is also404because the delete route only searchessuccess_callbackand this row was saved undercallbacks(pre-existing, unchanged)POST /config/updatewith{"litellm_settings":{"success_callback":["langfuse_otel"],"failure_callback":["langfuse_otel"]}, "environment_variables": {...}}returns200. Within the config sync interval the inventory is threelangfuse_otelrows (success,failure,success_and_failure) andlangsmithis gone, even though the YAML LangSmith callback is still registered. This is the ticket scenarioCache enabled does not leak internal hooks
cache: trueunderlitellm_settingson port 20387, two identical chat completions, the second carries anx-litellm-cache-keyheadercurl -s localhost:20387/callbacks/list -H "Authorization: Bearer $LITELLM_MASTER_KEY"showscache,_ProxyDBLoggerand the deployment hooks as activecurl -s localhost:20387/get/config/callbacks ...listslangsmithandlangfuse_otelonly, nocacherowError and auth paths
curl -s localhost:$PORT/v1/chat/completions ... -d '{"model":"does-not-exist","messages":[{"role":"user","content":"hi"}]}'returns400 Invalid model name passed in model=does-not-existand the inventory is unchangedcurl -s -o /dev/null -w "%{http_code}\n" localhost:$PORT/get/config/callbacksis401POST /user/newwithuser_roleinternal_userandinternal_user_viewerget401 Only proxy admin can be used ..., aproxy_admin_vieweruser gets200withLANGSMITH_API_KEY,LANGFUSE_PUBLIC_KEYandLANGFUSE_SECRET_KEYasREDACTEDandLANGSMITH_PROJECT,LANGFUSE_HOSTvisibleAfter (d63e655)
Real request reaches the YAML callbacks
curl -s localhost:$PORT/v1/chat/completions -H "Authorization: Bearer $LITELLM_MASTER_KEY" -H 'Content-Type: application/json' -d '{"model":"gpt-5.4-mini","messages":[{"role":"user","content":"Reply with exactly: ok"}],"metadata":{"tags":["lit5281-1789004664-$PORT"]}}'{"model": "gpt-5.4-mini", "content": "ok"}lit5281-1789004664-20383returns oneLLMRunwithstatus: successwhose output id matches the completion idcalls/stream_querycontains onelitellm_requestcall carrying that tagCallback inventory
for i in 1 2 3 4 5 6; do curl -s localhost:$PORT/get/config/callbacks -H "Authorization: Bearer $LITELLM_MASTER_KEY"; donelangsmith(success),langfuse_otel(success_and_failure), plusarize,s3andweave_otel(success_and_failure) with"read_only": true.s3is the YAMLs3_v2callback under the name the dashboard uses; at f3b76f1 that row was missing because the alias check looked the display name up in the integration registry--num_workers 4proxy on port 20385 whose log shows fourStarted server processlinesDashboard
Langsmithandlangfuse_otelwith the actions menu (Test, Edit, Delete) andarize,s3 Bucket (AWS)andweave_otelwith a "Read only" label in place of the menuDeleting and re-saving callbacks from the dashboard
curl -s -X POST localhost:$PORT/config/callback/delete -H "Authorization: Bearer $LITELLM_MASTER_KEY" -H 'Content-Type: application/json' -d '{"callback_name":"arize"}'404 {"detail":{"error":"Callback 'arize' not found in active configuration"}}, inventory unchanged, so the read-only row cannot be removed from the dashboard. Same404for"callback_name":"s3"and"s3_v2""callback_name":"langfuse_otel"is404for the same pre-existing reason as BeforePOST /config/updatereturns200. Within the config sync interval the inventory is the threelangfuse_otelrows,arizeandweave_otelread only, andlangsmithnow"read_only": trueinstead of disappearing, so the page keeps showing the still registered callbackCache enabled does not leak internal hooks
cache: trueon port 20388, two identical chat completions, the second carries anx-litellm-cache-keyheadercurl -s localhost:20388/callbacks/list ...showscache,_ProxyDBLogger,ServiceLoggingand the deployment hooks as activecurl -s localhost:20388/get/config/callbacks ...listslangsmith,langfuse_otel,arize(read only),weave_otel(read only), nocacherow and no internal hookError and auth paths
curl -s localhost:$PORT/v1/chat/completions ... -d '{"model":"does-not-exist","messages":[{"role":"user","content":"hi"}]}'returns400 Invalid model name passed in model=does-not-existand the inventory is unchangedcurl -s -o /dev/null -w "%{http_code}\n" localhost:$PORT/get/config/callbacksis401internal_userandinternal_user_viewerusers get401 Only proxy admin can be used ..., same as Before, so that 401 comes from route auth and never reaches this codeproxy_admin_vieweruser gets200with the same rows as the master key. Secrets areREDACTED(LANGSMITH_API_KEY,LANGFUSE_PUBLIC_KEY,LANGFUSE_SECRET_KEY, andAWS_ACCESS_KEY_ID,AWS_SECRET_ACCESS_KEYon the read-onlys3row), non-secrets stay visible (LANGSMITH_PROJECT,LANGFUSE_HOST,AWS_REGION_NAME), and thearizeandweave_otelrows carryread_only: truewith no variablesObservations from the run:
UNAVAILABLE; Arize ingestion unverifiedarizeinventory row does not depend on that exportcallbacksreturns 404 on both sides (pre-existing)Type
🐛 Bug Fix
Caveats (if any)
Low
CustomLoggerRegistryshow their class or function name/config/callback/deleteonly searchessuccess_callback(pre-existing, unchanged)cache,vector_store_pre_call_hook); a new auto-registered registry hook would need adding to that listFinal Attestation
The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR
d63e655 passes /live-pr-risk
Link to Devin session: https://app.devin.ai/sessions/e724cebb80f249c391598984099c3329
Open in Devin Desktop: https://app.devin.ai/desktop/session/e724cebb80f249c391598984099c3329?variant=devin
Requested by: @yucheng-berri