Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion ms_agent/skill/auto_skills.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# yapf: disable
import asyncio
import logging
import os
import re
from dataclasses import dataclass, field
from pathlib import Path
Expand Down Expand Up @@ -39,6 +40,15 @@ def _configure_logger_to_dir(log_dir: Path) -> None:
log_dir.mkdir(parents=True, exist_ok=True)
log_file = log_dir / 'ms_agent.log'

# Get current log level from environment
log_level_str = os.getenv('LOG_LEVEL', 'INFO').upper()
log_level = getattr(logging, log_level_str, logging.INFO)

# Update logger level to respect current LOG_LEVEL env var
logger.setLevel(log_level)
for handler in logger.handlers:
handler.setLevel(log_level)
Comment on lines +44 to +50
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

To avoid code duplication, you can use the new refresh_log_level function that you've added in ms_agent/utils/logger.py. This will make the code more maintainable.

You'll need to import it at the top of the file:

from ms_agent.utils.logger import get_logger, refresh_log_level
Suggested change
log_level_str = os.getenv('LOG_LEVEL', 'INFO').upper()
log_level = getattr(logging, log_level_str, logging.INFO)
# Update logger level to respect current LOG_LEVEL env var
logger.setLevel(log_level)
for handler in logger.handlers:
handler.setLevel(log_level)
log_level = refresh_log_level(logger)


# Check if file handler for this path already exists
for handler in logger.handlers:
if isinstance(handler, logging.FileHandler):
Expand All @@ -52,7 +62,7 @@ def _configure_logger_to_dir(log_dir: Path) -> None:

file_handler = logging.FileHandler(str(log_file), mode='a')
file_handler.setFormatter(logging.Formatter('[%(levelname)s:%(name)s] %(message)s'))
file_handler.setLevel(logger.level)
file_handler.setLevel(log_level)
logger.addHandler(file_handler)
logger.info(f'Logger configured to output to: {log_file}')

Expand Down
29 changes: 29 additions & 0 deletions ms_agent/utils/logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ def get_logger(log_file: Optional[str] = None,
logger = logging.getLogger(logger_name)
logger.propagate = False
if logger_name in init_loggers:
# Update log level dynamically to respect current LOG_LEVEL env var
logger.setLevel(log_level)
for handler in logger.handlers:
handler.setLevel(log_level)
add_file_handler_if_needed(logger, log_file, file_mode, log_level)
return logger

Expand Down Expand Up @@ -124,3 +128,28 @@ def add_file_handler_if_needed(logger, log_file, file_mode, log_level):
file_handler.setFormatter(logger_format)
file_handler.setLevel(log_level)
logger.addHandler(file_handler)


def refresh_log_level(target_logger=None):
"""
Refresh logger level from LOG_LEVEL environment variable.

This is useful when LOG_LEVEL is changed after the logger was initialized.

Args:
target_logger: Logger to refresh. If None, uses the default logger.

Returns:
The new log level (as int).
"""
if target_logger is None:
target_logger = logger

log_level_str = os.getenv('LOG_LEVEL', 'INFO').upper()
log_level_int = getattr(logging, log_level_str, logging.INFO)

target_logger.setLevel(log_level_int)
for handler in target_logger.handlers:
handler.setLevel(log_level_int)
Comment on lines +151 to +153
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This logic to set the level on the logger and its handlers is duplicated in the get_logger function (lines 57-59). To improve maintainability and avoid duplication, consider extracting this block into a private helper function, e.g., _set_level(logger, level), and call it from both get_logger and refresh_log_level.


return log_level_int
Loading