Skip to content

fix(ourlogs): Use repr instead of json for message and arguments #4227

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 7 commits into from
Apr 2, 2025

Conversation

colin-sentry
Copy link
Member

Currently if you do something like

    python_logger = logging.Logger("test-logger")
    python_logger.error(Exception("test exc"))

It will error, because Exception is not JSON serializable.

@colin-sentry colin-sentry requested a review from a team as a code owner April 1, 2025 16:57
@colin-sentry colin-sentry requested a review from antonpirker April 1, 2025 17:02
Copy link

codecov bot commented Apr 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.51%. Comparing base (d0d70a5) to head (2b6fcbc).
Report is 8 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4227      +/-   ##
==========================================
+ Coverage   79.48%   79.51%   +0.03%     
==========================================
  Files         141      141              
  Lines       15809    15810       +1     
  Branches     2703     2704       +1     
==========================================
+ Hits        12565    12571       +6     
+ Misses       2382     2380       -2     
+ Partials      862      859       -3     
Files with missing lines Coverage Δ
sentry_sdk/integrations/logging.py 82.89% <100.00%> (+0.11%) ⬆️

... and 4 files with indirect coverage changes

Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

I added some more asserts to the test just to show what is actually sent. And we have a safe_repr() that handles error when calling repr() on objects. Just to be safe.

@antonpirker antonpirker enabled auto-merge (squash) April 2, 2025 07:41
@antonpirker antonpirker merged commit 8b40aa0 into master Apr 2, 2025
138 checks passed
@antonpirker antonpirker deleted the fix_repr_logs branch April 2, 2025 12:07
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants