fix(otel): cap metric attributes so series count does not grow with traffic - #35166
Conversation
|
|
Greptile SummaryThe PR bounds OpenTelemetry metric attributes consistently across success and failure paths.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported caller-selectable
|
| Filename | Overview |
|---|---|
| litellm/integrations/otel/plumbing/metrics.py | Applies one bounded attribute ceiling to success and failure metrics, limits hidden parameters to model_id, and reports ineligible filter names. |
| tests/test_litellm/integrations/otel/test_otel_v2_metrics.py | Adds focused coverage proving per-request values no longer split metric series and both recording paths honor the ceiling. |
| litellm/integrations/otel/README.md | Documents the bounded-cardinality policy, excluded dimensions, and filter behavior. |
Reviews (2): Last reviewed commit: "fix(otel): cap metric attributes so seri..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
2affa60 to
6631566
Compare
…raffic `GenAIMetricRecorder._common_attributes` dumped the whole `hidden_params` object onto every metric datapoint as one label value. That object is per-request by construction: `response_cost`, `litellm_overhead_time_ms`, `cache_key`, `usage_object` and the provider's `additional_headers` rate-limit counters all move on every call. A unique label value is a new time series, and all six GenAI instruments share those attributes, so one request minted up to six series that would never be written to again That is the steady-state behavior of the feature rather than an abuse case, and it is wrong twice over. Hosted backends bill on series count, so recommending metrics be enabled would have meant a bill proportional to traffic. And a histogram whose every datapoint sits in its own series cannot be aggregated, so the dashboards would have looked populated while answering nothing Both paths now cap their attributes at METRIC_ATTRIBUTE_CEILING, which replaces the failure-only allowlist so the two paths cannot drift. The cap runs before the operator's `otel.attributes` filter, so an operator can narrow it and never widen it back to an unbounded label. Client-supplied and per-request metadata (`requester_metadata`, `spend_logs_metadata`, `user_api_key_end_user_id`, `requester_ip_address`) is metric-ineligible and stays on the span, which already carries it and where cardinality is free. `hidden_params` survives as a label but carries only `model_id` and `api_base`, which are bounded by the router's own deployment list and are the part a per-deployment panel reads Four tests fail against the previous behavior, the load-bearing one being that two requests differing only in per-request fields must land in one series rather than two
6631566 to
0376db4
Compare
|
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 0376db4. Configure here.
9a3bf20
into
litellm_otel_failure_path_metrics
* feat(otel): record the GenAI duration metric on failed requests `_record_metrics` ran only from `async_log_success_event`, so `gen_ai.client.operation.duration` counted only the requests that worked. Latency read off it during an incident was the latency of the surviving traffic, and with no error dimension anywhere there was no way to build a failure-rate panel or a success/failure split per model. A failed call now records the same duration histogram, tagged with the semconv `error.type` (the mapped provider exception's class name, bounded by construction; the message stays on the span). Success attributes are untouched, so an existing query can still isolate the old series with `error_type=""`. The other five instruments describe a completed generation and are skipped rather than filled with a fabricated zero: litellm hands the failure callback no `response_obj`, so there is no usage to split and no completion-token count, and it zeroes `response_cost` on failure. A proxy-gate rejection (auth / rate limit) records nothing, for the same reason it gets no span; no upstream call happened. `error.type` is stamped after the cardinality filter, like `gen_ai.token.type`, so an `otel.attributes` include/exclude list cannot strip the discriminator and silently merge failures into the success series. Resolves LIT-4955 * fix(otel): bound the failure metric's attribute set The failure datapoint reused the success path's full attribute set, which carries client-supplied fields (`metadata.requester_metadata`, `metadata.spend_logs_metadata`, the end-user id taken from the request's `user` field) and per-request ones (the `hidden_params` blob holding the provider's response headers). A failed request needs no provider spend, so nothing rate-limits a caller who puts a unique value in a field they control and mints one histogram series per request. A failure now carries a bounded allowlist: the operation enum, provider, request model, framework, the key/alias/team/org/user identifiers, and `error.type`. Every entry is a fixed enum or an operator-provisioned identifier, so the failure series count is bounded by the deployment's own key, team and user count while the labels still answer which team on which model is failing and how. The user email is left out as PII duplicating the user id already on the series. The operator's `otel.attributes` filter layers on top, so it narrows the allowlist further and never widens it. * fix(otel): cap metric attributes so series count does not grow with traffic (#35166) `GenAIMetricRecorder._common_attributes` dumped the whole `hidden_params` object onto every metric datapoint as one label value. That object is per-request by construction: `response_cost`, `litellm_overhead_time_ms`, `cache_key`, `usage_object` and the provider's `additional_headers` rate-limit counters all move on every call. A unique label value is a new time series, and all six GenAI instruments share those attributes, so one request minted up to six series that would never be written to again That is the steady-state behavior of the feature rather than an abuse case, and it is wrong twice over. Hosted backends bill on series count, so recommending metrics be enabled would have meant a bill proportional to traffic. And a histogram whose every datapoint sits in its own series cannot be aggregated, so the dashboards would have looked populated while answering nothing Both paths now cap their attributes at METRIC_ATTRIBUTE_CEILING, which replaces the failure-only allowlist so the two paths cannot drift. The cap runs before the operator's `otel.attributes` filter, so an operator can narrow it and never widen it back to an unbounded label. Client-supplied and per-request metadata (`requester_metadata`, `spend_logs_metadata`, `user_api_key_end_user_id`, `requester_ip_address`) is metric-ineligible and stays on the span, which already carries it and where cardinality is free. `hidden_params` survives as a label but carries only `model_id` and `api_base`, which are bounded by the router's own deployment list and are the part a per-deployment panel reads Four tests fail against the previous behavior, the load-bearing one being that two requests differing only in per-request fields must land in one series rather than two
TLDR
Problem this solves:
GenAIMetricRecorder._common_attributesdumped the wholehidden_paramsobject onto every metric datapoint as onesafe_dumpslabel value. That object is per-request by construction, so the label value is unique on essentially every call, and a unique label value is a new time seriesrequester_metadata,spend_logs_metadata,user_api_key_end_user_id,requester_ip_address), so a caller could mint series out of a field they controlHow it solves it:
METRIC_ATTRIBUTE_CEILING. That constant replaces the failure-only allowlist this PR is stacked on, so the two paths cannot drift apartotel.attributesfilter, so an operator can narrow it and never widen it back to an unbounded labelhidden_paramssurvives as a label but carries onlymodel_id, the router's own deployment id, which is bounded by the deployment list and is what a per-deployment panel joins on.api_baseis excluded despite naming the same deployment: it is a documented per-call parameter, so in SDK use it is caller-chosen, and a caller varying it would put the per-request cardinality straight backRelevant issues
Stacked on #35152, which established the bounded-attribute pattern on the failure path. Review that one first; this PR's own diff is the last commit.
Linear ticket
Resolves LIT-4967
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@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
A live proxy against real OpenAI (
gpt-5.6, real spend), OTel v2 metrics on with the console exporter, three IDENTICAL requests each time. Series count is read off what the exporter actually emitted, parsed out of the exported JSON rather than eyeballed.Before, on the base commit
Three identical requests, three separate series on every instrument. One request in, one series per instrument out.
The label doing it, read off the first real call:
response_cost,cache_key,litellm_overhead_time_ms,usage_objectand the provider'sadditional_headerscounters all move per call, so no two requests could ever share a series.After, on this branch
Same three requests, one series per instrument, and it stays one however much traffic runs.
hidden_paramssurvives as the deployment identity and nothing else:Four of the six instruments appear because these calls are non-streaming; the two streaming-only timing histograms are not recorded, which is pre-existing behaviour and unrelated to this change.
The ceiling really is a ceiling
An operator explicitly asking for the caller-controlled labels does not get them back, because the cap runs before the filter. Only the one requested attribute that is inside the ceiling survives.
Test-level evidence
Five tests fail against the previous behaviour. The load-bearing one is that two requests differing only in per-request fields have to land in ONE series rather than two, which is the defect stated as a property rather than as an attribute list:
With the fix:
Type
🐛 Bug Fix
Changes
METRIC_ATTRIBUTE_CEILINGreplacesFAILURE_ATTRIBUTE_KEYSand gainshidden_params;_bounded_attributesapplies it and bothrecordandrecord_failurego through it._common_attributesbuilds thehidden_paramslabel fromBOUNDED_HIDDEN_PARAM_KEYSonly.Two judgement calls a reviewer should weigh rather than take on faith.
The first is that this removes attributes an operator may be reading today, and it does so deliberately rather than behind a flag. The alternative was to leave the default unbounded and let operators opt into the cap, which gets the incentives backwards: the failure mode is a bill and a set of unusable histograms, both of which show up long after the config choice, and the data being removed cannot be aggregated anyway. An operator who wants per-request detail already has it on the span. If a reviewer disagrees, the cheap compromise is to keep the ceiling as the default and add an explicit unsafe-widening escape hatch, which I would rather not add until someone asks for it.
The second is keeping
hidden_paramsas a label name at all. Its contents are now a single field, solitellm.deployment.model_idwould be the honest name. I kept the existing key so that queries already grouping onhidden_paramskeep working, and because renaming a label is a separate mechanical change that reviews better on its own. Flagging it because the name now badly undersells what the label is.A review bot caught two things worth recording.
api_basewas in the bounded set in the first version of this PR and should not have been: I had reasoned about it as the deployment's base URL, which is true on a proxy reading it from config and false in SDK use where the caller passes it per call, and the general case is the one that decides whether a label is bounded. And the removal was originally silent, which is worse than the removal itself, hence the warning.Worth noting what this does NOT fix.
litellm/integrations/opentelemetry.py(the v1 integration) builds its metric attributes the same way and has the same defect. It is slated for deletion, and the v2 package is the live surface, so fixing it here and not there is deliberate; if v1's removal slips, that one needs the same cap.QA runbook
enable_metrics: trueunder theotelcallback, plus an OTLP endpoint or the console exporter/v1/chat/completionsrequest three timesgen_ai.client.*instruments, with a distincthidden_paramslabel; on this branch the three requests share one attribute set per instrument, so they aggregate into one seriesotel.attributes.exclude_list: ["metadata.user_api_key_hash"]and check the label is gone; then setinclude_list: ["hidden_params", "metadata.requester_ip_address"]and confirm the IP does NOT come back, because the ceiling runs firstFinal Attestation
Note
Medium Risk
Changes default metric label sets operators may rely on for dashboards, but bounds cardinality and prevents unusable/billing-heavy series; behavior is covered by extensive tests.
Overview
OTel v2 GenAI metrics no longer attach unbounded labels that grew one time series per request. Success and failure recording both pass through
METRIC_ATTRIBUTE_CEILING(replacing the failure-only allowlist) beforeotel.attributesinclude/exclude, so operators can narrow labels but cannot re-add client-controlled or per-request fields.The
hidden_paramsmetric label is now a JSON dump ofmodel_idonly (BOUNDED_HIDDEN_PARAM_KEYS); fullhidden_params, requester IP/metadata, spend logs metadata, and similar keys stay on spans only. Filter configs that name metric-ineligible span attributes log a one-time warning instead of failing silently.Tests were updated and extended for shared ceiling behavior, single-series aggregation across differing per-request fields, bounded
hidden_params, and the warning path.Reviewed by Cursor Bugbot for commit 0376db4. Bugbot is set up for automated code reviews on this repo. Configure here.