feat: BeforeCaptureScreenshot option

[bricefriha]

Relevant to #3353, I added a option to have a callback just before attaching a screenshot.

Note

I didn't had a closing keyword to this PR, as I don't believe that change alone fixes the core issue. However this change was demanded

[github-actions]

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 4ac2276

[bricefriha]

I did add a change log, maybe I did it wrong 🤔

update

it was an indentation issue, my bad

[bitsandfoxes]

Fwiw, PR titles we loosely follow a feat:, fix:, chore: convention. No need to add the issue number.

[bruno-garcia]

@romtsn how do we deal with this on Android for example?

[bruno-garcia]

Looks like we tried binding from Android but we never added Attachment on the .NET hint:

sentry-dotnet/src/Sentry/Platforms/Android/Extensions/HintExtensions.cs

Line 14 in 9b03645

//dotnetHint.Screenshot = (javaHint.Screenshot is { } screenshot) ? screenshot.ToAttachment() : null;

[bruno-garcia]

From in this PR:

• Add Hint support #2351

This:

Hints provide two specific functions:

They allow you to add or remove attachments during any of the mentioned callbacks. (Resolves #1469)

But unfortunately no docs anywhere to be found

[jamescrosswell]

The code looks good. I made a few styling suggestions.

I'm not entirely sure what this new option allows SDK users to do that they can't already do with SentryOptions.SetBeforeSend... The Android docs state:

Because capturing screenshots can be a resource-intensive operation on Android, it's limited to one screenshot every 2 seconds using a debouncing mechanism. This behavior can be overruled if you supply a BeforeCaptureCallback for screenshots in the SentryAndroidOptions.

The BeforeCaptureCallback also allows you to customize the behavior based on event data, so you can decide when to capture a screenshot and when not to. For example, you can decide to only capture screenshots of crashed and fatal events:

But it doesn't look like in the .NET implementation we're using the result of this callback in any way to determine whether a screenshot is added or not in SentryMauiScreenshotProcessor.cs.

I think it'd be worth adding an example of the new API to one of our samples, demonstrating how you can do something with it that can't already be achieved using SetBeforeSend. That'd be a good sanity check for us, as maintainers (to ensure we're building something meaningful), and would help demonstrate the value of the new functionality to SDK users.

[romtsn]

@romtsn how do we deal with this on Android for example?

we've added this callback because BeforeSend is already too late to determine whether we want to capture a screenshot or not, at that point the screenshot is already there and the performance cost is already paid. The callback is mainly targeting users who want to avoid capturing screenshots e.g. for errors (default behavior), and only do it for crashes.

FWIW, our callback does not return an event but a boolean that defines whether the screenshot should be captured or not, see: https://github.com/getsentry/sentry-java/blob/b5b093e5f805d0d02c7be344403804e7a7605398/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java#L77-L88

[jamescrosswell]

Just a couple of notes about comments but otherwise looks good, as long as CI is happy 👍🏻

It would be good to add some docs on this to the Sentry Docs as well.

[bricefriha]

Update

The callback works similarly to the Android SDK. It returns a boolean that determines whether to ignore the capture.

MAUI Sdk	Android Sdk

(details)

[jamescrosswell]

Functionally, looks good. I think just tweak the tests...

[jamescrosswell]

LGTM!

[bitsandfoxes]

Looks great to me!