From 71ead88bce1c56f431f14b1fe4f6e32a62fbf078 Mon Sep 17 00:00:00 2001 From: Ashwin Bharambe Date: Tue, 21 Oct 2025 11:08:25 -0700 Subject: [PATCH] fix(logging): move module-level initialization to explicit setup calls (#3874) - Moved environment variable parsing and `setup_logging()` call from module level to proper initialization points - Added explicit `setup_logging()` calls in `server.py::create_app()` and `library_client.py::AsyncLlamaStackAsLibraryClient.__init__()` Module-level side effects are bad practice and can cause issues with import order, testing, and circular dependencies. The previous implementation ran logging setup on every import of the log module, which is unpredictable and difficult to control. --------- Co-authored-by: Claude --- llama_stack/cli/llama.py | 5 +++++ llama_stack/core/library_client.py | 5 ++++- llama_stack/core/server/server.py | 5 ++++- llama_stack/log.py | 27 +++++++++++++++------------ 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/llama_stack/cli/llama.py b/llama_stack/cli/llama.py index 5ff15d8d7..aa8893bc0 100644 --- a/llama_stack/cli/llama.py +++ b/llama_stack/cli/llama.py @@ -6,6 +6,8 @@ import argparse +from llama_stack.log import setup_logging + from .stack import StackParser from .stack.utils import print_subcommand_description @@ -42,6 +44,9 @@ class LlamaCLIParser: def main(): + # Initialize logging from environment variables before any other operations + setup_logging() + parser = LlamaCLIParser() args = parser.parse_args() parser.run(args) diff --git a/llama_stack/core/library_client.py b/llama_stack/core/library_client.py index 328ca9c6e..c64b9a391 100644 --- a/llama_stack/core/library_client.py +++ b/llama_stack/core/library_client.py @@ -47,7 +47,7 @@ from llama_stack.core.stack import ( from llama_stack.core.utils.config import redact_sensitive_fields from llama_stack.core.utils.context import preserve_contexts_async_generator from llama_stack.core.utils.exec import in_notebook -from llama_stack.log import get_logger +from llama_stack.log import get_logger, setup_logging from llama_stack.providers.utils.telemetry.tracing import CURRENT_TRACE_CONTEXT, end_trace, setup_logger, start_trace from llama_stack.strong_typing.inspection import is_unwrapped_body_param @@ -200,6 +200,9 @@ class AsyncLlamaStackAsLibraryClient(AsyncLlamaStackClient): skip_logger_removal: bool = False, ): super().__init__() + # Initialize logging from environment variables first + setup_logging() + # when using the library client, we should not log to console since many # of our logs are intended for server-side usage if sinks_from_env := os.environ.get("TELEMETRY_SINKS", None): diff --git a/llama_stack/core/server/server.py b/llama_stack/core/server/server.py index db7584b01..dd21a72f9 100644 --- a/llama_stack/core/server/server.py +++ b/llama_stack/core/server/server.py @@ -56,7 +56,7 @@ from llama_stack.core.stack import ( from llama_stack.core.utils.config import redact_sensitive_fields from llama_stack.core.utils.config_resolution import Mode, resolve_config_or_distro from llama_stack.core.utils.context import preserve_contexts_async_generator -from llama_stack.log import get_logger +from llama_stack.log import get_logger, setup_logging from llama_stack.providers.datatypes import Api from llama_stack.providers.inline.telemetry.meta_reference.config import TelemetryConfig from llama_stack.providers.inline.telemetry.meta_reference.telemetry import ( @@ -374,6 +374,9 @@ def create_app() -> StackApp: Returns: Configured StackApp instance. """ + # Initialize logging from environment variables first + setup_logging() + config_file = os.getenv("LLAMA_STACK_CONFIG") if config_file is None: raise ValueError("LLAMA_STACK_CONFIG environment variable is required") diff --git a/llama_stack/log.py b/llama_stack/log.py index ff54b2f7c..8dc98cb08 100644 --- a/llama_stack/log.py +++ b/llama_stack/log.py @@ -166,14 +166,26 @@ class CustomFileHandler(logging.FileHandler): super().emit(record) -def setup_logging(category_levels: dict[str, int], log_file: str | None) -> None: +def setup_logging(category_levels: dict[str, int] | None = None, log_file: str | None = None) -> None: """ Configure logging based on the provided category log levels and an optional log file. + If category_levels or log_file are not provided, they will be read from environment variables. Parameters: - category_levels (Dict[str, int]): A dictionary mapping categories to their log levels. - log_file (str): Path to a log file to additionally pipe the logs into + category_levels (Dict[str, int] | None): A dictionary mapping categories to their log levels. + If None, reads from LLAMA_STACK_LOGGING environment variable and uses defaults. + log_file (str | None): Path to a log file to additionally pipe the logs into. + If None, reads from LLAMA_STACK_LOG_FILE environment variable. """ + # Read from environment variables if not explicitly provided + if category_levels is None: + category_levels = dict.fromkeys(CATEGORIES, DEFAULT_LOG_LEVEL) + env_config = os.environ.get("LLAMA_STACK_LOGGING", "") + if env_config: + category_levels.update(parse_environment_config(env_config)) + + if log_file is None: + log_file = os.environ.get("LLAMA_STACK_LOG_FILE") log_format = "%(asctime)s %(name)s:%(lineno)d %(category)s: %(message)s" class CategoryFilter(logging.Filter): @@ -278,12 +290,3 @@ def get_logger( log_level = _category_levels.get("root", DEFAULT_LOG_LEVEL) logger.setLevel(log_level) return logging.LoggerAdapter(logger, {"category": category}) - - -env_config = os.environ.get("LLAMA_STACK_LOGGING", "") -if env_config: - _category_levels.update(parse_environment_config(env_config)) - -log_file = os.environ.get("LLAMA_STACK_LOG_FILE") - -setup_logging(_category_levels, log_file)