Skip to content

Conversation

@jkebinger
Copy link
Contributor

Problem

InternalLogger instances were not being registered with Python's logging manager, causing several issues:

  • No parent logger: InternalLogger instances had parent = None
  • No handler propagation: logging.basicConfig() and parent logger handlers had no effect
  • Invisible internal logs: Users couldn't see Prefab's internal logs (config loading, checkpoint updates, etc.) without manual workarounds

Example of the issue:

import logging
logging.basicConfig(level=logging.DEBUG)

from prefab_cloud_python._internal_logging import InternalLogger
logger = InternalLogger('prefab_cloud_python.config_client')
logger.info('This message was invisible')  # Would not appear

Solution

This PR fixes the issue by properly integrating InternalLogger with Python's logging system:

1. Register with logging manager (lines 72-94)

  • Registers each InternalLogger instance with logging.Logger.manager.loggerDict
  • Sets up parent loggers in the hierarchy (adapted from Python's _fixupParents)
  • Ensures proper handler propagation through the logger tree

2. Fix prefab_internal attribute injection (lines 96-126)

  • Changed from overriding log() to overriding _log()
  • This is necessary because info(), debug(), etc. call _log() directly, not log()
  • Properly adds prefab_internal=True to the extra dict on all log records

Compatibility

Standard logging: Works with basicConfig() and manual handler configuration
Structlog: Works when structlog is configured to use stdlib logging
Maintains prefab_internal attribute: Still adds the extra attribute to identify Prefab's internal logs

Testing

Tested with both standard logging and structlog:

import logging
logging.basicConfig(level=logging.DEBUG)

from prefab_cloud_python._internal_logging import InternalLogger
logger = InternalLogger('prefab_cloud_python.config_client')
logger.info('This message now appears correctly')
# Output: INFO:prefab_cloud_python.config_client:This message now appears correctly

Impact

This fix allows users to see important Prefab internal logs such as:

  • Config version updates (highwater marks)
  • Checkpoint synchronization
  • SSE stream connections
  • Configuration initialization

Previously, these logs were invisible unless users manually configured handlers for Prefab's loggers.

🤖 Generated with Claude Code

@jkebinger jkebinger requested a review from a team as a code owner November 20, 2025 00:16
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>
@jkebinger jkebinger force-pushed the fix/internal-logger-hierarchy branch from 3f2bc95 to 340ce27 Compare November 20, 2025 00:23
Copy link
Contributor

@jdwyah jdwyah left a comment

Choose a reason for hiding this comment

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

thanks

@jkebinger jkebinger merged commit 0fa7895 into main Nov 20, 2025
9 of 10 checks passed
@jkebinger jkebinger mentioned this pull request Nov 20, 2025
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants