Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BugFix] - LoggingService changing default logging configuration #6681

Merged
merged 7 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading