From 45ef4cf347edb968eeed4e1be599a5de8106389f Mon Sep 17 00:00:00 2001 From: Diogo Sousa Date: Fri, 20 Sep 2024 12:41:46 +0100 Subject: [PATCH 1/5] Fix logging service changing global logging config --- .../openbb_core/app/logs/handlers_manager.py | 23 +++++++++----- .../openbb_core/app/logs/logging_service.py | 31 ++++++++----------- .../openbb_core/app/service/system_service.py | 2 ++ .../tests/app/logs/test_handlers_manager.py | 17 +++++----- 4 files changed, 40 insertions(+), 33 deletions(-) diff --git a/openbb_platform/core/openbb_core/app/logs/handlers_manager.py b/openbb_platform/core/openbb_core/app/logs/handlers_manager.py index 2d9a0efb927d..8e79d932f0ec 100644 --- a/openbb_platform/core/openbb_core/app/logs/handlers_manager.py +++ b/openbb_platform/core/openbb_core/app/logs/handlers_manager.py @@ -16,11 +16,18 @@ class HandlersManager: """Handlers Manager.""" - def __init__(self, settings: LoggingSettings): + def __init__(self, logger: logging.Logger, settings: LoggingSettings): """Initialize the HandlersManager.""" + self._logger = logger self._handlers = settings.handler_list self._settings = settings + def setup(self): + """Set the logger handlers and settings.""" + # Disable propagation to root logger to avoid duplicate logs + self._logger.propagate = False + self._logger.setLevel(self._settings.verbosity) + for handler_type in self._handlers: if handler_type == "stdout": self._add_stdout_handler() @@ -33,46 +40,46 @@ def __init__(self, settings: LoggingSettings): elif handler_type == "posthog": self._add_posthog_handler() else: - logging.getLogger().debug("Unknown log handler.") + self._logger.debug("Unknown log handler.") def _add_posthog_handler(self): """Add a Posthog handler.""" handler = PosthogHandler(settings=self._settings) formatter = FormatterWithExceptions(settings=self._settings) handler.setFormatter(formatter) - logging.getLogger().addHandler(handler) + self._logger.addHandler(handler) def _add_stdout_handler(self): """Add a stdout handler.""" handler = logging.StreamHandler(sys.stdout) formatter = FormatterWithExceptions(settings=self._settings) handler.setFormatter(formatter) - logging.getLogger().addHandler(handler) + self._logger.addHandler(handler) def _add_stderr_handler(self): """Add a stderr handler.""" handler = logging.StreamHandler(sys.stderr) formatter = FormatterWithExceptions(settings=self._settings) handler.setFormatter(formatter) - logging.getLogger().addHandler(handler) + self._logger.addHandler(handler) def _add_noop_handler(self): """Add a null handler.""" handler = logging.NullHandler() formatter = FormatterWithExceptions(settings=self._settings) handler.setFormatter(formatter) - logging.getLogger().addHandler(handler) + self._logger.addHandler(handler) def _add_file_handler(self): """Add a file handler.""" handler = PathTrackingFileHandler(settings=self._settings) formatter = FormatterWithExceptions(settings=self._settings) handler.setFormatter(formatter) - logging.getLogger().addHandler(handler) + self._logger.addHandler(handler) def update_handlers(self, settings: LoggingSettings): """Update the handlers with new settings.""" - logger = logging.getLogger() + logger = self._logger for hdlr in logger.handlers: if isinstance(hdlr, (PathTrackingFileHandler, PosthogHandler)): hdlr.settings = settings diff --git a/openbb_platform/core/openbb_core/app/logs/logging_service.py b/openbb_platform/core/openbb_core/app/logs/logging_service.py index 032069f7c786..9747a69013a3 100644 --- a/openbb_platform/core/openbb_core/app/logs/logging_service.py +++ b/openbb_platform/core/openbb_core/app/logs/logging_service.py @@ -19,13 +19,13 @@ class DummyProvider(BaseModel): - """Dummy Provider for error handling with logs""" + """Dummy Provider for error handling with logs.""" provider: str = "not_passed_to_kwargs" class LoggingService(metaclass=SingletonMeta): - """Logging Manager class responsible for managing logging settings and handling logs. + """Logging Service class responsible for managing logging settings and handling logs. Attributes ---------- @@ -59,6 +59,8 @@ class LoggingService(metaclass=SingletonMeta): Log startup information. """ + _logger = logging.getLogger("openbb.logging_service") + def __init__( self, system_settings: SystemSettings, @@ -118,20 +120,15 @@ def _setup_handlers(self) -> HandlersManager: HandlersManager Handlers Manager object. """ - logger = logging.getLogger(__name__) - logging.basicConfig( - level=self._logging_settings.verbosity, - format=FormatterWithExceptions.LOGFORMAT, - datefmt=FormatterWithExceptions.DATEFORMAT, - handlers=[], - force=True, + handlers_manager = HandlersManager( + self._logger, settings=self._logging_settings ) - handlers_manager = HandlersManager(settings=self._logging_settings) + handlers_manager.setup() - logger.info("Logging configuration finished") - logger.info("Logging set to %s", self._logging_settings.handler_list) - logger.info("Verbosity set to %s", self._logging_settings.verbosity) - logger.info( + self._logger.info("Logging configuration finished") + self._logger.info("Logging set to %s", self._logging_settings.handler_list) + self._logger.info("Verbosity set to %s", self._logging_settings.verbosity) + self._logger.info( "LOGFORMAT: %s%s", FormatterWithExceptions.LOGPREFIXFORMAT.replace("|", "-"), FormatterWithExceptions.LOGFORMAT.replace("|", "-"), @@ -160,8 +157,7 @@ class CredentialsDefinition(Enum): for c in credentials } - logger = logging.getLogger(__name__) - logger.info( + self._logger.info( "STARTUP: %s ", json.dumps( { @@ -223,7 +219,6 @@ def log( if "login" in route: self._log_startup(route, custom_headers) else: - logger = logging.getLogger(__name__) # Remove CommandContext if any kwargs.pop("cc", None) @@ -254,7 +249,7 @@ def log( default=to_jsonable_python, ) log_message = f"{message_label}: {log_message}" - log_level = logger.error if error else logger.info + log_level = self._logger.error if error else self._logger.info log_level( log_message, extra={"func_name_override": func.__name__}, diff --git a/openbb_platform/core/openbb_core/app/service/system_service.py b/openbb_platform/core/openbb_core/app/service/system_service.py index 4d9f9c6feeb3..dd4bfe9689d7 100644 --- a/openbb_platform/core/openbb_core/app/service/system_service.py +++ b/openbb_platform/core/openbb_core/app/service/system_service.py @@ -22,6 +22,8 @@ class SystemService(metaclass=SingletonMeta): "api_settings", "python_settings", "debug_mode", + "logging_handlers", + "logging_verbosity", } PRO_VALIDATION_HASH = "300ac59fdcc8f899e0bc5c18cda8652220735da1a00e2af365efe9d8e5fe8306" # pragma: allowlist secret diff --git a/openbb_platform/core/tests/app/logs/test_handlers_manager.py b/openbb_platform/core/tests/app/logs/test_handlers_manager.py index d46451c63146..b2ce9f30321d 100644 --- a/openbb_platform/core/tests/app/logs/test_handlers_manager.py +++ b/openbb_platform/core/tests/app/logs/test_handlers_manager.py @@ -51,11 +51,15 @@ def test_handlers_added_correctly(): MockFormatterWithExceptions, ): settings = Mock() + settings.verbosity = 20 settings.handler_list = ["stdout", "stderr", "noop", "file", "posthog"] - _ = HandlersManager(settings=settings) - - handlers = logging.getLogger().handlers + logger = logging.getLogger("test_handlers_added_correctly") + handlers_manager = HandlersManager(logger=logger, settings=settings) + handlers_manager.setup() + handlers = logger.handlers + assert not logger.propagate + assert logger.level == 20 assert len(handlers) >= 5 for handler in handlers: @@ -88,16 +92,15 @@ def test_update_handlers(): settings = Mock() settings.handler_list = ["file", "posthog"] settings.any_other_attr = "mock_settings" - handlers_manager = HandlersManager(settings=settings) + logger = logging.getLogger("test_update_handlers") + handlers_manager = HandlersManager(logger=logger, settings=settings) changed_settings = Mock() changed_settings.any_other_attr = "changed_settings" handlers_manager.update_handlers(settings=changed_settings) - handlers = logging.getLogger().handlers - - for hdlr in handlers: + for hdlr in logger.handlers: if isinstance(hdlr, (MockPosthogHandler, MockPathTrackingFileHandler)): assert hdlr.settings == changed_settings assert hdlr.formatter.settings == changed_settings # type: ignore[union-attr] From b154ba6731155c49c0ca478ebf96c9d3ced792eb Mon Sep 17 00:00:00 2001 From: Diogo Sousa Date: Fri, 20 Sep 2024 12:44:43 +0100 Subject: [PATCH 2/5] No needed --- openbb_platform/core/openbb_core/app/service/system_service.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/openbb_platform/core/openbb_core/app/service/system_service.py b/openbb_platform/core/openbb_core/app/service/system_service.py index dd4bfe9689d7..4d9f9c6feeb3 100644 --- a/openbb_platform/core/openbb_core/app/service/system_service.py +++ b/openbb_platform/core/openbb_core/app/service/system_service.py @@ -22,8 +22,6 @@ class SystemService(metaclass=SingletonMeta): "api_settings", "python_settings", "debug_mode", - "logging_handlers", - "logging_verbosity", } PRO_VALIDATION_HASH = "300ac59fdcc8f899e0bc5c18cda8652220735da1a00e2af365efe9d8e5fe8306" # pragma: allowlist secret From e72f097012fef2ff2aa781e13526d685e5afcd65 Mon Sep 17 00:00:00 2001 From: Diogo Sousa Date: Fri, 20 Sep 2024 13:04:16 +0100 Subject: [PATCH 3/5] Fix failing unit test --- .../tests/app/logs/test_logging_service.py | 68 +++++++++---------- 1 file changed, 32 insertions(+), 36 deletions(-) diff --git a/openbb_platform/core/tests/app/logs/test_logging_service.py b/openbb_platform/core/tests/app/logs/test_logging_service.py index 89e56a777cba..1a3bbe93edc6 100644 --- a/openbb_platform/core/tests/app/logs/test_logging_service.py +++ b/openbb_platform/core/tests/app/logs/test_logging_service.py @@ -51,6 +51,7 @@ def logging_service(): system_settings=mock_system_settings, user_settings=mock_user_settings, ) + _logging_service._logger = MagicMock() return _logging_service @@ -101,38 +102,35 @@ def test_logging_settings_setter(logging_service): def test_log_startup(logging_service): """Test the log_startup method.""" - with patch("logging.getLogger") as mock_get_logger: - mock_info = mock_get_logger.return_value.info - class MockCredentials(BaseModel): - username: str - password: str - - logging_service._user_settings = MagicMock( - preferences="your_preferences", - credentials=MockCredentials(username="username", password="password"), - ) - logging_service._system_settings = "your_system_settings" - - logging_service._log_startup( - route="test_route", custom_headers={"X-OpenBB-Test": "test"} - ) - - expected_log_data = { - "route": "test_route", - "PREFERENCES": "your_preferences", - "KEYS": { - "username": "defined", - "password": "defined", # pragma: allowlist secret - }, - "SYSTEM": "your_system_settings", - "custom_headers": {"X-OpenBB-Test": "test"}, - } - mock_info.assert_called_once_with( - "STARTUP: %s ", - json.dumps(expected_log_data), - ) - mock_get_logger.assert_called_once() + class MockCredentials(BaseModel): + username: str + password: str + + logging_service._user_settings = MagicMock( + preferences="your_preferences", + credentials=MockCredentials(username="username", password="password"), + ) + logging_service._system_settings = "your_system_settings" + + logging_service._log_startup( + route="test_route", custom_headers={"X-OpenBB-Test": "test"} + ) + + expected_log_data = { + "route": "test_route", + "PREFERENCES": "your_preferences", + "KEYS": { + "username": "defined", + "password": "defined", # pragma: allowlist secret + }, + "SYSTEM": "your_system_settings", + "custom_headers": {"X-OpenBB-Test": "test"}, + } + logging_service._logger.info.assert_called_once_with( + "STARTUP: %s ", + json.dumps(expected_log_data), + ) @pytest.mark.parametrize( @@ -190,7 +188,7 @@ def test_log( with patch( "openbb_core.app.logs.logging_service.LoggingSettings", MockLoggingSettings, - ), patch("logging.getLogger") as mock_get_logger: + ): if route == "login": with patch( "openbb_core.app.logs.logging_service.LoggingService._log_startup" @@ -221,15 +219,13 @@ def test_log( ) if expected_log_message.startswith("ERROR"): - mock_error = mock_get_logger.return_value.error - mock_error.assert_called_once_with( + logging_service._logger.error.assert_called_once_with( expected_log_message, extra={"func_name_override": "mock_func"}, exc_info=exec_info, ) if expected_log_message.startswith("CMD"): - mock_info = mock_get_logger.return_value.info - mock_info.assert_called_once_with( + logging_service._logger.info.assert_called_once_with( expected_log_message, extra={"func_name_override": "mock_func"}, exc_info=exec_info, From aa8066a3d60f5afba08101ebb808a118176e1838 Mon Sep 17 00:00:00 2001 From: Diogo Sousa Date: Mon, 23 Sep 2024 12:07:03 +0100 Subject: [PATCH 4/5] pylint --- openbb_platform/core/openbb_core/app/logs/logging_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openbb_platform/core/openbb_core/app/logs/logging_service.py b/openbb_platform/core/openbb_core/app/logs/logging_service.py index 9747a69013a3..39548423ff10 100644 --- a/openbb_platform/core/openbb_core/app/logs/logging_service.py +++ b/openbb_platform/core/openbb_core/app/logs/logging_service.py @@ -175,7 +175,7 @@ class CredentialsDefinition(Enum): ), ) - def log( + def log( # noqa: too-many-positional-arguments self, user_settings: UserSettings, system_settings: SystemSettings, From fef4c19ae60d2b86b531fb57ee661bcd80099229 Mon Sep 17 00:00:00 2001 From: Diogo Sousa Date: Mon, 23 Sep 2024 13:51:23 +0100 Subject: [PATCH 5/5] pylint --- openbb_platform/core/openbb_core/app/logs/logging_service.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openbb_platform/core/openbb_core/app/logs/logging_service.py b/openbb_platform/core/openbb_core/app/logs/logging_service.py index 39548423ff10..9c1900b0c022 100644 --- a/openbb_platform/core/openbb_core/app/logs/logging_service.py +++ b/openbb_platform/core/openbb_core/app/logs/logging_service.py @@ -175,7 +175,8 @@ class CredentialsDefinition(Enum): ), ) - def log( # noqa: too-many-positional-arguments + # pylint: disable=R0917 + def log( self, user_settings: UserSettings, system_settings: SystemSettings,