Skip to content

Commit

Permalink
[BugFix] - LoggingService changing default logging configuration (#6681)
Browse files Browse the repository at this point in the history
* Fix logging service changing global logging config

* No needed

* Fix failing unit test

* pylint

* pylint

---------

Co-authored-by: Igor Radovanovic <[email protected]>
Co-authored-by: Danglewood <[email protected]>
  • Loading branch information
3 people authored Sep 23, 2024
1 parent 12b4863 commit 24bc340
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 69 deletions.
23 changes: 15 additions & 8 deletions openbb_platform/core/openbb_core/app/logs/handlers_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down
32 changes: 14 additions & 18 deletions openbb_platform/core/openbb_core/app/logs/logging_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
----------
Expand Down Expand Up @@ -59,6 +59,8 @@ class LoggingService(metaclass=SingletonMeta):
Log startup information.
"""

_logger = logging.getLogger("openbb.logging_service")

def __init__(
self,
system_settings: SystemSettings,
Expand Down Expand Up @@ -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("|", "-"),
Expand Down Expand Up @@ -160,8 +157,7 @@ class CredentialsDefinition(Enum):
for c in credentials
}

logger = logging.getLogger(__name__)
logger.info(
self._logger.info(
"STARTUP: %s ",
json.dumps(
{
Expand All @@ -179,6 +175,7 @@ class CredentialsDefinition(Enum):
),
)

# pylint: disable=R0917
def log(
self,
user_settings: UserSettings,
Expand Down Expand Up @@ -223,7 +220,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)
Expand Down Expand Up @@ -254,7 +250,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__},
Expand Down
17 changes: 10 additions & 7 deletions openbb_platform/core/tests/app/logs/test_handlers_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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]
68 changes: 32 additions & 36 deletions openbb_platform/core/tests/app/logs/test_logging_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ def logging_service():
system_settings=mock_system_settings,
user_settings=mock_user_settings,
)
_logging_service._logger = MagicMock()

return _logging_service

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 24bc340

Please sign in to comment.