Add error handling for diagnostic logging setup (#42505)

* copilot tests

* simplify tests

* tests for second logger

* changelog

* changelog

* Update sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_diagnostics/diagnostic_logging.py

Co-authored-by: Copilot <[email protected]>

* lint

* singleton

* lint newline

* feedback: Add check to avoid lock expense, pass instead of info.

---------

Co-authored-by: Copilot <[email protected]>

Author: jeremydvoss

sdk/monitor/azure-monitor-opentelemetry/CHANGELOG.md

@@ -9,9 +9,13 @@
 ### Breaking Changes
 
 ### Bugs Fixed
+
 - Fixed issue #42337, removes warning messages for instrumentations that target multiple packages. The logic for dependency conflict detection has been enhanced by adding "instruments-any" feature. This feature is used when an instrumentation requires any of a set of dependencies rather than all. Follows upstream dependency conflict detection logic - https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/opentelemetry-instrumentation/src/opentelemetry/instrumentation/dependencies.py
   ([#42342](https://github.com/Azure/azure-sdk-for-python/pull/42342))
 
+- Add error handling for diagnostic logging setup
+  ([#42505](https://github.com/Azure/azure-sdk-for-python/pull/42505))
+
 ### Other Changes
 
 ## 1.6.13 (2025-07-30)

sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_diagnostics/diagnostic_logging.py

@@ -18,13 +18,17 @@
 )
 from azure.monitor.opentelemetry._version import VERSION
 
+# This logger is used for logging messages about the setup of AzureDiagnosticLogging
+_logger = logging.getLogger("azure.monitor.opentelemetry._diagnostics")
+
 _DIAGNOSTIC_LOGGER_FILE_NAME = "applicationinsights-extension.log"
 _SITE_NAME = _env_var_or_default("WEBSITE_SITE_NAME")
 _SUBSCRIPTION_ID_ENV_VAR = _env_var_or_default("WEBSITE_OWNER_NAME")
 _SUBSCRIPTION_ID = _SUBSCRIPTION_ID_ENV_VAR.split("+")[0] if _SUBSCRIPTION_ID_ENV_VAR else None
-_logger = logging.getLogger(__name__)
-_logger.propagate = False
-_logger.setLevel(logging.DEBUG)
+# This logger is used for logging the diagnostic logs themselves to a log file
+_diagnostic_file_logger = logging.getLogger(__name__)
+_diagnostic_file_logger.propagate = False
+_diagnostic_file_logger.setLevel(logging.DEBUG)
 _DIAGNOSTIC_LOG_PATH = _get_log_path()
 
 _DISTRO_DETECTS_ATTACH = "4100"
@@ -44,13 +48,21 @@
 
 
 class AzureDiagnosticLogging:
+    _instance = None
     _initialized = False
     _lock = threading.Lock()
 
+    def __new__(cls):
+        if cls._instance is None:
+            with cls._lock:
+                if cls._instance is None:
+                    cls._instance = super(AzureDiagnosticLogging, cls).__new__(cls)
+        return cls._instance
+
     @classmethod
     def _initialize(cls):
-        with AzureDiagnosticLogging._lock:
-            if not AzureDiagnosticLogging._initialized:
+        with cls._lock:
+            if not cls._initialized:
                 if _is_diagnostics_enabled() and _DIAGNOSTIC_LOG_PATH:
                     log_format = (
                         "{"
@@ -70,30 +82,46 @@ def _initialize(cls):
                         + "}"
                         + "}"
                     )
-                    if not exists(_DIAGNOSTIC_LOG_PATH):
-                        makedirs(_DIAGNOSTIC_LOG_PATH)
-                    f_handler = logging.FileHandler(join(_DIAGNOSTIC_LOG_PATH, _DIAGNOSTIC_LOGGER_FILE_NAME))
-                    formatter = logging.Formatter(fmt=log_format, datefmt="%Y-%m-%dT%H:%M:%S")
-                    f_handler.setFormatter(formatter)
-                    _logger.addHandler(f_handler)
-                    AzureDiagnosticLogging._initialized = True
+                    try:
+                        if not exists(_DIAGNOSTIC_LOG_PATH):
+                            try:
+                                makedirs(_DIAGNOSTIC_LOG_PATH)
+                            # Multi-thread can create a race condition for creating the log file
+                            except FileExistsError:
+                                pass
+                        f_handler = logging.FileHandler(join(_DIAGNOSTIC_LOG_PATH, _DIAGNOSTIC_LOGGER_FILE_NAME))
+                        formatter = logging.Formatter(fmt=log_format, datefmt="%Y-%m-%dT%H:%M:%S")
+                        f_handler.setFormatter(formatter)
+                        _diagnostic_file_logger.addHandler(f_handler)
+                        cls._initialized = True
+                    except Exception as e:  # pylint: disable=broad-except
+                        _logger.error("Failed to initialize Azure Monitor diagnostic logging: %s", e)
+                        cls._initialized = False
 
     @classmethod
     def debug(cls, message: str, message_id: str):
-        AzureDiagnosticLogging._initialize()
-        _logger.debug(message, extra={"msgId": message_id})
+        if not cls._initialized:
+            cls._initialize()
+        if cls._initialized:
+            _diagnostic_file_logger.debug(message, extra={"msgId": message_id})
 
     @classmethod
     def info(cls, message: str, message_id: str):
-        AzureDiagnosticLogging._initialize()
-        _logger.info(message, extra={"msgId": message_id})
+        if not cls._initialized:
+            cls._initialize()
+        if cls._initialized:
+            _diagnostic_file_logger.info(message, extra={"msgId": message_id})
 
     @classmethod
     def warning(cls, message: str, message_id: str):
-        AzureDiagnosticLogging._initialize()
-        _logger.warning(message, extra={"msgId": message_id})
+        if not cls._initialized:
+            cls._initialize()
+        if cls._initialized:
+            _diagnostic_file_logger.warning(message, extra={"msgId": message_id})
 
     @classmethod
     def error(cls, message: str, message_id: str):
-        AzureDiagnosticLogging._initialize()
-        _logger.error(message, extra={"msgId": message_id})
+        if not cls._initialized:
+            cls._initialize()
+        if cls._initialized:
+            _diagnostic_file_logger.error(message, extra={"msgId": message_id})

sdk/monitor/azure-monitor-opentelemetry/tests/diagnostics/test_diagnostic_logging.py

@@ -61,7 +61,7 @@ def set_up(
     is_diagnostics_enabled,
     subscription_id_env_var=TEST_SUBSCRIPTION_ID_PLUS,
 ) -> None:
-    diagnostic_logger._logger.handlers.clear()
+    diagnostic_logger._diagnostic_file_logger.handlers.clear()
     patch.dict(
         "os.environ",
         {
@@ -165,3 +165,86 @@ def test_subscription_id_no_plus(self, temp_file_path):
         diagnostic_logger.AzureDiagnosticLogging.info(MESSAGE1, "4200")
         diagnostic_logger.AzureDiagnosticLogging.info(MESSAGE2, "4301")
         check_file_for_messages(temp_file_path, "INFO", ((MESSAGE1, "4200"), (MESSAGE2, "4301")))
+
+    def test_initialize_file_handler_exception(self, temp_file_path):
+        """Test that initialization fails gracefully when FileHandler creation raises an exception."""
+        set_up(temp_file_path, is_diagnostics_enabled=True)
+        # Mock FileHandler to raise an exception
+        with patch("azure.monitor.opentelemetry._diagnostics.diagnostic_logging.logging.FileHandler") as mock_file_handler, \
+             patch("azure.monitor.opentelemetry._diagnostics.diagnostic_logging._logger") as mock_logger:
+            mock_file_handler.side_effect = OSError("Permission denied")
+            # Attempt to log, which will trigger initialization
+            diagnostic_logger.AzureDiagnosticLogging.info(MESSAGE1, "4200")
+            # Verify that initialization failed
+            assert diagnostic_logger.AzureDiagnosticLogging._initialized is False
+            check_file_is_empty(temp_file_path)
+            # Verify that the error was logged
+            mock_logger.error.assert_called_once()
+
+    def test_initialize_makedirs_exception_not_file_exists(self, temp_file_path):
+        """Test that initialization fails gracefully when makedirs raises a non-FileExistsError exception."""
+        set_up(temp_file_path, is_diagnostics_enabled=True)
+        # Mock makedirs to raise a PermissionError
+        with patch("azure.monitor.opentelemetry._diagnostics.diagnostic_logging.makedirs") as mock_makedirs, \
+             patch("azure.monitor.opentelemetry._diagnostics.diagnostic_logging.exists", return_value=False), \
+             patch("azure.monitor.opentelemetry._diagnostics.diagnostic_logging._logger") as mock_logger:
+            mock_makedirs.side_effect = PermissionError("Permission denied")
+            # Attempt to log, which will trigger initialization
+            diagnostic_logger.AzureDiagnosticLogging.info(MESSAGE1, "4200")
+            # Verify that initialization failed
+            assert diagnostic_logger.AzureDiagnosticLogging._initialized is False
+            check_file_is_empty(temp_file_path)
+            # Verify that the error was logged
+            mock_logger.error.assert_called_once()
+
+    def test_initialize_makedirs_file_exists_error_handled(self, temp_file_path):
+        """Test that FileExistsError from makedirs is handled gracefully and initialization continues."""
+        set_up(temp_file_path, is_diagnostics_enabled=True)
+        # Mock makedirs to raise FileExistsError (this should be handled gracefully)
+        with patch("azure.monitor.opentelemetry._diagnostics.diagnostic_logging.makedirs") as mock_makedirs, \
+             patch("azure.monitor.opentelemetry._diagnostics.diagnostic_logging.exists", return_value=False), \
+             patch("azure.monitor.opentelemetry._diagnostics.diagnostic_logging._logger") as mock_logger:
+            mock_makedirs.side_effect = FileExistsError("Directory already exists")
+            # Attempt to log, which will trigger initialization
+            diagnostic_logger.AzureDiagnosticLogging.info(MESSAGE1, "4200")
+            # Verify that initialization succeeded despite FileExistsError
+            assert diagnostic_logger.AzureDiagnosticLogging._initialized is True
+            check_file_for_messages(temp_file_path, "INFO", ((MESSAGE1, "4200"),))
+
+    def test_initialize_formatter_exception(self, temp_file_path):
+        """Test that initialization fails gracefully when Formatter creation raises an exception."""
+        set_up(temp_file_path, is_diagnostics_enabled=True)
+        # Mock Formatter to raise an exception
+        with patch("azure.monitor.opentelemetry._diagnostics.diagnostic_logging.logging.Formatter") as mock_formatter, \
+             patch("azure.monitor.opentelemetry._diagnostics.diagnostic_logging._logger") as mock_logger:
+            mock_formatter.side_effect = ValueError("Invalid format string")
+            # Attempt to log, which will trigger initialization
+            diagnostic_logger.AzureDiagnosticLogging.info(MESSAGE1, "4200")
+            # Verify that initialization failed
+            assert diagnostic_logger.AzureDiagnosticLogging._initialized is False
+            # Verify that the error was logged
+            mock_logger.error.assert_called_once()
+
+    def test_singleton_pattern(self, temp_file_path):
+        """Test that AzureDiagnosticLogging follows the singleton pattern."""
+        set_up(temp_file_path, is_diagnostics_enabled=True)
+        
+        # Create multiple instances
+        instance1 = diagnostic_logger.AzureDiagnosticLogging()
+        instance2 = diagnostic_logger.AzureDiagnosticLogging()
+        instance3 = diagnostic_logger.AzureDiagnosticLogging()
+        
+        # Verify all instances are the same object
+        assert instance1 is instance2
+        assert instance2 is instance3
+        assert instance1 is instance3
+        
+        # Verify they all have the same id (memory address)
+        assert id(instance1) == id(instance2) == id(instance3)
+        
+        # Verify class-level access still works
+        diagnostic_logger.AzureDiagnosticLogging.info(MESSAGE1, "4200")
+        assert diagnostic_logger.AzureDiagnosticLogging._initialized is True
+        
+        # Verify instance methods work (if they exist)
+        # Since this is primarily a class-based API, we just verify the singleton behavior