diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 55e3ead36..7ea307159 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -544,8 +544,11 @@ def set_streaming_query_agent_run( OTEL_INSTRUMENTED_MODULES = ( + "app.endpoints.authorized", + "app.endpoints.feedback", "app.endpoints.query", "app.endpoints.responses", + "utils.agents.query", "utils.quota_utils", "utils.responses", "utils.shields", diff --git a/tests/integration/test_feedback_otel_trace.py b/tests/integration/test_feedback_otel_trace.py new file mode 100644 index 000000000..bac9b569e --- /dev/null +++ b/tests/integration/test_feedback_otel_trace.py @@ -0,0 +1,95 @@ +"""Integration tests for OpenTelemetry span tree on POST /feedback. + +This module covers the component interaction the feedback handler produces: +the ``feedback.storage`` span is opened inside the ``feedback.submit`` span, +so the two must share a trace and ``feedback.storage`` must be parented to +``feedback.submit``. +""" + +from pathlib import Path + +import pytest +from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( + InMemorySpanExporter, +) +from pytest_mock import MockerFixture + +from app.endpoints.feedback import feedback_endpoint_handler +from authentication.interface import AuthTuple +from configuration import configuration +from models.api.requests import FeedbackRequest +from models.common.feedback import FeedbackCategory + +ROOT_SPAN_NAME = "feedback.submit" +STORAGE_SPAN_NAME = "feedback.storage" +FEEDBACK_CONVERSATION_ID = "12345678-abcd-0000-0123-456789abcdef" + + +@pytest.fixture(autouse=True) +def _clear_spans(otel_collector: InMemorySpanExporter) -> None: + """Clear collected spans before each test.""" + otel_collector.clear() + + +@pytest.mark.asyncio +@pytest.mark.usefixtures("test_config") +async def test_feedback_storage_span_nested_under_submit( + tmp_path: Path, + test_auth: AuthTuple, + otel_collector: InMemorySpanExporter, + mocker: MockerFixture, +) -> None: + """POST /feedback nests feedback.storage under feedback.submit. + + Feedback storage is pointed at a writable temp directory so the real + filesystem write drives a successful storage span. The test asserts only the + parent-child hierarchy: both spans exist, share a single trace, and + ``feedback.storage`` is parented to ``feedback.submit``. Individual span + attributes and events are covered at the unit level. + + Args: + tmp_path: Writable temp directory used as the feedback storage location. + test_auth: Authentication tuple from the real noop auth dependency. + otel_collector: In-memory OTEL exporter collecting finished spans. + mocker: pytest-mock fixture used to patch conversation retrieval. + """ + user_id, _, _, _ = test_auth + configuration.user_data_collection_configuration.feedback_storage = str(tmp_path) + + # The conversation must exist and belong to the authenticated user. + mock_conversation = mocker.Mock() + mock_conversation.user_id = user_id + mocker.patch( + "app.endpoints.feedback.retrieve_conversation", + return_value=mock_conversation, + ) + + result = await feedback_endpoint_handler( + feedback_request=FeedbackRequest( + conversation_id=FEEDBACK_CONVERSATION_ID, + user_question="What is Kubernetes?", + llm_response="Kubernetes is an open-source container orchestrator.", + user_feedback="The answer was too vague.", + sentiment=-1, + categories=[FeedbackCategory.INCORRECT, FeedbackCategory.INCOMPLETE], + ), + auth=test_auth, + _ensure_feedback_enabled=None, + ) + assert result.response == "feedback received" + + spans = otel_collector.get_finished_spans() + span_names = {span.name for span in spans} + missing = {ROOT_SPAN_NAME, STORAGE_SPAN_NAME} - span_names + assert not missing, f"Missing expected spans: {missing}" + + root = next(span for span in spans if span.name == ROOT_SPAN_NAME) + storage = next(span for span in spans if span.name == STORAGE_SPAN_NAME) + assert root.context is not None + assert storage.context is not None + + assert root.context.trace_id == storage.context.trace_id + assert storage.parent is not None, "feedback.storage should have a parent" + assert ( + storage.parent.span_id == root.context.span_id + ), "feedback.storage should be parented to feedback.submit" diff --git a/tests/unit/app/endpoints/test_query_otel.py b/tests/unit/app/endpoints/test_query_otel.py new file mode 100644 index 000000000..9b76e6f14 --- /dev/null +++ b/tests/unit/app/endpoints/test_query_otel.py @@ -0,0 +1,206 @@ +# pylint: disable=redefined-outer-name +"""OpenTelemetry unit tests for the /query REST API endpoint.""" + +from typing import Any + +import pytest +from fastapi import HTTPException, Request, status +from ogx_client import AsyncOgxClient +from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( + InMemorySpanExporter, +) +from pytest_mock import MockerFixture + +from app.endpoints.query import query_endpoint_handler +from configuration import AppConfig +from models.api.requests import QueryRequest +from models.api.responses.error import QuotaExceededResponse +from models.common.moderation import ShieldModerationPassed +from models.common.responses.responses_api_params import ResponsesApiParams +from models.common.turn_summary import TurnSummary +from quota.quota_exceed_error import QuotaExceedError +from utils.otel_tracing import SpanAttributes, SpanEvents + +MODULE = "app.endpoints.query" +QUERY_SPAN_NAME = "query.handle_request" +QUERY_TEXT = "What is Kubernetes?" + +# User ID must be a proper UUID. +MOCK_AUTH = ( + "00000001-0001-0001-0001-000000000001", + "mock_username", + False, + "mock_token", +) + + +@pytest.fixture(name="dummy_request") +def dummy_request_fixture() -> Request: + """Minimal FastAPI Request for query endpoint unit tests.""" + return Request(scope={"type": "http", "headers": []}) + + +@pytest.fixture(name="minimal_config") +def minimal_config_fixture() -> AppConfig: + """Minimal AppConfig for query endpoint OTEL tests.""" + cfg = AppConfig() + cfg.init_from_dict( + { + "name": "test", + "service": {"host": "localhost", "port": 8080}, + "ogx": { + "api_key": "test-key", + "url": "http://test.com:1234", + "use_as_library_client": False, + }, + "user_data_collection": {"transcripts_enabled": False}, + "mcp_servers": [], + "conversation_cache": {"type": "noop"}, + } + ) + return cfg + + +def _patch_query_success(mocker: MockerFixture) -> None: + """Patch the query handler dependencies for a successful turn.""" + mocker.patch(f"{MODULE}.check_configuration_loaded") + mocker.patch(f"{MODULE}.check_tokens_available") + mocker.patch(f"{MODULE}.validate_model_provider_override") + mocker.patch(f"{MODULE}.check_mcp_auth", new=mocker.AsyncMock()) + + mock_response_obj = mocker.Mock() + mock_response_obj.output = [] + mock_client = mocker.AsyncMock(spec=AsyncOgxClient) + mock_client.responses = mocker.Mock() + mock_client.responses.create = mocker.AsyncMock(return_value=mock_response_obj) + mock_holder = mocker.Mock() + mock_holder.get_client.return_value = mock_client + mocker.patch(f"{MODULE}.AsyncOgxClientHolder", return_value=mock_holder) + + mocker.patch( + f"{MODULE}.maybe_get_topic_summary", + new=mocker.AsyncMock(return_value=None), + ) + mocker.patch( + f"{MODULE}.run_shield_moderation", + new=mocker.AsyncMock(return_value=ShieldModerationPassed()), + ) + + mock_params = mocker.Mock(spec=ResponsesApiParams) + mock_params.model = "provider1/model1" + mock_params.conversation = "conv_123" + mock_params.tools = None + mock_params.model_dump.return_value = {"input": "test", "model": "provider1/model1"} + mocker.patch( + f"{MODULE}.prepare_responses_params", + new=mocker.AsyncMock(return_value=mock_params), + ) + + turn_summary = TurnSummary() + turn_summary.llm_response = "Kubernetes is a container orchestration platform" + mocker.patch( + f"{MODULE}.retrieve_agent_response", + new=mocker.AsyncMock(return_value=turn_summary), + ) + + mocker.patch(f"{MODULE}.normalize_conversation_id", return_value="123") + mocker.patch(f"{MODULE}.store_query_results") + mocker.patch(f"{MODULE}.consume_query_tokens") + mocker.patch(f"{MODULE}.get_available_quotas", return_value={}) + + +@pytest.mark.asyncio +async def test_query_root_span_attributes_and_events( + dummy_request: Request, + minimal_config: AppConfig, + mocker: MockerFixture, + otel: tuple[Any, InMemorySpanExporter], +) -> None: + """The /query root span carries setup attributes and all lifecycle events. + + The mocked success path validates the request, persists the turn, and + completes the LLM response, so the validation/turn-persisted/LLM-response + events are all recorded, and the anonymized user/input attributes are set. + """ + tracer, exporter = otel + mocker.patch(f"{MODULE}.configuration", minimal_config) + mocker.patch(f"{MODULE}.tracer", tracer) + mocker.patch( + f"{MODULE}.anonymize_value", side_effect=lambda value: f"[anon:{value}]" + ) + _patch_query_success(mocker) + + await query_endpoint_handler( + request=dummy_request, + query_request=QueryRequest( + query=QUERY_TEXT + ), # pyright: ignore[reportCallIssue] + auth=MOCK_AUTH, + mcp_headers={}, + ) + + root = next(s for s in exporter.get_finished_spans() if s.name == QUERY_SPAN_NAME) + attrs = dict(root.attributes or {}) + assert attrs[SpanAttributes.USER_ID] == f"[anon:{MOCK_AUTH[0]}]" + assert attrs[SpanAttributes.INPUT] == f"[anon:{QUERY_TEXT}]" + assert attrs[SpanAttributes.REQUEST_ATTACHMENTS_COUNT] == 0 + assert SpanAttributes.OUTPUT in attrs + assert SpanAttributes.SESSION_ID in attrs + + event_names = {event.name for event in root.events} + assert SpanEvents.VALIDATION_COMPLETED in event_names + assert SpanEvents.TURN_PERSISTED in event_names + assert SpanEvents.LLM_RESPONSE_COMPLETED in event_names + + +@pytest.mark.asyncio +async def test_query_quota_exceeded_records_attributes_without_events( + dummy_request: Request, + minimal_config: AppConfig, + mocker: MockerFixture, + otel: tuple[Any, InMemorySpanExporter], +) -> None: + """A 429 from the quota check leaves the root span attributes but no events. + + The quota check runs after the root attributes are recorded but before any + lifecycle event fires. When it raises HTTP 429 the request is aborted, so the + ``query.handle_request`` span is still exported with its user/input + attributes, but none of the validation/turn-persisted/LLM-response events are + recorded, and tracing does not crash on the error path. + """ + tracer, exporter = otel + mocker.patch(f"{MODULE}.configuration", minimal_config) + mocker.patch(f"{MODULE}.tracer", tracer) + mocker.patch( + f"{MODULE}.anonymize_value", side_effect=lambda value: f"[anon:{value}]" + ) + mocker.patch(f"{MODULE}.check_configuration_loaded") + mocker.patch(f"{MODULE}.check_mcp_auth", new=mocker.AsyncMock()) + + def _raise_quota_exceeded(*_args: object, **_kwargs: object) -> None: + error = QuotaExceedError(subject_id=MOCK_AUTH[0], subject_type="u", available=0) + raise HTTPException(**QuotaExceededResponse.from_exception(error).model_dump()) + + mocker.patch(f"{MODULE}.check_tokens_available", side_effect=_raise_quota_exceeded) + + with pytest.raises(HTTPException) as exc_info: + await query_endpoint_handler( + request=dummy_request, + query_request=QueryRequest( # pyright: ignore[reportCallIssue] + query=QUERY_TEXT + ), + auth=MOCK_AUTH, + mcp_headers={}, + ) + assert exc_info.value.status_code == status.HTTP_429_TOO_MANY_REQUESTS + + root = next(s for s in exporter.get_finished_spans() if s.name == QUERY_SPAN_NAME) + attrs = dict(root.attributes or {}) + assert attrs[SpanAttributes.USER_ID] == f"[anon:{MOCK_AUTH[0]}]" + assert attrs[SpanAttributes.INPUT] == f"[anon:{QUERY_TEXT}]" + assert attrs[SpanAttributes.REQUEST_ATTACHMENTS_COUNT] == 0 + + event_names = {event.name for event in root.events} + assert SpanEvents.VALIDATION_COMPLETED not in event_names + assert SpanEvents.TURN_PERSISTED not in event_names + assert SpanEvents.LLM_RESPONSE_COMPLETED not in event_names diff --git a/tests/unit/app/endpoints/test_responses_otel.py b/tests/unit/app/endpoints/test_responses_otel.py index b1de65e81..e6139a71e 100644 --- a/tests/unit/app/endpoints/test_responses_otel.py +++ b/tests/unit/app/endpoints/test_responses_otel.py @@ -25,11 +25,15 @@ from models.config import Action from tests.unit.app.endpoints.responses_otel_helpers import ( MOCK_AUTH, + MODEL, MODULE, + OTEL_CONV_ID, + ROOT_SPAN_NAME, assert_root_setup_attributes, find_span, make_turn_summary_with_tools, make_turn_summary_without_tools, + patch_handler_success_mocks, patch_responses_endpoint_setup, patch_responses_otel_tracers, run_responses_setup_smoke, @@ -252,3 +256,47 @@ async def test_streaming_root_span_closed_on_setup_error( ) find_span(exporter.get_finished_spans(), "responses.handle_request") + + @pytest.mark.asyncio + async def test_llm_failure_keeps_validation_without_response_event( + self, + dummy_request: Request, + minimal_config: AppConfig, + mocker: MockerFixture, + otel: tuple[Any, InMemorySpanExporter], + ) -> None: + """A failing LLM call keeps validation.completed but drops llm.response.completed. + + ``validation.completed`` fires before the model is called; the + LLM-response event only fires once the turn is finalized. Forcing + ``responses.create`` to raise aborts the request after validation, so the + root span is exported with the validation event but without the + LLM-response event, and tracing does not crash on the error path. + """ + tracer, exporter = otel + patch_responses_otel_tracers(mocker, tracer, minimal_config) + mock_client = patch_responses_endpoint_setup(mocker, minimal_config) + patch_handler_success_mocks(mocker) + mock_client.responses.create = mocker.AsyncMock( + side_effect=ApiException(status=None, reason="connection failed") + ) + + with pytest.raises(HTTPException): + await responses_endpoint_handler( + request=dummy_request, + responses_request=ResponsesRequest( + input=INPUT_TEXT, + model=MODEL, + stream=False, + store=False, + conversation=OTEL_CONV_ID, + generate_topic_summary=False, + ), + auth=MOCK_AUTH, + mcp_headers={}, + ) + + root = find_span(exporter.get_finished_spans(), ROOT_SPAN_NAME) + event_names = [event.name for event in root.events] + assert SpanEvents.VALIDATION_COMPLETED in event_names + assert SpanEvents.LLM_RESPONSE_COMPLETED not in event_names