diff --git a/server/src/agent_control_server/config.py b/server/src/agent_control_server/config.py index 9a1e754d..82ce14a4 100644 --- a/server/src/agent_control_server/config.py +++ b/server/src/agent_control_server/config.py @@ -244,6 +244,7 @@ class LoggingSettings(BaseSettings): model_config = SettingsConfigDict(**_COMMON_SETTINGS_CONFIG, env_prefix="AGENT_CONTROL_LOG_") + configure_logging: bool = _env_alias_field(True, "AGENT_CONTROL_CONFIGURE_LOGGING") level: str | None = None json_logs: bool = _env_alias_field(False, "AGENT_CONTROL_LOG_JSON") diff --git a/server/src/agent_control_server/logging_utils.py b/server/src/agent_control_server/logging_utils.py index 7c97744b..c41d4280 100644 --- a/server/src/agent_control_server/logging_utils.py +++ b/server/src/agent_control_server/logging_utils.py @@ -66,12 +66,20 @@ def _parse_json(json_flag: bool | None) -> bool: return LoggingSettings().json_logs +def should_configure_logging() -> bool: + """Return whether the server should install its own logging handlers.""" + return LoggingSettings().configure_logging + + def configure_logging( *, level: str | int | None = None, json: bool | None = None, default_level: str = "INFO", ) -> None: + if not should_configure_logging(): + return + resolved_level = level if level is not None else get_log_level_name(default_level) lvl = _parse_level(resolved_level) as_json = _parse_json(json) diff --git a/server/src/agent_control_server/main.py b/server/src/agent_control_server/main.py index 89a6275d..4aea5a6e 100644 --- a/server/src/agent_control_server/main.py +++ b/server/src/agent_control_server/main.py @@ -37,7 +37,7 @@ http_exception_handler, validation_exception_handler, ) -from .logging_utils import configure_logging, get_uvicorn_log_level_name +from .logging_utils import configure_logging, get_uvicorn_log_level_name, should_configure_logging from .observability.ingest import DirectEventIngestor from .observability.sinks import ( EventStoreControlEventSink, @@ -48,6 +48,7 @@ from .ui_assets import configure_ui_routes logger = logging.getLogger(__name__) +_logging_configured = False METRICS_PATH = "/metrics" PROMETHEUS_BUCKETS = [ @@ -74,6 +75,16 @@ def _default_log_level() -> str: return "DEBUG" if settings.debug else "INFO" +def _configure_logging_once() -> None: + global _logging_configured + + if _logging_configured or not should_configure_logging(): + return + + configure_logging(default_level=_default_log_level()) + _logging_configured = True + + def add_prometheus_metrics(app: FastAPI, metrics_prefix: str) -> None: """Configure Prometheus metrics for the FastAPI app.""" app.add_middleware( @@ -119,8 +130,7 @@ async def _shutdown_observability_sink(sink: object) -> None: @asynccontextmanager async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: """Lifespan context manager for FastAPI app startup and shutdown.""" - # Startup: Configure logging - configure_logging(default_level=_default_log_level()) + _configure_logging_once() # Install the request-auth provider selected by environment variables. from .auth_framework.config import configure_auth_from_env @@ -230,10 +240,6 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: allow_headers=settings.get_allow_headers(), ) -# Configure logging -configure_logging(default_level=_default_log_level()) - - @app.middleware("http") async def attach_version_header(request, call_next): # type: ignore[no-untyped-def] """Attach server version metadata to every response.""" @@ -403,12 +409,16 @@ async def health_check() -> HealthResponse: def run() -> None: """Run the server application.""" - uvicorn.run( - app, - host=settings.host, - port=settings.port, - log_level=get_uvicorn_log_level_name(_default_log_level()).lower(), - ) + _configure_logging_once() + uvicorn_kwargs: dict[str, Any] = { + "host": settings.host, + "port": settings.port, + "log_level": get_uvicorn_log_level_name(_default_log_level()).lower(), + } + if not should_configure_logging(): + uvicorn_kwargs["log_config"] = None + + uvicorn.run(app, **uvicorn_kwargs) if __name__ == "__main__": diff --git a/server/tests/test_config.py b/server/tests/test_config.py index 7029ce0b..51f48be1 100644 --- a/server/tests/test_config.py +++ b/server/tests/test_config.py @@ -2,6 +2,7 @@ from agent_control_server.config import ( AgentControlServerDatabaseConfig, + LoggingSettings, ObservabilitySettings, Settings, ) @@ -230,3 +231,17 @@ def test_observability_settings_ignore_legacy_env_vars(monkeypatch) -> None: # Then: the legacy env vars are ignored assert config.enabled is True assert config.stdout is False + + +def test_logging_settings_configure_logging_defaults_to_true() -> None: + config = LoggingSettings() + + assert config.configure_logging is True + + +def test_logging_settings_supports_host_owned_logging(monkeypatch) -> None: + monkeypatch.setenv("AGENT_CONTROL_CONFIGURE_LOGGING", "false") + + config = LoggingSettings() + + assert config.configure_logging is False diff --git a/server/tests/test_logging_utils.py b/server/tests/test_logging_utils.py index 750cd23d..5ed2e001 100644 --- a/server/tests/test_logging_utils.py +++ b/server/tests/test_logging_utils.py @@ -8,6 +8,7 @@ configure_logging, get_log_level_name, get_uvicorn_log_level_name, + should_configure_logging, ) @@ -138,3 +139,29 @@ def test_configure_logging_resets_uvicorn_handlers() -> None: logger.propagate = original_propagate root.handlers = root_handlers root.setLevel(root_level) + + +def test_should_configure_logging_can_be_disabled(monkeypatch) -> None: + monkeypatch.setenv("AGENT_CONTROL_CONFIGURE_LOGGING", "false") + + assert should_configure_logging() is False + + +def test_configure_logging_noops_when_host_owns_logging(monkeypatch) -> None: + monkeypatch.setenv("AGENT_CONTROL_CONFIGURE_LOGGING", "false") + + root = logging.getLogger() + handler = logging.StreamHandler() + original_handlers = list(root.handlers) + original_level = root.level + root.handlers = [handler] + root.setLevel(logging.ERROR) + + try: + configure_logging(level="INFO", json=False) + + assert root.handlers == [handler] + assert root.level == logging.ERROR + finally: + root.handlers = original_handlers + root.setLevel(original_level) diff --git a/server/tests/test_main_lifespan.py b/server/tests/test_main_lifespan.py index 69cabd5e..293fb957 100644 --- a/server/tests/test_main_lifespan.py +++ b/server/tests/test_main_lifespan.py @@ -1,7 +1,10 @@ from __future__ import annotations -from fastapi import FastAPI -from fastapi.testclient import TestClient +import json +import os +import subprocess +import sys +import textwrap from agent_control_server import main as main_module from agent_control_server.config import observability_settings, settings @@ -11,6 +14,63 @@ register_control_event_sink_factory, unregister_control_event_sink_factory, ) +from fastapi import FastAPI +from fastapi.testclient import TestClient + + +def test_importing_main_does_not_configure_host_owned_logging() -> None: + script = textwrap.dedent( + """ + import json + import logging + + root = logging.getLogger() + handler = logging.StreamHandler() + handler.setLevel(logging.ERROR) + root.handlers = [handler] + root.setLevel(logging.WARNING) + + before = { + "handler_types": [type(h).__name__ for h in root.handlers], + "handler_levels": [h.level for h in root.handlers], + "root_level": root.level, + } + import agent_control_server.main + after = { + "handler_types": [type(h).__name__ for h in root.handlers], + "handler_levels": [h.level for h in root.handlers], + "root_level": root.level, + } + print(json.dumps({"before": before, "after": after})) + """ + ) + + result = subprocess.run( + [sys.executable, "-c", script], + env={**os.environ, "AGENT_CONTROL_CONFIGURE_LOGGING": "false"}, + capture_output=True, + text=True, + check=True, + ) + + payload = json.loads(result.stdout.strip().splitlines()[-1]) + assert payload["before"] == payload["after"] + + +def test_configure_logging_once_is_idempotent(monkeypatch) -> None: + calls: list[str] = [] + + def fake_configure_logging(*, default_level: str) -> None: + calls.append(default_level) + + monkeypatch.setattr(main_module, "_logging_configured", False) + monkeypatch.setattr(main_module, "configure_logging", fake_configure_logging) + monkeypatch.setattr(settings, "debug", False) + + main_module._configure_logging_once() + main_module._configure_logging_once() + + assert calls == ["INFO"] def test_lifespan_initializes_observability_when_enabled(monkeypatch) -> None: @@ -230,3 +290,20 @@ def fake_run(app, host, port, log_level): assert called["host"] == "127.0.0.1" assert called["port"] == 9999 assert called["log_level"] == "debug" + + +def test_run_disables_uvicorn_log_config_when_host_owns_logging(monkeypatch) -> None: + called = {} + + def fake_run(app, **kwargs): # type: ignore[no-untyped-def] + called.update(kwargs) + + monkeypatch.setenv("AGENT_CONTROL_CONFIGURE_LOGGING", "false") + monkeypatch.setattr(main_module.uvicorn, "run", fake_run) + monkeypatch.setattr(settings, "host", "127.0.0.1") + monkeypatch.setattr(settings, "port", 9999) + monkeypatch.setattr(settings, "debug", False) + + main_module.run() + + assert called["log_config"] is None