fix(test): update Prometheus metric test patterns with new labels#20213
Open
shin-bot-litellm wants to merge 2 commits intomainfrom
Open
fix(test): update Prometheus metric test patterns with new labels#20213shin-bot-litellm wants to merge 2 commits intomainfrom
shin-bot-litellm wants to merge 2 commits intomainfrom
Conversation
…issues ## Problem Four tests in litellm_mapped_tests_core were failing: 1. test_register_model_with_scientific_notation - KeyError due to test isolation issues 2. test_search_uses_registry_credentials - Mock not being called due to incorrect patch path 3. test_send_email_missing_api_key - Real API calls despite mocking 4. test_stream_transformation_error_sync - Mock not effective, real API called ## Solution ### test_register_model_with_scientific_notation - Use unique model name to avoid conflicts with other tests - Clear LRU caches before test to prevent stale data - Clean up model_cost entry after test ### test_search_uses_registry_credentials - Use patch.object() on the actual base_llm_http_handler instance - String-based patching for instance methods can fail; direct object patching is more reliable ### test_send_email_missing_api_key - Directly inject mock HTTP client into logger instance - This bypasses any caching issues that could cause the fixture mock to be ineffective ### test_stream_transformation_error_sync - Patch litellm.completion directly instead of the handler module's litellm reference - This ensures the mock is effective regardless of import order ## Regression These tests were affected by LRU caching added in #19606 and HTTP client caching.
…_metrics ## Problem test_proxy_failure_metrics was failing in CI because the expected metric patterns didn't include labels that were recently added to Prometheus metrics: - client_ip (added in #19717) - user_agent (added in #19717) - model_id (added in #19678) ## Solution Update the expected metric patterns in test_proxy_failure_metrics to include the new labels in the correct alphabetical order (as Prometheus outputs them): - litellm_proxy_failed_requests_metric_total - litellm_proxy_total_requests_metric_total Labels are now: api_key_alias, client_ip, end_user, exception_class, exception_status, hashed_api_key, model_id, requested_model, route, team, team_alias, user, user_agent, user_email
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
Contributor
Greptile OverviewGreptile SummaryUpdated test patterns to include new Prometheus metric labels (
All changes are test-only fixes with no impact on production code. Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| tests/otel_tests/test_prometheus.py | Updated metric patterns with new labels (client_ip, user_agent, model_id) in alphabetical order to fix test failures |
| tests/test_litellm/test_utils.py | Improved test isolation by using unique model name and clearing caches before/after test execution |
| tests/test_litellm/enterprise/enterprise_callbacks/send_emails/test_resend_email.py | Fixed mock injection by directly setting the async HTTP client on logger instance to bypass caching |
Sequence Diagram
sequenceDiagram
participant Test as Test Suite
participant Proxy as LiteLLM Proxy
participant Prom as Prometheus Metrics
Note over Test,Prom: test_proxy_failure_metrics flow
Test->>Proxy: POST /chat/completions (bad request)
Proxy-->>Test: 429 Rate Limit Error
Proxy->>Prom: Record failure metric with labels:<br/>api_key_alias, client_ip, end_user,<br/>exception_class, exception_status,<br/>hashed_api_key, model_id, requested_model,<br/>route, team, team_alias, user, user_agent, user_email
Test->>Proxy: GET /metrics
Proxy-->>Test: Return Prometheus metrics
Note over Test: Verify metrics contain expected<br/>pattern with all labels in<br/>alphabetical order
Test->>Test: Assert litellm_proxy_failed_requests_metric_total<br/>contains all required labels
Test->>Test: Assert litellm_proxy_total_requests_metric_total<br/>contains all required labels
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes
test_proxy_failure_metricsinotel_testsCI job.Root Cause
PRs #19717 and #19678 added new labels (
client_ip,user_agent,model_id) to Prometheus metrics, but test patterns weren't updated. Prometheus outputs labels alphabetically, so new labels broke the substring match.Changes
test_proxy_failure_metricsto include all current labels in the correct alphabetical orderTesting
cc @ishaan-jaff