diff --git a/src/backend/api/event_utils.py b/src/backend/api/event_utils.py index 11905a8a..69ed1a83 100644 --- a/src/backend/api/event_utils.py +++ b/src/backend/api/event_utils.py @@ -4,7 +4,6 @@ # Third-party from applicationinsights import TelemetryClient -from applicationinsights.channel import SynchronousQueue, SynchronousSender, TelemetryChannel from dotenv import load_dotenv @@ -28,12 +27,8 @@ def _get_telemetry_client(): instrumentation_key = parts.get('InstrumentationKey') if instrumentation_key: - # Create a synchronous channel for immediate sending - sender = SynchronousSender() - queue = SynchronousQueue(sender) - channel = TelemetryChannel(None, queue) - - _telemetry_client = TelemetryClient(instrumentation_key, channel) + # Use the default (buffered/async) channel configuration + _telemetry_client = TelemetryClient(instrumentation_key) logging.info("Application Insights TelemetryClient initialized successfully") else: logging.error("Could not extract InstrumentationKey from connection string") @@ -59,7 +54,9 @@ def track_event_if_configured(event_name: str, event_data: dict): # Track the custom event client.track_event(event_name, properties=properties) - client.flush() # Ensure immediate sending + + # Flush to ensure events are sent immediately + client.flush() logging.debug(f"Tracked custom event: {event_name} with data: {event_data}") else: diff --git a/src/backend/app.py b/src/backend/app.py index 58ee1a29..17572652 100644 --- a/src/backend/app.py +++ b/src/backend/app.py @@ -161,8 +161,20 @@ def create_app() -> FastAPI: set_logger_provider(logger_provider) # Attach OpenTelemetry handler to Python's root logger - handler = LoggingHandler(logger_provider=logger_provider) - logging.getLogger().addHandler(handler) + # Ensure we don't accumulate stale handlers or mismatch logger providers + root_logger = logging.getLogger() + # Remove any existing OpenTelemetry logging handlers to avoid duplicates + for handler in list(root_logger.handlers): + if isinstance(handler, LoggingHandler): + root_logger.removeHandler(handler) + # Close the handler to release any associated exporter/background resources + try: + handler.close() + except Exception as exc: + # Guard against unexpected close failures + logging.error("Error while closing logging handler %r: %s", handler, exc) + # Add a fresh handler bound to the current logger_provider + root_logger.addHandler(LoggingHandler(logger_provider=logger_provider)) # Instrument ONLY FastAPI for HTTP request/response tracing # This is safe because it only wraps HTTP handlers, not internal async operations diff --git a/src/tests/backend/api/event_utils_test.py b/src/tests/backend/api/event_utils_test.py index 9e6e2b0f..9a4d0467 100644 --- a/src/tests/backend/api/event_utils_test.py +++ b/src/tests/backend/api/event_utils_test.py @@ -11,7 +11,7 @@ class TestTrackEventIfConfigured: def test_track_event_with_instrumentation_key(self): """Test tracking event when instrumentation key is set.""" - connection_string = "InstrumentationKey=test-key;IngestionEndpoint=https://test.com" + connection_string = "InstrumentationKey=12345678-1234-5678-1234-567812345678;IngestionEndpoint=https://test.com" with patch.dict(os.environ, {"APPLICATIONINSIGHTS_CONNECTION_STRING": connection_string}): with patch("backend.api.event_utils._get_telemetry_client") as mock_get_client: mock_client = MagicMock() @@ -36,7 +36,7 @@ def test_track_event_without_instrumentation_key(self): def test_track_event_with_empty_data(self): """Test tracking event with empty data.""" - connection_string = "InstrumentationKey=test-key;IngestionEndpoint=https://test.com" + connection_string = "InstrumentationKey=12345678-1234-5678-1234-567812345678;IngestionEndpoint=https://test.com" with patch.dict(os.environ, {"APPLICATIONINSIGHTS_CONNECTION_STRING": connection_string}): with patch("backend.api.event_utils._get_telemetry_client") as mock_get_client: mock_client = MagicMock() @@ -49,7 +49,7 @@ def test_track_event_with_empty_data(self): def test_track_event_with_complex_data(self): """Test tracking event with complex data.""" - connection_string = "InstrumentationKey=test-key;IngestionEndpoint=https://test.com" + connection_string = "InstrumentationKey=12345678-1234-5678-1234-567812345678;IngestionEndpoint=https://test.com" with patch.dict(os.environ, {"APPLICATIONINSIGHTS_CONNECTION_STRING": connection_string}): with patch("backend.api.event_utils._get_telemetry_client") as mock_get_client: mock_client = MagicMock() @@ -77,7 +77,7 @@ def test_track_event_with_complex_data(self): def test_track_event_client_returns_none(self): """Test tracking event when client initialization fails.""" - connection_string = "InstrumentationKey=test-key;IngestionEndpoint=https://test.com" + connection_string = "InstrumentationKey=12345678-1234-5678-1234-567812345678;IngestionEndpoint=https://test.com" with patch.dict(os.environ, {"APPLICATIONINSIGHTS_CONNECTION_STRING": connection_string}): with patch("backend.api.event_utils._get_telemetry_client") as mock_get_client: mock_get_client.return_value = None @@ -88,7 +88,7 @@ def test_track_event_client_returns_none(self): def test_track_event_with_exception(self): """Test tracking event when an exception occurs.""" - connection_string = "InstrumentationKey=test-key;IngestionEndpoint=https://test.com" + connection_string = "InstrumentationKey=12345678-1234-5678-1234-567812345678;IngestionEndpoint=https://test.com" with patch.dict(os.environ, {"APPLICATIONINSIGHTS_CONNECTION_STRING": connection_string}): with patch("backend.api.event_utils._get_telemetry_client") as mock_get_client: mock_client = MagicMock() diff --git a/src/tests/backend/app_test.py b/src/tests/backend/app_test.py index ea93d4e7..5dddb3c9 100644 --- a/src/tests/backend/app_test.py +++ b/src/tests/backend/app_test.py @@ -1,6 +1,9 @@ # pylint: disable=redefined-outer-name """Tests for the FastAPI application.""" +import logging +import os + from backend.app import create_app from fastapi import FastAPI @@ -8,6 +11,8 @@ from httpx import ASGITransport from httpx import AsyncClient +from opentelemetry.sdk._logs import LoggingHandler + import pytest @@ -34,3 +39,50 @@ async def test_backend_routes_exist(app: FastAPI): routes = [route.path for route in app.router.routes] backend_routes = [r for r in routes if r.startswith("/api")] assert backend_routes, "No backend routes found under /api prefix" + + +def test_logging_handler_deduplication(): + """Test that creating multiple apps doesn't accumulate LoggingHandler instances.""" + # Set up Application Insights connection string to trigger telemetry setup + # Use a valid UUID format for the instrumentation key + connection_string = "InstrumentationKey=12345678-1234-5678-1234-567812345678;IngestionEndpoint=https://test.com" + original_env = os.environ.get("APPLICATIONINSIGHTS_CONNECTION_STRING") + + try: + os.environ["APPLICATIONINSIGHTS_CONNECTION_STRING"] = connection_string + + # Get root logger and count existing LoggingHandlers + root_logger = logging.getLogger() + initial_handler_count = sum(1 for h in root_logger.handlers if isinstance(h, LoggingHandler)) + + # Create first app + create_app() + handler_count_after_first = sum(1 for h in root_logger.handlers if isinstance(h, LoggingHandler)) + + # Create second app + create_app() + handler_count_after_second = sum(1 for h in root_logger.handlers if isinstance(h, LoggingHandler)) + + # Assert only one LoggingHandler exists after multiple create_app() calls + assert handler_count_after_first == initial_handler_count + 1, \ + "First create_app() should add one LoggingHandler" + assert handler_count_after_second == handler_count_after_first, \ + "Second create_app() should not add another LoggingHandler (de-duplication should work)" + + # Clean up - remove the handler we added + for handler in list(root_logger.handlers): + if isinstance(handler, LoggingHandler): + root_logger.removeHandler(handler) + try: + handler.close() + except Exception: + logging.getLogger(__name__).debug( + "Failed to close LoggingHandler during test cleanup", + exc_info=True, + ) + finally: + # Restore original environment + if original_env is not None: + os.environ["APPLICATIONINSIGHTS_CONNECTION_STRING"] = original_env + else: + os.environ.pop("APPLICATIONINSIGHTS_CONNECTION_STRING", None)