-
Notifications
You must be signed in to change notification settings - Fork 98
LCORE-2981: Add OpenTelemetry instrumentation for POST /v1/streaming_query endpoint #2415
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |
| APIStatusError as LLSApiStatusError, | ||
| ) | ||
| from openai._exceptions import APIStatusError as OpenAIAPIStatusError | ||
| from opentelemetry import trace | ||
|
|
||
| from authentication import get_auth_dependency | ||
| from authentication.interface import AuthTuple | ||
|
|
@@ -65,6 +66,13 @@ | |
| ) | ||
| from utils.mcp_headers import McpHeaders, mcp_headers_dependency | ||
| from utils.mcp_oauth_probe import check_mcp_auth | ||
| from utils.otel_tracing import ( | ||
| SpanAttributes, | ||
| SpanEvents, | ||
| add_span_event, | ||
| anonymize_value, | ||
| set_span_attributes, | ||
| ) | ||
| from utils.query import ( | ||
| extract_provider_and_model_from_model_id, | ||
| handle_known_apistatus_errors, | ||
|
|
@@ -93,6 +101,7 @@ | |
| from utils.vector_search import build_rag_context | ||
|
|
||
| logger = get_logger(__name__) | ||
| tracer = trace.get_tracer(__name__) | ||
| router = APIRouter(tags=["streaming_query"]) | ||
|
|
||
| # Tracks background topic summary tasks for graceful shutdown. | ||
|
|
@@ -158,11 +167,55 @@ async def streaming_query_endpoint_handler( # pylint: disable=too-many-locals | |
| - 500: Internal Server Error - Configuration not loaded or other server errors | ||
| - 503: Service Unavailable - Unable to connect to OGX backend | ||
| """ | ||
| root_span = tracer.start_span("streaming_query.handle_request") | ||
| try: | ||
| return await _handle_streaming_query_with_tracing( | ||
| request, query_request, auth, mcp_headers, root_span | ||
| ) | ||
| except Exception: | ||
| root_span.end() | ||
| raise | ||
|
|
||
|
|
||
| async def _handle_streaming_query_with_tracing( # pylint: disable=too-many-locals | ||
| request: Request, | ||
| query_request: QueryRequest, | ||
| auth: AuthTuple, | ||
| mcp_headers: McpHeaders, | ||
| root_span: trace.Span, | ||
| ) -> StreamingResponse: | ||
| """Handle streaming query request with OTEL tracing instrumentation. | ||
|
|
||
| Parameters: | ||
| request: The incoming HTTP request. | ||
| query_request: Request payload containing query and optional parameters. | ||
| auth: Authentication tuple (user_id, username, skip_check, token). | ||
| mcp_headers: Headers to be passed to MCP servers. | ||
| root_span: OpenTelemetry root span for this request. | ||
|
|
||
| Returns: | ||
| StreamingResponse with SSE-formatted events. | ||
|
|
||
| Raises: | ||
| HTTPException: On authentication, authorization, quota, or model errors. | ||
| """ | ||
| check_configuration_loaded(configuration) | ||
|
|
||
| user_id, _user_name, _skip_userid_check, token = auth | ||
| started_at = datetime.datetime.now(datetime.UTC).strftime("%Y-%m-%dT%H:%M:%SZ") | ||
|
|
||
| # Set initial span attributes | ||
| set_span_attributes( | ||
| root_span, | ||
| { | ||
| SpanAttributes.USER_ID: anonymize_value(user_id), | ||
| SpanAttributes.INPUT: anonymize_value(query_request.query), | ||
| SpanAttributes.REQUEST_ATTACHMENTS_COUNT: ( | ||
| len(query_request.attachments) if query_request.attachments else 0 | ||
| ), | ||
| }, | ||
| ) | ||
|
|
||
| # Check MCP Auth | ||
| await check_mcp_auth(configuration, mcp_headers, token, request.headers) | ||
|
|
||
|
|
@@ -181,6 +234,9 @@ async def streaming_query_endpoint_handler( # pylint: disable=too-many-locals | |
| if query_request.attachments: | ||
| validate_attachments_metadata(query_request.attachments) | ||
|
|
||
| # Validation completed | ||
| add_span_event(root_span, SpanEvents.VALIDATION_COMPLETED) | ||
|
|
||
| # Retrieve conversation if conversation_id is provided | ||
| user_conversation = None | ||
| if query_request.conversation_id: | ||
|
|
@@ -291,6 +347,7 @@ async def streaming_query_endpoint_handler( # pylint: disable=too-many-locals | |
| responses_params=responses_params, | ||
| endpoint_path=endpoint_path, | ||
| image_attachments=image_attachments, | ||
| root_span=root_span, | ||
| ), | ||
| media_type=response_media_type, | ||
| ) | ||
|
|
@@ -316,6 +373,7 @@ async def streaming_query_endpoint_handler( # pylint: disable=too-many-locals | |
| responses_params=responses_params, | ||
| turn_summary=turn_summary, | ||
| background_topic_summary_tasks=_background_topic_summary_tasks, | ||
| root_span=root_span, | ||
| ), | ||
| media_type=response_media_type, | ||
| ) | ||
|
|
@@ -344,6 +402,7 @@ async def generate_response_with_compaction( | |
| responses_params: ResponsesApiParams, | ||
| endpoint_path: str, | ||
| image_attachments: Optional[list[Attachment]] = None, | ||
| root_span: Optional[trace.Span] = None, | ||
| ) -> AsyncIterator[str]: | ||
| """Stream a response for a conversation that requires compaction. | ||
|
|
||
|
|
@@ -359,79 +418,85 @@ async def generate_response_with_compaction( | |
| responses_params: The base Responses API parameters. | ||
| endpoint_path: API endpoint path used for metric labeling. | ||
| image_attachments: Image attachments for multimodal prompt construction. | ||
| root_span: OpenTelemetry root span for this request. | ||
|
|
||
| Yields: | ||
| SSE-formatted strings. | ||
| """ | ||
| media_type = context.query_request.media_type or MEDIA_TYPE_JSON | ||
| yield stream_start_event( | ||
| conversation_id=context.conversation_id, | ||
| request_id=context.request_id, | ||
| ) | ||
|
|
||
| compacted_original_input: Optional[ResponseInput] = None | ||
| try: | ||
| async for item in apply_compaction( | ||
| context.client, | ||
| responses_params, | ||
| configuration.inference, | ||
| configuration.compaction, | ||
| emit_events=True, | ||
| cache=configured_conversation_cache(), | ||
| user_id=context.user_id, | ||
| skip_user_id_check=context.skip_userid_check, | ||
| ): | ||
| if isinstance(item, CompactionStartedEvent): | ||
| yield stream_compaction_event(context.conversation_id) | ||
| elif isinstance(item, CompactionResult): | ||
| responses_params = item.params | ||
| compacted_original_input = item.original_input | ||
|
|
||
| generator, turn_summary = await retrieve_agent_response_generator( | ||
| responses_params=responses_params, | ||
| context=context, | ||
| endpoint_path=endpoint_path, | ||
| image_attachments=image_attachments, | ||
| ) | ||
| except HTTPException as e: | ||
| yield http_exception_stream_event(e) | ||
| return | ||
| except RuntimeError as e: # library mode wraps 413 into runtime error | ||
| error_response = ( | ||
| PromptTooLongResponse(model=responses_params.model) | ||
| if is_context_length_error(str(e)) | ||
| else InternalServerErrorResponse.generic() | ||
| ) | ||
| yield stream_http_error_event(error_response, media_type) | ||
| return | ||
| except APIConnectionError as e: | ||
| yield stream_http_error_event( | ||
| ServiceUnavailableResponse(backend_name="OGX", cause=str(e)), | ||
| media_type, | ||
| ) | ||
| return | ||
| except (LLSApiStatusError, OpenAIAPIStatusError) as e: | ||
| yield stream_http_error_event( | ||
| handle_known_apistatus_errors(e, responses_params.model), media_type | ||
| ) | ||
| return | ||
|
|
||
| # Combine inline RAG results (BYOK + Solr) with tool-based results | ||
| if context.moderation_result.decision == "passed": | ||
| turn_summary.referenced_documents = deduplicate_referenced_documents( | ||
| context.inline_rag_context.referenced_documents | ||
| + turn_summary.referenced_documents | ||
| media_type = context.query_request.media_type or MEDIA_TYPE_JSON | ||
| yield stream_start_event( | ||
| conversation_id=context.conversation_id, | ||
| request_id=context.request_id, | ||
| ) | ||
|
|
||
| # The start event was already emitted above; delegate the rest (re-yield, | ||
| # finalization, compacted-turn storage) to the shared generator. | ||
| async for event in generate_agent_response( | ||
| generator, | ||
| context, | ||
| responses_params, | ||
| turn_summary, | ||
| background_topic_summary_tasks=_background_topic_summary_tasks, | ||
| emit_start=False, | ||
| original_input=compacted_original_input, | ||
| ): | ||
| yield event | ||
| compacted_original_input: Optional[ResponseInput] = None | ||
| try: | ||
| async for item in apply_compaction( | ||
| context.client, | ||
| responses_params, | ||
| configuration.inference, | ||
| configuration.compaction, | ||
| emit_events=True, | ||
| cache=configured_conversation_cache(), | ||
| user_id=context.user_id, | ||
| skip_user_id_check=context.skip_userid_check, | ||
| ): | ||
| if isinstance(item, CompactionStartedEvent): | ||
| yield stream_compaction_event(context.conversation_id) | ||
| elif isinstance(item, CompactionResult): | ||
| responses_params = item.params | ||
| compacted_original_input = item.original_input | ||
|
|
||
| generator, turn_summary = await retrieve_agent_response_generator( | ||
| responses_params=responses_params, | ||
| context=context, | ||
| endpoint_path=endpoint_path, | ||
| image_attachments=image_attachments, | ||
| ) | ||
| except HTTPException as e: | ||
| yield http_exception_stream_event(e) | ||
| return | ||
| except RuntimeError as e: # library mode wraps 413 into runtime error | ||
| error_response = ( | ||
| PromptTooLongResponse(model=responses_params.model) | ||
| if is_context_length_error(str(e)) | ||
| else InternalServerErrorResponse.generic() | ||
| ) | ||
| yield stream_http_error_event(error_response, media_type) | ||
| return | ||
| except APIConnectionError as e: | ||
| yield stream_http_error_event( | ||
| ServiceUnavailableResponse(backend_name="OGX", cause=str(e)), | ||
| media_type, | ||
| ) | ||
| return | ||
| except (LLSApiStatusError, OpenAIAPIStatusError) as e: | ||
| yield stream_http_error_event( | ||
| handle_known_apistatus_errors(e, responses_params.model), media_type | ||
| ) | ||
| return | ||
|
|
||
| # Combine inline RAG results (BYOK + Solr) with tool-based results | ||
| if context.moderation_result.decision == "passed": | ||
| turn_summary.referenced_documents = deduplicate_referenced_documents( | ||
| context.inline_rag_context.referenced_documents | ||
| + turn_summary.referenced_documents | ||
| ) | ||
|
|
||
| # The start event was already emitted above; delegate the rest (re-yield, | ||
| # finalization, compacted-turn storage) to the shared generator. | ||
| async for event in generate_agent_response( | ||
| generator, | ||
| context, | ||
| responses_params, | ||
| turn_summary, | ||
| background_topic_summary_tasks=_background_topic_summary_tasks, | ||
| emit_start=False, | ||
| original_input=compacted_original_input, | ||
| root_span=root_span, | ||
| ): | ||
| yield event | ||
| finally: | ||
| if root_span is not None: | ||
| root_span.end() | ||
|
Comment on lines
+489
to
+502
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🌐 Web query:
💡 Result: In the OpenTelemetry Python SDK, the official specification for the Span.end method states that only the first call to end should modify the span [1][2][3]. Implementations are free to either ignore subsequent calls or raise an error when end is called more than once [2][4][5]. If your application triggers a second call to span.end, you may encounter warnings or logs such as "Calling end on an ended span" depending on the specific library implementation being used [6]. Because the Span object is intended to represent a single, discrete operation with a fixed duration, calling end multiple times is considered an anti-pattern and often indicates a logic error in the instrumentation code, such as improper handling of conditional paths (e.g., calling end on both success and failure paths, or within a context manager block that already handles closing the span) [6][2][7]. To resolve this: 1. Review your instrumentation logic to ensure that span.end is called exactly once per span lifecycle [6]. 2. If you are using a context manager (the Citations:
🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
fd -t f 'streaming_query\.py|streaming\.py' src
printf '%s\n' '--- relevant definitions and calls ---'
rg -n -C 5 'def generate_agent_response|generate_agent_response\(|root_span|end_span' src/app/endpoints/streaming_query.py src/utils/agents/streaming.py
printf '%s\n' '--- focused source ranges ---'
sed -n '220,355p' src/utils/agents/streaming.py
sed -n '450,525p' src/app/endpoints/streaming_query.pyRepository: lightspeed-core/lightspeed-stack Length of output: 23484 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- OpenTelemetry version pins ---'
rg -n -i -C 2 'opentelemetry|otel' pyproject.toml poetry.lock uv.lock requirements*.txt setup.cfg setup.py 2>/dev/null || true
printf '%s\n' '--- full shared generator control flow ---'
sed -n '164,345p' src/utils/agents/streaming.py
printf '%s\n' '--- compaction wrapper control flow ---'
sed -n '403,510p' src/app/endpoints/streaming_query.py
printf '%s\n' '--- available installed OpenTelemetry modules ---'
python3 - <<'PY'
try:
import inspect
import opentelemetry.sdk.trace
from opentelemetry.sdk.trace import ReadableSpan
print("sdk_trace:", inspect.getfile(opentelemetry.sdk.trace))
print("ReadableSpan.end source:")
print(inspect.getsource(ReadableSpan.end))
except Exception as exc:
print(type(exc).__name__ + ":", exc)
PYRepository: lightspeed-core/lightspeed-stack Length of output: 30922 🌐 Web query:
💡 Result: The warning "Calling end on an ended span" in the OpenTelemetry Python SDK indicates that the end method is being invoked more than once on the same Span object [1][2]. In the OpenTelemetry Python SDK, the Span.end method includes a thread-safe check to verify if the span has already been ended [1][2]. If a second call to end is made, the SDK logs this warning to alert you that the operation has already been completed [1][2]. This behavior aligns with the OpenTelemetry specification, which states that implementations should ignore subsequent calls to end [3][4]. Common causes for this warning include: 1. Double Instrumentation or Logic Errors: Often caused by custom code or instrumentation libraries that inadvertently call end multiple times on the same span [5]. For example, if an instrumentation wrapper ends a span upon encountering a specific result, but the main application logic or another decorator also calls end, the second call will trigger this warning [5]. 2. Context Manager Issues: If you are using a span as a context manager (e.g., Citations:
Give 🤖 Prompt for AI Agents |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Anonymization failure now fails the request.
anonymize_valueraisesValueErrorwhenOTEL_ANONYMIZATION_SECRETis unset andOTEL_SDK_DISABLEDis nottrue/1. This call runs before any request validation, so a deployment that enables the OTEL SDK without the secret returns 500 for everyPOST /v1/streaming_query. Tracing should not be able to break the endpoint.Fail soft here and log a warning instead.
🛡️ Proposed guard
📝 Committable suggestion
🤖 Prompt for AI Agents