feat: Add max_attachment_size to SentryOptions
#219
Conversation
|
|
|
||
| ## Fixes | ||
| ### Fixes |
There was a problem hiding this comment.
Quick fix for changelog blunder
| @@ -65,6 +65,7 @@ class SentryOptions : public RefCounted { | |||
| bool attach_screenshot = false; | |||
| sentry::Level screenshot_level = sentry::LEVEL_FATAL; | |||
| bool attach_scene_tree = false; | |||
| int64_t max_attachment_size = 20 * 1024 * 1024; // 20 MiB | |||
There was a problem hiding this comment.
Do we need to hard code a value here? Is this compressed or decompressed?
max size is 20MB compressed (100MB uncompressed) for the whole request. So if we're doing 1 request only with attachment that is it can be a lot bigger than 20MB before we compress the file (assuming something that compresses OK).
Sentry is working on allowing much larger (probably Gigabyte+ in size) attachments. We'd need to come back and change all SDKs hard coding this size. See:
Is also an issue that we only apply this to Android but not sentry-native. (Does it exist on Cocoa)? (cc @romtsn @philipphofmann )
I think best approach for now is to print to console if larger than X (20MB perhaps), that the limit is documented here: https://docs.sentry.io/concepts/data-management/size-limits/ and that because the attached file is larger than that, it's likely going to be dropped. We could hard code a value to drop if larger then 100MB decompressed, because that will be dropped for now, for sure, for now. Also debug log and print to console. I think this behavior should be aligned across SDKs though too, so we should add this to sentry-native then.
There was a problem hiding this comment.
@bruno-garcia I believe this option is meant to set a limit on the mobile SDKs that can cache these attachments on the drive and control space usage this way. I copied the default value from the Android SDK, but we could remove the default, set it to 0, and when it’s 0, the limit won’t apply. I’m adding it based on the documentation:
There was a problem hiding this comment.
Yes, we have the max attachment size on Cocoa: https://github.com/getsentry/sentry-cocoa/blob/bce9765eba9af4af7ad81ff7feb3684aed3ed684/Sources/Sentry/Public/SentryOptions.h#L248-L255
Yes, we should align this behaviour across the SDKs once we can upload larger attachments. On Cocoa, large attachements wouldn't work currently because we load the entire attachment into memory before sending it. We would need add streaming large files directly via the HTTP request instead of completely loading it into memory. We'll fix this once we get there.
|
During the last sync, we decided against introducing this option, so I'm closing this. |
max_attachment_sizeto SentryOptions