diff --git a/python/README.md b/python/README.md index 3c01a5c..923ef70 100644 --- a/python/README.md +++ b/python/README.md @@ -556,20 +556,54 @@ SageMakerEnvVars.CUSTOM_SCRIPT_FILENAME SageMakerEnvVars.SAGEMAKER_MODEL_PATH ``` -### Debug Logging +### Logging Control -```python -from model_hosting_container_standards.logging_config import enable_debug_logging +The package provides centralized logging control using standard SageMaker environment variables. + +**By default, the package uses ERROR level logging**, which effectively keeps it silent in production unless there are actual errors. + +#### Log Level Configuration + +```bash +# Set log level using SageMaker standard variable (recommended) +export SAGEMAKER_CONTAINER_LOG_LEVEL=DEBUG # or INFO, WARNING, ERROR (default) + +# Alternative: Use generic LOG_LEVEL variable +export LOG_LEVEL=INFO # Falls back to this if SAGEMAKER_CONTAINER_LOG_LEVEL not set +``` + +#### Log Levels -# Enable detailed handler resolution logging -enable_debug_logging() +- **ERROR (default)**: Only errors are logged - effectively silent in normal operation +- **WARNING**: Errors and warnings +- **INFO**: Informational messages, warnings, and errors +- **DEBUG**: Detailed debug information including handler resolution + +#### Log Format + +All package logs use a consistent format: ``` +[LEVEL] logger_name - filename:line: message +``` + +#### Examples ```bash -# Or via environment variable -export SAGEMAKER_CONTAINER_LOG_LEVEL=DEBUG +# Production: ERROR level by default (silent unless errors occur) +vllm serve model --dtype auto + +# Development: Enable INFO level logging +SAGEMAKER_CONTAINER_LOG_LEVEL=INFO vllm serve model --dtype auto + +# Debug mode: Enable detailed DEBUG logging +SAGEMAKER_CONTAINER_LOG_LEVEL=DEBUG vllm serve model --dtype auto + +# Using alternative LOG_LEVEL variable +LOG_LEVEL=DEBUG vllm serve model --dtype auto ``` +**Note**: These environment variables only control package logging. Your application's logging configuration is independent and unaffected. + ## Testing ### Quick Endpoint Testing diff --git a/python/model_hosting_container_standards/__init__.py b/python/model_hosting_container_standards/__init__.py index a698e3a..c866d87 100644 --- a/python/model_hosting_container_standards/__init__.py +++ b/python/model_hosting_container_standards/__init__.py @@ -5,4 +5,4 @@ - FastAPI: from .common.fastapi import EnvVars, ENV_CONFIG """ -__version__ = "0.1.7" +__version__ = "0.1.8" diff --git a/python/model_hosting_container_standards/common/custom_code_ref_resolver/file_loader.py b/python/model_hosting_container_standards/common/custom_code_ref_resolver/file_loader.py index eba2bba..b7c5345 100644 --- a/python/model_hosting_container_standards/common/custom_code_ref_resolver/file_loader.py +++ b/python/model_hosting_container_standards/common/custom_code_ref_resolver/file_loader.py @@ -27,7 +27,6 @@ """ import importlib.util -import logging from pathlib import Path from typing import Any, List, Optional @@ -36,8 +35,7 @@ HandlerNotFoundError, ModuleLoadError, ) - -logger = logging.getLogger(__name__) +from model_hosting_container_standards.logging_config import logger class FileLoader: diff --git a/python/model_hosting_container_standards/common/custom_code_ref_resolver/function_loader.py b/python/model_hosting_container_standards/common/custom_code_ref_resolver/function_loader.py index 710cc22..d989e0c 100644 --- a/python/model_hosting_container_standards/common/custom_code_ref_resolver/function_loader.py +++ b/python/model_hosting_container_standards/common/custom_code_ref_resolver/function_loader.py @@ -1,4 +1,3 @@ -import logging from typing import Any, Callable, Dict, List, Optional from model_hosting_container_standards.exceptions import ( @@ -6,13 +5,12 @@ HandlerNotCallableError, HandlerNotFoundError, ) +from model_hosting_container_standards.logging_config import logger from ..handler.spec import HandlerSpec from .file_loader import FileLoader from .module_loader import ModuleLoader -logger = logging.getLogger(__name__) - class FunctionLoader: """Unified function loader supporting multiple formats. diff --git a/python/model_hosting_container_standards/common/custom_code_ref_resolver/module_loader.py b/python/model_hosting_container_standards/common/custom_code_ref_resolver/module_loader.py index 2798da2..6c0f9d6 100644 --- a/python/model_hosting_container_standards/common/custom_code_ref_resolver/module_loader.py +++ b/python/model_hosting_container_standards/common/custom_code_ref_resolver/module_loader.py @@ -11,15 +11,13 @@ """ import importlib -import logging from typing import Any, Optional from model_hosting_container_standards.exceptions import ( HandlerNotFoundError, ModuleLoadError, ) - -logger = logging.getLogger(__name__) +from model_hosting_container_standards.logging_config import logger class ModuleLoader: diff --git a/python/model_hosting_container_standards/common/fastapi/routing.py b/python/model_hosting_container_standards/common/fastapi/routing.py index dd186e9..de76c2c 100644 --- a/python/model_hosting_container_standards/common/fastapi/routing.py +++ b/python/model_hosting_container_standards/common/fastapi/routing.py @@ -1,14 +1,12 @@ -import logging from dataclasses import dataclass from typing import Callable, List, Optional from fastapi import APIRouter, FastAPI from fastapi.routing import APIRoute +from ...logging_config import logger from ..handler import handler_registry -logger = logging.getLogger(__name__) - def normalize_prefix(prefix: str) -> str: """Normalize a URL prefix to ensure consistent path handling. diff --git a/python/model_hosting_container_standards/common/handler/resolver.py b/python/model_hosting_container_standards/common/handler/resolver.py index 9454f9d..84c26d2 100644 --- a/python/model_hosting_container_standards/common/handler/resolver.py +++ b/python/model_hosting_container_standards/common/handler/resolver.py @@ -34,7 +34,6 @@ def get_customer_script_handler(self, handler_type: str): ``` """ -import logging from abc import ABC, abstractmethod from typing import Any, Callable, Optional, Union @@ -44,11 +43,10 @@ def get_customer_script_handler(self, handler_type: str): HandlerResolutionError, InvalidHandlerSpecError, ) +from model_hosting_container_standards.logging_config import logger from .registry import handler_registry -logger = logging.getLogger(__name__) - class HandlerConfig(ABC): """Abstract base class for handler configuration. diff --git a/python/model_hosting_container_standards/common/transforms/base_factory.py b/python/model_hosting_container_standards/common/transforms/base_factory.py index c3b7aa3..45e3dfe 100644 --- a/python/model_hosting_container_standards/common/transforms/base_factory.py +++ b/python/model_hosting_container_standards/common/transforms/base_factory.py @@ -1,12 +1,10 @@ -import logging from typing import Any, Callable, Dict, Optional from fastapi import Request from model_hosting_container_standards.common import BaseApiTransform from model_hosting_container_standards.common.handler import handler_registry - -logger = logging.getLogger(__name__) +from model_hosting_container_standards.logging_config import logger def _resolve_transforms( diff --git a/python/model_hosting_container_standards/logging_config.py b/python/model_hosting_container_standards/logging_config.py index b6c0c87..6aa2b66 100644 --- a/python/model_hosting_container_standards/logging_config.py +++ b/python/model_hosting_container_standards/logging_config.py @@ -6,14 +6,21 @@ def get_logger(name: str = "model_hosting_container_standards") -> logging.Logger: - """Get a configured logger for the package.""" + """Get a configured logger for the package. + + The logger uses SAGEMAKER_CONTAINER_LOG_LEVEL (or LOG_LEVEL) to determine the log level. + If not set, defaults to ERROR level, which effectively disables most package logging. + + Returns: + Configured logger instance for the package. + """ logger = logging.getLogger(name) # Only configure if not already configured if not logger.handlers: - # Get log level from environment or default to INFO + # Get log level from environment, default to ERROR (effectively disabled) level = os.getenv( - "SAGEMAKER_CONTAINER_LOG_LEVEL", os.getenv("LOG_LEVEL", "INFO") + "SAGEMAKER_CONTAINER_LOG_LEVEL", os.getenv("LOG_LEVEL", "ERROR") ) # Set up handler with consistent format diff --git a/python/model_hosting_container_standards/sagemaker/handler_resolver.py b/python/model_hosting_container_standards/sagemaker/handler_resolver.py index 843bc2e..dfa1969 100644 --- a/python/model_hosting_container_standards/sagemaker/handler_resolver.py +++ b/python/model_hosting_container_standards/sagemaker/handler_resolver.py @@ -24,16 +24,14 @@ - Missing handlers return None (graceful degradation) """ -import logging from typing import Any, Callable, Optional, Union from ..common.handler.registry import handler_registry from ..common.handler.resolver import GenericHandlerResolver, HandlerConfig from ..exceptions import HandlerFileNotFoundError, HandlerNotFoundError +from ..logging_config import logger from .sagemaker_loader import SageMakerFunctionLoader -logger = logging.getLogger(__name__) - class SageMakerHandlerConfig(HandlerConfig): """SageMaker-specific handler configuration.""" diff --git a/python/model_hosting_container_standards/sagemaker/lora/factory.py b/python/model_hosting_container_standards/sagemaker/lora/factory.py index dae1bd4..7c51426 100644 --- a/python/model_hosting_container_standards/sagemaker/lora/factory.py +++ b/python/model_hosting_container_standards/sagemaker/lora/factory.py @@ -1,13 +1,9 @@ -import logging - from model_hosting_container_standards.common.transforms.base_factory import ( create_transform_decorator, ) from .transforms import resolve_lora_transform -logger = logging.getLogger(__name__) - def create_lora_transform_decorator(handler_type: str): return create_transform_decorator(handler_type, resolve_lora_transform) diff --git a/python/model_hosting_container_standards/sagemaker/lora/transforms/__init__.py b/python/model_hosting_container_standards/sagemaker/lora/transforms/__init__.py index 3d73614..47a759e 100644 --- a/python/model_hosting_container_standards/sagemaker/lora/transforms/__init__.py +++ b/python/model_hosting_container_standards/sagemaker/lora/transforms/__init__.py @@ -1,13 +1,11 @@ -import logging from typing import List +from ....logging_config import logger from ..constants import LoRAHandlerType from .inject_to_body import InjectToBodyApiTransform from .register import RegisterLoRAApiTransform from .unregister import UnregisterLoRAApiTransform -logger = logging.getLogger(__name__) - def resolve_lora_transform(handler_type: str) -> type: """Get the appropriate transformer class based on the LoRA handler type. diff --git a/python/model_hosting_container_standards/sagemaker/sagemaker_router.py b/python/model_hosting_container_standards/sagemaker/sagemaker_router.py index 45b66ec..0171709 100644 --- a/python/model_hosting_container_standards/sagemaker/sagemaker_router.py +++ b/python/model_hosting_container_standards/sagemaker/sagemaker_router.py @@ -1,16 +1,14 @@ -import logging from typing import Optional from fastapi import APIRouter # Import routing utilities (generic) from ..common.fastapi.routing import RouteConfig, create_router +from ..logging_config import logger # Import LoRA-specific route configuration from .lora.routes import get_lora_route_config -logger = logging.getLogger(__name__) - def get_sagemaker_route_config(handler_type: str) -> Optional[RouteConfig]: """Get route configuration for SageMaker handler types. diff --git a/python/model_hosting_container_standards/sagemaker/sessions/handlers.py b/python/model_hosting_container_standards/sagemaker/sessions/handlers.py index c074468..f75da69 100644 --- a/python/model_hosting_container_standards/sagemaker/sessions/handlers.py +++ b/python/model_hosting_container_standards/sagemaker/sessions/handlers.py @@ -1,16 +1,14 @@ -import logging from datetime import datetime, timezone from http import HTTPStatus from fastapi import Request, Response from fastapi.exceptions import HTTPException +from ...logging_config import logger from .manager import SessionManager from .models import SageMakerSessionHeader, SessionRequestType from .utils import get_session_id_from_request -logger = logging.getLogger(__name__) - def get_handler_for_request_type(request_type: SessionRequestType): """Map session request type to the appropriate handler function. diff --git a/python/model_hosting_container_standards/sagemaker/sessions/transform.py b/python/model_hosting_container_standards/sagemaker/sessions/transform.py index 8290b78..c7ee449 100644 --- a/python/model_hosting_container_standards/sagemaker/sessions/transform.py +++ b/python/model_hosting_container_standards/sagemaker/sessions/transform.py @@ -1,5 +1,4 @@ import json -import logging from http import HTTPStatus from typing import Optional @@ -13,8 +12,6 @@ from .models import SessionRequest from .utils import get_session, get_session_id_from_request -logger = logging.getLogger(__name__) - def _parse_session_request(request_data: dict) -> Optional[SessionRequest]: """Parse and validate if request is a session management request. diff --git a/python/model_hosting_container_standards/sagemaker/sessions/utils.py b/python/model_hosting_container_standards/sagemaker/sessions/utils.py index 8926ae0..3c47f48 100644 --- a/python/model_hosting_container_standards/sagemaker/sessions/utils.py +++ b/python/model_hosting_container_standards/sagemaker/sessions/utils.py @@ -1,14 +1,12 @@ -import logging from http import HTTPStatus from fastapi import Request from fastapi.exceptions import HTTPException +from ...logging_config import logger from .manager import SessionManager from .models import SageMakerSessionHeader -logger = logging.getLogger(__name__) - def get_session_id_from_request(raw_request: Request): """Extract the session ID from the request headers. diff --git a/python/pyproject.toml b/python/pyproject.toml index 5a939ed..43a9e4c 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "model-hosting-container-standards" -version = "0.1.7" +version = "0.1.8" description = "Python toolkit for standardized model hosting container implementations with Amazon SageMaker integration" authors = ["Amazon Web Services"] readme = "README.md" diff --git a/python/tests/test_logging_config.py b/python/tests/test_logging_config.py new file mode 100644 index 0000000..2e1deb8 --- /dev/null +++ b/python/tests/test_logging_config.py @@ -0,0 +1,163 @@ +"""Unit tests for logging_config module.""" + +import logging +import os +from unittest.mock import patch + +from model_hosting_container_standards.logging_config import get_logger + + +class TestGetLogger: + """Test get_logger function.""" + + def test_default_error_level(self): + """Test that logger defaults to ERROR level when no env var set.""" + with patch.dict(os.environ, {}, clear=True): + # Create a unique logger name to avoid conflicts + test_logger = get_logger("test_default_logger") + try: + assert len(test_logger.handlers) > 0 + assert test_logger.level == logging.ERROR + finally: + # Clean up the test logger + test_logger.handlers.clear() + + def test_sagemaker_container_log_level_debug(self): + """Test that SAGEMAKER_CONTAINER_LOG_LEVEL=DEBUG sets DEBUG level.""" + with patch.dict(os.environ, {"SAGEMAKER_CONTAINER_LOG_LEVEL": "DEBUG"}): + test_logger = get_logger("test_debug_logger") + try: + assert len(test_logger.handlers) > 0 + assert test_logger.level == logging.DEBUG + finally: + test_logger.handlers.clear() + + def test_sagemaker_container_log_level_info(self): + """Test that SAGEMAKER_CONTAINER_LOG_LEVEL=INFO sets INFO level.""" + with patch.dict(os.environ, {"SAGEMAKER_CONTAINER_LOG_LEVEL": "INFO"}): + test_logger = get_logger("test_info_logger") + try: + assert len(test_logger.handlers) > 0 + assert test_logger.level == logging.INFO + finally: + test_logger.handlers.clear() + + def test_sagemaker_container_log_level_warning(self): + """Test that SAGEMAKER_CONTAINER_LOG_LEVEL=WARNING sets WARNING level.""" + with patch.dict(os.environ, {"SAGEMAKER_CONTAINER_LOG_LEVEL": "WARNING"}): + test_logger = get_logger("test_warning_logger") + try: + assert len(test_logger.handlers) > 0 + assert test_logger.level == logging.WARNING + finally: + test_logger.handlers.clear() + + def test_sagemaker_container_log_level_error(self): + """Test that SAGEMAKER_CONTAINER_LOG_LEVEL=ERROR sets ERROR level.""" + with patch.dict(os.environ, {"SAGEMAKER_CONTAINER_LOG_LEVEL": "ERROR"}): + test_logger = get_logger("test_error_logger") + try: + assert len(test_logger.handlers) > 0 + assert test_logger.level == logging.ERROR + finally: + test_logger.handlers.clear() + + def test_log_level_fallback(self): + """Test that LOG_LEVEL is used when SAGEMAKER_CONTAINER_LOG_LEVEL not set.""" + with patch.dict(os.environ, {"LOG_LEVEL": "DEBUG"}, clear=True): + test_logger = get_logger("test_fallback_logger") + try: + assert len(test_logger.handlers) > 0 + assert test_logger.level == logging.DEBUG + finally: + test_logger.handlers.clear() + + def test_sagemaker_takes_priority_over_log_level(self): + """Test that SAGEMAKER_CONTAINER_LOG_LEVEL takes priority over LOG_LEVEL.""" + with patch.dict( + os.environ, + {"SAGEMAKER_CONTAINER_LOG_LEVEL": "INFO", "LOG_LEVEL": "DEBUG"}, + ): + test_logger = get_logger("test_priority_logger") + try: + assert len(test_logger.handlers) > 0 + assert test_logger.level == logging.INFO + finally: + test_logger.handlers.clear() + + def test_case_insensitive_log_level(self): + """Test that log level is case-insensitive.""" + with patch.dict(os.environ, {"SAGEMAKER_CONTAINER_LOG_LEVEL": "debug"}): + test_logger = get_logger("test_case_logger") + try: + assert len(test_logger.handlers) > 0 + assert test_logger.level == logging.DEBUG + finally: + test_logger.handlers.clear() + + def test_logger_not_reconfigured_if_already_configured(self): + """Test that logger is not reconfigured if it already has handlers.""" + with patch.dict(os.environ, {"SAGEMAKER_CONTAINER_LOG_LEVEL": "INFO"}): + # Create a unique logger name + test_logger = get_logger("test_reconfig_logger") + try: + initial_handler_count = len(test_logger.handlers) + + # Call get_logger again + test_logger_again = get_logger("test_reconfig_logger") + + # Should be the same logger instance with same handlers + assert test_logger is test_logger_again + assert len(test_logger_again.handlers) == initial_handler_count + finally: + # Clean up the test logger + test_logger.handlers.clear() + + def test_logger_name_parameter(self): + """Test that logger name parameter is respected.""" + with patch.dict(os.environ, {"SAGEMAKER_CONTAINER_LOG_LEVEL": "INFO"}): + custom_name = "custom_test_logger" + test_logger = get_logger(custom_name) + try: + assert test_logger.name == custom_name + finally: + # Clean up the test logger + test_logger.handlers.clear() + + def test_default_logger_name(self): + """Test that default logger name is used when not specified.""" + with patch.dict(os.environ, {"SAGEMAKER_CONTAINER_LOG_LEVEL": "INFO"}): + test_logger = get_logger() + try: + assert test_logger.name == "model_hosting_container_standards" + finally: + # Clean up the test logger + test_logger.handlers.clear() + + def test_logger_has_handler_and_formatter(self): + """Test that logger has proper handler and formatter configured.""" + with patch.dict(os.environ, {"SAGEMAKER_CONTAINER_LOG_LEVEL": "INFO"}): + test_logger = get_logger("test_format_logger") + try: + assert len(test_logger.handlers) == 1 + handler = test_logger.handlers[0] + assert isinstance(handler, logging.StreamHandler) + assert handler.formatter is not None + # Check format string contains expected elements + format_str = handler.formatter._fmt + assert "%(levelname)s" in format_str + assert "%(name)s" in format_str + assert "%(filename)s" in format_str + assert "%(lineno)d" in format_str + assert "%(message)s" in format_str + finally: + test_logger.handlers.clear() + + def test_logger_propagate_false(self): + """Test that logger propagate is set to False to avoid duplicate logs.""" + with patch.dict(os.environ, {"SAGEMAKER_CONTAINER_LOG_LEVEL": "INFO"}): + test_logger = get_logger("test_propagate_logger") + try: + assert test_logger.propagate is False + finally: + test_logger.handlers.clear()