From 3685020735d2aa6c79680b21118967a65f4835a7 Mon Sep 17 00:00:00 2001 From: Harmanpreet-Microsoft Date: Mon, 16 Mar 2026 18:25:27 +0530 Subject: [PATCH 01/10] Refactor telemetry client initialization and event tracking in event_utils module --- src/backend/api/event_utils.py | 14 +++++--------- src/backend/app.py | 7 +++++-- src/tests/backend/api/event_utils_test.py | 3 --- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/backend/api/event_utils.py b/src/backend/api/event_utils.py index 11905a8a..c98d52d0 100644 --- a/src/backend/api/event_utils.py +++ b/src/backend/api/event_utils.py @@ -4,8 +4,6 @@ # Third-party from applicationinsights import TelemetryClient -from applicationinsights.channel import SynchronousQueue, SynchronousSender, TelemetryChannel - from dotenv import load_dotenv load_dotenv() @@ -28,12 +26,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 +53,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..4ef00a7d 100644 --- a/src/backend/app.py +++ b/src/backend/app.py @@ -161,8 +161,11 @@ 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) + # Guard against duplicate handlers in hot-reload or test scenarios + root_logger = logging.getLogger() + if not any(isinstance(h, LoggingHandler) for h in root_logger.handlers): + handler = LoggingHandler(logger_provider=logger_provider) + root_logger.addHandler(handler) # 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..9bf8b4ed 100644 --- a/src/tests/backend/api/event_utils_test.py +++ b/src/tests/backend/api/event_utils_test.py @@ -20,7 +20,6 @@ def test_track_event_with_instrumentation_key(self): track_event_if_configured("TestEvent", {"key": "value"}) mock_client.track_event.assert_called_once_with("TestEvent", properties={"key": "value"}) - mock_client.flush.assert_called_once() def test_track_event_without_instrumentation_key(self): """Test tracking event when instrumentation key is not set.""" @@ -45,7 +44,6 @@ def test_track_event_with_empty_data(self): track_event_if_configured("TestEvent", {}) mock_client.track_event.assert_called_once_with("TestEvent", properties={}) - mock_client.flush.assert_called_once() def test_track_event_with_complex_data(self): """Test tracking event with complex data.""" @@ -73,7 +71,6 @@ def test_track_event_with_complex_data(self): } mock_client.track_event.assert_called_once_with("ComplexEvent", properties=expected_properties) - mock_client.flush.assert_called_once() def test_track_event_client_returns_none(self): """Test tracking event when client initialization fails.""" From 936272c1ef4dda2d0c62355a27efb7630cc40f88 Mon Sep 17 00:00:00 2001 From: Harmanpreet-Microsoft Date: Mon, 16 Mar 2026 18:28:45 +0530 Subject: [PATCH 02/10] Fix import order and clean up whitespace in event_utils module --- src/backend/api/event_utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/backend/api/event_utils.py b/src/backend/api/event_utils.py index c98d52d0..de72567b 100644 --- a/src/backend/api/event_utils.py +++ b/src/backend/api/event_utils.py @@ -3,9 +3,10 @@ import os # Third-party -from applicationinsights import TelemetryClient from dotenv import load_dotenv +from applicationinsights import TelemetryClient + load_dotenv() # Global telemetry client (initialized once) @@ -53,7 +54,7 @@ def track_event_if_configured(event_name: str, event_data: dict): # Track the custom event client.track_event(event_name, properties=properties) - + # Flush to ensure events are sent immediately client.flush() From fac619685f58180201abe79dfd5a0d4e4cc56980 Mon Sep 17 00:00:00 2001 From: Harmanpreet-Microsoft Date: Mon, 16 Mar 2026 18:31:18 +0530 Subject: [PATCH 03/10] Refactor import statements in event_utils module --- src/backend/api/event_utils.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/api/event_utils.py b/src/backend/api/event_utils.py index de72567b..61de1863 100644 --- a/src/backend/api/event_utils.py +++ b/src/backend/api/event_utils.py @@ -3,9 +3,8 @@ import os # Third-party -from dotenv import load_dotenv - from applicationinsights import TelemetryClient +from dotenv import load_dotenv load_dotenv() From 2390c725fe931b6776761fc6316abd687bf16927 Mon Sep 17 00:00:00 2001 From: Harmanpreet-Microsoft Date: Mon, 16 Mar 2026 18:36:44 +0530 Subject: [PATCH 04/10] Enhance OpenTelemetry logging handler management and update tests to ensure flush is called --- src/backend/app.py | 11 +++++++---- src/tests/backend/api/event_utils_test.py | 3 +++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/backend/app.py b/src/backend/app.py index 4ef00a7d..69ea3c8a 100644 --- a/src/backend/app.py +++ b/src/backend/app.py @@ -161,11 +161,14 @@ def create_app() -> FastAPI: set_logger_provider(logger_provider) # Attach OpenTelemetry handler to Python's root logger - # Guard against duplicate handlers in hot-reload or test scenarios + # Ensure we don't accumulate stale handlers or mismatch logger providers root_logger = logging.getLogger() - if not any(isinstance(h, LoggingHandler) for h in root_logger.handlers): - handler = LoggingHandler(logger_provider=logger_provider) - root_logger.addHandler(handler) + # Remove any existing OpenTelemetry logging handlers to avoid duplicates + for handler in list(root_logger.handlers): + if isinstance(handler, LoggingHandler): + root_logger.removeHandler(handler) + # 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 9bf8b4ed..9e6e2b0f 100644 --- a/src/tests/backend/api/event_utils_test.py +++ b/src/tests/backend/api/event_utils_test.py @@ -20,6 +20,7 @@ def test_track_event_with_instrumentation_key(self): track_event_if_configured("TestEvent", {"key": "value"}) mock_client.track_event.assert_called_once_with("TestEvent", properties={"key": "value"}) + mock_client.flush.assert_called_once() def test_track_event_without_instrumentation_key(self): """Test tracking event when instrumentation key is not set.""" @@ -44,6 +45,7 @@ def test_track_event_with_empty_data(self): track_event_if_configured("TestEvent", {}) mock_client.track_event.assert_called_once_with("TestEvent", properties={}) + mock_client.flush.assert_called_once() def test_track_event_with_complex_data(self): """Test tracking event with complex data.""" @@ -71,6 +73,7 @@ def test_track_event_with_complex_data(self): } mock_client.track_event.assert_called_once_with("ComplexEvent", properties=expected_properties) + mock_client.flush.assert_called_once() def test_track_event_client_returns_none(self): """Test tracking event when client initialization fails.""" From df015b9ea83f4737cea1c09caf32165b54603411 Mon Sep 17 00:00:00 2001 From: Harmanpreet-Microsoft Date: Mon, 16 Mar 2026 18:45:10 +0530 Subject: [PATCH 05/10] Add handler closure in logging setup and update test imports --- src/backend/app.py | 6 ++++++ src/tests/backend/app_test.py | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/src/backend/app.py b/src/backend/app.py index 69ea3c8a..592c38c6 100644 --- a/src/backend/app.py +++ b/src/backend/app.py @@ -167,6 +167,12 @@ def create_app() -> FastAPI: 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 AttributeError: + # Some handlers may not have a close method + pass # Add a fresh handler bound to the current logger_provider root_logger.addHandler(LoggingHandler(logger_provider=logger_provider)) diff --git a/src/tests/backend/app_test.py b/src/tests/backend/app_test.py index ea93d4e7..44651b69 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 From 8a64e5cea6eb6be0657cc84ad7a53ed2dd4aae63 Mon Sep 17 00:00:00 2001 From: Harmanpreet-Microsoft Date: Mon, 16 Mar 2026 18:53:14 +0530 Subject: [PATCH 06/10] Enhance error handling during logging handler closure and clean up test imports --- src/backend/app.py | 6 +++--- src/tests/backend/app_test.py | 5 ----- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/backend/app.py b/src/backend/app.py index 592c38c6..17572652 100644 --- a/src/backend/app.py +++ b/src/backend/app.py @@ -170,9 +170,9 @@ def create_app() -> FastAPI: # Close the handler to release any associated exporter/background resources try: handler.close() - except AttributeError: - # Some handlers may not have a close method - pass + 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)) diff --git a/src/tests/backend/app_test.py b/src/tests/backend/app_test.py index 44651b69..ea93d4e7 100644 --- a/src/tests/backend/app_test.py +++ b/src/tests/backend/app_test.py @@ -1,9 +1,6 @@ # pylint: disable=redefined-outer-name """Tests for the FastAPI application.""" -import logging -import os - from backend.app import create_app from fastapi import FastAPI @@ -11,8 +8,6 @@ from httpx import ASGITransport from httpx import AsyncClient -from opentelemetry.sdk._logs import LoggingHandler - import pytest From 8a668fd40a513f3af38446873c5dd9e9a063fd66 Mon Sep 17 00:00:00 2001 From: Harmanpreet-Microsoft Date: Mon, 16 Mar 2026 19:06:31 +0530 Subject: [PATCH 07/10] Add test for LoggingHandler deduplication in app creation --- src/tests/backend/app_test.py | 48 +++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/tests/backend/app_test.py b/src/tests/backend/app_test.py index ea93d4e7..b8ecf076 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,46 @@ 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 + connection_string = "InstrumentationKey=test-key;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 + app1 = create_app() + handler_count_after_first = sum(1 for h in root_logger.handlers if isinstance(h, LoggingHandler)) + + # Create second app + app2 = 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: + pass + 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) From 2b48edb3e17b5d0853df210b244b25653964d6a2 Mon Sep 17 00:00:00 2001 From: Harmanpreet-Microsoft Date: Mon, 16 Mar 2026 19:11:36 +0530 Subject: [PATCH 08/10] Update instrumentation key in event_utils tests and logging handler deduplication test --- src/tests/backend/api/event_utils_test.py | 10 +++++----- src/tests/backend/app_test.py | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) 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 b8ecf076..43be9aea 100644 --- a/src/tests/backend/app_test.py +++ b/src/tests/backend/app_test.py @@ -44,7 +44,8 @@ async def test_backend_routes_exist(app: FastAPI): 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 - connection_string = "InstrumentationKey=test-key;IngestionEndpoint=https://test.com" + # 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: From 11528185b19c3ad0a751ab223b72c9210903d83e Mon Sep 17 00:00:00 2001 From: Harmanpreet-Microsoft Date: Mon, 16 Mar 2026 19:16:57 +0530 Subject: [PATCH 09/10] Refactor logging handler deduplication test to remove unnecessary app variable assignments and enhance error logging during cleanup --- src/tests/backend/app_test.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/tests/backend/app_test.py b/src/tests/backend/app_test.py index 43be9aea..5dddb3c9 100644 --- a/src/tests/backend/app_test.py +++ b/src/tests/backend/app_test.py @@ -56,11 +56,11 @@ def test_logging_handler_deduplication(): initial_handler_count = sum(1 for h in root_logger.handlers if isinstance(h, LoggingHandler)) # Create first app - app1 = create_app() + create_app() handler_count_after_first = sum(1 for h in root_logger.handlers if isinstance(h, LoggingHandler)) # Create second app - app2 = create_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 @@ -76,7 +76,10 @@ def test_logging_handler_deduplication(): try: handler.close() except Exception: - pass + logging.getLogger(__name__).debug( + "Failed to close LoggingHandler during test cleanup", + exc_info=True, + ) finally: # Restore original environment if original_env is not None: From fb094c8689d3ac08a284ed9ac19ef2f72dafb116 Mon Sep 17 00:00:00 2001 From: Harmanpreet-Microsoft Date: Mon, 16 Mar 2026 19:19:18 +0530 Subject: [PATCH 10/10] Add missing import for os module in event_utils.py --- src/backend/api/event_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/backend/api/event_utils.py b/src/backend/api/event_utils.py index 61de1863..69ed1a83 100644 --- a/src/backend/api/event_utils.py +++ b/src/backend/api/event_utils.py @@ -4,6 +4,7 @@ # Third-party from applicationinsights import TelemetryClient + from dotenv import load_dotenv load_dotenv()