-
Notifications
You must be signed in to change notification settings - Fork 548
feat(ourlogs): canonicalize paths from the logger integration #4336
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
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #4336 +/- ##
==========================================
- Coverage 80.29% 80.29% -0.01%
==========================================
Files 142 142
Lines 15934 15937 +3
Branches 2726 2727 +1
==========================================
+ Hits 12794 12796 +2
- Misses 2263 2267 +4
+ Partials 877 874 -3
|
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.
LGTM but please address comment before merging @colin-sentry. A test case would also be awesome
sentry_sdk/integrations/logging.py
Outdated
@@ -374,7 +375,10 @@ def _capture_log_from_record(client, record): | |||
if record.lineno: | |||
attrs["code.line.number"] = record.lineno | |||
if record.pathname: | |||
attrs["code.file.path"] = record.pathname | |||
if record.pathname.startswith(project_root): |
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.
project_root
might be None
so we need to be more careful here:
if record.pathname.startswith(project_root): | |
if project_root is not None and record.pathname.startswith(project_root): |
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.
done, test added too
d946062
to
81797d1
Compare
We'd like to allow linking to the 'source code' line in the logs page - this canonicalizes the path relative to the project root (if one is defined)
Solves LOGS-58