[78] Use latest Sentry

[sakuraehikaru]

Related Issue: #78

Summary of Problem:

We should use the newest Sentry SDK to minimize chances of problems in reporting.

Proposed Solution:

• Updated various Sentry version markers
• Removed autoProguardConfig and updated syntax (needed the equal signs now)
  • see https://docs.sentry.io/platforms/android/migration/
• Bumped version of Steamclog (not sure if this is done right 🤔)

[ssawchenko]

Seems reasonable - although I haven't looked at the Sentry upgrade steps yet. Did you test out sending crash reports via the sample app to make sure that nothing went wrong with the library upgrade? In the past I've had some issues with proguard and sentry not playing well together. Also it looks like the changelog needs updating :)

As for bumping the version number we haven't really discussed a release workflow or schedule for the library, but I'll keep the bumped version in mind when we discuss that. I've created a ticket for that here: #80

[sakuraehikaru]

Hi @ssawchenko and @jacobminer the changelog is updated, and I've verified that the demo app works. Sentry and proguard also don't seem to have a problem in this update. However, just to be sure, how do we spot a Sentry and proguard issue? Is there an obvious symptom like the app not building, or is it more subtle which I could have totally missed? Thanks!

[jacobminer]

@jeremychiang Proguard is likely only run on release builds, but we can also probably just make sure we add the Proguard files from this documentation
I think the only lines we need to add/update should be in the proguard-rules.pro file

-keep class io.sentry.android.core.SentryAndroidOptions
-keep class io.sentry.android.ndk.SentryNdk

I think the most typical symptom of a Proguard issue would be a crash in a release build at runtime, which is pretty unfortunate. 😅

[jacobminer]

Once the proguard file is updated, this looks good to go!

[sakuraehikaru]

Hi @jacobminer, those 2 lines seem specific to "Multi-Dex Support", and I couldn't find the usage of multiDexKeepProguard in either the app nor steamclog gradle files.

Furthermore, at the top of the proguard documentation, it says:

The Android SDK ships with ProGuard rules automatically defined; no further configuration is needed.

And in the migration guide here, it says:

The autoProguardConfig flag has been removed, the Android SDK >= 2.0.0 version already merges the ProGuard rules automatically

so I am wondering if we still need those 2 lines, can you help me confirm? Thanks!

[jacobminer]

@jeremychiang Seems reasonable to me!

[sakuraehikaru]

Hi @ssawchenko, I think this is ready to be merged. Just wanted to get another final confirmation from you, especially on this topic.

[ssawchenko]

However, just to be sure, how do we spot a Sentry and proguard issue? Is there an obvious symptom like the app not building, or is it more subtle which I could have totally missed? Thanks!

I have setup both debug and release builds of the sample app to use proguard:

    buildTypes {
        debug {
            debuggable true
            minifyEnabled true // Always minify so that we can test proguard
            proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro'
        }
        release {
            minifyEnabled true
            proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro'
        }
    }

So by enabling crash reporting and then sending off a test crash on the sample app you should in theory be testing out the proguard setup on a debug build.

I believe on Sentry there is actually a red warning box when you drill into the issue that indicates that the symbols are not available, but I also look down at the stack trace and make sure that each item has a properly named functions:

For example, if the proguard mappings were not correctly uploaded the functions above would say something like MainActivity in A at line 140
instead of
MainActivity in testNonFatalWithThrowable at line 140

It looks like the latest Sentry reports are correct, so this should be good to go :)

[sakuraehikaru]

Thanks @ssawchenko. I will merge and tag this release.

[ssawchenko]

Hi @ssawchenko, I think this is ready to be merged. Just wanted to get another final confirmation from you, especially on this topic.

As for the migration steps, I think that looks good to me. I'm also a little unsure about the mulitdex details Sentry has listed. Since we have a min SDK of 21 on all of our projects, we don't need to explicitly setup multidex on the project itself, but I can't find verification in the Sentry documentation that indicates if that means we do not need those extra steps. The sample app isn't big enough to trigger the count limit, but I'll have a look at CIDirect tomorrow to see if it may be applicable there.

Since we weren't taking it into account in Steamclog previously, I'd say we move ahead with committing this and we can address multidex support (if required) here - #81