feat: add TRACE log level and patch_logging()#84
Conversation
…gging Ports the TRACE log level support from netius (hivesolutions/netius#49) to the appier framework, enabling protocol-level verbose logging below DEBUG for fine-grained debugging of low-level operations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis change introduces a new TRACE logging level (finer-grained than DEBUG) for detailed protocol-level debugging. Key additions include a TRACE constant, a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
/dev/null does not exist on Windows, causing test failures on CI. Use tempfile.mkstemp() for cross-platform compatibility. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1858dc7713
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| logging of protocol-level operations, this is meant to be | ||
| used for fine-grained debugging of low-level operations | ||
| like raw byte transfers and frame parsing """ | ||
|
|
||
| LEVELS = ("DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL") |
There was a problem hiding this comment.
After introducing TRACE, MemoryHandler still uses LEVELS = ("DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"), so emit() cannot index "TRACE" in get_messages_l() and silently skips per-level storage for trace records. In practice, get_latest(level="TRACE") (and flush_to_file(..., level="TRACE")) always returns no trace messages even when trace logs were emitted, which makes the new level partially unusable for log inspection.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new ultra-verbose TRACE logging level to Appier (ported from netius) and wires it into Python’s logging system via a new patch_logging() helper, while expanding logging-related test coverage.
Changes:
- Added
TRACElog level (DEBUG - 5) pluspatch_logging()to register the level name andLogger.trace()method (and auto-call it during_load_logging()). - Added
App.is_trace()andDummyLogger.trace(), and taughtApp._level()to resolve"TRACE"similarly to"SILENT". - Adjusted
in_signature()to always return abooland added extensive logging tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/appier/log.py |
Adds TRACE, patch_logging(), Logger.trace() implementation, and tweaks in_signature() |
src/appier/base.py |
Integrates TRACE into level resolution and logging initialization; adds is_trace() |
src/appier/__init__.py |
Re-exports TRACE/SILENT and patch_logging() |
src/appier/test/log.py |
Adds tests for TRACE level semantics, patching behavior, and some logging helpers |
CHANGELOG.md |
Documents the new TRACE support in Unreleased |
Comments suppressed due to low confidence (1)
src/appier/log.py:84
TRACEis introduced as a new level, butLEVELS(used byMemoryHandler.get_messages_l()/emit()and thereforeget_latest(level=...)) does not include "TRACE". This means TRACE records will not be added to any per-threshold queue andget_latest(level="TRACE")will always return an empty list; the/loggingdebug endpoint (which passes the requestlevelstring through toget_latest) will also be unable to return TRACE logs. Consider adding "TRACE" toLEVELS(before "DEBUG") and extending tests to coverMemoryHandler.get_latest(level="TRACE").
TRACE = logging.DEBUG - 5
""" The trace level used for extremely detailed and verbose
logging of protocol-level operations, this is meant to be
used for fine-grained debugging of low-level operations
like raw byte transfers and frame parsing """
LEVELS = ("DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL")
""" The sequence of levels from the least sever to the
most sever this sequence may be used to find all the
levels that are considered more sever that a level """
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def in_signature(callable, name): | ||
| has_full = hasattr(inspect, "getfullargspec") | ||
| if has_full: | ||
| spec = inspect.getfullargspec(callable) | ||
| else: | ||
| spec = inspect.getargspec(callable) | ||
| args, _varargs, kwargs = spec[:3] | ||
| return (args and name in args) or (kwargs and "secure" in kwargs) | ||
| return bool((args and name in args) or (kwargs and "secure" in kwargs)) |
There was a problem hiding this comment.
in_signature() is checking the var-keyword parameter incorrectly: kwargs from spec[:3] is the name of the **kwargs parameter (a string) or None, not a collection of accepted kwarg names. As written, (kwargs and "secure" in kwargs) will usually be False even when **kwargs is present, and it hard-codes "secure" instead of using the name argument. This can cause smtp_handler() to incorrectly decide that logging.handlers.SMTPHandler.__init__ does not support the secure argument on Python versions where it is accepted via **kwargs or as a kw-only arg. Consider updating the logic to return True when name is present in positional/kw-only args, or when **kwargs is present at all (and avoid the hard-coded "secure").
![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
🧹 Nitpick comments (1)
CHANGELOG.md (1)
18-20: Consider documenting the rotating handler test fix.The PR objectives mention a separate commit that "Fixes rotating handler tests to use tempfile.mkstemp() instead of /dev/null for cross-platform (Windows) compatibility." This cross-platform compatibility improvement could be worth documenting in the Fixed section. As per coding guidelines, CHANGELOG.md should be updated for changes.
📝 Suggested addition to the Fixed section
### Fixed -* +* Rotating handler tests now use `tempfile.mkstemp()` instead of `/dev/null` for cross-platform (Windows) compatibility🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 18 - 20, Add a bullet to the "Fixed" section of CHANGELOG.md documenting the test change that makes rotating handler tests cross-platform: mention that tests now use tempfile.mkstemp() instead of hardcoding /dev/null (reference the rotating handler tests change) and briefly state this fixes Windows compatibility; keep the entry concise and follow the existing changelog bullet style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/appier/base.py`:
- Around line 3674-3677: The is_trace method incorrectly treats level==0 as
unset by using "if not self.level"; change the unset check to explicitly test
for None (e.g., "if self.level is None") so logging.NOTSET (0) is handled as a
valid level and the subsequent comparison against log.TRACE still runs; update
the is_trace function (referencing is_trace, self.level, and log.TRACE)
accordingly.
In `@src/appier/log.py`:
- Around line 301-302: The trace method currently uses the parameter name
"object", shadowing the built-in and triggering Ruff A002; rename the parameter
in the trace function signature (e.g., to "obj" or "value") and update any
internal references inside trace (and any callers if they rely on keyword
argument naming) to use the new name so the builtin is no longer shadowed.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Around line 18-20: Add a bullet to the "Fixed" section of CHANGELOG.md
documenting the test change that makes rotating handler tests cross-platform:
mention that tests now use tempfile.mkstemp() instead of hardcoding /dev/null
(reference the rotating handler tests change) and briefly state this fixes
Windows compatibility; keep the entry concise and follow the existing changelog
bullet style.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69dc1140-349f-4be0-ba03-6c5d9f1f30f8
📒 Files selected for processing (5)
CHANGELOG.mdsrc/appier/__init__.pysrc/appier/base.pysrc/appier/log.pysrc/appier/test/log.py
| def is_trace(self): | ||
| if not self.level: | ||
| return False | ||
| return self.level <= log.TRACE |
There was a problem hiding this comment.
is_trace() should not treat level 0 as unset.
if not self.level returns False for logging.NOTSET (0), but 0 should still satisfy trace-enabled checks.
🔧 Proposed fix
def is_trace(self):
- if not self.level:
+ if self.level == None:
return False
return self.level <= log.TRACE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/appier/base.py` around lines 3674 - 3677, The is_trace method incorrectly
treats level==0 as unset by using "if not self.level"; change the unset check to
explicitly test for None (e.g., "if self.level is None") so logging.NOTSET (0)
is handled as a valid level and the subsequent comparison against log.TRACE
still runs; update the is_trace function (referencing is_trace, self.level, and
log.TRACE) accordingly.
| def trace(self, object): | ||
| pass |
There was a problem hiding this comment.
Rename object parameter to avoid builtin shadowing.
This triggers Ruff A002 and is easy to fix.
✏️ Proposed fix
- def trace(self, object):
+ def trace(self, message):
pass📝 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.
| def trace(self, object): | |
| pass | |
| def trace(self, message): | |
| pass |
🧰 Tools
🪛 Ruff (0.15.7)
[error] 301-301: Function argument object is shadowing a Python builtin
(A002)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/appier/log.py` around lines 301 - 302, The trace method currently uses
the parameter name "object", shadowing the built-in and triggering Ruff A002;
rename the parameter in the trace function signature (e.g., to "obj" or "value")
and update any internal references inside trace (and any callers if they rely on
keyword argument naming) to use the new name so the builtin is no longer
shadowed.
Port TRACE log level support from appier (hivesolutions/appier#84) to colony. Adds TRACE constant (logging.DEBUG - 5 = 5) and SILENT constant (logging.CRITICAL + 1 = 51) for fine-grained protocol-level debugging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat: add TRACE log level and patch_logging() Port TRACE log level support from appier (hivesolutions/appier#84) to colony. Adds TRACE constant (logging.DEBUG - 5 = 5) and SILENT constant (logging.CRITICAL + 1 = 51) for fine-grained protocol-level debugging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add trace() method and *args/**kwargs to PluginManager log methods Adds trace() convenience method to PluginManager and makes all log methods (debug, info, warning, error, critical) accept *args and **kwargs for lazy evaluation support. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add TRACE-aware logging format with pathname and lineno When the log level is set to TRACE, automatically switches to a more detailed format that includes file path and line number for fine-grained debugging of low-level operations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: new module starting * fix: handle GLOBAL_CONFIG override for TRACE logging format The GLOBAL_CONFIG always has a hardcoded logging_format value, so the .get() default never triggers. Added explicit check to switch to the trace format when the config value matches the default and the level is TRACE. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: new logging format for trace * chore: new black format * chore: new logging format for trace Passes stacklevel=2 in all PluginManager log methods (trace, debug, info, warning, error, critical) so that pathname and lineno in the format string reflect the actual caller, not the internal log method. Guards stacklevel for Python < 3.8 where it is not supported, and makes DEFAULT_LOGGING_FORMAT_TRACE conditional on Python 3.8+. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: trace reference * fix: small spelling error in logging_util.py docstring --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
TRACEconstant (logging.DEBUG - 5 = 5) for fine-grained protocol-level debuggingpatch_logging()to register the TRACE level with Python's logging (idempotent, called automatically in_load_logging())is_trace()method toAppandtrace()toDummyLogger"TRACE"special case inApp._level()(matching the existing"SILENT"pattern)in_signature()return inbool()for type correctnessTest plan
pytest src/appier/test/log.py -v)🤖 Generated with Claude Code
Summary by CodeRabbit
TRACElog level for ultra-fine-grained protocol-level debugging with deeper visibility into application behavior beyond standard DEBUG logging