feat: Connect DYN_LOG level to TLLM_LOG_LEVEL #3451
Conversation
WalkthroughIntroduces pre-import configuration to set TLLM_LOG_LEVEL from DYN_LOG before importing TensorRT-LLM. Adds mapping and configuration functions to the logging module, integrates TensorRT-LLM logging setup into existing configure_dynamo_logging, and respects overrides via existing environment variables or a skip flag. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Env as Process Env
participant Main as dynamo/trtllm/main.py
participant L as logging.py
participant TRT as tensorrt_llm
Note over Main: Module import start
Main->>Env: Check TLLM_LOG_LEVEL
alt Not set
Main->>L: map_dyn_log_to_tllm_level(DYN_LOG)
L-->>Main: TLLM level string
Main->>Env: Set TLLM_LOG_LEVEL
else Already set
Note over Main: Preserve existing value
end
Main->>TRT: import tensorrt_llm...
Note over TRT: Initializes with established log level
sequenceDiagram
autonumber
participant App as Application
participant Log as configure_dynamo_logging
participant L as logging.py
participant Env as Process Env
App->>Log: configure_dynamo_logging(...)
alt DYN_SKIP_TRTLLM_LOG_FORMATTING not set
Log->>L: configure_trtllm_logging(dyn_level)
L->>Env: If TLLM_LOG_LEVEL unset, set from map_dyn_log_to_tllm_level(DYN_LOG)
else Skip flag set
Note over Log,Env: Do not modify TLLM_LOG_LEVEL
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/src/dynamo/trtllm/main.py(1 hunks)lib/bindings/python/src/dynamo/runtime/logging.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/src/dynamo/trtllm/main.py (1)
lib/bindings/python/src/dynamo/runtime/logging.py (1)
map_dyn_log_to_tllm_level(198-222)
🪛 Ruff (0.13.3)
lib/bindings/python/src/dynamo/runtime/logging.py
225-225: Unused function argument: dyn_level
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: trtllm (amd64)
- GitHub Check: Build and Test - dynamo
| # Configure TLLM_LOG_LEVEL before importing tensorrt_llm | ||
| # This must happen before any tensorrt_llm imports | ||
| if "TLLM_LOG_LEVEL" not in os.environ: | ||
| # This import is safe because it doesn't trigger tensorrt_llm imports | ||
| from dynamo.runtime.logging import map_dyn_log_to_tllm_level | ||
|
|
||
| dyn_log = os.environ.get("DYN_LOG", "info") | ||
| tllm_level = map_dyn_log_to_tllm_level(dyn_log) | ||
| os.environ["TLLM_LOG_LEVEL"] = tllm_level | ||
|
|
There was a problem hiding this comment.
Respect DYN_SKIP_TRTLLM_LOG_FORMATTING before touching TLLM_LOG_LEVEL.
This pre-import block sets TLLM_LOG_LEVEL even when callers explicitly opt out via DYN_SKIP_TRTLLM_LOG_FORMATTING, making the skip flag ineffective. Gate the block on the same flag so users can truly bypass Dynamo’s TRT-LLM logging setup.
-# Configure TLLM_LOG_LEVEL before importing tensorrt_llm
-# This must happen before any tensorrt_llm imports
-if "TLLM_LOG_LEVEL" not in os.environ:
+# Configure TLLM_LOG_LEVEL before importing tensorrt_llm
+# This must happen before any tensorrt_llm imports
+if (
+ "TLLM_LOG_LEVEL" not in os.environ
+ and os.getenv("DYN_SKIP_TRTLLM_LOG_FORMATTING") not in ("1", "true", "TRUE")
+):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Configure TLLM_LOG_LEVEL before importing tensorrt_llm | |
| # This must happen before any tensorrt_llm imports | |
| if "TLLM_LOG_LEVEL" not in os.environ: | |
| # This import is safe because it doesn't trigger tensorrt_llm imports | |
| from dynamo.runtime.logging import map_dyn_log_to_tllm_level | |
| dyn_log = os.environ.get("DYN_LOG", "info") | |
| tllm_level = map_dyn_log_to_tllm_level(dyn_log) | |
| os.environ["TLLM_LOG_LEVEL"] = tllm_level | |
| # Configure TLLM_LOG_LEVEL before importing tensorrt_llm | |
| # This must happen before any tensorrt_llm imports | |
| if ( | |
| "TLLM_LOG_LEVEL" not in os.environ | |
| and os.getenv("DYN_SKIP_TRTLLM_LOG_FORMATTING") not in ("1", "true", "TRUE") | |
| ): | |
| # This import is safe because it doesn't trigger tensorrt_llm imports | |
| from dynamo.runtime.logging import map_dyn_log_to_tllm_level | |
| dyn_log = os.environ.get("DYN_LOG", "info") | |
| tllm_level = map_dyn_log_to_tllm_level(dyn_log) | |
| os.environ["TLLM_LOG_LEVEL"] = tllm_level |
🤖 Prompt for AI Agents
In components/src/dynamo/trtllm/main.py around lines 11 to 20, the pre-import
block sets TLLM_LOG_LEVEL even when callers set DYN_SKIP_TRTLLM_LOG_FORMATTING;
update the guard so the block runs only when DYN_SKIP_TRTLLM_LOG_FORMATTING is
not set and TLLM_LOG_LEVEL is also unset: check os.environ for
DYN_SKIP_TRTLLM_LOG_FORMATTING first and return/skip if present, otherwise
proceed to import map_dyn_log_to_tllm_level, compute tllm_level from DYN_LOG and
set os.environ["TLLM_LOG_LEVEL"] as before.
| # Only set TLLM_LOG_LEVEL if it's not already set | ||
| # This allows users to override it explicitly if needed | ||
| if "TLLM_LOG_LEVEL" not in os.environ: | ||
| dyn_log = os.environ.get("DYN_LOG", "info") | ||
| tllm_level = map_dyn_log_to_tllm_level(dyn_log) | ||
| os.environ["TLLM_LOG_LEVEL"] = tllm_level | ||
| logging.debug(f"Set TLLM_LOG_LEVEL to {tllm_level} based on DYN_LOG") |
There was a problem hiding this comment.
Use the dyn_level argument (fixes Ruff ARG001).
configure_trtllm_logging ignores its dyn_level parameter and re-reads DYN_LOG, which triggers Ruff ARG001 (unused argument) and will fail CI. Derive the TensorRT-LLM level from the provided dyn_level instead of reading the env again.
def configure_trtllm_logging(dyn_level: int):
@@
- if "TLLM_LOG_LEVEL" not in os.environ:
- dyn_log = os.environ.get("DYN_LOG", "info")
- tllm_level = map_dyn_log_to_tllm_level(dyn_log)
+ if "TLLM_LOG_LEVEL" not in os.environ:
+ dyn_level_name = logging.getLevelName(dyn_level)
+ tllm_level = map_dyn_log_to_tllm_level(dyn_level_name)
os.environ["TLLM_LOG_LEVEL"] = tllm_level
logging.debug(f"Set TLLM_LOG_LEVEL to {tllm_level} based on DYN_LOG")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Only set TLLM_LOG_LEVEL if it's not already set | |
| # This allows users to override it explicitly if needed | |
| if "TLLM_LOG_LEVEL" not in os.environ: | |
| dyn_log = os.environ.get("DYN_LOG", "info") | |
| tllm_level = map_dyn_log_to_tllm_level(dyn_log) | |
| os.environ["TLLM_LOG_LEVEL"] = tllm_level | |
| logging.debug(f"Set TLLM_LOG_LEVEL to {tllm_level} based on DYN_LOG") | |
| def configure_trtllm_logging(dyn_level: int): | |
| # Only set TLLM_LOG_LEVEL if it's not already set | |
| # This allows users to override it explicitly if needed | |
| if "TLLM_LOG_LEVEL" not in os.environ: | |
| dyn_level_name = logging.getLevelName(dyn_level) | |
| tllm_level = map_dyn_log_to_tllm_level(dyn_level_name) | |
| os.environ["TLLM_LOG_LEVEL"] = tllm_level | |
| logging.debug(f"Set TLLM_LOG_LEVEL to {tllm_level} based on DYN_LOG") |
Overview:
This fixes the logging within TRTLLM engine. The code will set TLLM_LOG_LEVEL=INFO by default which would allow the print_iter_log to appear.
Without this TRTLLM logs will not appear during inference.
Before
After
Summary by CodeRabbit