mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-05 08:23:17 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Haiku 4.5
parent
3a2cba43dc
commit
0b67b642cb
@@ -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")
|
||||
|
||||
|
||||
@@ -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"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user