Skip to content

Conversation

@st0012
Copy link
Contributor

@st0012 st0012 commented Apr 8, 2023

  1. Exclude sql.active_record payload's connection and binds because they could cause problems when being serialised.
  2. Exclude non-primitive type objects because at this stage they're likely added by the user or other libraries, which could also be problematic to serialise.

I know 2) also excludes most data declared in IGNORED_KEYS, but I think it's still worth keeping that list to make it explicit on what we try to exclude.

st0012 added 2 commits April 8, 2023 21:32
As explained in #1183 (comment),
sometimes other gems could inject their own data into the payload, which
may cause problems when we try to serialize them.

It's possible that Array and Hash also contain such objects, but I'm not
excluding them yet because:

1. I haven't seen other gems injecting their own data into the payload
   in this way.
2. Some Rails events (e.g. `deliver.action_mailer`) use Array or Hash in
   the payload, so ignore them now could reduce the data users can get.
@st0012 st0012 added the bug fix label Apr 8, 2023
@st0012 st0012 added this to the 5.9.0 milestone Apr 8, 2023
@st0012 st0012 self-assigned this Apr 8, 2023
@github-actions
Copy link

github-actions bot commented Apr 8, 2023

Fails
🚫 Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Improve ActiveSupport breadcrumb logger's data filtering ([#2030](https://github.com/getsentry/sentry-ruby/pull/2030))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against f48259c

@codecov
Copy link

codecov bot commented Apr 8, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01 ⚠️

Comparison is base (d45f720) 98.58% compared to head (f48259c) 98.58%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2030      +/-   ##
==========================================
- Coverage   98.58%   98.58%   -0.01%     
==========================================
  Files         157      157              
  Lines       10127    10142      +15     
==========================================
+ Hits         9984     9998      +14     
- Misses        143      144       +1     
Impacted Files Coverage Δ
.../sentry/rails/instrument_payload_cleanup_helper.rb 100.00% <100.00%> (ø)
...ry/rails/breadcrumbs/active_support_logger_spec.rb 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@st0012 st0012 closed this Apr 8, 2023
@sl0thentr0py sl0thentr0py deleted the fix-#1183 branch October 7, 2025 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants