Skip to content
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

FEAT-#4622: Unify data type of log_level in logging module #6992

Merged
merged 2 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions modin/logging/class_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

from typing import Dict, Optional

from .config import LogLevel
from .logger_decorator import enable_logging


Expand All @@ -38,7 +39,7 @@ def __init_subclass__(
cls,
modin_layer: Optional[str] = None,
class_name: Optional[str] = None,
log_level: str = "info",
log_level: LogLevel = LogLevel.INFO,
**kwargs: Dict,
) -> None:
"""
Expand All @@ -51,8 +52,8 @@ def __init_subclass__(
class_name : str, optional
The name of the class the decorator is being applied to.
Composed from the decorated class name if not specified.
log_level : str, default: "info"
The log level (INFO, DEBUG, WARNING, etc.).
log_level : LogLevel, default: LogLevel.INFO
The log level (LogLevel.INFO, LogLevel.DEBUG, LogLevel.WARNING, etc.).
**kwargs : dict
"""
modin_layer = modin_layer or cls._modin_logging_layer
Expand Down
22 changes: 16 additions & 6 deletions modin/logging/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import threading
import time
import uuid
from enum import IntEnum
from logging.handlers import RotatingFileHandler
from pathlib import Path
from typing import Optional
Expand All @@ -36,6 +37,16 @@
__LOGGER_CONFIGURED__: bool = False


class LogLevel(IntEnum): # noqa: PR01
"""Enumerator to specify the valid values of LogLevel accepted by Logger.setLevel()."""

DEBUG = 10
INFO = 20
WARNING = 30
ERROR = 40
CRITICAL = 50


class ModinFormatter(logging.Formatter): # noqa: PR01
"""Implement custom formatter to log at microsecond granularity."""

Expand Down Expand Up @@ -97,7 +108,7 @@ def bytes_int_to_str(num_bytes: int, suffix: str = "B") -> str:


def _create_logger(
namespace: str, job_id: str, log_name: str, log_level: int
namespace: str, job_id: str, log_name: str, log_level: LogLevel
) -> logging.Logger:
"""
Create and configure logger as Modin expects it to be.
Expand All @@ -110,8 +121,7 @@ def _create_logger(
Part of path to where logs are stored.
log_name : str
Name of the log file to create.
log_level : int
Log level as accepted by `Logger.setLevel()`.
log_level : LogLevel

Returns
-------
Expand Down Expand Up @@ -159,7 +169,7 @@ def configure_logging() -> None:
"modin.logger.default",
job_id,
"trace",
logging.INFO if LogMode.get() == "enable_api_only" else logging.DEBUG,
LogLevel.INFO if LogMode.get() == "enable_api_only" else LogLevel.DEBUG,
)

logger.info(f"OS Version: {platform.platform()}")
Expand All @@ -174,7 +184,7 @@ def configure_logging() -> None:
if LogMode.get() != "enable_api_only":
mem_sleep = LogMemoryInterval.get()
mem_logger = _create_logger(
"modin_memory.logger", job_id, "memory", logging.DEBUG
"modin_memory.logger", job_id, "memory", LogLevel.DEBUG
)

svmem = psutil.virtual_memory()
Expand All @@ -186,7 +196,7 @@ def configure_logging() -> None:
)
mem.start()

_create_logger("modin.logger.errors", job_id, "error", logging.INFO)
_create_logger("modin.logger.errors", job_id, "error", LogLevel.INFO)

__LOGGER_CONFIGURED__ = True

Expand Down
17 changes: 6 additions & 11 deletions modin/logging/logger_decorator.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,12 @@
"""

from functools import wraps
from logging import Logger
from types import FunctionType, MethodType
from typing import Any, Callable, Dict, Optional, Tuple, Type, Union

from modin.config import LogMode

from .config import get_logger
from .config import LogLevel, get_logger

_MODIN_LOGGER_NOWRAP = "__modin_logging_nowrap__"

Expand All @@ -50,7 +49,7 @@ def disable_logging(func: Callable) -> Callable:
def enable_logging(
modin_layer: Union[str, Callable, Type] = "PANDAS-API",
name: Optional[str] = None,
log_level: str = "info",
log_level: LogLevel = LogLevel.INFO,
) -> Callable:
"""
Log Decorator used on specific Modin functions or classes.
Expand All @@ -63,8 +62,8 @@ def enable_logging(
name : str, optional
The name of the object the decorator is being applied to.
Composed from the decorated object name if not specified.
log_level : str, default: "info"
The log level (INFO, DEBUG, WARNING, etc.).
log_level : LogLevel, default: LogLevel.INFO
The log level (LogLevel.INFO, LogLevel.DEBUG, LogLevel.WARNING, etc.).

Returns
-------
Expand All @@ -77,9 +76,6 @@ def enable_logging(
# def func()
return enable_logging()(modin_layer)

log_level = log_level.lower()
assert hasattr(Logger, log_level.lower()), f"Invalid log level: {log_level}"

def decorator(obj: Any) -> Any:
"""Decorate function or class to add logs to Modin API function(s)."""
if isinstance(obj, type):
Expand Down Expand Up @@ -129,8 +125,7 @@ def run_and_log(*args: Tuple, **kwargs: Dict) -> Any:
return obj(*args, **kwargs)

logger = get_logger()
logger_level = getattr(logger, log_level)
logger_level(start_line)
logger.log(log_level, start_line)
try:
result = obj(*args, **kwargs)
except BaseException as e:
Expand All @@ -146,7 +141,7 @@ def run_and_log(*args: Tuple, **kwargs: Dict) -> Any:
e._modin_logged = True # type: ignore[attr-defined]
raise
finally:
logger_level(stop_line)
logger.log(log_level, stop_line)
return result

# make sure we won't decorate multiple times
Expand Down
12 changes: 6 additions & 6 deletions modin/test/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ def __init__(self, namespace):
self.messages = collections.defaultdict(list)
self.namespace = namespace

def info(self, message, *args, **kw):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In previous impl, as log_level was a string the logger was called as below,

logger_level = getattr(logger, log_level)
logger_level(start_line)

now as log level is not a string(but int) this has been changed to call
logger.log(log_level,message)

self.messages["info"].append(message.format(*args, **kw))
def log(self, log_level, message, *args, **kw):
self.messages[log_level].append(message.format(*args, **kw))

def exception(self, message, *args, **kw):
self.messages["exception"].append(message.format(*args, **kw))
Expand Down Expand Up @@ -81,8 +81,8 @@ def func(do_raise):
with pytest.raises(ValueError):
func(do_raise=True)

assert "func" in get_log_messages()["info"][0]
assert "START" in get_log_messages()["info"][0]
assert "func" in get_log_messages()[logging.INFO][0]
assert "START" in get_log_messages()[logging.INFO][0]
assert get_log_messages("modin.logger.errors")["exception"] == [
"STOP::PANDAS-API::func"
]
Expand Down Expand Up @@ -137,7 +137,7 @@ def method4(self):
Bar().method1()
Bar().method4()

assert get_log_messages()["info"] == [
assert get_log_messages()[logging.INFO] == [
"START::CUSTOM::Foo.method1",
"STOP::CUSTOM::Foo.method1",
"START::CUSTOM::Foo.method2",
Expand All @@ -164,7 +164,7 @@ def method2(self):
Bar().method1()
Bar().method2()

assert get_log_messages()["info"] == [
assert get_log_messages()[logging.INFO] == [
"START::CUSTOM::Foo.method1",
"STOP::CUSTOM::Foo.method1",
"START::CUSTOM::Foo.method1",
Expand Down