fix: suppress non-axolotl logs unless it's warning or higher#2724
Merged
Conversation
Contributor
|
""" WalkthroughThe logging configuration was updated by introducing a custom filter and logger class to control log record emission based on environment variables. The root logger's level is now set via an environment variable, and the axolotl logger's level is also configurable. The filter ensures only relevant log records are allowed, and all loggers use the new logger class. Changes
Sequence Diagram(s)sequenceDiagram
participant Env as Environment
participant Config as configure_logging()
participant LoggerClass as AxolotlLogger
participant Filter as AxolotlOrWarnErrorFilter
participant Logger
Env->>Config: Provide LOG_LEVEL, AXOLOTL_LOG_LEVEL
Config->>LoggerClass: Set as global logger class
Config->>Logger: Create root and axolotl loggers
Config->>Logger: Attach AxolotlOrWarnErrorFilter
Logger->>Filter: Filter log records based on env levels
Filter-->>Logger: Allow or drop record
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
NanoCode012
force-pushed
the
fix/logging-level
branch
from
6f7fc82 to
60a7db5
Compare
Collaborator
Author
|
New: Root logs are gone like from, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/axolotl/logging_config.py(4 hunks)src/axolotl/utils/config/__init__.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: PyTest (3.11, 2.7.0)
- GitHub Check: PyTest from Source Dist (3.11, 2.7.0)
- GitHub Check: PyTest from Source Dist (3.11, 2.5.1)
- GitHub Check: PyTest (3.11, 2.5.1)
- GitHub Check: PyTest from Source Dist (3.11, 2.6.0)
- GitHub Check: PyTest (3.11, 2.6.0)
- GitHub Check: pre-commit
🔇 Additional comments (6)
src/axolotl/logging_config.py (5)
15-28: LGTM! Well-designed filter implementation.The
AxolotlOrWarnErrorFiltercorrectly implements the filtering logic to allow WARNING+ messages from any logger while restricting INFO messages to only axolotl-prefixed loggers. The logic is clear and aligns with the PR objectives to reduce log noise.
31-38: LGTM! Clean logger class implementation.The
AxolotlLoggerclass properly inherits from the standardLoggerand automatically applies the custom filter. The implementation is minimal and focused, following the single responsibility principle.
86-86: LGTM! Root logger level change aligns with objectives.Changing the root logger default from "INFO" to "WARNING" effectively reduces noise from non-axolotl libraries while still respecting the LOG_LEVEL environment variable. This addresses the core issue mentioned in the PR description.
90-90: LGTM! Axolotl logger level maintains flexibility.The axolotl logger now defaults to "INFO" instead of "DEBUG" while still respecting the LOG_LEVEL environment variable. This provides a good balance between informativeness and noise reduction.
101-101: LGTM! Proper integration of custom logger class.Setting the global logger class ensures that all subsequent
logging.getLogger()calls will createAxolotlLoggerinstances with the filtering behavior. This is the correct way to apply the custom logger globally.src/axolotl/utils/config/__init__.py (1)
24-24: LGTM! Dynamic logger naming improves modularity.Changing from hardcoded
"axolotl"to__name__makes the logger name dynamic based on the module path (axolotl.utils.config). This aligns perfectly with the newAxolotlOrWarnErrorFilterwhich allows INFO+ messages for loggers starting with "axolotl", and it follows logging best practices for hierarchical logger naming.
Collaborator
Author
|
Current logs: |
NanoCode012
changed the title
Contributor
|
@NanoCode012 I ended up landing the rank-0 logging PR separately if you're up for merging it in and updating this PR |
Collaborator
Author
|
After merged main in |
Collaborator
Author
|
Fixed logger to respect the env Verified with |
salmanmohammadi
approved these changes
May 30, 2025
salmanmohammadi
left a comment
salmanmohammadi
left a comment
Contributor
There was a problem hiding this comment.
,
/\^/`\
| \/ |
| | |
\ \ /
'\\//'
||
||
||
|| ,
|\ || |\
| | || | |
| | || / /
\ \||/ /
`\\//`
^^^^^^^^
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
By default, we set Axolotl to LOG all debugs which can be annoying to users and doesn't respect the LOG_LEVEL env.
Secondly, we had other loggers set to INFO, so moving it up to WARNING to not show their non-axolotl INFO logs
Motivation and Context
How has this been tested?
Screenshots (if appropriate)
Types of changes
Social Handles (Optional)
Summary by CodeRabbit