Skip to content
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

feat(process-value): Add object level trimming #3877

Merged
merged 8 commits into from
Aug 1, 2024

Conversation

iambriccardo
Copy link
Member

@iambriccardo iambriccardo commented Jul 31, 2024

This PR adds an object-level trimming control, which works slightly differently from before. Instead of avoiding trimming altogether, this implementation performs all the traversal as if it were trimming but doesn't perform the actual trimming. This way, any arbitrary node in the traversal path can treat a child node with trim = disabled as if it were trimmed, but its contents in reality are not.

In our example, we want to limit an array of spans to 800 KiB, but we don't want the trimming to happen inside the span. Rather, we want to let the system trim the entire spans in the array. To achieve this, we had to implement this system in which a span signals how many bytes were used but no data is trimmed. Then, the array sees that the remaining size is 0 and will start dropping entire spans.

Closes: #3880

let mut field_attrs = parse_field_attributes(index, bi.ast(), &mut is_tuple_struct);
// In case we have `fake_trim` set on the type, we want to override this field attribute
// manually.
if type_attrs.fake_trim {
Copy link
Member Author

Choose a reason for hiding this comment

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

It would be nice not to use field_attrs and instead create a new type but it might make it a much bigger PR and maybe confuse whoever uses the Processor interface.

@iambriccardo iambriccardo changed the title feat(process-value): Add option to disable trimming at the object level feat(process-value): Add object level trimming Jul 31, 2024
@iambriccardo iambriccardo marked this pull request as ready for review July 31, 2024 13:01
@iambriccardo iambriccardo requested a review from a team as a code owner July 31, 2024 13:01
Copy link
Member

@jjbayer jjbayer left a comment

Choose a reason for hiding this comment

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

@iambriccardo could you please link this PR to the previous two attempts?

relay-event-schema/src/processor/attrs.rs Outdated Show resolved Hide resolved
relay-event-normalization/src/trimming.rs Show resolved Hide resolved
relay-event-normalization/src/trimming.rs Outdated Show resolved Hide resolved
relay-event-derive/src/lib.rs Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@jjbayer jjbayer merged commit ac0bde3 into master Aug 1, 2024
24 checks passed
@jjbayer jjbayer deleted the riccardo/fix/trimming-spans branch August 1, 2024 13:43
jjbayer added a commit that referenced this pull request Aug 2, 2024
jjbayer added a commit that referenced this pull request Aug 2, 2024
This seems to not have solved the problem and introduced new failures.

Reverts #3877
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.

Fix trimming of spans
2 participants