-
Notifications
You must be signed in to change notification settings - Fork 5.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Core] Ray Core / Ray Data logging configuration leads to unexpected behavior #48732
Comments
To be clear about the expected behavior some examples / modifications of the repro: 1. Ray Data Import then Ray Core Init [UNEXPECTED BEHAVIOR]Script: print("BEFORE")
report_logger(logging.getLogger("ray.data"))
print()
import ray.data
ray.init(logging_config=ray.LoggingConfig(encoding="JSON", log_level="INFO"))
print("AFTER:")
report_logger(logging.getLogger("ray.data")) Output:
Notes: 2. Ray Core Init then Ray Data Import [EXPECTED BEHAVIOR]Script: print("BEFORE")
report_logger(logging.getLogger("ray.data"))
print()
ray.init(logging_config=ray.LoggingConfig(encoding="JSON", log_level="INFO"))
import ray.data
print("AFTER:")
report_logger(logging.getLogger("ray.data")) Output:
Notes: 3. Only Ray Data Import [EXPECTED BEHAVIOR]Script: print("BEFORE")
report_logger(logging.getLogger("ray.data"))
print()
import ray.data
print("AFTER:")
report_logger(logging.getLogger("ray.data")) Output:
Notes: 4. Only Ray Core Init [EXPECTED BEHAVIOR]Script: print("BEFORE")
report_logger(logging.getLogger("ray.data"))
print()
ray.init(logging_config=ray.LoggingConfig(encoding="JSON", log_level="INFO"))
print("AFTER:")
report_logger(logging.getLogger("ray.data")) Output:
Notes: |
@omatthew98 why don't we just do
|
Update from some more offline discussion with @alexeykudinkin: An alternative to this would be to just wrap this configuration at the ray core level by:
|
I am trying to understand the structured logging behavior as well. One of my question is |
Yeah I think that understanding is correct. From what I have read, configuring the parent logger after the child logger (so the order of #1), the result will be only the parent logger is configured. If you configure the child logger after the parent logger (so the order of #2), then the two loggers will be configured as expected. I think this is is just an implementation detail for python's logging module that doesn't seem particularly well documented. |
…behavior (#48958) ### Issue In the Ray codebase, [logging.config.dictConfig](https://docs.python.org/3/library/logging.config.html#logging.config.dictConfig) may be called multiple times. However, we found that if a logger’s child loggers are set before the logger is set via `dictConfig`, it may cause issues. * [Example1](https://gist.github.com/kevin85421/24849e06c61f221fd95063a4ce81ca8f) (incremental: False): The logger `Ray.data` loses its original handler and uses the `Ray` logger’s handler after the Ray logger is set via `dictConfig`. ``` 2024-11-27 04:32:06,213 - Ray.data - INFO - This is an INFO log from Ray.data. 2024-11-27 04:32:06,213 - Ray.data - WARNING - This is a WARNING log from Ray.data. 2024-11-27 04:32:06,213 - Ray.data - INFO - Ray data propagate False abc Ray - DEBUG - This is a DEBUG log from Ray. abc Ray - ERROR - This is an ERROR log from Ray. abc Ray.data - INFO - Another INFO log from Ray.data. abc Ray.data - INFO - Ray data propagate True ``` * [Example2](https://gist.github.com/kevin85421/9cf6ee70ceec42be3de888174d0c8e6a) (incremental: True): It looks like `Ray.data`’s handlers are removed after the `Ray` logger is set via `dictConfig`. ``` 2024-11-27 04:35:25,379 - Ray.data - INFO - This is an INFO log from Ray.data. 2024-11-27 04:35:25,379 - Ray.data - WARNING - This is a WARNING log from Ray.data. 2024-11-27 04:35:25,379 - Ray.data - INFO - Ray data propagate False This is an ERROR log from Ray. 2024-11-27 04:35:25,379 - Ray.data - INFO - Another INFO log from Ray.data. 2024-11-27 04:35:25,379 - Ray.data - INFO - Ray data propagate False ``` * CPython implementation * Case 1: `incremental` is `False` * If an existing logger is also a child logger of a new logger, the child logger’s handlers will be reset, and its `propagate` attribute will be set to true. * In [Example1](https://gist.github.com/kevin85421/24849e06c61f221fd95063a4ce81ca8f), `Ray.data` is not only an existing logger but also a child logger of Ray. Therefore, its handlers will be reset, and propagate will be set to true. * See the function for more details: https://github.com/python/cpython/blob/71ede1142ddad2d31cc966b8fe4a5aff664f4d53/Lib/logging/config.py#L193-L196 * Case 2: `incremental` is `True` * No handlers & filters will be added to the new logger. * See the function for more details: https://github.com/python/cpython/blob/71ede1142ddad2d31cc966b8fe4a5aff664f4d53/Lib/logging/config.py#L906-L915 ### Solution Instead of using `dictConfig` to set the root logger and the Ray logger, call other functions to set the loggers explicitly. ## Related issue number Closes #48732 <!-- For example: "Closes #1234" --> ## Checks * Test 1 ```python import ray import logging import ray.data ray.init(logging_config=ray.LoggingConfig(encoding="TEXT", log_level="INFO")) root_logger = logging.getLogger() root_logger.info("root logger") ray_logger = logging.getLogger("ray") ray_logger.info("ray logger") ray_data_logger = logging.getLogger("ray.data") ray_data_logger.info("ray data logger") @ray.remote def f(): root_logger = logging.getLogger() root_logger.info("root logger") ray_data_logger = logging.getLogger("ray.data") ray_data_logger.info("ray data logger") ray.get(f.remote()) ``` <img width="1440" alt="image" src="https://github.com/user-attachments/assets/e522a257-28c5-4b3c-ad62-c41e4cd61664"> * Test 2 ```python import ray import logging def report_logger(logger): # Collect this logger and its parents loggers = [] current_logger = logger while current_logger: loggers.append(current_logger) if not current_logger.parent or current_logger.parent == current_logger: break current_logger = current_logger.parent # Report the configuration of each logger in the hierarchy print(f"Logging configuration for '{logger.name}' and its hierarchy:") for log in reversed(loggers): # Start from the root and go down to the given logger print(f"\nLogger: {log.name or 'root'} (Level: {logging.getLevelName(log.level)})") if log.handlers: print(" Handlers:") for handler in log.handlers: print(f" - {handler.__class__.__name__} (Level: {logging.getLevelName(handler.level)})") else: print(" No handlers configured") print("BEFORE") report_logger(logging.getLogger("ray.data")) print() import ray.data ray.init(logging_config=ray.LoggingConfig(encoding="TEXT", log_level="INFO")) print("AFTER:") report_logger(logging.getLogger("ray.data")) ``` <img width="1189" alt="image" src="https://github.com/user-attachments/assets/9129b22a-f436-40ca-9f42-f1ecacf6c515"> Signed-off-by: kaihsun <kaihsun@anyscale.com> Signed-off-by: Kai-Hsun Chen <kaihsun@apache.org> Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com>
…behavior (ray-project#48958) ### Issue In the Ray codebase, [logging.config.dictConfig](https://docs.python.org/3/library/logging.config.html#logging.config.dictConfig) may be called multiple times. However, we found that if a logger’s child loggers are set before the logger is set via `dictConfig`, it may cause issues. * [Example1](https://gist.github.com/kevin85421/24849e06c61f221fd95063a4ce81ca8f) (incremental: False): The logger `Ray.data` loses its original handler and uses the `Ray` logger’s handler after the Ray logger is set via `dictConfig`. ``` 2024-11-27 04:32:06,213 - Ray.data - INFO - This is an INFO log from Ray.data. 2024-11-27 04:32:06,213 - Ray.data - WARNING - This is a WARNING log from Ray.data. 2024-11-27 04:32:06,213 - Ray.data - INFO - Ray data propagate False abc Ray - DEBUG - This is a DEBUG log from Ray. abc Ray - ERROR - This is an ERROR log from Ray. abc Ray.data - INFO - Another INFO log from Ray.data. abc Ray.data - INFO - Ray data propagate True ``` * [Example2](https://gist.github.com/kevin85421/9cf6ee70ceec42be3de888174d0c8e6a) (incremental: True): It looks like `Ray.data`’s handlers are removed after the `Ray` logger is set via `dictConfig`. ``` 2024-11-27 04:35:25,379 - Ray.data - INFO - This is an INFO log from Ray.data. 2024-11-27 04:35:25,379 - Ray.data - WARNING - This is a WARNING log from Ray.data. 2024-11-27 04:35:25,379 - Ray.data - INFO - Ray data propagate False This is an ERROR log from Ray. 2024-11-27 04:35:25,379 - Ray.data - INFO - Another INFO log from Ray.data. 2024-11-27 04:35:25,379 - Ray.data - INFO - Ray data propagate False ``` * CPython implementation * Case 1: `incremental` is `False` * If an existing logger is also a child logger of a new logger, the child logger’s handlers will be reset, and its `propagate` attribute will be set to true. * In [Example1](https://gist.github.com/kevin85421/24849e06c61f221fd95063a4ce81ca8f), `Ray.data` is not only an existing logger but also a child logger of Ray. Therefore, its handlers will be reset, and propagate will be set to true. * See the function for more details: https://github.com/python/cpython/blob/71ede1142ddad2d31cc966b8fe4a5aff664f4d53/Lib/logging/config.py#L193-L196 * Case 2: `incremental` is `True` * No handlers & filters will be added to the new logger. * See the function for more details: https://github.com/python/cpython/blob/71ede1142ddad2d31cc966b8fe4a5aff664f4d53/Lib/logging/config.py#L906-L915 ### Solution Instead of using `dictConfig` to set the root logger and the Ray logger, call other functions to set the loggers explicitly. ## Related issue number Closes ray-project#48732 <!-- For example: "Closes ray-project#1234" --> ## Checks * Test 1 ```python import ray import logging import ray.data ray.init(logging_config=ray.LoggingConfig(encoding="TEXT", log_level="INFO")) root_logger = logging.getLogger() root_logger.info("root logger") ray_logger = logging.getLogger("ray") ray_logger.info("ray logger") ray_data_logger = logging.getLogger("ray.data") ray_data_logger.info("ray data logger") @ray.remote def f(): root_logger = logging.getLogger() root_logger.info("root logger") ray_data_logger = logging.getLogger("ray.data") ray_data_logger.info("ray data logger") ray.get(f.remote()) ``` <img width="1440" alt="image" src="https://github.com/user-attachments/assets/e522a257-28c5-4b3c-ad62-c41e4cd61664"> * Test 2 ```python import ray import logging def report_logger(logger): # Collect this logger and its parents loggers = [] current_logger = logger while current_logger: loggers.append(current_logger) if not current_logger.parent or current_logger.parent == current_logger: break current_logger = current_logger.parent # Report the configuration of each logger in the hierarchy print(f"Logging configuration for '{logger.name}' and its hierarchy:") for log in reversed(loggers): # Start from the root and go down to the given logger print(f"\nLogger: {log.name or 'root'} (Level: {logging.getLevelName(log.level)})") if log.handlers: print(" Handlers:") for handler in log.handlers: print(f" - {handler.__class__.__name__} (Level: {logging.getLevelName(handler.level)})") else: print(" No handlers configured") print("BEFORE") report_logger(logging.getLogger("ray.data")) print() import ray.data ray.init(logging_config=ray.LoggingConfig(encoding="TEXT", log_level="INFO")) print("AFTER:") report_logger(logging.getLogger("ray.data")) ``` <img width="1189" alt="image" src="https://github.com/user-attachments/assets/9129b22a-f436-40ca-9f42-f1ecacf6c515"> Signed-off-by: kaihsun <kaihsun@anyscale.com> Signed-off-by: Kai-Hsun Chen <kaihsun@apache.org> Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com> Signed-off-by: Connor Sanders <connor@elastiflow.com>
…behavior (ray-project#48958) ### Issue In the Ray codebase, [logging.config.dictConfig](https://docs.python.org/3/library/logging.config.html#logging.config.dictConfig) may be called multiple times. However, we found that if a logger’s child loggers are set before the logger is set via `dictConfig`, it may cause issues. * [Example1](https://gist.github.com/kevin85421/24849e06c61f221fd95063a4ce81ca8f) (incremental: False): The logger `Ray.data` loses its original handler and uses the `Ray` logger’s handler after the Ray logger is set via `dictConfig`. ``` 2024-11-27 04:32:06,213 - Ray.data - INFO - This is an INFO log from Ray.data. 2024-11-27 04:32:06,213 - Ray.data - WARNING - This is a WARNING log from Ray.data. 2024-11-27 04:32:06,213 - Ray.data - INFO - Ray data propagate False abc Ray - DEBUG - This is a DEBUG log from Ray. abc Ray - ERROR - This is an ERROR log from Ray. abc Ray.data - INFO - Another INFO log from Ray.data. abc Ray.data - INFO - Ray data propagate True ``` * [Example2](https://gist.github.com/kevin85421/9cf6ee70ceec42be3de888174d0c8e6a) (incremental: True): It looks like `Ray.data`’s handlers are removed after the `Ray` logger is set via `dictConfig`. ``` 2024-11-27 04:35:25,379 - Ray.data - INFO - This is an INFO log from Ray.data. 2024-11-27 04:35:25,379 - Ray.data - WARNING - This is a WARNING log from Ray.data. 2024-11-27 04:35:25,379 - Ray.data - INFO - Ray data propagate False This is an ERROR log from Ray. 2024-11-27 04:35:25,379 - Ray.data - INFO - Another INFO log from Ray.data. 2024-11-27 04:35:25,379 - Ray.data - INFO - Ray data propagate False ``` * CPython implementation * Case 1: `incremental` is `False` * If an existing logger is also a child logger of a new logger, the child logger’s handlers will be reset, and its `propagate` attribute will be set to true. * In [Example1](https://gist.github.com/kevin85421/24849e06c61f221fd95063a4ce81ca8f), `Ray.data` is not only an existing logger but also a child logger of Ray. Therefore, its handlers will be reset, and propagate will be set to true. * See the function for more details: https://github.com/python/cpython/blob/71ede1142ddad2d31cc966b8fe4a5aff664f4d53/Lib/logging/config.py#L193-L196 * Case 2: `incremental` is `True` * No handlers & filters will be added to the new logger. * See the function for more details: https://github.com/python/cpython/blob/71ede1142ddad2d31cc966b8fe4a5aff664f4d53/Lib/logging/config.py#L906-L915 ### Solution Instead of using `dictConfig` to set the root logger and the Ray logger, call other functions to set the loggers explicitly. ## Related issue number Closes ray-project#48732 <!-- For example: "Closes ray-project#1234" --> ## Checks * Test 1 ```python import ray import logging import ray.data ray.init(logging_config=ray.LoggingConfig(encoding="TEXT", log_level="INFO")) root_logger = logging.getLogger() root_logger.info("root logger") ray_logger = logging.getLogger("ray") ray_logger.info("ray logger") ray_data_logger = logging.getLogger("ray.data") ray_data_logger.info("ray data logger") @ray.remote def f(): root_logger = logging.getLogger() root_logger.info("root logger") ray_data_logger = logging.getLogger("ray.data") ray_data_logger.info("ray data logger") ray.get(f.remote()) ``` <img width="1440" alt="image" src="https://github.com/user-attachments/assets/e522a257-28c5-4b3c-ad62-c41e4cd61664"> * Test 2 ```python import ray import logging def report_logger(logger): # Collect this logger and its parents loggers = [] current_logger = logger while current_logger: loggers.append(current_logger) if not current_logger.parent or current_logger.parent == current_logger: break current_logger = current_logger.parent # Report the configuration of each logger in the hierarchy print(f"Logging configuration for '{logger.name}' and its hierarchy:") for log in reversed(loggers): # Start from the root and go down to the given logger print(f"\nLogger: {log.name or 'root'} (Level: {logging.getLevelName(log.level)})") if log.handlers: print(" Handlers:") for handler in log.handlers: print(f" - {handler.__class__.__name__} (Level: {logging.getLevelName(handler.level)})") else: print(" No handlers configured") print("BEFORE") report_logger(logging.getLogger("ray.data")) print() import ray.data ray.init(logging_config=ray.LoggingConfig(encoding="TEXT", log_level="INFO")) print("AFTER:") report_logger(logging.getLogger("ray.data")) ``` <img width="1189" alt="image" src="https://github.com/user-attachments/assets/9129b22a-f436-40ca-9f42-f1ecacf6c515"> Signed-off-by: kaihsun <kaihsun@anyscale.com> Signed-off-by: Kai-Hsun Chen <kaihsun@apache.org> Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com> Signed-off-by: hjiang <dentinyhao@gmail.com>
What happened + What you expected to happen
We use
logging.config.dictConfig(config)
to configure the ray data logger (here), but this is also how ray core configures the ray logger (here).For both of these logging configs, we use
disable_existing_loggers: False
. The behavior for this is described as (logging docs):This description makes it seem like these two logging calls are commutative (regardless of ordering they will produce the same result), but that is not exactly how the python logging module works. If we configure the ray module logger then the ray.data module logger, the results are expected and both are configured. If we instead configure the ray.data module then configure the ray module logger, then the ray.data logging configuration is clobbered. This happens because when configuring the parent logger of a module (e.g. ray module logger is the parent logger of the ray.data module logger), the various handlers associated with the child logger are not guaranteed to be preserved.
Our end goal should be a state where the call order of the logging configurations should not affect the logging behavior.
Versions / Dependencies
ray==2.39.0
Reproduction script
Issue Severity
High: It blocks me from completing my task.
The text was updated successfully, but these errors were encountered: