Skip to content

Commit

Permalink
Merge pull request #406 from lsst-sqre/tickets/DM-48798/disable-logging
Browse files Browse the repository at this point in the history
DM-48798: Config option to disable monkeys logging to file
  • Loading branch information
fajpunk authored Feb 13, 2025
2 parents 10cc31e + e736ff9 commit d30d7fc
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 3 deletions.
9 changes: 9 additions & 0 deletions src/mobu/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,15 @@ class Configuration(BaseSettings):
title="Log level of the application's logger",
)

log_monkeys_to_file: bool = Field(
True,
title="Log monkey messages to a file",
description=(
"Log monkey messages to a file instead of doing whatever the"
" normal global logger does"
),
)

metrics: MetricsConfiguration = Field(
default_factory=metrics_configuration_factory,
title="Metrics configuration",
Expand Down
12 changes: 12 additions & 0 deletions src/mobu/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"GafaelfawrWebError",
"GitHubFileNotFoundError",
"MonkeyNotFoundError",
"NotRetainingLogsError",
"SubprocessError",
]

Expand Down Expand Up @@ -94,6 +95,17 @@ def __init__(self, monkey: str) -> None:
super().__init__(msg, ErrorLocation.path, ["monkey"])


class NotRetainingLogsError(ClientRequestError):
"""Mobu is not configured to retain logs."""

error = "mobu_not_retaining_logs"
status_code = status.HTTP_404_NOT_FOUND

def __init__(self) -> None:
msg = "Mobu is not configured to retain monkey logs"
super().__init__(msg)


class NotebookRepositoryError(Exception):
"""The repository containing notebooks to run is not valid."""

Expand Down
13 changes: 12 additions & 1 deletion src/mobu/handlers/external.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from safir.slack.webhook import SlackRouteErrorHandler

from mobu.config import Configuration
from mobu.exceptions import NotRetainingLogsError

from ..dependencies.config import config_dependency
from ..dependencies.context import RequestContext, context_dependency
Expand Down Expand Up @@ -170,15 +171,25 @@ async def get_monkey(
description="Returns the monkey log output as a file",
response_class=StreamingResponse,
responses={
404: {"description": "Monkey or flock not found", "model": ErrorModel}
404: {
"description": (
"Monkey or flock not found, or Mobu is not configured to"
" retain logs."
),
"model": ErrorModel,
},
},
summary="Log for monkey",
)
def get_monkey_log(
flock: str,
monkey: str,
config: Annotated[Configuration, Depends(config_dependency)],
context: Annotated[RequestContext, Depends(context_dependency)],
) -> StreamingResponse:
if not config.log_monkeys_to_file:
raise NotRetainingLogsError

logfile = context.manager.get_flock(flock).get_monkey(monkey).logfile()

# We can't use FileResponse because the log file is constantly changing
Expand Down
8 changes: 6 additions & 2 deletions src/mobu/services/monkey.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,17 @@ def __init__(
self._user = user

self._state = MonkeyState.IDLE
self._logfile = NamedTemporaryFile()
self._logger = self._build_logger(self._logfile)
self._global_logger = logger.bind(
monkey=self._name, user=self._user.username
)
self._job: Job | None = None

self._logfile = NamedTemporaryFile()
if self._config.log_monkeys_to_file:
self._logger = self._build_logger(self._logfile)
else:
self._logger = self._global_logger

# Determine the business class from the type of configuration we got,
# which in turn will be based on Pydantic validation of the value of
# the type field.
Expand Down
6 changes: 6 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,12 @@ def _enable_github_ci_app(
config_dependency.set_path(config_path("base"))


@pytest.fixture
def _disable_file_logging() -> None:
"""Disable monkey file logging."""
config_dependency.set_path(config_path("base_no_file_logging"))


@pytest.fixture
def _enable_github_refresh_app(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
Expand Down
11 changes: 11 additions & 0 deletions tests/data/config/base_no_file_logging.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
slackAlerts: true
environmentUrl: "https://example.com"
sentryEnvironment: "pytest"
availableServices:
- some_service
- some_other_service
metrics:
enabled: false
mock: true
appName: mobu
logMonkeysToFile: false
21 changes: 21 additions & 0 deletions tests/handlers/flock_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,3 +353,24 @@ async def test_errors(client: AsyncClient, respx_mock: respx.Router) -> None:
"msg": ANY,
"type": "union_tag_invalid",
}


@pytest.mark.asyncio
@pytest.mark.usefixtures("_disable_file_logging")
async def test_file_logging_disabled(
client: AsyncClient, respx_mock: respx.Router
) -> None:
mock_gafaelfawr(respx_mock)

config = {
"name": "test",
"count": 1,
"user_spec": {"username_prefix": "bot-mobu-testuser"},
"scopes": ["exec:notebook"],
"business": {"type": "EmptyLoop"},
}
r = await client.put("/mobu/flocks", json=config)
assert r.status_code == 201

r = await client.get("/mobu/flocks/test/monkeys/bot-mobu-testuser1/log")
assert r.status_code == 404

0 comments on commit d30d7fc

Please sign in to comment.