From 85d98e1dd6d16b6ee82a39d28ca2dcfdf11d2d1b Mon Sep 17 00:00:00 2001 From: Gene Der Su Date: Wed, 11 Sep 2024 12:06:40 -0700 Subject: [PATCH] cherrypick #47600 for serve logger fix (#47613) otherwise `ray start` might exit immediately with no error output. --- python/ray/serve/_private/logging_utils.py | 18 ++++++++++---- python/ray/serve/tests/test_logging.py | 28 ++++++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/python/ray/serve/_private/logging_utils.py b/python/ray/serve/_private/logging_utils.py index 010428410636..d85329ded166 100644 --- a/python/ray/serve/_private/logging_utils.py +++ b/python/ray/serve/_private/logging_utils.py @@ -279,6 +279,7 @@ def configure_component_logger( component_type: Optional[ServeComponentType] = None, max_bytes: Optional[int] = None, backup_count: Optional[int] = None, + stream_handler_only: bool = False, ): """Configure a logger to be used by a Serve component. @@ -292,14 +293,22 @@ def configure_component_logger( logger.setLevel(logging_config.log_level) logger.handlers.clear() - # Only add stream handler if RAY_SERVE_LOG_TO_STDERR is True. - if RAY_SERVE_LOG_TO_STDERR: + # Only add stream handler if RAY_SERVE_LOG_TO_STDERR is True or if + # `stream_handler_only` is set to True. + if RAY_SERVE_LOG_TO_STDERR or stream_handler_only: stream_handler = logging.StreamHandler() stream_handler.setFormatter(ServeFormatter(component_name, component_id)) stream_handler.addFilter(log_to_stderr_filter) stream_handler.addFilter(ServeContextFilter()) logger.addHandler(stream_handler) + # Skip setting up file handler and stdout/stderr redirect if `stream_handler_only` + # is set to True. Logger such as default serve logger can be configured outside the + # context of a Serve component, we don't want those logs to redirect into serve's + # logger and log files. + if stream_handler_only: + return + if logging_config.logs_dir: logs_dir = logging_config.logs_dir else: @@ -344,8 +353,8 @@ def configure_component_logger( # Remove unwanted attributes from the log record. file_handler.addFilter(ServeLogAttributeRemovalFilter()) - # Redirect print, stdout, and stderr to Serve logger. - if not RAY_SERVE_LOG_TO_STDERR: + # Redirect print, stdout, and stderr to Serve logger, only when it's on the replica. + if not RAY_SERVE_LOG_TO_STDERR and component_type == ServeComponentType.REPLICA: builtins.print = redirected_print sys.stdout = StreamToLogger(logger, logging.INFO, sys.stdout) sys.stderr = StreamToLogger(logger, logging.INFO, sys.stderr) @@ -362,6 +371,7 @@ def configure_default_serve_logger(): logging_config=LoggingConfig(), max_bytes=LOGGING_ROTATE_BYTES, backup_count=LOGGING_ROTATE_BACKUP_COUNT, + stream_handler_only=True, ) diff --git a/python/ray/serve/tests/test_logging.py b/python/ray/serve/tests/test_logging.py index 31b66685d388..6bb3b20f8571 100644 --- a/python/ray/serve/tests/test_logging.py +++ b/python/ray/serve/tests/test_logging.py @@ -27,7 +27,9 @@ ServeFormatter, StreamToLogger, configure_component_logger, + configure_default_serve_logger, get_serve_logs_dir, + redirected_print, ) from ray.serve._private.utils import get_component_file_name from ray.serve.context import _get_global_client @@ -871,5 +873,31 @@ def __call__(self): assert "cannot pickle" not in f.read() +@pytest.mark.skipif(sys.platform == "win32", reason="Fail to create temp dir.") +@pytest.mark.parametrize( + "ray_instance", + [ + {"RAY_SERVE_LOG_TO_STDERR": "0"}, + ], + indirect=True, +) +def test_configure_default_serve_logger_with_stderr_redirect( + serve_and_ray_shutdown, ray_instance, tmp_dir +): + """Test configuring default serve logger with stderr redirect. + + Default serve logger should only be configured with one StreamToLogger handler, and + print, stdout, and stderr should NOT be overridden and redirected to the logger. + """ + + configure_default_serve_logger() + serve_logger = logging.getLogger("ray.serve") + assert len(serve_logger.handlers) == 1 + assert isinstance(serve_logger.handlers[0], logging.StreamHandler) + assert print != redirected_print + assert not isinstance(sys.stdout, StreamToLogger) + assert not isinstance(sys.stderr, StreamToLogger) + + if __name__ == "__main__": sys.exit(pytest.main(["-v", "-s", __file__]))