Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modularise bugsnag-android into JVM, NDK, and ANR artefacts #522

Merged
merged 23 commits into from
Jul 18, 2019

Conversation

fractalwrench
Copy link
Contributor

@fractalwrench fractalwrench commented Jul 11, 2019

Goal

The current version of bugsnag-android includes JVM, NDK, and ANR error detection by default. We should modularise these into 3 separate artefacts, so that it's possible for users to exclude code/resources they do not use from their APK.

This should be an invisible change to anyone using bugsnag-android and bugsnag-android-ndk.

Changeset

  • Created 3 modules which publish a JVM, NDK, and ANR artefact
  • Created bugsnag-android and bugsnag-android-ndk module which publish the artefact intended for public consumption
  • Updated CI/mazerunner to install to mavenLocal rather than copying AARs
  • Updated contributing/readme instructions to account for new artefact structure
  • Removed AAR size badge
  • Altered ProGuard rules to retain BugsnagPlugin implementations
  • Extracted shared gradle logic for library projects (e.g. lintoptions) to root-level build.gradle
  • Disabled NDK crash detection by default for all artefacts
  • Updated example app to use mavenLocal rather than compiling module directly, as this was somewhat error-prone in location SO files
  • Migrated ANR code to separate module and SO file
  • Add an publicly accessible BugsnagPluginInterface that allows for registration of plugins
  • Create plugins for ANR + NDK module using BugsnagPluginInterface, which the Client attempts to load if the right Configuration option has been set

Tests

Installed the artefacts into a local maven repository, and tested:

  • depending on bugsnag-android allows JVM, NDK, and ANR errors to be captured
  • both bugsnag-plugin-android-anr and bugsnag-plugin-android-ndk can be excluded from the APK if their respective configuration options are disabled
  • When ANR/NDK detection are disabled using the bugsnag-android dependency, no shared libraries are loaded (improves init time from around 80ms to 50ms on a Nexus 5x)

Remaining work

Copy link
Contributor

@kattrali kattrali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mostly follow so far, and the changes make sense though with some naming questions:

  • Is bugsnag-android-ndk going to be published? If so this is a fairly significant break. If not, then bugsnag-android-ndk could continue to be an alias for bugsnag-android.
  • Were other naming conventions considered, like having a bugsnag-plugin-* namespace for the plugin artefacts? That would resolve the above point and separate reporting libs from plugin libs.

bugsnag-android-anr/README.md Outdated Show resolved Hide resolved
bugsnag-android-core/README.md Outdated Show resolved Hide resolved
bugsnag-android-ndk/README.md Outdated Show resolved Hide resolved
gradle.properties Outdated Show resolved Hide resolved
examples/sdk-app-example/build.gradle Show resolved Hide resolved
@fractalwrench fractalwrench force-pushed the artefact-modularisation branch 5 times, most recently from e2e8f9f to 4e7321d Compare July 11, 2019 15:31
@fractalwrench
Copy link
Contributor Author

As discussed I will adapt this PR to do the following:

bugsnag-android: meta package that depends on all 3 artefacts
bugsnag-android-ndk: meta package that depends on all 3 artefacts (identical to bugsnag-android)

bugsnag-plugin-android-jvm: JVM exception handling
bugsnag-plugin-android-anr: ANR detection
bugsnag-plugin-android-ndk: NDK handlers

@fractalwrench fractalwrench force-pushed the artefact-modularisation branch 2 times, most recently from c781975 to 04e9c0b Compare July 12, 2019 10:56
@fractalwrench fractalwrench changed the title Artefact modularisation Modularise bugsnag-android into JVM, NDK, and ANR artefacts Jul 12, 2019
Copy link
Contributor

@kattrali kattrali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a general point of organization, what happens if someone excludes the "jvm" plugin because they only want NDK and/or ANR reporting? It seems like the core constructs of error/session reporting are in the JVM plugin, but it may be a misunderstanding.

@fractalwrench fractalwrench marked this pull request as ready for review July 12, 2019 12:44
Copy link
Contributor

@kattrali kattrali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The modularization split looks good. I got a bit further into the implementation of the plugins and added a few questions.

bugsnag-android-core/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved
bugsnag-plugin-android-ndk/src/main/jni/utils/serializer.h Outdated Show resolved Hide resolved
gradle.properties Outdated Show resolved Hide resolved
features/fixtures/mazerunner/build.gradle Show resolved Hide resolved
bugsnag-plugin-android-anr/src/main/jni/bugsnag_anr.h Outdated Show resolved Hide resolved
bugsnag-plugin-android-anr/src/main/jni/utils/string.c Outdated Show resolved Hide resolved
bugsnag-plugin-android-anr/src/main/jni/bugsnag_anr.c Outdated Show resolved Hide resolved
@fractalwrench fractalwrench merged commit cbd480d into next Jul 18, 2019
@fractalwrench fractalwrench deleted the artefact-modularisation branch July 18, 2019 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants