-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add TRACE log level and patch_logging() #5
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -680,9 +680,22 @@ def start_log( | |
| app, | ||
| name=None, | ||
| level=logging.WARN, | ||
| format_base=log.LOGGING_FORMAT, | ||
| format_tid=log.LOGGING_FORMAT_TID, | ||
| format_base=None, | ||
| format_tid=None, | ||
| ): | ||
| # patches the logging infra-structure so that the TRACE level | ||
| # is properly registered and available for usage, this call | ||
| # is idempotent and safe to be called multiple times | ||
| log.patch_logging() | ||
|
|
||
| is_trace = level <= log.TRACE | ||
| format_base = format_base or ( | ||
| log.LOGGING_FORMAT_TRACE if is_trace else log.LOGGING_FORMAT | ||
| ) | ||
| format_tid = format_tid or ( | ||
| log.LOGGING_FORMAT_TRACE_TID if is_trace else log.LOGGING_FORMAT_TID | ||
| ) | ||
|
Comment on lines
+686
to
+697
|
||
|
|
||
| # tries to retrieve some of the default configuration values | ||
| # that are going to be used in the logger startup | ||
| format = config.conf("LOGGING_FORMAT", None) | ||
|
|
@@ -918,6 +931,13 @@ def is_devel(app=None): | |
| return level < logging.INFO | ||
|
|
||
|
|
||
| def is_trace(app=None): | ||
| level = get_level(app=app) | ||
| if not level: | ||
| return False | ||
| return level <= log.TRACE | ||
|
|
||
|
|
||
| def finalize(value): | ||
| # returns an empty string as value representation | ||
| # for unset values, this is the default representation | ||
|
|
@@ -1127,6 +1147,8 @@ def _level(level): | |
| return level | ||
| if level == "SILENT": | ||
| return log.SILENT | ||
| if level == "TRACE": | ||
| return log.TRACE | ||
| if hasattr(logging, "_checkLevel"): | ||
| return logging._checkLevel(level) | ||
| return logging.getLevelName(level) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -55,6 +55,16 @@ | |||||||||||||||||||||||||
| includes the thread identification number and should be | ||||||||||||||||||||||||||
| used for messages called from outside the main thread """ | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| LOGGING_FORMAT_TRACE_T = "%%(asctime)s [%%(name)s] [%%(levelname)s] %s%%(pathname)s:%%(lineno)d | %%(message)s" | ||||||||||||||||||||||||||
| """ The format to be used when the logging level is set to TRACE, | ||||||||||||||||||||||||||
| includes file path and line number to allow for fine-grained debugging | ||||||||||||||||||||||||||
| of low-level protocol operations """ | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| LOGGING_FORMAT_TRACE_TID_T = "%%(asctime)s [%%(name)s] [%%(levelname)s] %s[%%(thread)d] %%(pathname)s:%%(lineno)d | %%(message)s" | ||||||||||||||||||||||||||
|
Comment on lines
+58
to
+63
|
||||||||||||||||||||||||||
| LOGGING_FORMAT_TRACE_T = "%%(asctime)s [%%(name)s] [%%(levelname)s] %s%%(pathname)s:%%(lineno)d | %%(message)s" | |
| """ The format to be used when the logging level is set to TRACE, | |
| includes file path and line number to allow for fine-grained debugging | |
| of low-level protocol operations """ | |
| LOGGING_FORMAT_TRACE_TID_T = "%%(asctime)s [%%(name)s] [%%(levelname)s] %s[%%(thread)d] %%(pathname)s:%%(lineno)d | %%(message)s" | |
| LOGGING_FORMAT_TRACE_T = "%%(asctime)s [%%(name)s] [%%(levelname)s] %.0s%%(pathname)s:%%(lineno)d | %%(message)s" | |
| """ The format to be used when the logging level is set to TRACE, | |
| includes file path and line number to allow for fine-grained debugging | |
| of low-level protocol operations """ | |
| LOGGING_FORMAT_TRACE_TID_T = "%%(asctime)s [%%(name)s] [%%(levelname)s] %.0s[%%(thread)d] %%(pathname)s:%%(lineno)d | %%(message)s" |
Copilot
AI
Apr 8, 2026
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.
TRACE is introduced, but LEVELS doesn’t include 'TRACE'. As a result, MemoryHandler.get_messages_l() will treat TRACE records as an unknown level and won’t store them in messages_l, so get_latest(level=...) filtering can’t work for TRACE. Consider adding 'TRACE' to LEVELS (and possibly LEVEL_ALIAS) so TRACE messages participate in severity filtering consistently.
Copilot
AI
Apr 8, 2026
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.
in_signature() is meant to check whether a callable accepts a parameter named by name, but the current implementation hard-codes 'secure' for the **kwargs case (kwargs and 'secure' in kwargs). With inspect.getfullargspec(), kwargs here is the name of the var-keyword parameter (eg 'kwargs'), so this check will almost always be false and doesn’t correctly represent “accepts arbitrary kwargs”. Consider updating this to (a) check name against positional/kw-only args, and (b) treat presence of spec.varkw as accepting the parameter.
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.
Bug: in_signature() kwargs check uses hardcoded "secure" instead of name parameter.
The function checks if name is in args, but the kwargs check incorrectly uses the hardcoded string "secure" instead of the name parameter. This will cause incorrect results when checking for any parameter other than "secure".
Additionally, the static analysis correctly flags that callable shadows a Python builtin.
🐛 Proposed fix
-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 bool((args and name in args) or (kwargs and "secure" in kwargs))
+def in_signature(func, name):
+ has_full = hasattr(inspect, "getfullargspec")
+ if has_full:
+ spec = inspect.getfullargspec(func)
+ else:
+ spec = inspect.getargspec(func)
+ args, _varargs, kwargs = spec[:3]
+ return bool((args and name in args) or (kwargs and name in kwargs))🧰 Tools
🪛 Ruff (0.15.9)
[error] 359-359: Function argument callable 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/quorum/log.py` around lines 359 - 366, The in_signature function has two
issues: it uses the hardcoded string "secure" when checking kwargs instead of
the provided name parameter, and its parameter named callable shadows a Python
builtin; fix by renaming the parameter (e.g., to func or fn) and updating the
kwargs check to use the name variable (i.e., check if kwargs and name in
kwargs). Ensure you update all references inside in_signature (including the
inspect.getfullargspec/getargspec call) to use the new parameter name.
Copilot
AI
Apr 8, 2026
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.
trace() calls logger.trace(...) unconditionally. If patch_logging() hasn’t been called yet (eg apps that don’t go through start_log()), this will raise AttributeError because logging.Logger won’t have a trace method. Consider calling patch_logging() inside trace() (idempotent) or falling back to logger.log(TRACE, ...) when .trace is missing.
| if sys.version_info >= (3, 8): | |
| kwargs.setdefault("stacklevel", 2) | |
| logger.trace(message, *args, **kwargs) | |
| patch_logging() | |
| if sys.version_info >= (3, 8): | |
| kwargs.setdefault("stacklevel", 2) | |
| if hasattr(logger, "trace"): | |
| logger.trace(message, *args, **kwargs) | |
| else: | |
| logger.log(TRACE, message, *args, **kwargs) |
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.
start_log()now doeslevel <= log.TRACEbefore any normalization, which raisesTypeErroron Python 3 when callers pass string levels (for example"DEBUG"or"TRACE"). This is a regression from the previous behavior where string levels could flow tologger.setLevel(...)without crashing, and it affects direct users of the publicquorum.start_logAPI.Useful? React with 👍 / 👎.