From 70a2e054f65da81876c0015edf808375b16d9b74 Mon Sep 17 00:00:00 2001 From: Roja Reddy Sareddy Date: Sun, 27 Sep 2026 17:56:39 -0700 Subject: [PATCH 1/3] fix(core): stop hijacking stdout logging on import get_sagemaker_config_logger() attached a StreamHandler(sys.stdout) to the "sagemaker.config" logger and set propagate=False the first time it ran (which happens during config resolution at import). A library forcing its own stdout handler and disabling propagation overrides the application's logging configuration and pollutes stdout just by importing sagemaker. Follow the standard library-logging pattern instead: - Install a NullHandler on the top-level "sagemaker" logger at import so records are safely discarded until the application configures logging. - Have get_sagemaker_config_logger() only set a default INFO level (when unset) and let records propagate to the application's handlers, rather than attaching a stdout handler or disabling propagation. Config-substitution messages are still emitted on the "sagemaker.config" logger; applications that configure logging (e.g. logging.basicConfig) receive them through their own handlers and formatting. Closes #4387 --- sagemaker-core/src/sagemaker/core/__init__.py | 10 +++++ .../src/sagemaker/core/config/config_utils.py | 29 ++++--------- .../tests/unit/config/test_config_logging.py | 41 +++++++++++++++++++ 3 files changed, 60 insertions(+), 20 deletions(-) create mode 100644 sagemaker-core/tests/unit/config/test_config_logging.py diff --git a/sagemaker-core/src/sagemaker/core/__init__.py b/sagemaker-core/src/sagemaker/core/__init__.py index fe829739b3..8d6cf6c7db 100644 --- a/sagemaker-core/src/sagemaker/core/__init__.py +++ b/sagemaker-core/src/sagemaker/core/__init__.py @@ -1,8 +1,18 @@ """SageMaker Core package for low-level resource management and SDK foundations.""" +import logging as _logging + from sagemaker.core.utils.utils import enable_textual_rich_console_and_traceback from sagemaker.core.deprecations import register_removed_module_finder +# A library must not hijack stdout or emit to an unconfigured root logger. Attach a +# NullHandler to the top-level "sagemaker" logger so SDK log records are discarded by +# default until the application configures logging (see #4387). sagemaker-core is the +# universal dependency of every v3 package, so installing it here covers all of them. +_sagemaker_root_logger = _logging.getLogger("sagemaker") +if not any(isinstance(_h, _logging.NullHandler) for _h in _sagemaker_root_logger.handlers): + _sagemaker_root_logger.addHandler(_logging.NullHandler()) + enable_textual_rich_console_and_traceback() # Install the meta-path finder that gives actionable migration guidance for v2 diff --git a/sagemaker-core/src/sagemaker/core/config/config_utils.py b/sagemaker-core/src/sagemaker/core/config/config_utils.py index e0f0ba2a42..b03150829d 100644 --- a/sagemaker-core/src/sagemaker/core/config/config_utils.py +++ b/sagemaker-core/src/sagemaker/core/config/config_utils.py @@ -19,7 +19,6 @@ from collections import deque import logging -import sys from typing import Callable, List, TYPE_CHECKING import re from copy import deepcopy @@ -31,31 +30,21 @@ def get_sagemaker_config_logger(): - """Return a logger with the name 'sagemaker.config' - - If the logger to be returned has no level or handlers set, this will get level and handler - attributes. (So if the SDK user has setup loggers in a certain way, that setup will not be - changed by this function.) It is safe to make repeat calls to this function. + """Return the ``sagemaker.config`` logger. + + Ensures the logger has a sensible default level (INFO) when the user has not + configured one, but does **not** attach handlers or disable propagation. The + SDK is a library, so it must not hijack ``sys.stdout`` or override the + application's logging configuration; log records propagate to whatever the + application has configured. A ``NullHandler`` on the top-level ``sagemaker`` + logger (installed at import) safely discards records until then. It is safe to + call this function repeatedly. """ sagemaker_config_logger = logging.getLogger("sagemaker.config") - sagemaker_logger = logging.getLogger("sagemaker") if sagemaker_config_logger.level == logging.NOTSET: sagemaker_config_logger.setLevel(logging.INFO) - # check sagemaker_logger here as well, so that if handlers were set for the parent logger - # already, we dont change behavior for the child logger - if len(sagemaker_config_logger.handlers) == 0 and len(sagemaker_logger.handlers) == 0: - # use sys.stdout so logs dont show up with a red background in a notebook - handler = logging.StreamHandler(sys.stdout) - - formatter = logging.Formatter("%(name)s %(levelname)-4s - %(message)s") - handler.setFormatter(formatter) - sagemaker_config_logger.addHandler(handler) - - # if a handler is being added, we dont want the root handler to also process the same events - sagemaker_config_logger.propagate = False - return sagemaker_config_logger diff --git a/sagemaker-core/tests/unit/config/test_config_logging.py b/sagemaker-core/tests/unit/config/test_config_logging.py new file mode 100644 index 0000000000..7394db472c --- /dev/null +++ b/sagemaker-core/tests/unit/config/test_config_logging.py @@ -0,0 +1,41 @@ +"""Unit tests locking in that the SDK does not hijack stdout logging on import (#4387). + +The library must not attach a stdout StreamHandler to the ``sagemaker.config`` logger +or disable its propagation, and it must install a NullHandler on the top-level +``sagemaker`` logger so records are safely discarded until the application configures +logging. +""" +import logging + +from sagemaker.core.config.config_utils import get_sagemaker_config_logger + + +def test_config_logger_does_not_attach_stdout_handler(): + logger = get_sagemaker_config_logger() + stream_handlers = [ + h + for h in logger.handlers + if isinstance(h, logging.StreamHandler) and not isinstance(h, logging.NullHandler) + ] + assert stream_handlers == [] + + +def test_config_logger_does_not_disable_propagation(): + logger = logging.getLogger("sagemaker.config") + logger.propagate = True + get_sagemaker_config_logger() + # The library must let records propagate to the application's logging config. + assert logger.propagate is True + + +def test_config_logger_defaults_to_info_level_when_unset(): + logger = logging.getLogger("sagemaker.config") + logger.setLevel(logging.NOTSET) + assert get_sagemaker_config_logger().level == logging.INFO + + +def test_root_sagemaker_logger_has_nullhandler_after_import(): + import sagemaker.core # noqa: F401 # importing installs the NullHandler + + root = logging.getLogger("sagemaker") + assert any(isinstance(h, logging.NullHandler) for h in root.handlers) From 2f00a11f49920b45f554de6cdf76508c589001d4 Mon Sep 17 00:00:00 2001 From: Roja Reddy Sareddy Date: Mon, 28 Sep 2026 11:19:36 -0700 Subject: [PATCH 2/3] style(core): apply black to test_config_logging.py Add the blank line after the module docstring that black --check (used by the codestyle-doc-tests CI job) requires. --- sagemaker-core/tests/unit/config/test_config_logging.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sagemaker-core/tests/unit/config/test_config_logging.py b/sagemaker-core/tests/unit/config/test_config_logging.py index 7394db472c..e06458e868 100644 --- a/sagemaker-core/tests/unit/config/test_config_logging.py +++ b/sagemaker-core/tests/unit/config/test_config_logging.py @@ -5,6 +5,7 @@ ``sagemaker`` logger so records are safely discarded until the application configures logging. """ + import logging from sagemaker.core.config.config_utils import get_sagemaker_config_logger From bbd8b1b4931990a39250c8405ed6364b77b0f01b Mon Sep 17 00:00:00 2001 From: Roja Reddy Sareddy Date: Mon, 28 Sep 2026 14:33:42 -0700 Subject: [PATCH 3/3] fix(core): gate import-time rich logging to fully stop stdout hijack Address review feedback: stopping the sagemaker.config StreamHandler alone did not eliminate stdout output on import, because get_textual_rich_logger() still called logging.basicConfig(level=INFO, handlers=[RichHandler(...)]) at import (via the module-level loggers in utils.py, resources.py, and intelligent_defaults_helper.py). That installed a RichHandler on the root logger and forced the root level to INFO, so config-substitution messages still printed to stdout (now propagated to that root handler) and the application's own logging config was overridden. Make the rich logging opt-in: add is_rich_logging_enabled() (gated by the SAGEMAKER_ENABLE_RICH_LOGGING env var) and only call basicConfig / install the rich console+traceback when opted in. A default "import sagemaker" now leaves the root logger level and handlers untouched and sys.excepthook as the stdlib default, so combined with the NullHandler and the sagemaker.config change, nothing is emitted to stdout on import. Closes #4387 --- sagemaker-core/src/sagemaker/core/__init__.py | 2 +- .../src/sagemaker/core/utils/utils.py | 60 +++++++-- .../tests/unit/utils/test_rich_logging.py | 114 ++++++++++++++++++ 3 files changed, 165 insertions(+), 11 deletions(-) create mode 100644 sagemaker-core/tests/unit/utils/test_rich_logging.py diff --git a/sagemaker-core/src/sagemaker/core/__init__.py b/sagemaker-core/src/sagemaker/core/__init__.py index 8d6cf6c7db..8c3fb33d91 100644 --- a/sagemaker-core/src/sagemaker/core/__init__.py +++ b/sagemaker-core/src/sagemaker/core/__init__.py @@ -13,7 +13,7 @@ if not any(isinstance(_h, _logging.NullHandler) for _h in _sagemaker_root_logger.handlers): _sagemaker_root_logger.addHandler(_logging.NullHandler()) -enable_textual_rich_console_and_traceback() +enable_textual_rich_console_and_traceback() # opt-in; no-op unless SAGEMAKER_ENABLE_RICH_LOGGING is set # Install the meta-path finder that gives actionable migration guidance for v2 # modules removed in v3. sagemaker-core is the universal dependency of every v3 diff --git a/sagemaker-core/src/sagemaker/core/utils/utils.py b/sagemaker-core/src/sagemaker/core/utils/utils.py index dfc815190a..d807523935 100644 --- a/sagemaker-core/src/sagemaker/core/utils/utils.py +++ b/sagemaker-core/src/sagemaker/core/utils/utils.py @@ -137,12 +137,47 @@ def get_textual_rich_theme() -> Theme: ) +RICH_LOGGING_OPT_IN_ENV_VAR = "SAGEMAKER_ENABLE_RICH_LOGGING" + +_TRUTHY_ENV_VALUES = frozenset({"1", "true", "yes", "on"}) + + +def is_rich_logging_enabled() -> bool: + """Whether the user opted in to sagemaker-core's rich console and tracebacks. + + Reconfiguring the global rich console and calling ``rich.traceback.install()`` + override ``sys.excepthook`` and restyle the process-global console, so they are + opt-in: merely importing the SDK must not change tracebacks or console styling. + Enable by setting the ``SAGEMAKER_ENABLE_RICH_LOGGING`` environment variable to + one of ``1``/``true``/``yes``/``on`` (case-insensitive). + + Returns: + bool: True if rich console/traceback output has been opted into. + """ + return os.environ.get(RICH_LOGGING_OPT_IN_ENV_VAR, "").strip().lower() in _TRUTHY_ENV_VALUES + + textual_rich_console_and_traceback_enabled = False -def enable_textual_rich_console_and_traceback(): - """Reconfigure the global textual rich console with the customized theme and enable textual rich error traceback""" +def enable_textual_rich_console_and_traceback(force: bool = False): + """Reconfigure the global rich console and install rich error tracebacks. + + This overrides ``sys.excepthook`` (via ``rich.traceback.install``) and restyles + the process-global rich console. Because those are process-wide side effects, it + is opt-in and a no-op unless the user opts in via the + ``SAGEMAKER_ENABLE_RICH_LOGGING`` environment variable + (see :func:`is_rich_logging_enabled`) or the caller passes ``force=True``. This + keeps ``import sagemaker`` free of global traceback/console side effects by + default. + + Args: + force (bool): Enable regardless of the environment variable, for callers + that explicitly want rich output. Defaults to False. + """ global textual_rich_console_and_traceback_enabled + if not (force or is_rich_logging_enabled()): + return if not textual_rich_console_and_traceback_enabled: theme = get_textual_rich_theme() reconfigure(theme=theme) @@ -159,23 +194,28 @@ def get_rich_handler(): def get_textual_rich_logger(name: str, log_level: str = "INFO") -> logging.Logger: - """Get a logger with textual rich handler. + """Get a logger, attaching a rich handler only when rich logging is opted in. + + Rich logging (a ``RichHandler`` on the root logger via ``logging.basicConfig``, + plus the themed console/traceback) is opt-in, so that importing the SDK does not + reconfigure the root logger or change process-wide log formatting/level. When the + user has not opted in (see :func:`is_rich_logging_enabled`), this returns the named + logger without configuring handlers or levels, leaving logging to the application. Args: name (str): The name of the logger - log_level (str): The log level to set. + log_level (str): The log level to set when rich logging is enabled. Accepted values are: "DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL". Defaults to the value of "INFO". Return: - logging.Logger: A textial rich logger. + logging.Logger: The requested logger. """ enable_textual_rich_console_and_traceback() - handler = get_rich_handler() - logging.basicConfig(level=getattr(logging, log_level), handlers=[handler]) - logger = logging.getLogger(name) - - return logger + if is_rich_logging_enabled(): + handler = get_rich_handler() + logging.basicConfig(level=getattr(logging, log_level), handlers=[handler]) + return logging.getLogger(name) logger = get_textual_rich_logger(__name__) diff --git a/sagemaker-core/tests/unit/utils/test_rich_logging.py b/sagemaker-core/tests/unit/utils/test_rich_logging.py new file mode 100644 index 0000000000..e875ea9ec1 --- /dev/null +++ b/sagemaker-core/tests/unit/utils/test_rich_logging.py @@ -0,0 +1,114 @@ +"""Unit tests for opt-in rich console/traceback behavior in sagemaker.core.utils.utils. + +These lock in that importing the SDK does not override sys.excepthook or restyle the +process-global rich console unless the user explicitly opts in via +SAGEMAKER_ENABLE_RICH_LOGGING (or a force=True call). +""" + +import os +from unittest.mock import patch + +import pytest + +from sagemaker.core.utils import utils +from sagemaker.core.utils.utils import ( + RICH_LOGGING_OPT_IN_ENV_VAR, + enable_textual_rich_console_and_traceback, + is_rich_logging_enabled, +) + + +@pytest.fixture(autouse=True) +def _reset_latch(): + """Reset the one-shot 'already enabled' latch around every test.""" + saved = utils.textual_rich_console_and_traceback_enabled + utils.textual_rich_console_and_traceback_enabled = False + try: + yield + finally: + utils.textual_rich_console_and_traceback_enabled = saved + + +@pytest.mark.parametrize("value", ["1", "true", "TRUE", "Yes", "on", " on "]) +def test_is_rich_logging_enabled_truthy(value): + with patch.dict(os.environ, {RICH_LOGGING_OPT_IN_ENV_VAR: value}): + assert is_rich_logging_enabled() is True + + +@pytest.mark.parametrize("value", ["", "0", "false", "no", "off", "nope"]) +def test_is_rich_logging_enabled_falsey(value): + with patch.dict(os.environ, {RICH_LOGGING_OPT_IN_ENV_VAR: value}): + assert is_rich_logging_enabled() is False + + +def test_disabled_by_default_is_noop(): + # Env var absent -> importing/using the SDK must not touch the global console + # or install rich tracebacks (no sys.excepthook override). + with patch.dict(os.environ, {}, clear=False): + os.environ.pop(RICH_LOGGING_OPT_IN_ENV_VAR, None) + with ( + patch.object(utils, "reconfigure") as mock_reconfigure, + patch.object(utils, "install") as mock_install, + ): + enable_textual_rich_console_and_traceback() + mock_reconfigure.assert_not_called() + mock_install.assert_not_called() + assert utils.textual_rich_console_and_traceback_enabled is False + + +def test_enabled_when_opted_in_via_env(): + with patch.dict(os.environ, {RICH_LOGGING_OPT_IN_ENV_VAR: "true"}): + with ( + patch.object(utils, "reconfigure") as mock_reconfigure, + patch.object(utils, "install") as mock_install, + ): + enable_textual_rich_console_and_traceback() + mock_reconfigure.assert_called_once() + mock_install.assert_called_once() + assert utils.textual_rich_console_and_traceback_enabled is True + + +def test_force_enables_regardless_of_env(): + with patch.dict(os.environ, {}, clear=False): + os.environ.pop(RICH_LOGGING_OPT_IN_ENV_VAR, None) + with ( + patch.object(utils, "reconfigure") as mock_reconfigure, + patch.object(utils, "install") as mock_install, + ): + enable_textual_rich_console_and_traceback(force=True) + mock_reconfigure.assert_called_once() + mock_install.assert_called_once() + + +def test_enable_is_idempotent_when_opted_in(): + with patch.dict(os.environ, {RICH_LOGGING_OPT_IN_ENV_VAR: "1"}): + with ( + patch.object(utils, "reconfigure") as mock_reconfigure, + patch.object(utils, "install") as mock_install, + ): + enable_textual_rich_console_and_traceback() + enable_textual_rich_console_and_traceback() + # The one-shot latch prevents re-installing on the second call. + mock_reconfigure.assert_called_once() + mock_install.assert_called_once() + + +def test_get_logger_does_not_call_basicconfig_when_opted_out(): + # Getting a module logger must not reconfigure the root logger by default. + with patch.dict(os.environ, {}, clear=False): + os.environ.pop(RICH_LOGGING_OPT_IN_ENV_VAR, None) + with patch.object(utils.logging, "basicConfig") as mock_basic_config: + returned = utils.get_textual_rich_logger("sagemaker.core.test.optout") + mock_basic_config.assert_not_called() + assert returned is utils.logging.getLogger("sagemaker.core.test.optout") + + +def test_get_logger_calls_basicconfig_when_opted_in(): + with patch.dict(os.environ, {RICH_LOGGING_OPT_IN_ENV_VAR: "1"}): + with ( + patch.object(utils, "reconfigure"), + patch.object(utils, "install"), + patch.object(utils.logging, "basicConfig") as mock_basic_config, + ): + utils.get_textual_rich_logger("sagemaker.core.test.optin") + mock_basic_config.assert_called_once()