Changing attachments after sentry_init

[thanhldnv]

Description
I'm reading the source of Sentry SDK, and I see a comment relate using sentry_options_t *options.

/**
 * Initializes the Sentry SDK with the specified options.
 *
 * This takes ownership of the options.  After the options have been set
 * they cannot be modified any more.
 * Depending on the configured transport and backend, this function might not be
 * fully thread-safe.
 * Returns 0 on success.
 */
SENTRY_API int sentry_init(sentry_options_t *options);

My application need to attach the log file when a crash occurs. My logger works like unix logrotate, it will generate a new file every day or when the file size exceeds a limit. Then I need to update the new log file path via sentry_options_add_attachment so I understand this function will change the options object.
So can I use sentry_options_add_attachment after sentry_init and is it safe ?
As I tested this way Sentry still work well.

[Swatinem]

Well, in theory, you could do that. But 1) its not threadsafe that way, 2) you can’t remove files after you added them.

There was #433 but that stalled because there were concerns about the API design and how it fits into other language SDKs.

[thanhldnv]

@Swatinem Thanks for the confirmation.

[Swatinem]

Also as that PR notes, the way that the crashpad backend works, the attachments in that case indeed are fixed at the time you start the backend, and they can’t be changed after the fact (without an unreasonable amount of effort to add that ability to crashpad).

[brianmichel]

Just wanted to add my plus one to this, since I'm discovering the same thing, and also wanting to have the ability to change attachments at runtime. As we're building https://github.com/thebrowsercompany/swift-sentry we want to mirror the Cocoa APIs to have a somewhat drop in replacement. I was a bit confused when I saw that adding an attachment required a sentry_options_t which we discard after initialization.

After reading the comments available on #433 it's unclear what API concerns still exist, or what is blocking it (other than the questions that exist which have no reply).

It would be exceptional to have this API!

[supervacuus]

After reading the comments available on #433 it's unclear what API concerns still exist, or what is blocking it (other than the questions that exist which have no reply).

Hi @brianmichel, the API concerns are mostly related to downstream SDKs that include the Native SDK to handle crashes on the "native" layer. Those SDKs must provide attachments via "byte-arrays" (in addition to lazily loaded paths). Since the PR was created in response to the needs of downstream SDKs, this still needs to be solved.

The more significant issue for users of our crashpad backend is that the crashpad_handler has no way of updating attachments after the process starts. This happens in sentry_init(), which makes attachments for crash events when using this backend essentially equally flexible to our current situation.

Of course, changing this in crashpad is an option, but it would require a cross-platform IPC mechanism that we'd have to introduce between the client and the handler, which increases the maintenance effort of our fork considerably.

Last but not least, there are currently minimal resources available to work on new features of the Native SDK, so I cannot say when topics like these will get prioritized again.

Because it wasn't mentioned in this issue yet, the current workaround on the sentry-native side is the following:

• attachments specified in the options are not loaded before the envelope is prepared to be sent
• this allows you to use the on_crash or before_send hooks (depending on whether you need different attachments for crash and non-crash events) to prepare the dynamic attachments to be moved/copied to the specified paths (or to zip an, at the point of initialization, unknown set of attachments and specify that zip via the options)
• of course, doing this in the mentioned hooks needs to be done very carefully since that happens from within the crash-handling context (i.e., it shouldn't crash and on Linux should be signal-safe) and is also not an option when running crashpad on macOS.