Update Sentry sdk to v2.0

[claucookie]

Title: Update Sentry SDK to v2.0 Release Candidate

Issue: Making progress the against Encrypting Logs Issue #10698

Description:
We want to be able to link Sentry events (unhandled or caught exceptions) to future Encrypted logs we are gonna submit to Wordpress API.

This PR allows us to attach additional information to any Sentry event before it gets sent to Sentry server.

• Implemented already: Logic to encrypt the logs Issue/10698 encryption utils cleanup #11230 Add Encryption Functionality and Tests #10738
• Not implemented yet: The logic to generate the UUID and sending the encrypted log. (Next PR)

Why not using Sentry v1.X? Because there is a race condition when sending the event for caught exceptions. If we try to attach additional data or tags or anything before the event is sent, it seems the logic is not waiting for the info to be attached and sends the event anyway without it.

Why V2.0? With Sentry v2.0, we gain the ability to update the event with the future UUID we are going to generate in the next PR. E.g.

SentryAndroid.init(context.applicationContext) {
            it.dsn = BuildConfig.SENTRY_DSN
            it.beforeSend = BeforeSendCallback { event, hint ->
                // Todo: Update event with a UUID from the Logs we want to encrypt.
                return@BeforeSendCallback event
            }
        }

Testing instructions:
Enable CrashLogging momentarily for your Debug build by doing the following

# CrashloggingUtils.kt
fun shouldEnableCrashLogging(context: Context): Boolean {
        if (PackageUtils.isDebugBuild()) {
            return true  // <-- Change this from false to true (dont forget to revert it)
        }

        val prefs = PreferenceManager.getDefaultSharedPreferences(context)
        val hasUserOptedOut = !prefs.getBoolean(context.getString(R.string.pref_key_send_crash), true)
        return !hasUserOptedOut
    }

Build the project, deploy the app nd check out the logcat to see if the following messages are displayed:

2020-02-13 15:31:26.847 6970-6970/org.wordpress.android D/Sentry: Auto-init: false
2020-02-13 15:31:26.847 6970-6970/org.wordpress.android I/Sentry: Retrieving auto-init from AndroidManifest.xml

If there are no other errors, then it should be fine.

If we attach info to events, how is it gonna look like? Let's see a dummy Logs id sent as Additional Data

PR submission checklist:

• I have considered adding unit tests where possible.
• I have considered adding accessibility improvements for my changes.
• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[jkmassel]

The code for this looks good (I left one minor comment)

But I'm hitting a couple of issues that are concerning:

I'm not seeing any events coming in Sentry when I build and run the app in release mode, even if I override the shouldEnableCrashLogging callback. It seems to work fine in development mode on-device though if I do the same. Are you seeing the same issue?

Does this configuration support multi-dex? I noted that the config suggestion from the docs wasn't present here – will that matter?

Let me know, and thanks!!

[jkmassel]

I'm not sure we need to clear the breadcrumbs if we're closing down Sentry – in the older version clearing out the context would tear down Sentry completely so that we could re-init it later.

WDYT?

[claucookie]

That's a very good question 😄.
Just a bit of context. When I was looking for an alternative to .clearContext() I couldn't find any equivalent method in the v2.0 Sentry sdk.
Then, I checked what that method was doing inside to see if I could find an equivalent logic. By doing so I bumped into the v1.X implementation, which internally is calling this method contextManager.clear() :

# v1.X 
public synchronized void clear() {
        setLastEventId(null);
        clearBreadcrumbs();
        clearUser();
        clearTags();
        clearExtra();
    }

Since my thought was to preserve the behavior, I tried to see if the v2.0 sdk contained methods to clear those values, but the only one I could find is clearBreadcrumbs().

However, I'm having second thoughts since you well pointed out that clearContext() would tear down Sentry completely.

🤔 I would say that just calling clearBreadcrumbs() doesn't add any value. So I think we can remove that line of code (L44).

[jkmassel]

Sounds great!

[claucookie]

@jkmassel Answering your other questions here :)

I'm not seeing any events coming in Sentry when I build and run the app in release mode, even if I override the shouldEnableCrashLogging callback. It seems to work fine in development mode on-device though if I do the same. Are you seeing the same issue?

I am going to run the release build and have a look.

Does this configuration support multi-dex? I noted that the config suggestion from the docs wasn't present here – will that matter?

I missed that bit in the docs, not sure how. Anyway, We definitely use multidex in the project, but we haven't set up any rules for the release yet using:
multiDexKeepProguard file(...)
I think it would be a good idea to create a rules file and include the rules to keep the classes they mention.

[claucookie]

@jkmassel Answering your other questions here :)

I'm not seeing any events coming in Sentry when I build and run the app in release mode, even if I override the shouldEnableCrashLogging callback. It seems to work fine in development mode on-device though if I do the same. Are you seeing the same issue?

I am going to run the release build and have a look.

Does this configuration support multi-dex? I noted that the config suggestion from the docs wasn't present here – will that matter?

I missed that bit in the docs, not sure how. Anyway, We definitely use multidex in the project, but we haven't set up any rules for the release yet using:
multiDexKeepProguard file(...)
I think it would be a good idea to create a rules file and include the rules to keep the classes they mention.

Sorry for my late response, regarding events coming in Sentry when building the release. Yes, I do see the log messages from Sentry when running the release build:

2020-02-17 16:23:15.364 29448-29448/? D/Sentry: Auto-init: false
2020-02-17 16:23:15.364 29448-29448/? I/Sentry: Retrieving auto-init from AndroidManifest.xml
2020-02-17 16:25:56.729 29779-29779/? D/Sentry: Auto-init: false
2020-02-17 16:25:56.729 29779-29779/? I/Sentry: Retrieving auto-init from AndroidManifest.xml
2020-02-17 16:27:42.992 30892-30892/? D/Sentry: Auto-init: false
2020-02-17 16:27:42.992 30892-30892/? I/Sentry: Retrieving auto-init from AndroidManifest.xml

What's the configuration on your side? Did you update the wp.sentry.dsn property inside the gradle. properties? It might still have a dummy value in your local. 🤔

Also, the crashes go through and I can see them in the Sentry web console.

[jkmassel]

What's the configuration on your side? Did you update the wp.sentry.dsn property inside the gradle. properties? It might still have a dummy value in your local. 🤔

It's working now! I think it might've been something to do with our production credentials system – I re-applied them and was able to get it consistently working. Sorry about that!!

[jkmassel]

This works! Merging.