-
Notifications
You must be signed in to change notification settings - Fork 124
Fix gateway launchd log growth #921
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,59 @@ | ||||||||||||||||||||||||||
| #!/usr/bin/env bash | ||||||||||||||||||||||||||
| set -euo pipefail | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ROOT_DIR="${AWORLD_GATEWAY_ROOT:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)}" | ||||||||||||||||||||||||||
| LOG_DIR="${AWORLD_GATEWAY_LAUNCHD_LOG_DIR:-$HOME/Documents/logs}" | ||||||||||||||||||||||||||
| OUT_LOG="${AWORLD_GATEWAY_LAUNCHD_OUT_LOG:-$LOG_DIR/aworld-gateway.launchd.out.log}" | ||||||||||||||||||||||||||
| ERR_LOG="${AWORLD_GATEWAY_LAUNCHD_ERR_LOG:-$LOG_DIR/aworld-gateway.launchd.err.log}" | ||||||||||||||||||||||||||
| MAX_BYTES="${AWORLD_GATEWAY_LAUNCHD_LOG_MAX_BYTES:-16777216}" | ||||||||||||||||||||||||||
| BACKUPS="${AWORLD_GATEWAY_LAUNCHD_LOG_BACKUPS:-5}" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| file_size_bytes() { | ||||||||||||||||||||||||||
| local file="$1" | ||||||||||||||||||||||||||
| if stat -f %z "$file" >/dev/null 2>&1; then | ||||||||||||||||||||||||||
| stat -f %z "$file" | ||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||
| stat -c %s "$file" | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+11
to
+18
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| rotate_log_if_needed() { | ||||||||||||||||||||||||||
| local file="$1" | ||||||||||||||||||||||||||
| local size | ||||||||||||||||||||||||||
| local index | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| [[ -f "$file" ]] || return 0 | ||||||||||||||||||||||||||
| size="$(file_size_bytes "$file")" | ||||||||||||||||||||||||||
| [[ "$size" =~ ^[0-9]+$ ]] || return 0 | ||||||||||||||||||||||||||
| (( size >= MAX_BYTES )) || return 0 | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| for ((index = BACKUPS; index >= 1; index--)); do | ||||||||||||||||||||||||||
| if (( index == BACKUPS )); then | ||||||||||||||||||||||||||
| rm -f "$file.$index" | ||||||||||||||||||||||||||
| elif [[ -f "$file.$index" ]]; then | ||||||||||||||||||||||||||
| mv "$file.$index" "$file.$((index + 1))" | ||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||
| done | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| mv "$file" "$file.1" | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| mkdir -p "$LOG_DIR" "$ROOT_DIR/logs" | ||||||||||||||||||||||||||
| rotate_log_if_needed "$OUT_LOG" | ||||||||||||||||||||||||||
| rotate_log_if_needed "$ERR_LOG" | ||||||||||||||||||||||||||
| exec >>"$OUT_LOG" 2>>"$ERR_LOG" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export AWORLD_DISABLE_CONSOLE_LOG="${AWORLD_DISABLE_CONSOLE_LOG:-true}" | ||||||||||||||||||||||||||
| export AWORLD_GATEWAY_CONSOLE_LOG="${AWORLD_GATEWAY_CONSOLE_LOG:-false}" | ||||||||||||||||||||||||||
| export AWORLD_GATEWAY_UVICORN_LOG_LEVEL="${AWORLD_GATEWAY_UVICORN_LOG_LEVEL:-warning}" | ||||||||||||||||||||||||||
| export AWORLD_LOG_PATH="${AWORLD_LOG_PATH:-$ROOT_DIR/logs}" | ||||||||||||||||||||||||||
| export AWORLD_GATEWAY_LOG_PATH="${AWORLD_GATEWAY_LOG_PATH:-$ROOT_DIR/logs/gateway.log}" | ||||||||||||||||||||||||||
| export PYTHONPATH="$ROOT_DIR/aworld-cli/src:$ROOT_DIR${PYTHONPATH:+:$PYTHONPATH}" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| cd "$ROOT_DIR" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if command -v aworld-cli >/dev/null 2>&1; then | ||||||||||||||||||||||||||
| exec aworld-cli gateway server "$@" | ||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| exec "${PYTHON:-python3}" -m aworld_cli.main gateway server "$@" | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |||||||||||||
|
|
||||||||||||||
| import asyncio | ||||||||||||||
| import importlib | ||||||||||||||
| import logging | ||||||||||||||
| import os | ||||||||||||||
| import sys | ||||||||||||||
| from types import ModuleType | ||||||||||||||
|
|
@@ -51,7 +52,7 @@ def test_gateway_channels_list_contains_placeholder_channels( | |||||||||||||
| assert rows["web"]["implemented"] is False | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| def test_enable_aworld_console_logging_for_gateway_reconfigures_disabled_logger( | ||||||||||||||
| def test_suppress_gateway_console_logging_reconfigures_enabled_logger( | ||||||||||||||
| monkeypatch: pytest.MonkeyPatch, | ||||||||||||||
| ) -> None: | ||||||||||||||
| class FakeLogger: | ||||||||||||||
|
|
@@ -83,15 +84,31 @@ def __init__( | |||||||||||||
|
|
||||||||||||||
| import aworld.logs.util as log_util | ||||||||||||||
|
|
||||||||||||||
| fake_logger = FakeLogger(disable_console=True) | ||||||||||||||
| fake_logger = FakeLogger(disable_console=False) | ||||||||||||||
| monkeypatch.setattr(log_util, "logger", fake_logger) | ||||||||||||||
| monkeypatch.setenv("AWORLD_DISABLE_CONSOLE_LOG", "true") | ||||||||||||||
| monkeypatch.setenv("AWORLD_DISABLE_CONSOLE_LOG", "false") | ||||||||||||||
| monkeypatch.delenv("AWORLD_GATEWAY_CONSOLE_LOG", raising=False) | ||||||||||||||
|
|
||||||||||||||
| gateway_cli._suppress_gateway_console_logging() | ||||||||||||||
|
|
||||||||||||||
| assert os.environ["AWORLD_DISABLE_CONSOLE_LOG"] == "true" | ||||||||||||||
| assert os.environ["AWORLD_GATEWAY_CONSOLE_LOG"] == "false" | ||||||||||||||
| assert fake_logger.disable_console is True | ||||||||||||||
| assert fake_logger.calls[-1]["disable_console"] is True | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| def test_configure_gateway_file_logging_can_suppress_console_propagation( | ||||||||||||||
| monkeypatch: pytest.MonkeyPatch, | ||||||||||||||
| tmp_path: Path, | ||||||||||||||
| ) -> None: | ||||||||||||||
| monkeypatch.setenv("AWORLD_GATEWAY_CONSOLE_LOG", "false") | ||||||||||||||
|
|
||||||||||||||
| gateway_cli._configure_gateway_file_logging(base_dir=tmp_path) | ||||||||||||||
|
|
||||||||||||||
| gateway_cli._enable_aworld_console_logging_for_gateway() | ||||||||||||||
| assert logging.getLogger("aworld.gateway").propagate is False | ||||||||||||||
|
|
||||||||||||||
| assert os.environ["AWORLD_DISABLE_CONSOLE_LOG"] == "false" | ||||||||||||||
| assert fake_logger.disable_console is False | ||||||||||||||
| assert fake_logger.calls[-1]["disable_console"] is False | ||||||||||||||
| monkeypatch.setenv("AWORLD_GATEWAY_CONSOLE_LOG", "true") | ||||||||||||||
| gateway_cli._configure_gateway_file_logging(base_dir=tmp_path) | ||||||||||||||
|
Comment on lines
+110
to
+111
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test
Suggested change
|
||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| def test_configure_gateway_file_logging_writes_to_dedicated_gateway_log( | ||||||||||||||
|
|
@@ -136,7 +153,7 @@ def test_get_gateway_logger_returns_named_child_logger( | |||||||||||||
| assert logger.name == f"{GATEWAY_LOGGER_NAME}.child_component" | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| def test_serve_gateway_enables_console_logging_before_loading_agents( | ||||||||||||||
| def test_serve_gateway_suppresses_console_logging_before_loading_agents( | ||||||||||||||
| monkeypatch: pytest.MonkeyPatch, | ||||||||||||||
| tmp_path: Path, | ||||||||||||||
| ) -> None: | ||||||||||||||
|
|
@@ -145,6 +162,7 @@ def test_serve_gateway_enables_console_logging_before_loading_agents( | |||||||||||||
|
|
||||||||||||||
| async def fake_load_all_agents(*, remote_backends, local_dirs, agent_files): | ||||||||||||||
| observed["disable_console_env"] = os.environ.get("AWORLD_DISABLE_CONSOLE_LOG", "") | ||||||||||||||
| observed["gateway_console_env"] = os.environ.get("AWORLD_GATEWAY_CONSOLE_LOG", "") | ||||||||||||||
| observed["quiet_boot_env"] = os.environ.get("AWORLD_GATEWAY_QUIET_BOOT", "") | ||||||||||||||
| raise RuntimeError("stop after env check") | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -163,7 +181,8 @@ async def fake_load_all_agents(*, remote_backends, local_dirs, agent_files): | |||||||||||||
| ) | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| assert observed["disable_console_env"] == "false" | ||||||||||||||
| assert observed["disable_console_env"] == "true" | ||||||||||||||
| assert observed["gateway_console_env"] == "false" | ||||||||||||||
| assert observed["quiet_boot_env"] == "true" | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
|
|
@@ -311,11 +330,12 @@ def get_started_channel(self, channel_name: str): | |||||||||||||
| return telegram_adapter | ||||||||||||||
|
|
||||||||||||||
| class FakeUvicornConfig: | ||||||||||||||
| def __init__(self, *, app, host, port): | ||||||||||||||
| def __init__(self, *, app, host, port, log_level): | ||||||||||||||
| calls["uvicorn_config"] = { | ||||||||||||||
| "app": app, | ||||||||||||||
| "host": host, | ||||||||||||||
| "port": port, | ||||||||||||||
| "log_level": log_level, | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| class FakeUvicornServer: | ||||||||||||||
|
|
@@ -434,8 +454,9 @@ def get_started_channel(self, channel_name: str): | |||||||||||||
| return None | ||||||||||||||
|
|
||||||||||||||
| class FakeUvicornConfig: | ||||||||||||||
| def __init__(self, *, app, host, port): | ||||||||||||||
| def __init__(self, *, app, host, port, log_level): | ||||||||||||||
| calls["uvicorn_app"] = app | ||||||||||||||
| calls["uvicorn_log_level"] = log_level | ||||||||||||||
|
|
||||||||||||||
| class FakeUvicornServer: | ||||||||||||||
| def __init__(self, config): | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To ensure robust environment variable restoration,
_suppress_gateway_console_logging()should be called inside thetryblock. If an exception (such as aKeyboardInterruptor other asynchronous exception) occurs after_suppress_gateway_console_logging()is called but before entering thetryblock, the environment variables will not be restored in thefinallyblock.