fix(langfuse): detach explicit traces from external parents - #34234
fix(langfuse): detach explicit traces from external parents#34234ShellWen wants to merge 1 commit into
Conversation
Greptile SummaryThis PR changes Langfuse OTEL parenting when request metadata provides an explicit trace ID. The main changes are:
Confidence Score: 4/5Explicit trace handling can break nested observation parenting and should be corrected before merging
litellm/integrations/langfuse/langfuse_otel.py
|
| Filename | Overview |
|---|---|
| litellm/integrations/langfuse/langfuse_otel.py | Adds context detachment for explicit trace IDs, but also removes internal parent spans and activates for empty IDs |
| tests/test_litellm/integrations/test_langfuse_otel.py | Tests direct context selection but does not verify actual span hierarchy or malformed trace ID behavior |
Reviews (1): Last reviewed commit: "fix(langfuse): detach explicit traces fr..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if metadata.get("trace_id") is not None: | ||
| return None, None |
There was a problem hiding this comment.
This override discards both the external context and any default_span supplied by an internal caller. Nested operations carrying the same explicit trace_id, including guardrail spans, can therefore become root spans instead of children of the request span, breaking the observation hierarchy within the selected trace
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!
| if metadata.get("trace_id") is not None: | ||
| return None, None |
There was a problem hiding this comment.
An empty trace_id, such as one populated from an unset environment variable, satisfies this condition and discards valid parent propagation. The span then carries an empty langfuse.trace.id, which can leave it orphaned or rejected instead of preserving the existing OpenTelemetry trace
There was a problem hiding this comment.
Pull request overview
This PR adjusts the Langfuse OpenTelemetry integration to prevent explicit Langfuse trace selection (via request metadata) from inheriting an unrelated external parent context (for example, an inbound traceparent header), which can produce traces whose parent observations do not exist within the selected Langfuse trace
Changes:
- Override
LangfuseOtelLogger._get_span_contextto short-circuit context propagation when an explicit Langfuse trace ID is provided via metadata - Add regression tests to ensure explicit trace IDs do not call into the base OpenTelemetry context propagation logic, while requests without explicit trace IDs retain the prior propagation behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
litellm/integrations/langfuse/langfuse_otel.py |
Overrides span context resolution to detach explicit Langfuse trace selection from external parent propagation |
tests/test_litellm/integrations/test_langfuse_otel.py |
Adds unit tests validating the changed context-propagation behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _get_span_context(self, kwargs, default_span: Optional[Span] = None): | ||
| metadata = self._extract_langfuse_metadata(kwargs) | ||
| if metadata.get("trace_id") is not None: | ||
| return None, None | ||
| return super()._get_span_context(kwargs, default_span) |
| def test_explicit_trace_id_does_not_inherit_external_parent_context(self): | ||
| logger = object.__new__(LangfuseOtelLogger) | ||
| kwargs = { | ||
| "litellm_params": { | ||
| "metadata": {"trace_id": "0123456789abcdef0123456789abcdef"}, | ||
| "proxy_server_request": { | ||
| "headers": { | ||
| "traceparent": "00-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-01" | ||
| } | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| with patch( | ||
| "litellm.integrations.opentelemetry.OpenTelemetry._get_span_context" | ||
| ) as parent_context: | ||
| assert logger._get_span_context(kwargs) == (None, None) | ||
|
|
||
| parent_context.assert_not_called() | ||
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
TLDR
Problem this solves:
How it solves it:
Relevant issues
Linear ticket
Pre-Submission checklist
Screenshots / Proof of Fix
Live validation is pending an image build from this branch
Before this fix, an explicit Langfuse
trace_idcombined with an incomingtraceparentproduced a trace whose generation referenced a parent observation that could not exist in the selected traceType
🐛 Bug Fix
Changes
Langfuse OTEL now starts a root context when request metadata supplies an explicit
trace_idRequests without an explicit Langfuse Trace ID retain the existing OpenTelemetry parent context behavior
Final Attestation