ref: Event processing

[limbonaut]

• Revise event processing to support Android. This fixes missing screenshots and view hierarchy on Android.
• Introduce event processors (internally). This interface can also be later exposed in the public API if needed.

Depends on:

• feat: Initial Android support #169

#skip-changelog

[limbonaut]

I'm not sure if we need a changelog entry for this.

[bruno-garcia]

@sentry review

[seer-by-sentry]

On it! We are reviewing the PR and will provide feedback shortly.

[seer-by-sentry]

PR Description

This pull request introduces a new event processing pipeline to the Sentry Godot SDK. The primary goal is to provide a more modular and extensible way to process Sentry events before they are sent to the Sentry backend. This allows for easier addition of new features like automatic screenshot and view hierarchy attachment, as well as more flexible customization through event processors.

Click to see more

Key Technical Changes

1. Event Processor Framework: A new SentryEventProcessor base class is introduced, along with concrete implementations for attaching screenshots (ScreenshotProcessor) and view hierarchies (ViewHierarchyProcessor).
2. Centralized Event Processing: The process_event function in sentry/processing/process_event.cpp centralizes the event processing logic, including context injection, applying event processors, and executing the before_send callback.
3. Context Merging: The merge_context function is added to SentryEvent to allow merging dictionary contexts, providing a way to update existing context information.
4. Android API Updates: The Android plugin API is updated to use addFileAttachment for more flexible attachment handling, including specifying filename and content type.
5. Native SDK Changes: The native SDK event handling is updated to use the new process_event pipeline.

Architecture Decisions

The architectural decision was made to use a chain-of-responsibility pattern for event processors. This allows for multiple processors to be applied to an event in a defined order, with each processor having the option to modify or discard the event. This pattern promotes modularity and extensibility.

Dependencies and Interactions

This pull request modifies the core event handling logic and interacts with the following parts of the system:

• SentryOptions: To configure event processors and the before_send callback.
• SentryEvent: To add the merge_context function and provide a base class for event objects.
• Android and Native SDKs: To integrate the new event processing pipeline.
• Godot Engine: To capture screenshots and view hierarchies.

Risk Considerations

1. Thread Safety: The event processing pipeline and the management of event processors in SentryOptions may require thread safety considerations, especially if event processing or processor management can occur on different threads.
2. Performance: The addition of event processors and the before_send callback could impact performance, especially if the processors or callback are computationally expensive. Performance testing should be conducted to ensure that the impact is acceptable.
3. Backward Compatibility: The changes to the Android API for attachments may require updates to existing integrations.
4. Event Processor Errors: Errors within event processors or the before_send callback could potentially lead to event loss or unexpected behavior. Robust error handling and logging should be implemented.

Notable Implementation Details

1. The ScreenshotProcessor and ViewHierarchyProcessor capture screenshots and view hierarchies by writing them to the user:// directory and then attaching them to the event. This approach relies on the Godot Engine's file system and may have limitations in certain environments.
2. The merge_context function in NativeEvent directly modifies the underlying Sentry value, which could have unintended consequences if the value is shared or used elsewhere.
3. The before_send callback is executed after all event processors have been applied, which allows the callback to make final modifications to the event before it is sent to Sentry.

[limbonaut]

I addressed points that I found useful from the bot's review.

[bruno-garcia]

@romtsn maybe can peek at the kotlin? hoping @tustanivsky or @vaind can peek at the C++ 🙏

[romtsn]

I'm thinking this is done too early - e.g. if we exit early in one of the ifs below, like get_level or before_capture checks, we'd still memoize the last_screenshot_frame here although the screenshot hasn't been taken.

[limbonaut]

Addressed in 61e0e58

[romtsn]

kotlin LGTM, a couple of questions to the cpp code, but I'm no expert there so take it or leave it :)

[tustanivsky]

C++ looks good as well - really like this new approach with event processors!