refactor: logging configuration to require Settings instance (#1031)

Author: gcharest

app/infrastructure/logging/__init__.py

@@ -51,7 +51,6 @@ def process_item(item_id: str):
 # Core logging setup
 from infrastructure.logging.setup import (
     configure_logging,
-    logger,  # Module-level logger for backward compat
     # Deprecated - kept for backward compatibility
     get_module_logger,
 )
@@ -76,7 +75,6 @@ def process_item(item_id: str):
 __all__ = [
     # Setup
     "configure_logging",
-    "logger",
     # Deprecated - backward compatibility only
     "get_module_logger",
     # Context

app/infrastructure/logging/setup.py

@@ -23,10 +23,10 @@
 import sys
 import structlog
 from structlog.stdlib import BoundLogger
-from typing import Optional
-from infrastructure.services.providers import get_settings
+from typing import TYPE_CHECKING
 
-settings = get_settings()
+if TYPE_CHECKING:
+    from infrastructure.configuration import Settings
 
 
 def _is_test_environment() -> bool:
@@ -39,8 +39,9 @@ def _is_test_environment() -> bool:
 
 
 def configure_logging(
-    log_level: Optional[str] = None,
-    is_production: Optional[bool] = None,
+    settings: "Settings",
+    log_level: str | None = None,
+    is_production: bool | None = None,
 ) -> BoundLogger:
     """Configure structured logging with enhanced processors.
 
@@ -51,6 +52,7 @@ def configure_logging(
     - Test environment detection for log suppression
 
     Args:
+        settings: Settings instance (REQUIRED). Must be passed explicitly.
         log_level: Optional override for log level (DEBUG, INFO, WARNING, etc).
             Defaults to settings.LOG_LEVEL if not provided.
         is_production: Optional override for production mode. Defaults to
@@ -61,11 +63,14 @@ def configure_logging(
 
     Example:
         # At application startup
-        logger = configure_logging()
+        from infrastructure.services import get_settings
+        settings = get_settings()
+        logger = configure_logging(settings=settings)
 
         # With overrides for testing
-        logger = configure_logging(log_level="DEBUG", is_production=False)
+        logger = configure_logging(settings=settings, log_level="DEBUG", is_production=False)
     """
+
     # Suppress all logging during tests
     if _is_test_environment():
         # Set root logger to suppress all output during tests
@@ -139,11 +144,6 @@ def configure_logging(
     return structlog.stdlib.get_logger()
 
 
-# Module-level logger - used for backward compatibility with legacy code
-# New code should use: structlog.get_logger()
-logger: BoundLogger = configure_logging()
-
-
 # =============================================================================
 # DEPRECATED FUNCTIONS - For backward compatibility only
 # =============================================================================
@@ -182,11 +182,11 @@ def get_module_logger() -> BoundLogger:
     # Auto-detect calling module for backward compatibility
     current_frame = inspect.currentframe()
     if current_frame is None:
-        return logger
+        return structlog.get_logger()
 
     frame = current_frame.f_back
     if frame is None:
-        return logger
+        return structlog.get_logger()
 
     module = inspect.getmodule(frame)
     if module:
@@ -196,6 +196,6 @@ def get_module_logger() -> BoundLogger:
             "component": parts[-1],
             "module_path": module_name,
         }
-        return logger.bind(**context)
+        return structlog.get_logger().bind(**context)
 
-    return logger.bind(component="unknown")
+    return structlog.get_logger().bind(component="unknown")

app/tests/unit/infrastructure/conftest.py

@@ -1,4 +1,19 @@
 """Module-level fixtures for infrastructure tests (Level 2)."""
 
+import pytest
+from unittest.mock import Mock
+
+from infrastructure.configuration import Settings
+
+
+@pytest.fixture
+def mock_settings():
+    """Mock Settings instance for testing."""
+    settings = Mock(spec=Settings)
+    settings.LOG_LEVEL = "INFO"
+    settings.is_production = False
+    return settings
+
+
 # Note: Command-framework-specific fixtures are in Level 3 conftest
 # (tests/unit/infrastructure/commands/conftest.py)

app/tests/unit/infrastructure/logging/conftest.py

@@ -0,0 +1,15 @@
+"""Fixtures for infrastructure.logging tests."""
+
+import pytest
+from unittest.mock import Mock
+
+from infrastructure.configuration import Settings
+
+
+@pytest.fixture
+def mock_settings():
+    """Mock Settings instance for testing."""
+    settings = Mock(spec=Settings)
+    settings.LOG_LEVEL = "INFO"
+    settings.is_production = False
+    return settings

app/tests/unit/infrastructure/logging/test_setup.py

@@ -38,54 +38,54 @@ def test_returns_true_during_test_run(self):
 class TestConfigureLogging:
     """Test suite for configure_logging function."""
 
-    def test_configure_logging_returns_bound_logger(self):
+    def test_configure_logging_returns_bound_logger(self, mock_settings):
         """configure_logging returns a BoundLogger instance."""
-        result = configure_logging()
+        result = configure_logging(settings=mock_settings)
 
         assert result is not None
         assert hasattr(result, "info")
         assert hasattr(result, "debug")
         assert hasattr(result, "warning")
         assert hasattr(result, "error")
 
-    def test_configure_logging_default_parameters(self):
+    def test_configure_logging_default_parameters(self, mock_settings):
         """configure_logging works with default parameters."""
-        logger = configure_logging()
+        logger = configure_logging(settings=mock_settings)
 
         assert logger is not None
 
-    def test_configure_logging_with_log_level(self):
+    def test_configure_logging_with_log_level(self, mock_settings):
         """configure_logging accepts log_level parameter."""
         # In test environment, logging is suppressed, but function should still work
-        logger = configure_logging(log_level="DEBUG")
+        logger = configure_logging(settings=mock_settings, log_level="DEBUG")
         assert logger is not None
 
-        logger = configure_logging(log_level="INFO")
+        logger = configure_logging(settings=mock_settings, log_level="INFO")
         assert logger is not None
 
-        logger = configure_logging(log_level="WARNING")
+        logger = configure_logging(settings=mock_settings, log_level="WARNING")
         assert logger is not None
 
-    def test_configure_logging_with_is_production(self):
+    def test_configure_logging_with_is_production(self, mock_settings):
         """configure_logging accepts is_production parameter."""
         # In test environment, logging is suppressed, but parameters should be accepted
-        logger = configure_logging(is_production=True)
+        logger = configure_logging(settings=mock_settings, is_production=True)
         assert logger is not None
 
-        logger = configure_logging(is_production=False)
+        logger = configure_logging(settings=mock_settings, is_production=False)
         assert logger is not None
 
-    def test_configure_logging_idempotent(self):
+    def test_configure_logging_idempotent(self, mock_settings):
         """Multiple configure_logging calls are safe."""
-        logger1 = configure_logging()
-        logger2 = configure_logging()
+        logger1 = configure_logging(settings=mock_settings)
+        logger2 = configure_logging(settings=mock_settings)
 
         assert logger1 is not None
         assert logger2 is not None
 
-    def test_configure_logging_suppresses_in_test_env(self):
+    def test_configure_logging_suppresses_in_test_env(self, mock_settings):
         """In test environment, root logger level is set high to suppress output."""
-        configure_logging()
+        configure_logging(settings=mock_settings)
 
         # In test environment, root logger should be set to suppress output
         root_logger = logging.getLogger()
@@ -97,9 +97,9 @@ def test_configure_logging_suppresses_in_test_env(self):
 class TestGetModuleLogger:
     """Test suite for get_module_logger function (deprecated)."""
 
-    def test_get_module_logger_returns_bound_logger(self):
+    def test_get_module_logger_returns_bound_logger(self, mock_settings):
         """get_module_logger returns a BoundLogger with expected methods."""
-        configure_logging()
+        configure_logging(settings=mock_settings)
 
         with warnings.catch_warnings(record=True) as w:
             warnings.simplefilter("always")
@@ -114,16 +114,16 @@ def test_get_module_logger_returns_bound_logger(self):
         assert hasattr(logger, "info")
         assert hasattr(logger, "bind")
 
-    def test_get_module_logger_emits_deprecation_warning(self):
+    def test_get_module_logger_emits_deprecation_warning(self, mock_settings):
         """get_module_logger emits DeprecationWarning."""
-        configure_logging()
+        configure_logging(settings=mock_settings)
 
         with pytest.warns(DeprecationWarning, match="deprecated"):
             get_module_logger()
 
-    def test_get_module_logger_still_works(self):
+    def test_get_module_logger_still_works(self, mock_settings):
         """get_module_logger still returns functional logger for backward compat."""
-        configure_logging()
+        configure_logging(settings=mock_settings)
 
         with warnings.catch_warnings():
             warnings.simplefilter("ignore", DeprecationWarning)
@@ -140,9 +140,9 @@ def test_get_module_logger_still_works(self):
 class TestLoggingBestPractices:
     """Tests demonstrating structlog best practices."""
 
-    def test_standard_structlog_pattern(self):
+    def test_standard_structlog_pattern(self, mock_settings):
         """Standard structlog.get_logger() pattern works."""
-        configure_logging()
+        configure_logging(settings=mock_settings)
 
         # This is the recommended pattern
         logger = structlog.get_logger()
@@ -151,29 +151,29 @@ def test_standard_structlog_pattern(self):
         assert hasattr(logger, "info")
         assert hasattr(logger, "bind")
 
-    def test_bind_for_context(self):
+    def test_bind_for_context(self, mock_settings):
         """Use .bind() for adding context."""
-        configure_logging()
+        configure_logging(settings=mock_settings)
 
         logger = structlog.get_logger()
         log = logger.bind(user_id="123", operation="test")
 
         assert log is not None
         assert hasattr(log, "info")
 
-    def test_chained_binds(self):
+    def test_chained_binds(self, mock_settings):
         """Multiple .bind() calls can be chained."""
-        configure_logging()
+        configure_logging(settings=mock_settings)
 
         logger = structlog.get_logger()
         log = logger.bind(request_id="req-123")
         log = log.bind(user_id="user-456")
 
         assert log is not None
 
-    def test_logging_methods_dont_raise(self):
+    def test_logging_methods_dont_raise(self, mock_settings):
         """Logging methods execute without raising exceptions."""
-        configure_logging()
+        configure_logging(settings=mock_settings)
 
         logger = structlog.get_logger()
         log = logger.bind(component="test")
@@ -184,9 +184,9 @@ def test_logging_methods_dont_raise(self):
         log.warning("warning message")
         log.error("error message", error_code="E001")
 
-    def test_exception_logging(self):
+    def test_exception_logging(self, mock_settings):
         """Exception logging works correctly."""
-        configure_logging()
+        configure_logging(settings=mock_settings)
 
         logger = structlog.get_logger()
 

app/tests/unit/infrastructure/test_logging.py

@@ -48,33 +48,33 @@ def test_is_test_environment_without_pytest(self):
                     # Re-import pytest since we removed it
                     import pytest as _  # noqa: F401
 
-    def test_configure_logging_returns_bound_logger(self):
+    def test_configure_logging_returns_bound_logger(self, mock_settings):
         """configure_logging returns a logger instance."""
-        logger = configure_logging()
+        logger = configure_logging(settings=mock_settings)
         # Should return a structlog logger (may be BoundLoggerLazyProxy)
         assert logger is not None
         assert hasattr(logger, "bind")
 
-    def test_configure_logging_in_test_environment(self):
+    def test_configure_logging_in_test_environment(self, mock_settings):
         """configure_logging suppresses logs in test environment."""
         # We're in a test environment, so logging should be at CRITICAL+1
-        configure_logging()
+        configure_logging(settings=mock_settings)
         root_level = logging.root.level
         assert root_level == logging.CRITICAL + 1
 
-    def test_configure_logging_with_custom_log_level(self):
+    def test_configure_logging_with_custom_log_level(self, mock_settings):
         """configure_logging accepts custom log level."""
-        logger = configure_logging(log_level="DEBUG")
+        logger = configure_logging(settings=mock_settings, log_level="DEBUG")
         # Should return a logger instance
         assert logger is not None
         assert hasattr(logger, "bind")
 
-    def test_configure_logging_with_custom_production_flag(self):
+    def test_configure_logging_with_custom_production_flag(self, mock_settings):
         """configure_logging accepts custom production flag."""
         # Note: We can't easily verify the processor setup, but we can verify
         # it doesn't raise an error with different production flags
-        logger1 = configure_logging(is_production=True)
-        logger2 = configure_logging(is_production=False)
+        logger1 = configure_logging(settings=mock_settings, is_production=True)
+        logger2 = configure_logging(settings=mock_settings, is_production=False)
 
         assert logger1 is not None
         assert logger2 is not None