[Fix] Preserve key_alias and team_id metadata in /user/daily/activity/aggregated after key deletion or regeneration#20684
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile OverviewGreptile SummaryThis PR fixes loss of
The changes integrate with the existing analytics aggregation flow ( Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_endpoints/common_daily_activity.py | Adds deleted-keys fallback in get_api_key_metadata() to preserve key_alias/team_id for historical analytics; currently swallows all exceptions when querying deleted table, which can silently hide real failures. |
| litellm/proxy/management_endpoints/key_management_endpoints.py | Persists current key row to deleted table before regenerating token hash and tightens a user_id check in list_keys; regeneration path ignores persistence failures, which can reintroduce the metadata-loss problem. |
| tests/test_litellm/proxy/management_endpoints/test_common_daily_activity.py | Adds unit tests covering get_api_key_metadata() active/deleted fallback behavior and integration into aggregated activity; tests largely mock Prisma calls and validate metadata propagation. |
Sequence Diagram
sequenceDiagram
autonumber
participant Client
participant API as /user/daily/activity/aggregated
participant Daily as common_daily_activity._aggregate_spend_records
participant Active as litellm_verificationtoken
participant Deleted as litellm_deletedverificationtoken
participant Key as /key/*/regenerate
Client->>API: GET aggregated activity
API->>Daily: query spend rows + aggregate
Daily->>Active: find_many(token in api_keys)
alt some keys missing
Daily->>Deleted: find_many(token in missing_keys, order deleted_at desc)
Deleted-->>Daily: deleted key rows
end
Daily-->>API: SpendAnalytics response with key_alias/team_id metadata
API-->>Client: api_key_breakdown includes metadata
Client->>Key: POST regenerate key
Key->>Deleted: create_many(old key row persisted)
Key->>Active: update(token hash to new)
Key-->>Client: new key + updated params
| try: | ||
| deleted_key_records = ( | ||
| await prisma_client.db.litellm_deletedverificationtoken.find_many( | ||
| where={"token": {"in": list(missing_keys)}}, | ||
| order={"deleted_at": "desc"}, | ||
| ) | ||
| ) | ||
| # Use the most recent deleted record for each token (ordered by deleted_at desc) | ||
| for k in deleted_key_records: | ||
| if k.token not in result: | ||
| result[k.token] = { | ||
| "key_alias": k.key_alias, | ||
| "team_id": k.team_id, | ||
| } | ||
| except Exception: | ||
| verbose_proxy_logger.debug( | ||
| "Failed to fetch deleted key metadata for missing keys" | ||
| ) |
There was a problem hiding this comment.
Broad exception hides bugs
get_api_key_metadata() swallows any exception when querying litellm_deletedverificationtoken and just returns partial metadata. This can silently mask real issues (e.g., query/schema errors) and lead to key_alias/team_id unexpectedly staying null with no actionable signal. At minimum, log the exception object (or re-raise non-“table missing” errors) so production failures don’t get silently ignored.
| # Save the old key record to deleted table before regeneration | ||
| # This preserves key_alias and team_id metadata for historical spend records | ||
| try: | ||
| await _persist_deleted_verification_tokens( | ||
| keys=[_key_in_db], | ||
| prisma_client=prisma_client, | ||
| user_api_key_dict=user_api_key_dict, | ||
| litellm_changed_by=litellm_changed_by, | ||
| ) | ||
| except Exception: | ||
| verbose_proxy_logger.debug( | ||
| "Failed to persist old key record to deleted table during regeneration" | ||
| ) |
There was a problem hiding this comment.
Regeneration can lose history
In regenerate_key_fn(), persisting the old key into the deleted table is wrapped in except Exception and failures are ignored. If _persist_deleted_verification_tokens() fails (DB outage, constraint violation, etc.), the function proceeds to overwrite the token hash anyway, permanently losing the old hash→metadata mapping that this PR is trying to preserve. This needs to fail the regeneration (or otherwise guarantee persistence) when the persistence step can’t be completed.
|
@greptile review given the previous comments have been resolved in the new commit |
Greptile OverviewGreptile SummaryThis PR fixes a critical data loss issue where Key changes:
Previous thread resolution:
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_endpoints/common_daily_activity.py | Adds fallback to deleted keys table when fetching API key metadata, with broad exception handling that could mask real errors |
| litellm/proxy/management_endpoints/key_management_endpoints.py | Persists old key record before regeneration to preserve metadata; includes unrelated fix for user_id comparison |
| tests/test_litellm/proxy/management_endpoints/test_common_daily_activity.py | Comprehensive unit tests covering active keys, deleted keys, mixed scenarios, error handling, and end-to-end aggregation |
Sequence Diagram
sequenceDiagram
participant User
participant API as /user/daily/activity/aggregated
participant GDAA as get_daily_activity_aggregated
participant GAKM as get_api_key_metadata
participant ActiveDB as litellm_verificationtoken
participant DeletedDB as litellm_deletedverificationtoken
User->>API: Request activity report
API->>GDAA: Fetch aggregated data
GDAA->>GAKM: Get metadata for api_keys
GAKM->>ActiveDB: Query active keys
ActiveDB-->>GAKM: Return found keys
alt Some keys not found
GAKM->>DeletedDB: Query deleted keys (fallback)
DeletedDB-->>GAKM: Return deleted key metadata
end
GAKM-->>GDAA: Return complete metadata
GDAA-->>API: Activity data with preserved metadata
API-->>User: Report with key_alias and team_id
Note over User,DeletedDB: Key Regeneration Flow
participant RegAPI as /key/regenerate
participant RegFn as regenerate_key_fn
participant Persist as _persist_deleted_verification_tokens
User->>RegAPI: Regenerate key
RegAPI->>RegFn: Process regeneration
RegFn->>Persist: Save old key to deleted table
alt Persist fails
Persist--XRegFn: Raise exception
RegFn--XRegAPI: Abort regeneration
RegAPI--XUser: Error - metadata not lost
else Persist succeeds
Persist-->>RegFn: Success
RegFn->>ActiveDB: Update token hash
ActiveDB-->>RegFn: Updated
RegFn-->>User: New token
end
| admin_team_ids = None | ||
|
|
||
| if not user_id and user_api_key_dict.user_role not in [ | ||
| if user_id is None and user_api_key_dict.user_role not in [ |
There was a problem hiding this comment.
Unrelated to PR scope
This fix (user_id is None vs not user_id) is correct but unrelated to preserving key metadata after deletion/regeneration.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
does not change the flow.
There was a problem hiding this comment.
Where did this change come from? looks like you're about to cause a regression: #20623
…egeneration_and_deletion
Relevant issues
Issue: When an API key is deleted or regenerated, the /user/daily/activity/aggregated endpoint returns null for key_alias and team_id in the api_key_breakdown metadata. The spend metrics (tokens, cost, requests) remain intact, but the identity metadata needed to attribute those metrics to a specific user or team is lost — breaking audit and cost-allocation workflows.
Cause: The get_api_key_metadata() function in common_daily_activity.py resolves key_alias and team_id by querying only the active litellm_verificationtoken table at request time. When a key is deleted, its row is removed from that table (moved to litellm_deletedverificationtoken), so the lookup returns nothing. When a key is regenerated, the old token hash is overwritten with a new one in-place, so the old hash — still referenced by historical spend records — no longer matches any row.
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unitCI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Type
🐛 Bug Fix
✅ Test
Changes
Fix: Two changes were made. First, get_api_key_metadata() now falls back to the litellm_deletedverificationtoken table for any keys not found in the active table, retrieving the most recent deleted record. Second, regenerate_key_fn() now persists the old key record to the deleted table before replacing the token hash, ensuring the old hash and its metadata remain queryable for historical spend attribution.