feat: Include more lines of code for additional context in structured logging output#3667
Open
edgarrmondragon wants to merge 3 commits into
Open
feat: Include more lines of code for additional context in structured logging output#3667edgarrmondragon wants to merge 3 commits into
edgarrmondragon wants to merge 3 commits into
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideExtend structured exception logging to include surrounding source lines for each traceback frame, and update tests to validate the new contextual data in the structured formatter output. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3667 +/- ##
=======================================
Coverage 94.13% 94.13%
=======================================
Files 73 73
Lines 6203 6207 +4
Branches 763 763
=======================================
+ Hits 5839 5843 +4
Misses 270 270
Partials 94 94
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1ff649a to
cb60fe8
Compare
Documentation build overview
26 files changed ·
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- When building the
linescontext slice, consider making the context window size a named constant and using explicit bounds (e.g.,start = max(lineno - 2, 1),end = lineno + 1) so the intent is clearer and edge cases (top-of-file frames) are handled more transparently than relying on negative slice indices.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When building the `lines` context slice, consider making the context window size a named constant and using explicit bounds (e.g., `start = max(lineno - 2, 1)`, `end = lineno + 1`) so the intent is clearer and edge cases (top-of-file frames) are handled more transparently than relying on negative slice indices.
## Individual Comments
### Comment 1
<location path="singer_sdk/logging.py" line_range="121" />
<code_context>
- tb.tb_lineno,
- ),
+ "line": linecache.getline(filename, lineno),
+ "lines": linecache.getlines(filename)[lineno - 2 : lineno + 1],
}
frames.append(frame_info)
</code_context>
<issue_to_address>
**issue:** Guard against negative slice indices when lineno is 1 or 2.
For `lineno` 1 or 2, `lineno - 2` is negative, so the slice pulls lines from the *end* of the file instead of just the first lines. Clamp the start index to 0 (e.g. `start = max(0, lineno - 2)` and slice `[start:lineno + 1]`) so early frames only include valid leading context.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
d4c1576 to
24f3285
Compare
… logging output Signed-off-by: Edgar Ramírez Mondragón <edgarrm358@gmail.com>
Signed-off-by: Edgar Ramírez Mondragón <edgarrm358@gmail.com>
…bove and below Signed-off-by: Edgar Ramírez Mondragón <edgarrm358@gmail.com>
24f3285 to
caa0a3c
Compare
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
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.
Related
Summary by Sourcery
Include additional source code context lines in structured log exception frames.
New Features:
Enhancements:
Tests: