Send attachment data inline when possible
#3654
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jjbayer
reviewed
May 28, 2024
jjbayer
reviewed
May 28, 2024
jjbayer
approved these changes
May 28, 2024
loewenheim
reviewed
May 29, 2024
jjbayer
approved these changes
May 29, 2024
Dav1dde
reviewed
May 29, 2024
Swatinem
added a commit
to getsentry/sentry
that referenced
this pull request
May 29, 2024
This will be used in relay starting with this PR: getsentry/relay#3654
Dav1dde
approved these changes
May 29, 2024
For `invidual_attachments`, we can now send the `data` inline if it is small enough. This avoids having to store temporary `attachment_chunks` in the processing redis store.
Swatinem
force-pushed
the
swatinem/skip-single-attachment-chunks
branch
from
2615cb9 to
9991ff0
Compare
|
Thanks for all the feedback! I rebased/squashed the PR, added documentation to the function, and cleaned up the flow in the caller. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For
invidual_attachments, we can now send thedatainline if it is small enough. This avoids having to store temporaryattachment_chunksin the processing redis store.Right now, every "event" attachment is sent with outlined chunks, even for things like logfiles, even though only minidumps and other processing-relevant attachments need to be treated that way.
It might be possible to also inline logfiles with more logic. That way, such attachments could be saved as
EventAttachmentdirectly without putting load on the attachments store. Doing that however would create problems when such events are discarded within the rest of the pipeline.Support for consuming the
dataproperty is being added in getsentry/sentry#71522 to the Sentry codebase.Developing this, I noticed that
extract_kafka_messages_for_eventis a free function returning anIteratorpurely for testing purposes. These tests pretty much just tested thesend_individual_attachmentslogic in a very convoluted way, and are now pretty useless.