feat(prometheus): add api_provider label to token, latency, request and cache metrics - #32126
Conversation
…nd cache metrics The token (input/output/total), latency (llm_api, time_to_first_token, request_total, request_queue_time), proxy request (total/failed) and cache metrics were emitted from the same call sites as litellm_spend_metric and litellm_requests_metric, which already carry api_provider, yet these were missing it. That left no way to break tokens, latency, request counts or cache hits down by upstream provider even though the provider is already on the payload as custom_llm_provider. Add api_provider to each metric's label allow-list. The success path already populates enum_values.api_provider from standard_logging_payload, so those metrics emit it with no further plumbing. The cache label is added to the shared _cache_metric_labels list, so alongside litellm_cache_hits_metric and litellm_cache_misses_metric it also covers litellm_cached_tokens_metric and the provider prompt-cache read/creation token metrics; the label-presence test asserts all of them. For the client-side failure path, where a deployment may not have been resolved, derive it best-effort from litellm_params.custom_llm_provider, a partial standard_logging_object, or inference from the requested model name via litellm.get_llm_provider, falling back to empty rather than guessing. Resolves LIT-4178
yucheng-berri
added
the
oss-adoption
📝 WalkthroughWalkthroughAdds an ChangesAPI Provider Label Propagation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Proxy
participant PrometheusLogger
participant LiteLLM as litellm.get_llm_provider
Proxy->>PrometheusLogger: async_post_call_failure_hook(request_data)
PrometheusLogger->>PrometheusLogger: _extract_api_provider_from_request_data(request_data)
alt custom_llm_provider present
PrometheusLogger-->>PrometheusLogger: resolve from litellm_params/standard_logging_object
else model name present
PrometheusLogger->>LiteLLM: get_llm_provider(model)
LiteLLM-->>PrometheusLogger: provider or BadRequestError (swallowed)
end
PrometheusLogger->>PrometheusLogger: build UserAPIKeyLabelValues(api_provider)
PrometheusLogger-->>Proxy: emit litellm_proxy_failed_requests_metric, litellm_proxy_total_requests_metric
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
Greptile SummaryThis PR adds the
Confidence Score: 5/5The code changes are logically correct and well-tested; prior review threads have already surfaced and discussed the label-schema migration concern at length, so the maintainer is informed on that front. The new helper method has a sound priority chain and safe fallbacks, all branches are exercised by unit tests, and the label-list additions are consistent with how the rest of the file is structured. The concerns raised in previous review rounds are tracked and the author is aware of them. No files require additional attention beyond what was discussed in prior threads.
|
| Filename | Overview |
|---|---|
| litellm/types/integrations/prometheus.py | Adds api_provider to label lists for 10+ long-lived metrics; correct but this is a breaking label-schema change for existing deployments (covered in prior review threads) |
| litellm/integrations/prometheus.py | Adds _extract_api_provider_from_request_data with well-structured priority chain (litellm_params → standard_logging_object → model inference) and safe fallback; populates api_provider in the failure hook enum_values |
| tests/enterprise/litellm_enterprise/enterprise_callbacks/test_prometheus_logging_callbacks.py | Existing mock-based tests updated to include api_provider="openai" where required by the new label signature; changes are additive and don't weaken existing assertions |
| tests/test_litellm/integrations/test_prometheus_labels.py | New end-to-end tests drive the real logger and assert api_provider in collected samples; registry teardown uses a private prometheus_client attribute (flagged in prior threads) |
Reviews (4): Last reviewed commit: "fix(prometheus): satisfy ruff BLE001 bud..." | Re-trigger Greptile
Greptile SummaryThis PR adds the
Confidence Score: 3/5Merging as-is will silently break existing Grafana dashboards and alert rules the moment the first scrape arrives after upgrade, because adding labels to existing Prometheus metrics creates new time series while old data retains the original label set. The failure-path helper and the test suite are well-constructed, but the label additions to 10+ live metrics are unconditional. The project already has a documented opt-in mechanism for exactly this situation — the rate-limit and stream labels are both gated behind module-level flags specifically to preserve existing dashboard compatibility across upgrades. litellm/types/integrations/prometheus.py — all the label-list additions should be reviewed against the opt-in flag pattern before merge.
|
| Filename | Overview |
|---|---|
| litellm/types/integrations/prometheus.py | Adds api_provider to 10+ existing metric label lists unconditionally, including latency, token, request-counter, and cache metrics. This changes the cardinality of long-lived series and will break existing dashboards on upgrade without an opt-in flag. |
| litellm/integrations/prometheus.py | Adds _extract_api_provider_from_request_data static method with a well-ordered fallback chain and wires the result into the failure hook's UserAPIKeyLabelValues. Logic is clean and error-safe. |
| tests/test_litellm/integrations/test_prometheus_labels.py | Comprehensive test additions covering label-list presence, factory plumbing, failure-path extraction, and end-to-end emit wiring. The _clear_prometheus_registry helper uses a private prometheus_client attribute. |
Reviews (1): Last reviewed commit: "feat(prometheus): add api_provider label..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| UserAPIKeyLabelNames.END_USER.value, | ||
| UserAPIKeyLabelNames.USER.value, | ||
| UserAPIKeyLabelNames.MODEL_ID.value, | ||
| UserAPIKeyLabelNames.API_PROVIDER.value, |
There was a problem hiding this comment.
Adding api_provider to these established metrics (token, latency, proxy request counters, cache) changes their label cardinality. In Prometheus, a metric's label set is fixed at registration time — upgrading litellm registers new timeseries that carry the extra label, while the old labelset timeseries stop receiving data. Any existing Grafana panel or PromQL alert that queries litellm_llm_api_latency_metric, litellm_total_tokens_metric, litellm_proxy_failed_requests_metric, or the cache metrics by their current label set will show a gap at the upgrade boundary, and aggregations will silently produce incorrect totals until dashboards are updated.
Other label additions in this codebase (stream label, rate-limit labels) were gated behind litellm.prometheus_emit_stream_label / litellm.prometheus_emit_rate_limit_labels precisely to let operators adopt the new cardinality on their own schedule. The same mechanism should apply here — a litellm.prometheus_emit_api_provider_label (defaulting to False) would let get_labels_for_metric include api_provider only when explicitly enabled, keeping existing dashboards intact until users are ready to migrate.
Rule Used: What: avoid backwards-incompatible changes without... (source)
| from prometheus_client import REGISTRY | ||
|
|
||
| for collector in list(REGISTRY._collector_to_names.keys()): | ||
| try: | ||
| REGISTRY.unregister(collector) | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
REGISTRY._collector_to_names is a private implementation detail of prometheus_client, not part of the public API. Its name and structure have changed across minor releases and could change again. If the attribute is renamed or its type changes, this helper silently becomes a no-op (the try/except swallows any AttributeError), leaving metrics registered and causing subsequent tests to fail with ValueError: Duplicated timeseries. Using a per-test CollectorRegistry instance passed explicitly to metric constructors would be more robust.
| UserAPIKeyLabelNames.MODEL_ID.value, | ||
| UserAPIKeyLabelNames.API_PROVIDER.value, | ||
| ] |
There was a problem hiding this comment.
The project already has an established pattern for exactly this scenario: litellm.prometheus_emit_rate_limit_labels and litellm.prometheus_emit_stream_label are opt-in flags used precisely because "existing dashboards keyed on this metric's historical label set [must] keep matching after upgrade" (see the comment in litellm_proxy_failed_requests_metric). Adding api_provider unconditionally to 10+ long-lived metrics — including litellm_llm_api_latency_metric, litellm_total_tokens_metric, litellm_cache_hits_metric and the others — changes their time-series fingerprint on the first scrape after upgrade. Every Grafana panel, alert rule, and recording rule that references these metrics by label equality will immediately start matching nothing (old series have no api_provider label; new series carry it), causing silent gaps or broken alerting for all existing deployments. The same label addition should be gated on an opt-in flag (e.g. litellm.prometheus_emit_api_provider_label, defaulting to False) and wired through PrometheusMetricLabels.get_labels the same way the rate-limit and stream labels are today.
Rule Used: What: avoid backwards-incompatible changes without... (source)
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!
| def _clear_prometheus_registry() -> None: | ||
| from prometheus_client import REGISTRY | ||
|
|
||
| for collector in list(REGISTRY._collector_to_names.keys()): | ||
| try: | ||
| REGISTRY.unregister(collector) | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
REGISTRY._collector_to_names is a private implementation detail of prometheus_client — the leading underscore signals it is not part of the public API and may be renamed or removed without notice. The prometheus_client library does expose CollectorRegistry.unregister, so the teardown logic itself is sound; only the attribute used to enumerate collectors is fragile.
Greptile SummaryThis PR adds
Confidence Score: 4/5Safe to merge with awareness that adding new labels to long-running metrics causes a time-series gap in existing Prometheus deployments; the code logic and test coverage are solid. The provider extraction helper is well-structured with a sensible fallback chain and correct exception handling. The nine metrics that gain api_provider are all in the label-schema category, not the core request path, and the success-path wiring required no new production plumbing. The main concern is that existing Prometheus/Grafana deployments will see old time series (without api_provider) stop receiving data and fresh series appear after upgrading, producing a visible gap in dashboards and potentially causing alert flap during the transition window. A changelog entry or feature flag to opt out would reduce the risk for existing users. litellm/types/integrations/prometheus.py — the label additions affect nine metrics and the time-series discontinuity impact is concentrated here.
|
| Filename | Overview |
|---|---|
| litellm/integrations/prometheus.py | Adds _extract_api_provider_from_request_data static method and wires api_provider into the failure-path enum_values; logic is correct with proper fallback chain and exception handling. |
| litellm/types/integrations/prometheus.py | Adds api_provider label to 9 metrics (latency, token, request counter, and cache metrics via shared _cache_metric_labels); the change is a breaking label-schema update for existing Prometheus deployments. |
| tests/test_litellm/integrations/test_prometheus_labels.py | Adds 6 new unit tests covering label presence, value flow through the factory, failure-path extraction, and end-to-end emit wiring; tests are mock-only with no real network calls, but the registry helper accesses a private prometheus_client attribute. |
Reviews (3): Last reviewed commit: "feat(prometheus): add api_provider label..." | Re-trigger Greptile
| def _clear_prometheus_registry() -> None: | ||
| from prometheus_client import REGISTRY | ||
|
|
||
| for collector in list(REGISTRY._collector_to_names.keys()): | ||
| try: | ||
| REGISTRY.unregister(collector) | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
REGISTRY._collector_to_names is an undocumented private attribute of prometheus_client's CollectorRegistry. It has changed in past minor releases and could silently break these tests if prometheus_client is upgraded. A more version-stable alternative is to use a fresh CollectorRegistry per test (pass it to PrometheusLogger if the constructor supports it) or iterate over list(REGISTRY._names_to_collectors.values()) and unregister by collector reference — but both of those are still private. The safest pattern is to instantiate an isolated CollectorRegistry(auto_describe=False) and pass it through rather than mutating the global singleton.
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!
| UserAPIKeyLabelNames.END_USER.value, | ||
| UserAPIKeyLabelNames.USER.value, | ||
| UserAPIKeyLabelNames.MODEL_ID.value, |
There was a problem hiding this comment.
Adding api_provider to metrics that have been emitting without it (e.g. litellm_llm_api_latency_metric, litellm_proxy_total_requests_metric, the token metrics) is a breaking change for any deployment that has been scraping litellm's Prometheus endpoint. Prometheus identifies a time series by its metric name plus its full label set, so every existing series for these metrics will stop receiving new data points after the upgrade and a fresh set of series (with api_provider included) will begin. Dashboards and alert rules that reference the old series will see a data gap at the upgrade boundary. Per the repo's backwards-compatibility policy, a feature flag (e.g. litellm.prometheus_emit_api_provider_label) that defaults to True for new installs but can be set to False for existing ones would let current users opt out of the label addition until they are ready to migrate their dashboards. At minimum, this change should be prominently called out in the changelog as a breaking label-schema change.
Rule Used: What: avoid backwards-incompatible changes without... (source)
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/test_litellm/integrations/test_prometheus_labels.py (1)
159-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer constructing/injecting a real object over binding methods onto a
MagicMock.
prometheus_loggeris a bareMagicMock()with_cached_metric_labels/label_filtersset manually andget_labels_for_metricbound via__get__. Any attribute the real method touches that isn't explicitly stubbed silently resolves to anotherMagicMockinstead of failing, which can mask future integration breakage. A minimal real instance (bypassing__init__side effects) or a small dependency-injected fake would be more faithful to the production object.As per path instructions, "Prefer dependency injection over monkeypatching class attributes in tests; pass mocked dependencies into classes instead."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_litellm/integrations/test_prometheus_labels.py` around lines 159 - 172, Replace the bare MagicMock-based setup for prometheus_logger in test_prometheus_labels with a minimal real PrometheusLogger instance or a small injected fake, so get_labels_for_metric is exercised against a faithful object instead of a mock that auto-creates missing attributes. Keep the test focused on PrometheusLogger.get_labels_for_metric and prometheus_label_factory, and initialize only the specific dependencies/state the real method needs rather than binding the method onto MagicMock via __get__.Source: Path instructions
litellm/types/integrations/prometheus.py (1)
334-371: 🚀 Performance & Scalability | 🔵 TrivialNew
api_providerlabel is unconditional, unlike the neighboring rate-limit labels on the same metric.The comment directly above (lines 362-365) explains that
rate_limit_category/rate_limit_typewere deliberately kept opt-in onlitellm_proxy_failed_requests_metricspecifically to avoid changing this metric's historical label set for existing dashboards/alerts.api_provideris added unconditionally here instead, solitellm_proxy_total_requests_metricandlitellm_proxy_failed_requests_metric(two of the highest-volume metrics) get a new label dimension by default on upgrade — this is presumably intentional per the PR's goal, but worth calling out for anyone relying on a fixed label/cardinality set for these two metrics.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@litellm/types/integrations/prometheus.py` around lines 334 - 371, The new api_provider label is being added unconditionally to litellm_proxy_total_requests_metric and litellm_proxy_failed_requests_metric, which changes the existing label set for these high-volume metrics. Update the metric label definitions in prometheus.py so api_provider follows the same opt-in pattern as the rate-limit labels, and only gets appended in get_labels() when explicitly enabled. Keep the existing symbol names aligned with UserAPIKeyLabelNames and the two metric lists so dashboards and alerts retain their historical cardinality by default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@litellm/types/integrations/prometheus.py`:
- Around line 334-371: The new api_provider label is being added unconditionally
to litellm_proxy_total_requests_metric and litellm_proxy_failed_requests_metric,
which changes the existing label set for these high-volume metrics. Update the
metric label definitions in prometheus.py so api_provider follows the same
opt-in pattern as the rate-limit labels, and only gets appended in get_labels()
when explicitly enabled. Keep the existing symbol names aligned with
UserAPIKeyLabelNames and the two metric lists so dashboards and alerts retain
their historical cardinality by default.
In `@tests/test_litellm/integrations/test_prometheus_labels.py`:
- Around line 159-172: Replace the bare MagicMock-based setup for
prometheus_logger in test_prometheus_labels with a minimal real PrometheusLogger
instance or a small injected fake, so get_labels_for_metric is exercised against
a faithful object instead of a mock that auto-creates missing attributes. Keep
the test focused on PrometheusLogger.get_labels_for_metric and
prometheus_label_factory, and initialize only the specific dependencies/state
the real method needs rather than binding the method onto MagicMock via __get__.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cba8b9f9-54ac-41ff-bfc5-59802ee64f27
📒 Files selected for processing (3)
litellm/integrations/prometheus.pylitellm/types/integrations/prometheus.pytests/test_litellm/integrations/test_prometheus_labels.py
…el assertions - Suppress the strict-rule BLE001 budget breach with a justified noqa; the broad except in the failure-path provider extraction is intentional defense-in-depth (covered by test_extract_api_provider_swallows_unknown_model_but_logs_unexpected_errors), not dead code to delete - Update tests/enterprise assertions for litellm_tokens_metric, litellm_input_tokens_metric, litellm_output_tokens_metric, the three latency metrics, and the proxy request counters to expect the new api_provider label, matching what litellm_mapped_enterprise_tests caught in CI
|
ci/circleci: image_gen_testing is failing here, but it is also failing on the current tip of |
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit be63de1. Configure here.
| try: | ||
| return litellm.get_llm_provider(model=model)[1] or None | ||
| except litellm.exceptions.BadRequestError: | ||
| return None |
There was a problem hiding this comment.
Failure path misses provider fields
Medium Severity
_extract_api_provider_from_request_data only reads custom_llm_provider from nested litellm_params and from a top-level standard_logging_object. On the proxy client failure path, post_call_failure_hook drops litellm_logging_obj without copying that payload onto request_data, so resolved provider data that still lives on the logging object never reaches the extractor and api_provider can stay empty when model-name inference fails.
Reviewed by Cursor Bugbot for commit be63de1. Configure here.
Chat completions, embeddings, image generation and audio all land in the same litellm_requests_metric and litellm_spend_metric series today. Embedding traffic is usually orders of magnitude higher volume and orders of magnitude lower cost than chat, so mixing them makes both the request rate and the spend series hard to reason about. The value is already on standard_logging_payload as call_type, so this is label plumbing rather than new collection. Async and sync entry points report different call types for the same operation, acompletion against completion, and the proxy is async while the SDK usually is not. Emitting both spellings would split every proxy's chat traffic in two, so each async call type is collapsed onto its sync twin. The alias table is derived from CallTypes at import time and matches on member names rather than values: stripping a leading "a" from the value would turn add_message into dd_message and anthropic_messages into nthropic_messages, both of which are sync call types that legitimately start with one. 16 metrics gain the label. Remaining-quota gauges and the configured tpm/rpm limits deliberately do not, since headroom belongs to a deployment rather than to a call type and splitting it would emit several series each claiming to describe the same number. A test pins that exclusion. Same shape as BerriAI#32126, which added api_provider to the metrics emitted from call sites that already held the value.
3 participants
Relevant issues
LIT-4178
Linear ticket
Pre-Submission checklist
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
Before
After
Type
🐛 Bug Fix
Changes
A subset of Prometheus metrics carried no api_provider label even though sibling metrics emitted from the same call sites (litellm_spend_metric, litellm_requests_metric) already did, so there was no way to break token usage, latency, request counts or cache hits down by upstream provider. This adds api_provider to the token metrics (litellm_input_tokens_metric, litellm_output_tokens_metric, litellm_total_tokens_metric), the latency metrics (litellm_llm_api_latency_metric, litellm_llm_api_time_to_first_token_metric, litellm_request_total_latency_metric, litellm_request_queue_time_seconds), the proxy request counters (litellm_proxy_total_requests_metric, litellm_proxy_failed_requests_metric) and the cache metrics.
The cache label is added to the shared _cache_metric_labels list, so the change covers all five metrics that use it: litellm_cache_hits_metric, litellm_cache_misses_metric, litellm_cached_tokens_metric, litellm_provider_cache_read_input_tokens_metric and litellm_provider_cache_creation_input_tokens_metric. The label-presence test asserts every one of them.
On the success path no new plumbing is needed; enum_values.api_provider is already populated from standard_logging_payload["custom_llm_provider"], so the value flows through as soon as the label is in the metric's list. For the client-side failure path, where a request can fail before a deployment resolves, the provider is derived best-effort from litellm_params.custom_llm_provider, then a partial standard_logging_object, then inference from the requested model name via litellm.get_llm_provider, falling back to empty rather than guessing.
Credit
Adopted from #32043 by @shivijain2323. Mirrored onto a
litellm_branch so CircleCI and the internal lint workflow run. All commits preserve the original author metadata. A newoss-adoptionlabel was added to this PR to mark it as an adopted external contribution.Summary by CodeRabbit
New Features
api_providerlabels where available, improving visibility into which provider handled a request.Bug Fixes
api_providerinstead of leaving it blank.Note
Medium Risk
Prometheus metric label schemas change at logger init, which can affect dashboards and series cardinality; runtime risk is low because provider inference is best-effort and errors are swallowed.
Overview
Adds
api_providerto Prometheus label sets for token, latency, proxy request, and cache metrics so they align with spend/request metrics and can be sliced by upstream provider (LIT-4178).On success, values already flow from
custom_llm_providerviaUserAPIKeyLabelValues; this PR only extends each metric’s allowed labels (including shared_cache_metric_labels). On client-side failure, a new_extract_api_provider_from_request_datahelper sets the label fromlitellm_params.custom_llm_provider, partialstandard_logging_object, orget_llm_provider(model)inference, returning empty when unknown so labeling never breaks the failure path.Tests cover label definitions, factory wiring, the extractor, and end-to-end emits on success and failure hooks.
Reviewed by Cursor Bugbot for commit be63de1. Bugbot is set up for automated code reviews on this repo. Configure here.