Changes from omni.log to python logging

[pascal-roth]

Description

Changes from omni.log to an own python logger for IsaacLab. The logging information are formatted as follows:

14:09:39 [manager_based_env.py] WARNING: The render interval (1) is smaller than the decimation (2). Multiple render calls will happen for each environment step. If this is not intended, set the render interval to be equal to the decimation.

All logs are saved to a temp file. Carb initialized a logging handler:

<_CarbLogHandler <stderr> (NOTSET)>

which is removed when configuring our handler.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with ./isaaclab.sh --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have updated the changelog and the corresponding version in the extension's config/extension.toml file
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[greptile-apps]

Greptile Overview

Greptile Summary

This PR migrates IsaacLab from Omniverse's omni.log to Python's standard logging module across 59 files. The changes include:

• New logging infrastructure in simulation_context.py and utils.py with ColoredFormatter for console output and RateLimitFilter to throttle duplicate warnings
• Configuration options added to SimulationCfg: logging_level (default: WARNING) and save_logs_to_file (default: True)
• Consistent migration pattern: All files replace omni.log.warn() → logger.warning(), omni.log.verbose() → logger.debug(), omni.log.error() → logger.error(), omni.log.info() → logger.info()
• Logs saved to temp directory with timestamp, format: isaaclab_YYYY-MM-DD_HH-MM-SS.log

Key issue identified: The _setup_logger() method unconditionally adds handlers to the root logger without checking if they already exist, which could cause duplicate log messages if SimulationContext is re-instantiated or if other libraries also configure the root logger.

Confidence Score: 3/5

• This PR is mostly safe to merge but requires addressing the duplicate handler issue to prevent log message duplication
• The migration from omni.log to Python logging is implemented consistently across all 59 files with proper mappings (verbose→debug, warn→warning). However, the _setup_logger() method has a critical flaw where it unconditionally adds handlers to the root logger without checking for existing handlers. This could cause duplicate log messages if SimulationContext is re-instantiated or if other code also configures the root logger. The core functionality is sound, but this issue should be fixed before merging to avoid production logging problems.
• source/isaaclab/isaaclab/sim/simulation_context.py - Fix duplicate handler issue in _setup_logger()

Important Files Changed

File Analysis

Filename	Score	Overview
source/isaaclab/isaaclab/sim/simulation_context.py	2/5	Adds logger setup with potential duplicate handler issue when SimulationContext is re-instantiated
source/isaaclab/isaaclab/sim/utils.py	4/5	Adds ColoredFormatter and RateLimitFilter classes with consistent logger usage throughout
source/isaaclab/isaaclab/sim/simulation_cfg.py	5/5	Adds logging configuration fields (logging_level and save_logs_to_file) to SimulationCfg

Sequence Diagram

sequenceDiagram
    participant User
    participant SimContext as SimulationContext
    participant Logger as Root Logger
    participant Console as Console Handler
    participant File as File Handler
    participant Module as Module Code

    User->>SimContext: __init__(cfg)
    SimContext->>SimContext: _setup_logger()
    SimContext->>Logger: getLogger() [root]
    SimContext->>Logger: setLevel(cfg.logging_level)
    SimContext->>Console: Create StreamHandler(sys.stdout)
    SimContext->>Console: Add ColoredFormatter
    SimContext->>Console: Add RateLimitFilter(5s)
    SimContext->>Logger: addHandler(Console)
    
    alt save_logs_to_file=True
        SimContext->>File: Create FileHandler(temp_dir)
        SimContext->>File: Add Formatter with line numbers
        SimContext->>Logger: addHandler(File)
    end
    
    SimContext-->>SimContext: return root_logger
    
    Note over Module: During execution
    Module->>Logger: getLogger(__name__)
    Module->>Logger: logger.warning("message")
    Logger->>Console: Check RateLimitFilter
    Console->>Console: Rate limit WARNING messages
    Logger->>Console: Format with ColoredFormatter
    Console->>User: Display colored log
    Logger->>File: Write to temp file (DEBUG level)

Loading

_{59 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic: Handlers are added to root logger without checking if they already exist. If SimulationContext is re-instantiated (or if other code also configures the root logger), duplicate handlers will cause duplicate log messages.

Consider checking for existing handlers before adding:

def _setup_logger(self):
    root_logger = logging.getLogger()
    
    # Only configure if not already configured
    if not root_logger.handlers:
        root_logger.setLevel(self.cfg.logging_level)
        # ... rest of handler setup
    else:
        # Just update the level if already configured
        root_logger.setLevel(self.cfg.logging_level)
    
    return root_logger

[greptile-apps]

Greptile Overview

Greptile Summary

This PR migrates IsaacLab from omni.log to Python's standard logging module, implementing custom formatters and filters for colored output and rate limiting.

Key changes:

• Replaced all omni.log.warn() calls with self.logger.warning() in SimulationContext
• Added _setup_logger() method that configures the root logger with ColoredFormatter and RateLimitFilter
• Implemented optional file logging to temp directory
• Added logging level configuration via SimulationCfg.logging_level

Critical issues found:

1. Missing import: datetime is used on line 1012 but never imported, causing immediate runtime failure when file logging is enabled
2. Handler management: Unconditionally removes ALL handlers from root logger, breaking logging from other libraries and components
3. Resource leak: File handlers are never closed, causing file handle leaks on repeated instantiation
4. Global rate limiting: RateLimitFilter applies to ALL WARNING messages system-wide, potentially hiding critical warnings from third-party libraries
5. Duplicate handler issue: As noted in previous comments, re-instantiating SimulationContext will keep adding handlers despite the removal logic

Confidence Score: 1/5

• This PR has critical bugs that will cause runtime failures and resource leaks
• The missing datetime import on line 1012 will cause an immediate NameError when file logging is enabled (which is the default per simulation_cfg.py:417). Additionally, the unconditional removal of all root logger handlers and the lack of file handler cleanup create serious issues for production use
• source/isaaclab/isaaclab/sim/simulation_context.py requires immediate fixes before merge

Important Files Changed

File Analysis

Filename	Score	Overview
source/isaaclab/isaaclab/sim/simulation_context.py	2/5	Replaces omni.log with Python logging. Critical issues: missing datetime import causes runtime error, unconditional handler removal breaks other logging, file handle leaks, and rate limiting affects all WARNING messages globally

Sequence Diagram

sequenceDiagram
    participant App as Application
    participant SC as SimulationContext
    participant Logger as Root Logger
    participant Handler as StreamHandler/FileHandler
    participant CF as ColoredFormatter
    participant RLF as RateLimitFilter
    participant File as Log File

    App->>SC: __init__()
    SC->>SC: _setup_logger()
    SC->>Logger: logging.getLogger()
    Logger-->>SC: root_logger
    SC->>Logger: setLevel(cfg.logging_level)
    
    Note over SC,Logger: Critical: Removes ALL existing handlers
    SC->>Logger: removeHandler() for each handler
    
    SC->>Handler: Create StreamHandler(sys.stdout)
    SC->>CF: ColoredFormatter()
    SC->>RLF: RateLimitFilter(interval_seconds=5)
    SC->>Handler: setFormatter(ColoredFormatter)
    SC->>Handler: addFilter(RateLimitFilter)
    SC->>Logger: addHandler(StreamHandler)
    
    alt cfg.save_logs_to_file
        Note over SC,File: Missing datetime import causes error here
        SC->>File: Create temp file path
        SC->>Handler: Create FileHandler(log_file_path)
        SC->>Handler: setFormatter(file_formatter)
        SC->>Logger: addHandler(FileHandler)
        Note over Handler,File: File handle never closed (leak)
    end
    
    SC-->>App: logger instance
    
    Note over App,RLF: Runtime: WARNING messages rate limited globally
    App->>Logger: logger.warning("message")
    Logger->>RLF: filter(record)
    RLF-->>Logger: True/False (rate limited)
    alt Not rate limited
        Logger->>Handler: emit(record)
        Handler->>CF: format(record)
        CF-->>Handler: colored message
        Handler->>File: write to stdout/file
    end

Loading

_{1 file reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

syntax: datetime is used but never imported in this file

Suggested change

	log_file_path = os.path.join(temp_dir, f"isaaclab_{datetime.now().strftime('%Y-%m-%d_%H-%M-%S')}.log")
	log_file_path = os.path.join(temp_dir, f"isaaclab_{time.strftime('%Y-%m-%d_%H-%M-%S')}.log")

[greptile-apps]

logic: Removing all handlers on the root logger unconditionally can break logging from other parts of the application (e.g., libraries, other components). A previous comment already flagged similar concerns about duplicate handlers. Instead of removing all handlers, check if IsaacLab handlers already exist before adding new ones

[greptile-apps]

logic: RateLimitFilter applies rate limiting globally to ALL WARNING messages from any module. This means warnings from important third-party libraries will also be rate limited, potentially hiding critical issues

[greptile-apps]

logic: File handler is never closed. If SimulationContext is instantiated multiple times (common in test suites or when resetting environments), file handles will leak. Consider storing the handler in an instance variable and closing it in a cleanup method, or using WatchedFileHandler with proper cleanup