From 0b67b642cbc85ed9064159f308afb47eb86ef698 Mon Sep 17 00:00:00 2001 From: Harshit28j Date: Sat, 7 Mar 2026 04:32:23 +0530 Subject: [PATCH] Fix OTEL span redundancy, orphaned guardrail traces, and missing response IDs Addresses 4 critical OpenTelemetry span issues in LiteLLM: Issue #3: Remove redundant attributes from raw_gen_ai_request spans - Removed self.set_attributes() call that was duplicating all parent span attributes (gen_ai.*, metadata.*) onto the raw span - Raw span now only contains provider-specific llm.{provider}.* attributes - Reduces storage and eliminates search confusion from duplicate data Issue #4: Prevent attribute duplication on litellm_proxy_request parent span - When litellm_request child span exists, removed redundant set_attributes() call on the parent proxy span - Child span already carries all attributes; parent duplication doubles storage and complicates search Issue #5: Fix orphaned guardrail traces - Guardrail spans were created with context=None when no parent proxy span existed, resulting in orphaned root spans (separate trace_id) - Added _resolve_guardrail_context() helper to ensure guardrails always have a valid parent (litellm_request or proxy span) - Applied fix to both _handle_success and _handle_failure paths Issue #8: Add gen_ai.response.id for embeddings and image generation - EmbeddingResponse and ImageResponse types don't have provider response IDs - Added fallback to standard_logging_payload["id"] (litellm call ID) for correlation across LiteLLM UI, Phoenix traces, and provider logs - Completions still use provider ID (e.g. "chatcmpl-xxx") when available Tests added: - TestRawSpanAttributeIsolation: Verify raw span has no gen_ai/metadata attrs - TestNoParentSpanDuplication: Verify parent span doesn't get duplicated attrs - TestGuardrailSpanParenting: Verify guardrails are children (not orphaned) - TestResponseIdFallback: Verify response ID set for all call types All existing OTEL tests pass (73 passed, 14 pre-existing protocol failures). Co-Authored-By: Claude Haiku 4.5 --- litellm/integrations/opentelemetry.py | 68 +++- .../integrations/test_opentelemetry.py | 338 ++++++++++++++++++ 2 files changed, 390 insertions(+), 16 deletions(-) diff --git a/litellm/integrations/opentelemetry.py b/litellm/integrations/opentelemetry.py index 7cdd338c4f..a77a6f73b1 100644 --- a/litellm/integrations/opentelemetry.py +++ b/litellm/integrations/opentelemetry.py @@ -735,13 +735,10 @@ class OpenTelemetry(CustomLogger): self._maybe_log_raw_request( kwargs, response_obj, start_time, end_time, span ) - # Ensure proxy-request parent span is annotated with the actual operation kind - if ( - parent_span is not None - and hasattr(parent_span, "name") - and parent_span.name == LITELLM_PROXY_REQUEST_SPAN_NAME - ): - self.set_attributes(parent_span, kwargs, response_obj) + # Do NOT duplicate attributes onto the parent proxy-request span. + # The child litellm_request span already carries all attributes; + # copying them to the parent doubles storage and complicates + # search (Issue #4). else: # Do not create primary span (keep hierarchy shallow when parent exists) from opentelemetry.trace import Status, StatusCode @@ -757,8 +754,12 @@ class OpenTelemetry(CustomLogger): kwargs, response_obj, start_time, end_time, parent_span ) - # 3. Guardrail span - self._create_guardrail_span(kwargs=kwargs, context=ctx) + # 3. Guardrail span — ensure guardrails are always parented to an + # existing span so they never become orphaned root spans (Issue #5). + guardrail_ctx = self._resolve_guardrail_context( + span=span, parent_span=parent_span, fallback_ctx=ctx + ) + self._create_guardrail_span(kwargs=kwargs, context=guardrail_ctx) # 4. Metrics & cost recording self._record_metrics(kwargs, response_obj, start_time, end_time) @@ -1145,6 +1146,27 @@ class OpenTelemetry(CustomLogger): ) otel_logger.emit(log_record) + @staticmethod + def _resolve_guardrail_context( + span: Optional[Any], + parent_span: Optional[Any], + fallback_ctx: Optional[Any], + ) -> Optional[Any]: + """ + Return a valid OTEL context for guardrail child spans so they are + never orphaned (Issue #5). Priority: + 1. The litellm_request span that was just created + 2. The parent proxy-request span + 3. The original fallback context (may be None — last resort) + """ + from opentelemetry import trace as _trace + + if span is not None: + return _trace.set_span_in_context(span) + if parent_span is not None: + return _trace.set_span_in_context(parent_span) + return fallback_ctx + def _create_guardrail_span( self, kwargs: Optional[dict], context: Optional[Context] ): @@ -1250,6 +1272,7 @@ class OpenTelemetry(CustomLogger): "USE_OTEL_LITELLM_REQUEST_SPAN" ) + span = None if should_create_primary_span: # Span 1: Request sent to litellm SDK otel_tracer: Tracer = self.get_tracer_to_use_for_request(kwargs) @@ -1275,8 +1298,11 @@ class OpenTelemetry(CustomLogger): self.set_attributes(parent_otel_span, kwargs, response_obj) self._record_exception_on_span(span=parent_otel_span, kwargs=kwargs) - # Create span for guardrail information - self._create_guardrail_span(kwargs=kwargs, context=_parent_context) + # Create span for guardrail information — ensure proper parenting (Issue #5) + guardrail_ctx = self._resolve_guardrail_context( + span=span, parent_span=parent_otel_span, fallback_ctx=_parent_context + ) + self._create_guardrail_span(kwargs=kwargs, context=guardrail_ctx) # Do NOT end parent span - it should be managed by its creator # External spans (from Langfuse, user code, HTTP headers, global context) must not be closed by LiteLLM @@ -1579,12 +1605,20 @@ class OpenTelemetry(CustomLogger): value=optional_params.get("user"), ) - # The unique identifier for the completion. - if response_obj and response_obj.get("id"): + # The unique identifier for the LLM call. + # Completions have a provider response ID (e.g. "chatcmpl-xxx"), + # but Embeddings and Image-gen responses do not. Fall back to + # the litellm call ID so every call type can be correlated + # across LiteLLM UI, Phoenix traces, and provider logs (Issue #8). + response_id = ( + (response_obj.get("id") if response_obj else None) + or standard_logging_payload.get("id") + ) + if response_id: self.safe_set_attribute( span=span, key="gen_ai.response.id", - value=response_obj.get("id"), + value=response_id, ) # The model used to generate the response. @@ -1808,8 +1842,10 @@ class OpenTelemetry(CustomLogger): def set_raw_request_attributes(self, span: Span, kwargs, response_obj): try: - self.set_attributes(span, kwargs, response_obj) - kwargs.get("optional_params", {}) + # Only set provider-specific raw payload attributes on this span. + # The parent litellm_request span already carries the standard + # gen_ai.* / metadata.* attributes — duplicating them here doubles + # storage and adds noise (Issue #3). litellm_params = kwargs.get("litellm_params", {}) or {} custom_llm_provider = litellm_params.get("custom_llm_provider", "Unknown") diff --git a/tests/test_litellm/integrations/test_opentelemetry.py b/tests/test_litellm/integrations/test_opentelemetry.py index 96b0fae5b8..7f3b44e142 100644 --- a/tests/test_litellm/integrations/test_opentelemetry.py +++ b/tests/test_litellm/integrations/test_opentelemetry.py @@ -2336,3 +2336,341 @@ class TestOpenTelemetrySemanticConventions138(unittest.TestCase): error_spans = [s for s in spans if s.status.status_code == StatusCode.ERROR] self.assertTrue(error_spans, "Expected at least one span with ERROR status") + + +class TestRawSpanAttributeIsolation(unittest.TestCase): + """Issue #3: raw_gen_ai_request span should only contain provider-specific + llm.{provider}.* attributes, not the duplicated gen_ai.* / metadata.* attrs.""" + + @patch("litellm.turn_off_message_logging", False) + def test_raw_span_does_not_duplicate_parent_attributes(self): + span_exporter = InMemorySpanExporter() + tracer_provider = TracerProvider() + tracer_provider.add_span_processor(SimpleSpanProcessor(span_exporter)) + + otel = OpenTelemetry(tracer_provider=tracer_provider) + otel.message_logging = True + + mock_tracer = tracer_provider.get_tracer(__name__) + otel.get_tracer_to_use_for_request = MagicMock(return_value=mock_tracer) + + raw_span = mock_tracer.start_span("raw_gen_ai_request") + + kwargs = { + "litellm_params": {"custom_llm_provider": "vertex_ai"}, + "optional_params": {"temperature": 0.7}, + "original_response": '{"predictions": [1,2,3]}', + "additional_args": { + "complete_input_dict": {"instances": [{"content": "hello"}]} + }, + "standard_logging_object": { + "id": "test-id", + "call_type": "embedding", + "metadata": {"user_api_key_hash": "abc123"}, + "hidden_params": {}, + }, + } + response_obj = {"model": "text-embedding-004", "usage": {"total_tokens": 5}} + + otel.set_raw_request_attributes(raw_span, kwargs, response_obj) + raw_span.end() + + spans = span_exporter.get_finished_spans() + raw = [s for s in spans if s.name == "raw_gen_ai_request"][0] + attr_keys = set(raw.attributes.keys()) if raw.attributes else set() + + # Provider-specific attributes SHOULD be present + self.assertTrue( + any(k.startswith("llm.vertex_ai.") for k in attr_keys), + f"Expected llm.vertex_ai.* attributes, got: {attr_keys}", + ) + # Standard gen_ai / metadata attributes should NOT be present + self.assertFalse( + any(k.startswith("gen_ai.") for k in attr_keys), + f"raw span should not contain gen_ai.* attributes, got: {attr_keys}", + ) + self.assertFalse( + any(k.startswith("metadata.") for k in attr_keys), + f"raw span should not contain metadata.* attributes, got: {attr_keys}", + ) + + +class TestNoParentSpanDuplication(unittest.TestCase): + """Issue #4: When litellm_request child span exists, the parent + litellm_proxy_request span should NOT get set_attributes() called.""" + + HERE = os.path.dirname(__file__) + + @patch.dict(os.environ, {"USE_OTEL_LITELLM_REQUEST_SPAN": "true"}, clear=False) + def test_parent_proxy_span_not_duplicated(self): + span_exporter = InMemorySpanExporter() + tracer_provider = TracerProvider() + tracer_provider.add_span_processor(SimpleSpanProcessor(span_exporter)) + + otel = OpenTelemetry(tracer_provider=tracer_provider) + + with open( + os.path.join(self.HERE, "open_telemetry", "data", "captured_kwargs.json") + ) as f: + kwargs = json.load(f) + with open( + os.path.join(self.HERE, "open_telemetry", "data", "captured_response.json") + ) as f: + response_obj = json.load(f) + + # Simulate proxy flow: create a parent proxy span + tracer = tracer_provider.get_tracer(__name__) + from litellm.integrations.opentelemetry import LITELLM_PROXY_REQUEST_SPAN_NAME + + parent_span = tracer.start_span(name=LITELLM_PROXY_REQUEST_SPAN_NAME) + # Inject parent span into kwargs so _get_span_context finds it + kwargs["litellm_params"]["metadata"]["litellm_parent_otel_span"] = parent_span + + start = datetime.utcnow() + end = start + timedelta(seconds=1) + otel._handle_success(kwargs, response_obj, start, end) + + spans = span_exporter.get_finished_spans() + proxy_spans = [ + s for s in spans if s.name == LITELLM_PROXY_REQUEST_SPAN_NAME + ] + self.assertEqual(len(proxy_spans), 1, "Should have exactly one proxy span") + + proxy_attrs = proxy_spans[0].attributes or {} + # The parent proxy span should NOT have gen_ai.request.model set + self.assertNotIn( + "gen_ai.request.model", + proxy_attrs, + "Parent proxy span should NOT duplicate gen_ai.request.model (Issue #4)", + ) + + +class TestGuardrailSpanParenting(unittest.TestCase): + """Issue #5: Guardrail spans must not be orphaned — they should always + be children of the litellm_request span (or parent span).""" + + def test_guardrail_span_is_child_of_litellm_request(self): + """When no parent proxy span exists, guardrail spans should be + children of the litellm_request span, not orphaned root spans.""" + span_exporter = InMemorySpanExporter() + tracer_provider = TracerProvider() + tracer_provider.add_span_processor(SimpleSpanProcessor(span_exporter)) + + otel = OpenTelemetry(tracer_provider=tracer_provider) + otel.tracer = tracer_provider.get_tracer(__name__) + + guardrail_info = { + "guardrail_name": "pii_filter", + "guardrail_mode": "pre_call", + "guardrail_response": "ok", + "start_time": time.time(), + "end_time": time.time() + 0.1, + } + + kwargs = { + "model": "gpt-4", + "messages": [{"role": "user", "content": "Hello"}], + "optional_params": {}, + "litellm_params": {"custom_llm_provider": "openai", "metadata": {}}, + "standard_logging_object": { + "id": "test-guardrail-id", + "call_type": "completion", + "metadata": {}, + "hidden_params": {}, + "guardrail_information": [guardrail_info], + }, + } + response_obj = { + "id": "chatcmpl-test", + "choices": [ + { + "finish_reason": "stop", + "index": 0, + "message": {"content": "Hi!", "role": "assistant"}, + } + ], + "usage": { + "prompt_tokens": 5, + "completion_tokens": 2, + "total_tokens": 7, + }, + } + + start = datetime.utcnow() + end = start + timedelta(seconds=1) + otel._handle_success(kwargs, response_obj, start, end) + + spans = span_exporter.get_finished_spans() + guardrail_spans = [s for s in spans if s.name == "guardrail"] + litellm_spans = [s for s in spans if s.name == "litellm_request"] + + self.assertTrue(guardrail_spans, "Expected at least one guardrail span") + self.assertTrue(litellm_spans, "Expected a litellm_request span") + + litellm_span = litellm_spans[0] + for gs in guardrail_spans: + # All spans should share the same trace_id (not orphaned) + self.assertEqual( + gs.context.trace_id, + litellm_span.context.trace_id, + "Guardrail span should share trace_id with litellm_request (not orphaned)", + ) + # Guardrail should be a child of the litellm_request span + self.assertIsNotNone( + gs.parent, + "Guardrail span should have a parent (not be a root span)", + ) + self.assertEqual( + gs.parent.span_id, + litellm_span.context.span_id, + "Guardrail span should be a child of litellm_request", + ) + + def test_guardrail_span_parented_on_failure(self): + """Guardrail spans should also be properly parented in the failure path.""" + span_exporter = InMemorySpanExporter() + tracer_provider = TracerProvider() + tracer_provider.add_span_processor(SimpleSpanProcessor(span_exporter)) + + otel = OpenTelemetry(tracer_provider=tracer_provider) + otel.tracer = tracer_provider.get_tracer(__name__) + + guardrail_info = { + "guardrail_name": "content_filter", + "guardrail_mode": "pre_call", + "guardrail_response": "blocked", + "start_time": time.time(), + "end_time": time.time() + 0.05, + } + + kwargs = { + "model": "gpt-4", + "messages": [{"role": "user", "content": "Hello"}], + "optional_params": {}, + "litellm_params": {"custom_llm_provider": "openai", "metadata": {}}, + "standard_logging_object": { + "id": "test-fail-id", + "call_type": "completion", + "metadata": {}, + "hidden_params": {}, + "guardrail_information": [guardrail_info], + }, + "exception": Exception("test error"), + } + + start = datetime.utcnow() + end = start + timedelta(seconds=1) + otel._handle_failure(kwargs, None, start, end) + + spans = span_exporter.get_finished_spans() + guardrail_spans = [s for s in spans if s.name == "guardrail"] + + self.assertTrue(guardrail_spans, "Expected at least one guardrail span") + for gs in guardrail_spans: + self.assertIsNotNone( + gs.parent, + "Guardrail span should have a parent on failure path too", + ) + + +class TestResponseIdFallback(unittest.TestCase): + """Issue #8: gen_ai.response.id should be set for embeddings and image gen + using standard_logging_payload['id'] as fallback.""" + + def test_response_id_from_response_obj(self): + """When response_obj has an id, it should be used.""" + otel = OpenTelemetry() + mock_span = MagicMock() + + kwargs = { + "model": "gpt-4", + "optional_params": {}, + "litellm_params": {"custom_llm_provider": "openai"}, + "standard_logging_object": { + "id": "litellm-call-id-123", + "call_type": "completion", + "metadata": {}, + }, + } + response_obj = { + "id": "chatcmpl-provider-id-456", + "choices": [ + { + "finish_reason": "stop", + "index": 0, + "message": {"content": "Hi", "role": "assistant"}, + } + ], + "usage": { + "prompt_tokens": 5, + "completion_tokens": 2, + "total_tokens": 7, + }, + } + + otel.set_attributes(mock_span, kwargs, response_obj) + + # Should use provider response ID, not litellm call ID + mock_span.set_attribute.assert_any_call( + "gen_ai.response.id", "chatcmpl-provider-id-456" + ) + + def test_response_id_fallback_for_embeddings(self): + """When response_obj has no id (embeddings), fallback to + standard_logging_payload['id'].""" + otel = OpenTelemetry() + mock_span = MagicMock() + + kwargs = { + "model": "text-embedding-ada-002", + "optional_params": {}, + "litellm_params": {"custom_llm_provider": "openai"}, + "standard_logging_object": { + "id": "litellm-embed-call-789", + "call_type": "embedding", + "metadata": {}, + }, + } + # Embedding response has no "id" field + response_obj = { + "object": "list", + "data": [{"embedding": [0.1, 0.2], "index": 0}], + "model": "text-embedding-ada-002", + "usage": {"prompt_tokens": 5, "total_tokens": 5}, + } + + otel.set_attributes(mock_span, kwargs, response_obj) + + # Should fallback to litellm call ID + mock_span.set_attribute.assert_any_call( + "gen_ai.response.id", "litellm-embed-call-789" + ) + + def test_response_id_fallback_for_image_gen(self): + """When response_obj has no id (image gen), fallback to + standard_logging_payload['id'].""" + otel = OpenTelemetry() + mock_span = MagicMock() + + kwargs = { + "model": "dall-e-3", + "optional_params": {}, + "litellm_params": {"custom_llm_provider": "openai"}, + "standard_logging_object": { + "id": "litellm-img-call-101", + "call_type": "image_generation", + "metadata": {}, + }, + } + # Image response has no "id" field + response_obj = { + "created": 1234567890, + "data": [{"url": "https://example.com/img.png"}], + } + + otel.set_attributes(mock_span, kwargs, response_obj) + + # Should fallback to litellm call ID + mock_span.set_attribute.assert_any_call( + "gen_ai.response.id", "litellm-img-call-101" + )