-
-
Notifications
You must be signed in to change notification settings - Fork 494
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
Use allowlist to filter ActiveSupport breadcrumbs' data #2048
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #2048 +/- ##
=======================================
Coverage 83.23% 83.23%
=======================================
Files 119 119
Lines 5662 5662
=======================================
Hits 4713 4713
Misses 949 949
☔ View full report in Codecov by Sentry. |
The failures are caused by latest Rails changes. I will open another PR to address it. |
bb5ca15
to
4d47eab
Compare
@sl0thentr0py Once this and #2036 is merged, I'd like to release v5.10.0. WDYT? |
Since the number of potentially hard-to-serialise data is growing, we should use allowlist to filter the data.
4d47eab
to
29854b9
Compare
@sl0thentr0py I've fixed conflicts in this PR, would you mind giving it a look? |
@st0012 do you think we can remove this line now?
|
@sl0thentr0py Probably not because users could still accidentally add data that's not serialisable, which we'd want to spot early with |
ok let's keep it in for now, but I would actually prefer to get rid of that eventually because ideally there should be just one large JSON serialization step in client/transport. We can make it clearer in the docs/docstrings what people should pass into |
Since the number of potentially hard-to-serialise data is growing, we should use allowlist to filter the data.
This should properly resolve #1183