feat(otel): add team_metadata, http.route, and model names to inference spans - #29319
Conversation
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryStamps four request-identity attributes (
Confidence Score: 4/5Safe to merge; the change only adds new span attributes and cannot remove or alter any attribute already emitted. The implementation is narrow and well-tested. The only noteworthy observations are that No files require special attention;
|
| Filename | Overview |
|---|---|
| litellm/integrations/opentelemetry.py | Adds _set_inference_identity_attributes and _team_metadata_json helpers; calls the former at the end of the main set_attributes block to stamp http.route, litellm.team.metadata, litellm.model_group, and litellm.provider.model on every inference span. Logic is sound; minor note that http.route value already appears under metadata.user_api_key_request_route from the existing metadata loop, and serialization failures in _team_metadata_json are swallowed silently. |
| tests/test_litellm/integrations/test_opentelemetry.py | Adds TestOpenTelemetryInferenceIdentityAttributes with five focused unit tests using InMemorySpanExporter — covers all four new attributes, empty-dict and missing-key drop behavior, and the _team_metadata_json static helper. No real network calls; follows existing test patterns. |
Reviews (1): Last reviewed commit: "feat(otel): stamp team_metadata, http.ro..." | Re-trigger Greptile
| ) | ||
|
|
||
| @staticmethod | ||
| def _team_metadata_json(value: Any) -> Optional[str]: | ||
| """JSON-serialize a team's metadata dict for a single span attribute. |
There was a problem hiding this comment.
Silent serialization failure drops attribute without any log
_team_metadata_json catches all exceptions from safe_dumps and returns None, silently suppressing the attribute. This is consistent with the file's OTEL error-handling style, but a verbose_logger.debug(...) call in the except branch would make it easier to diagnose cases where team_metadata is present but never stamps on spans (e.g., a non-JSON-serializable custom object stored in team metadata).
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!
| LLM-call span is filterable by the route it came in on, the team's | ||
| metadata, and both the user-facing (model_group alias) and the | ||
| dispatched (provider) model names. Empty/absent values are skipped. | ||
| """ | ||
| metadata = standard_logging_payload.get("metadata") or {} | ||
|
|
||
| http_route = metadata.get("user_api_key_request_route") |
There was a problem hiding this comment.
http.route is also already emitted as metadata.user_api_key_request_route
The metadata loop at the start of set_attributes (around line 2076) iterates over every key in standard_logging_payload["metadata"] and writes each one as metadata.{key}, so user_api_key_request_route is already on every LLM-call span as metadata.user_api_key_request_route. The new code stamps the same value again as http.route. This is likely intentional (the two keys have different semantics: one is a raw metadata dump, the other is a proper OTel HTTP semantic-convention attribute), but it's worth calling out that traces will carry the route under both keys rather than just the new one.
Greptile SummaryThis PR backports four request-identity attributes (
Confidence Score: 4/5Safe to merge; the change is additive (new span attributes only) and touches no auth, routing, or data-mutation paths. The new helper and its call site are tightly scoped to attribute stamping, use safe dict access throughout, and are covered by dedicated in-memory span tests. The two concerns worth addressing before merge are: (1) the helper has no per-method exception guard, so an unexpected throw would silently drop all subsequent attributes in the same span (cost, completions, finish reasons, etc.), and (2) the litellm/integrations/opentelemetry.py — specifically the
|
| Filename | Overview |
|---|---|
| litellm/integrations/opentelemetry.py | Adds _set_inference_identity_attributes and _team_metadata_json helpers, called from set_attributes. The new code is tightly scoped to attribute stamping with safe dict access throughout; one minor concern is that it sits inside the outer try/except with no per-method guard, so an unexpected throw would skip all remaining attribute setting for that span. |
| tests/test_litellm/integrations/test_opentelemetry.py | New TestOpenTelemetryInferenceIdentityAttributes class with five unit tests using real in-memory OTEL spans; covers the happy path, provider-model fallback, empty team_metadata suppression, missing route suppression, and the _team_metadata_json helper directly. No real network calls; test isolation is correct. |
Reviews (2): Last reviewed commit: "feat(otel): stamp team_metadata, http.ro..." | Re-trigger Greptile
| self._set_inference_identity_attributes( | ||
| span=span, | ||
| standard_logging_payload=standard_logging_payload, | ||
| litellm_params=litellm_params, | ||
| ) |
There was a problem hiding this comment.
No per-method exception guard on
_set_inference_identity_attributes
The call site sits inside the large try block that wraps all of set_attributes (catch at line 2413). If this helper raises unexpectedly, control jumps straight to handle_callback_failure, silently skipping every attribute set that follows: cost breakdown, LLM request/response fields, completions data, finish reasons, etc. The existing sibling helpers (_set_tools_attributes, _set_functions_attributes) each carry their own try/except precisely to avoid this blast radius. Wrapping this call in a local try/except (or adding the guard inside _set_inference_identity_attributes) would make the failure mode consistent and self-contained.
There was a problem hiding this comment.
Leaving this without its own guard on purpose. The helper only does .get() on dicts and calls safe_dumps, which never raises (it handles circular refs and unserializable objects internally and falls back to str/default=str), so there is no realistic throw path. The sibling set_tools_attributes / system-instruction helpers wrap their own try/except because they do nontrivial parsing (iterating tool defs, json-decoding content) that genuinely can raise; this helper doesn't. Adding error handling for an impossible path would also cut against the repo guideline of not guarding impossible scenarios. If it ever does throw, the outer try/except still keeps the request itself from failing, which is the property that matters.
Generated by Claude Code
| or {} | ||
| ) | ||
| team_metadata = self._team_metadata_json( | ||
| raw_metadata.get("user_api_key_team_metadata") | ||
| ) | ||
| if team_metadata: | ||
| self.safe_set_attribute( |
There was a problem hiding this comment.
or-chain silently skips litellm_metadata when metadata is non-empty but lacks team info
raw_metadata = litellm_params.get("metadata") or litellm_params.get("litellm_metadata") or {} treats any truthy metadata dict as the authoritative source and never inspects litellm_metadata. If a caller populates litellm_params["metadata"] with non-team keys (e.g. {"proxy_server_request": ...}) and separately sets litellm_params["litellm_metadata"]["user_api_key_team_metadata"], the team attribute is silently dropped. Since both keys are alternate names for the full metadata dict in practice (not partial contributors), the current logic is likely correct for real traffic, but the assumption isn't documented and a defensive merge ({**litellm_params.get("litellm_metadata", {}), **litellm_params.get("metadata", {})}) or explicit comment would clarify intent.
79e8929 to
3610279
Compare
3610279 to
2f9354f
Compare
| or litellm_params.get("litellm_metadata") | ||
| or {} | ||
| ) | ||
| team_metadata = self._team_metadata_json( |
There was a problem hiding this comment.
Medium: Team metadata exposure
This serializes the entire team metadata dict onto every inference span. Team metadata is also used for callback settings and callback_vars such as Langfuse/LangSmith/GCS credentials, so a normal user on that team can cause those stored values to be exported to the configured OpenTelemetry backend by making an inference request.
Please emit an allowlist of non-sensitive team metadata fields, or redact credential-bearing subtrees like logging[].callback_vars and callback_settings.callback_vars before setting this span attribute.
PR overviewThis pull request enriches OpenTelemetry inference spans with additional context, including team metadata, HTTP route information, and model names. The change is centered in the LiteLLM OpenTelemetry integration code that populates span attributes for inference requests. There is one open security concern: the current team metadata serialization may export sensitive callback-related values, such as backend credentials, to the configured OpenTelemetry collector when a team member makes an inference request. No issues have been addressed yet, so the PR still needs a redaction or allowlist approach before this metadata is safely emitted. Open issues (1)
Fixed/addressed: 0 · PR risk: 7/10 |
3be3c1d
into
litellm_internal_staging
Relevant issues
Brings a subset of the otelv2 work in #28909 to the current (v1) OpenTelemetry integration. #28909 added these identity attributes only under v2; this stamps them on the inference spans the current integration already emits.
Linear ticket
Pre-Submission checklist
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewChanges
Every inference span emitted by
OpenTelemetry.set_attributesnow carries four request-identity attributes so traces are filterable by team, route, and model:http.route— the normalized route template the request came in on (e.g./v1/chat/completions), read fromuser_api_key_request_route. Previously this lived only on the proxy SERVER root span; now it is on the LLM-call span too.litellm.team.metadata— the team's free-form metadata dict, JSON-serialized. Read from the raw request metadata inlitellm_paramsbecauseuser_api_key_team_metadatais dropped from the standard logging payload metadata. An empty/missing dict is dropped rather than stamping a useless"{}".litellm.model_group— the user-facing model the caller requested (the litellm alias).litellm.provider.model— the model litellm dispatched to the provider (the router deployment's model name), falling back to the payload model on the SDK path where no router renaming happened.Per discussion, the model attributes use explicit
litellm.*keys rather than overloading the existinggen_ai.request.model, so this PR does not change the meaning of any attribute already emitted; it only adds new ones.The work is isolated to a small helper,
_set_inference_identity_attributes, called fromset_attributes, plus a_team_metadata_jsonhelper for the empty-dict handling.Type
🆕 New Feature
Screenshots / Proof of Fix
Ran the local proxy on
:4011withcallbacks: ["otel"](console exporter), apre_callguardrail attached to amock-gptmodel, and the master key. A pre-call guardrail injected the team's metadata intodata["metadata"]["user_api_key_team_metadata"]so the request carries the same shape/team/newproduces.Config (
/tmp/otel_pr29319/config.yaml):Start the proxy and send a request:
The OTEL console exporter prints the
litellm_request(inference) span. All four new attributes are present:Caveat on this run: my local DB stack was unavailable, so the team and the virtual key were not created through
/team/new+/key/generate; the sameuser_api_key_team_metadatadict that those endpoints would have produced was injected by the guardrail'sasync_pre_call_hook. The OTEL stamper reads that dict fromlitellm_params["metadata"]regardless of who put it there, so the code path the PR adds is exercised end-to-end.Changes