Try to get a more reasonable stacklevel#4656
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces stacklevel handling to logging functions to provide more accurate caller information in logs. The core logic in tmt/log.py correctly adjusts the stack level. The comment suggesting to refactor repeated stacklevel logic into a decorator has been kept as it aligns with general code quality improvements and is not contradicted by existing rules.
Note: Security Review did not run due to the size of the PR.
|
What is a “stacklevel”, and what is it good for? If there’s a vhange to output, would it be possible to inckude an example or two to demonstrate the change, e.g. before/after? |
It's related to the stack trace and so far I don't think we use it anywhere yet (will do in #4642). It basically controls how many levels up the stack trace do you navigate (from inside |
Hmm, imo it's better to put _log in this mr , #4642 is mainly about warning file , and Warnings classes,plus the mr will be merged before #4642.Not strong opinion though:) |
skycastlelily
left a comment
There was a problem hiding this comment.
LGTM in general:)
thrix
left a comment
There was a problem hiding this comment.
The PR fixes source location reporting in log records.
Python's logging module records the filename, line number, and function name of the code that emitted each log message. Without stacklevel, every log record in tmt would show tmt/log.py
(the internal Logger._log method) as the source, because that's where self._logger._log() is actually called.
With this patch, log records correctly point to the actual caller - e.g., the plugin or step code that called logger.verbose() or self.info(). This matters when:
- Using log formatters that include %(filename)s:%(lineno)d (standard Python logging pattern)
- Debugging with file-based log handlers that show source locations
- Any tooling that inspects LogRecord.pathname / LogRecord.lineno to trace where a message originated
Before this PR, all log records would point to the same internal location regardless of which code emitted them. After it, each record correctly identifies its origin in the business logic.
The PR description notes this was split out from #4642, which specifically needs accurate caller information.
@LecrisUT I would make the MR description a bit more descriptive, for example this AI summary is a lot better for me then the current description.
I do not like the verbosity there at all. If there is a concise version I could consider, but I do not want the commit message to be so full of tangents Edit: tried to manually improve the message. |
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Try to provide a more reasonable
stacklevelto the innerlogging.Logger._log, skipping any functions that are wrappers around the loggers. The references in the logger's record should point to the most relevant function that requested the log command, or any such indirect commands like theguest.run.Found that this would be useful in #4642 for getting a more accurate source of the caller and I split it out from there to simplify the review.
The stack can be further improved as it goes, as shown in the second commit of this PR, but that can be gradually addressed.
Pull Request Checklist