-
Notifications
You must be signed in to change notification settings - Fork 2
Fix InternalLogger to properly register with logging hierarchy #127
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
Merged
Conversation
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
InternalLogger instances were not being registered with Python's logging manager, causing them to have no parent logger and preventing handler propagation. This meant that: - basicConfig() had no effect on InternalLogger instances - They couldn't inherit handlers from parent loggers - Users couldn't see Prefab's internal logs without manual configuration This commit fixes the issue by: 1. Registering InternalLogger instances with logging.Logger.manager.loggerDict during __init__, ensuring they participate in the logging hierarchy 2. Setting up parent loggers properly (adapted from Python's logging internals) so handlers propagate correctly 3. Fixing the prefab_internal extra attribute by overriding _log() instead of log(), since info(), debug(), etc. call _log() directly The fix is compatible with both: - Standard logging (logging.basicConfig, manual handler configuration) - Structlog (when configured to use stdlib logging) The prefab_internal=True extra attribute is still added to all log records from InternalLogger instances, allowing users to filter/identify Prefab's internal messages. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
3f2bc95 to
340ce27
Compare
jdwyah
approved these changes
Nov 20, 2025
Contributor
jdwyah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks
Merged
jkebinger
added a commit
to ReforgeHQ/sdk-python
that referenced
this pull request
Nov 20, 2025
InternalLogger instances were not being registered with Python's logging manager, causing them to have no parent logger and preventing handler propagation. This meant that: - basicConfig() had no effect on InternalLogger instances - They couldn't inherit handlers from parent loggers - Users couldn't see SDK internal logs without manual configuration This commit fixes the issue by: 1. Registering InternalLogger instances with logging.Logger.manager.loggerDict during __init__, ensuring they participate in the logging hierarchy 2. Setting up parent loggers properly (adapted from Python's logging internals) so handlers propagate correctly 3. Fixing the prefab_internal extra attribute by overriding _log() instead of log(), since info(), debug(), etc. call _log() directly The fix is compatible with both: - Standard logging (logging.basicConfig, manual handler configuration) - Structlog (when configured to use stdlib logging) The prefab_internal=True extra attribute is still added to all log records from InternalLogger instances, allowing users to filter/identify SDK internal messages. Ported from: prefab-cloud/prefab-cloud-python#127 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
jkebinger
added a commit
to ReforgeHQ/sdk-python
that referenced
this pull request
Nov 20, 2025
* Fix InternalLogger to properly register with logging hierarchy InternalLogger instances were not being registered with Python's logging manager, causing them to have no parent logger and preventing handler propagation. This meant that: - basicConfig() had no effect on InternalLogger instances - They couldn't inherit handlers from parent loggers - Users couldn't see SDK internal logs without manual configuration This commit fixes the issue by: 1. Registering InternalLogger instances with logging.Logger.manager.loggerDict during __init__, ensuring they participate in the logging hierarchy 2. Setting up parent loggers properly (adapted from Python's logging internals) so handlers propagate correctly 3. Fixing the prefab_internal extra attribute by overriding _log() instead of log(), since info(), debug(), etc. call _log() directly The fix is compatible with both: - Standard logging (logging.basicConfig, manual handler configuration) - Structlog (when configured to use stdlib logging) The prefab_internal=True extra attribute is still added to all log records from InternalLogger instances, allowing users to filter/identify SDK internal messages. Ported from: prefab-cloud/prefab-cloud-python#127 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Bump version to 1.1.1 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
InternalLoggerinstances were not being registered with Python's logging manager, causing several issues:parent = Nonelogging.basicConfig()and parent logger handlers had no effectExample of the issue:
Solution
This PR fixes the issue by properly integrating
InternalLoggerwith Python's logging system:1. Register with logging manager (lines 72-94)
logging.Logger.manager.loggerDict_fixupParents)2. Fix
prefab_internalattribute injection (lines 96-126)log()to overriding_log()info(),debug(), etc. call_log()directly, notlog()prefab_internal=Trueto theextradict on all log recordsCompatibility
✅ Standard logging: Works with
basicConfig()and manual handler configuration✅ Structlog: Works when structlog is configured to use stdlib logging
✅ Maintains
prefab_internalattribute: Still adds the extra attribute to identify Prefab's internal logsTesting
Tested with both standard logging and structlog:
Impact
This fix allows users to see important Prefab internal logs such as:
Previously, these logs were invisible unless users manually configured handlers for Prefab's loggers.
🤖 Generated with Claude Code