Upgrade sentry to 5.0.1

oguzkocer commented 3 years ago

This PR upgrades Sentry to 5.0.1 and Sentry plugin to 2.0.0. This was a required change for Gradle 7.1.1 & Android Gradle Plugin 4.2.2 upgrade.

I took this opportunity to ignore the .idea/compiler.xml file. We have had an internal Slack discussion about this in the past and we decided to ignore it. However, it looks like we didn't make the change in this library. I typically don't like to mix & match changes, but this was such a small change that I decided to include it. Please let me know if you prefer it to be a separate PR, or if you don't want to ignore it in this library.
I initially ported all the extra functions in CrashLogging but then I realized they were not being used. I dropped anything unused which should help us port crash logging to the Tracks library (if we want) which has all the bells & whistles.
I also dropped setTag("version", BuildConfig.VERSION_NAME) and explained the reason as a code comment so we can discuss it in place.

To test:

Test Sentry integration - I am actually not familiar with the current setup, so I am hoping @aforcier & @cameronvoell can help me with testing.

peril-automattic[bot] commented 3 years ago

You can test the changes on this Pull Request by downloading the APK here.

aforcier commented 3 years ago

@oguzkocer I'm not at all familiar with the Sentry config, but it looks like something's off, the demo app crashes on launch for me (debug or release):

9051-9051/com.automattic.loop E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.automattic.loop, PID: 9051
    java.lang.RuntimeException: Unable to get provider io.sentry.android.core.SentryInitProvider: java.lang.IllegalArgumentException: DSN is required. Use empty string to disable SDK.
        at android.app.ActivityThread.installProvider(ActivityThread.java:7251)
        at android.app.ActivityThread.installContentProviders(ActivityThread.java:6787)
        at android.app.ActivityThread.handleBindApplication(ActivityThread.java:6704)
        at android.app.ActivityThread.access$1300(ActivityThread.java:237)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1913)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loop(Looper.java:223)
        at android.app.ActivityThread.main(ActivityThread.java:7664)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)

Seems to be happening extremely early, even before App.onCreate().

Side note: It took me a while to figure out what the right configure command was to decrypt the secrets:

./gradlew configure? ./gradlew applyConfiguration? bundle exec fastlane run configure_update?

It turned out the answer was ./gradlew updateConfiguration. Might it make sense to document this in https://github.com/Automattic/configure or something? (Though even this seems to do weird stuff, like copy gradle.properties.enc to the project root folder.)

Also .configure-files/automattic.jks.enc seems unused?

oguzkocer commented 3 years ago

@aforcier I am sorry about the crash, I've fixed that in 9dfd8c6dfa2d61c3bb4f401c229feac831393e22.

It turned out the answer was ./gradlew updateConfiguration. Might it make sense to document this in https://github.com/Automattic/configure or something? (Though even this seems to do weird stuff, like copy gradle.properties.enc to the project root folder.)

The correct task to use is applyConfiguration as this will copy the files from the secrets repo using the provided hash, source and destination. updateConfiguration is to update the .configure file to the latest hash and to update the encrypted files.

I've also opened a PR to add this information to the plugin, please feel free to review that and provide feedback: https://github.com/Automattic/configure/pull/20

Note that there is a known bug in the plugin where it doesn't copy the correct binary file after a version update. It just assumes that the binary version is correct. Unfortunately we never got around to fixing this. So, there is a chance you might have hit this issue. If you did, you can remove the vendor/ folder to fix it.

Also .configure-files/automattic.jks.enc seems unused?

Yeah, it seems like it's not used anymore. We can fix that the next time we update the configuration files. I don't think we want to combine it into this PR, but I can open a separate PR if it's bothering you.

@aforcier This is now ready for another look whenever you get a chance. Thanks for the review 🙇‍♂️

aforcier commented 3 years ago

Seems to be working @oguzkocer! The demo app builds now, and a thrown exception in the release build gets logged correctly in Sentry:

https://sentry.io/share/issue/d177bb6d6a92405dbecca32536006799/

(BTW could we rename that Sentry project from portkey-android to stories-android to match the new repository name? I'd do it but my Sentry rank isn't high enough.)

The correct task to use is applyConfiguration as this will copy the files from the secrets repo using the provided hash, source and destination. updateConfiguration is to update the .configure file to the latest hash and to update the encrypted files.

Note that there is a known bug in the plugin where it doesn't copy the correct binary file after a version update. It just assumes that the binary version is correct. Unfortunately we never got around to fixing this. So, there is a chance you might have hit this issue. If you did, you can remove the vendor/ folder to fix it.

Ah. That failed for me before, cleaning things up and deleting vendor/ seems to have fixed it, thanks!

And thanks for updating the configure repo README 💯

Yeah, it seems like it's not used anymore. We can fix that the next time we update the configuration files. I don't think we want to combine it into this PR, but I can open a separate PR if it's bothering you.

Sounds good, no big deal, next time we update the configuration files is fine. At the time a lot of things seemed in a weird state so I thought I'd mention it, now that everything else is working fine it's not too concerning.

Anyway, :shipit: Thanks @oguzkocer !

oguzkocer commented 3 years ago

(BTW could we rename that Sentry project from portkey-android to stories-android to match the new repository name? I'd do it but my Sentry rank isn't high enough.)

@aforcier I've made this change and updated your role to be an admin.

Thanks again for the review 🙇‍♂️

aforcier commented 3 years ago

Thanks @oguzkocer ! 🙌

Automattic / stories-android

Upgrade sentry to 5.0.1 #707