Add max_nesting of 10 to breadcrumbs data serialization#2583
Add max_nesting of 10 to breadcrumbs data serialization#2583sl0thentr0py merged 1 commit intomasterfrom
Conversation
62bd21c to
240f514
Compare
szokeasaurusrex
left a comment
There was a problem hiding this comment.
If I understand correctly, this change causes the entire breadcrumb data to be deleted if the nesting is at least 5. Why not just strip the portions which are too deeply nested?
I suppose if Relay is throwing away the entire data when there is too much nesting, this is okay, but if relay only removes the part that is too deeply nested, the SDK should have the same behavior.
|
valid, but that will require generalized databag trimming like python which is not implemented in ruby (and we don't plan to). |
240f514 to
b3300ea
Compare
|
alright, let's do this, I'll make the depth 10, so we allow some leeway instead of dropping the whole thing. |
b3300ea to
70161d0
Compare
This is [enforced in [relay](https://github.com/getsentry/relay/blob/bcbfa7e7db6e4a8f3f0383270fbb2b5cfe668770/relay-event-schema/src/protocol/breadcrumb.rs#L111) at a depth of 5. We allow upto 10 to have some leeway but if we have a really large data object we just drop it to avoid SystemStackError. Closes #2393 and #2397
70161d0 to
387f2a2
Compare
szokeasaurusrex
left a comment
There was a problem hiding this comment.
Nice! So if I understand correctly, the limit of 10 is so that if the depth of 5 is only slightly exceeded, we allow Relay's logic to trim down to a depth of 5 (preserving the portion that is less than 5 deep), but if we exceed the limit of 10, we drop the whole thing to avoid the error in Ruby?
I think that is reasonable. Although, if we can avoid the recursion error altogether, that would be the best solution (i.e. by replacing the recursive function which causes the error with an iterative one). Would this be feasible?
|
yes with 10 we will drop the whole thing |
This is trimmed in relay at a depth of 5.
We allow upto 10 to have some leeway but if we have a really large data object we just drop it to avoid
SystemStackError.Closes #2393 and #2397