feat: Adds SAGP as an experimental expo plugin feature #4440
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 86d6d2c+dirty | 1291.62 ms | 1296.80 ms | 5.18 ms |
| 52a8031+dirty | 1255.96 ms | 1273.00 ms | 17.04 ms |
| e5c9b8b+dirty | 1276.90 ms | 1280.92 ms | 4.02 ms |
| acadc0f+dirty | 1271.12 ms | 1272.28 ms | 1.16 ms |
| 52c0562+dirty | 1233.94 ms | 1226.29 ms | -7.65 ms |
| 8e80789+dirty | 1235.66 ms | 1223.78 ms | -11.88 ms |
| 0db0c72+dirty | 1258.88 ms | 1262.52 ms | 3.64 ms |
| 9433f35+dirty | 1232.24 ms | 1232.74 ms | 0.50 ms |
| e2b64fe+dirty | 1285.78 ms | 1297.56 ms | 11.78 ms |
| c639edf+dirty | 1223.63 ms | 1227.98 ms | 4.35 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 86d6d2c+dirty | 2.92 MiB | 3.37 MiB | 464.31 KiB |
| 52a8031+dirty | 2.92 MiB | 3.38 MiB | 475.71 KiB |
| e5c9b8b+dirty | 2.92 MiB | 3.43 MiB | 524.50 KiB |
| acadc0f+dirty | 2.92 MiB | 3.39 MiB | 487.34 KiB |
| 52c0562+dirty | 2.92 MiB | 3.69 MiB | 794.15 KiB |
| 8e80789+dirty | 2.92 MiB | 3.67 MiB | 772.00 KiB |
| 0db0c72+dirty | 2.92 MiB | 3.40 MiB | 492.71 KiB |
| 9433f35+dirty | 2.92 MiB | 3.41 MiB | 503.55 KiB |
| e2b64fe+dirty | 2.92 MiB | 3.41 MiB | 499.97 KiB |
| c639edf+dirty | 2.92 MiB | 3.64 MiB | 742.55 KiB |
Previous results on branch: antonis/experimental-expo-sagp
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a05231a+dirty | 1225.98 ms | 1234.84 ms | 8.86 ms |
| c5d03fc+dirty | 1230.90 ms | 1235.12 ms | 4.22 ms |
| e165be8+dirty | 1228.63 ms | 1245.41 ms | 16.78 ms |
| 28fa62b+dirty | 1214.88 ms | 1214.82 ms | -0.06 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a05231a+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
| c5d03fc+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
| e165be8+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
| 28fa62b+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 86d6d2c+dirty | 1267.55 ms | 1286.21 ms | 18.66 ms |
| 52a8031+dirty | 1280.88 ms | 1289.78 ms | 8.90 ms |
| e5c9b8b+dirty | 1258.57 ms | 1267.32 ms | 8.75 ms |
| acadc0f+dirty | 1264.38 ms | 1290.06 ms | 25.68 ms |
| 52c0562+dirty | 1219.34 ms | 1221.25 ms | 1.91 ms |
| 8e80789+dirty | 1228.04 ms | 1237.20 ms | 9.16 ms |
| 0db0c72+dirty | 1275.02 ms | 1285.84 ms | 10.82 ms |
| 9433f35+dirty | 1246.94 ms | 1271.45 ms | 24.52 ms |
| e2b64fe+dirty | 1232.22 ms | 1255.20 ms | 22.98 ms |
| c639edf+dirty | 1236.18 ms | 1235.04 ms | -1.14 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 86d6d2c+dirty | 2.36 MiB | 2.82 MiB | 462.82 KiB |
| 52a8031+dirty | 2.36 MiB | 2.82 MiB | 469.44 KiB |
| e5c9b8b+dirty | 2.36 MiB | 2.87 MiB | 520.43 KiB |
| acadc0f+dirty | 2.36 MiB | 2.83 MiB | 480.37 KiB |
| 52c0562+dirty | 2.36 MiB | 3.14 MiB | 793.36 KiB |
| 8e80789+dirty | 2.36 MiB | 3.10 MiB | 759.43 KiB |
| 0db0c72+dirty | 2.36 MiB | 2.84 MiB | 487.01 KiB |
| 9433f35+dirty | 2.36 MiB | 2.85 MiB | 499.80 KiB |
| e2b64fe+dirty | 2.36 MiB | 2.85 MiB | 495.80 KiB |
| c639edf+dirty | 2.36 MiB | 3.08 MiB | 736.63 KiB |
Previous results on branch: antonis/experimental-expo-sagp
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a05231a+dirty | 1232.37 ms | 1242.12 ms | 9.75 ms |
| c5d03fc+dirty | 1214.23 ms | 1214.45 ms | 0.21 ms |
| e165be8+dirty | 1226.43 ms | 1225.46 ms | -0.97 ms |
| 28fa62b+dirty | 1222.57 ms | 1223.91 ms | 1.34 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a05231a+dirty | 2.63 MiB | 3.68 MiB | 1.05 MiB |
| c5d03fc+dirty | 2.63 MiB | 3.69 MiB | 1.05 MiB |
| e165be8+dirty | 2.63 MiB | 3.69 MiB | 1.05 MiB |
| 28fa62b+dirty | 2.63 MiB | 3.69 MiB | 1.05 MiB |
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 52c0562+dirty | 401.23 ms | 435.65 ms | 34.42 ms |
| 5f03ae9+dirty | 375.36 ms | 401.26 ms | 25.90 ms |
| 63ed251+dirty | 485.02 ms | 531.16 ms | 46.14 ms |
| a989877+dirty | 383.04 ms | 400.92 ms | 17.88 ms |
| 1332acb+dirty | 385.00 ms | 404.80 ms | 19.80 ms |
| dadc233+dirty | 363.19 ms | 370.37 ms | 7.18 ms |
| c639edf+dirty | 363.39 ms | 414.78 ms | 51.39 ms |
| 9cab16b+dirty | 370.82 ms | 416.37 ms | 45.55 ms |
| 488c9c5+dirty | 448.98 ms | 531.62 ms | 82.64 ms |
| ac41368+dirty | 395.91 ms | 451.17 ms | 55.26 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 52c0562+dirty | 7.15 MiB | 8.39 MiB | 1.24 MiB |
| 5f03ae9+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
| 63ed251+dirty | 7.15 MiB | 8.35 MiB | 1.20 MiB |
| a989877+dirty | 7.15 MiB | 8.35 MiB | 1.20 MiB |
| 1332acb+dirty | 7.15 MiB | 8.37 MiB | 1.22 MiB |
| dadc233+dirty | 7.15 MiB | 8.04 MiB | 910.84 KiB |
| c639edf+dirty | 7.15 MiB | 8.35 MiB | 1.20 MiB |
| 9cab16b+dirty | 7.15 MiB | 8.35 MiB | 1.20 MiB |
| 488c9c5+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
| ac41368+dirty | 7.15 MiB | 8.39 MiB | 1.24 MiB |
Previous results on branch: antonis/experimental-expo-sagp
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| e165be8+dirty | 403.17 ms | 433.17 ms | 30.00 ms |
| a05231a+dirty | 410.49 ms | 464.94 ms | 54.45 ms |
| c5d03fc+dirty | 400.53 ms | 450.84 ms | 50.31 ms |
| 28fa62b+dirty | 467.78 ms | 451.33 ms | -16.45 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| e165be8+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
| a05231a+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
| c5d03fc+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
| 28fa62b+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9cab16b | 463.30 ms | 455.06 ms | -8.24 ms |
| 43e66e0 | 373.32 ms | 366.57 ms | -6.75 ms |
| e73d82f | 475.82 ms | 506.55 ms | 30.73 ms |
| 4a6664f | 548.79 ms | 585.00 ms | 36.21 ms |
| 728164b | 414.34 ms | 449.22 ms | 34.88 ms |
| ac41368 | 451.47 ms | 453.67 ms | 2.20 ms |
| 3853f43 | 329.68 ms | 346.32 ms | 16.64 ms |
| 4297324 | 536.61 ms | 542.48 ms | 5.87 ms |
| 9385d74 | 432.71 ms | 423.56 ms | -9.15 ms |
| b8ff156 | 438.80 ms | 454.14 ms | 15.34 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9cab16b | 17.74 MiB | 20.08 MiB | 2.34 MiB |
| 43e66e0 | 17.74 MiB | 20.09 MiB | 2.35 MiB |
| e73d82f | 17.73 MiB | 20.07 MiB | 2.33 MiB |
| 4a6664f | 17.73 MiB | 19.94 MiB | 2.21 MiB |
| 728164b | 17.73 MiB | 19.85 MiB | 2.12 MiB |
| ac41368 | 17.73 MiB | 20.11 MiB | 2.38 MiB |
| 3853f43 | 17.73 MiB | 19.81 MiB | 2.08 MiB |
| 4297324 | 17.74 MiB | 20.08 MiB | 2.34 MiB |
| 9385d74 | 17.74 MiB | 20.09 MiB | 2.35 MiB |
| b8ff156 | 17.74 MiB | 20.09 MiB | 2.35 MiB |
Previous results on branch: antonis/experimental-expo-sagp
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| e165be8 | 451.06 ms | 466.90 ms | 15.84 ms |
| a05231a | 459.71 ms | 465.70 ms | 5.99 ms |
| c5d03fc | 422.04 ms | 418.83 ms | -3.21 ms |
| 28fa62b | 420.92 ms | 423.72 ms | 2.80 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| e165be8 | 17.75 MiB | 20.11 MiB | 2.37 MiB |
| a05231a | 17.75 MiB | 20.11 MiB | 2.36 MiB |
| c5d03fc | 17.75 MiB | 20.11 MiB | 2.37 MiB |
| 28fa62b | 17.75 MiB | 20.11 MiB | 2.37 MiB |
antonis
requested review from
krystofwoldrich and
lucas-zimerman
as code owners
| import { withAppBuildGradle, withProjectBuildGradle } from '@expo/config-plugins'; | ||
|
|
||
| export interface SentryAndroidGradlePluginOptions { | ||
| sentryAndroidGradlePluginVersion?: string; |
There was a problem hiding this comment.
Let's bake in a version that's compatible with current RN SDK and keep it our responsibility instead of users having to pick SAGP version.
There was a problem hiding this comment.
Updated with 2539a24 to hardcode the SAGP version and use enableAndroidGradlePlugin as a flag to enable.
Comment on lines
19
to
28
| export function withSentryAndroidGradlePlugin(config: any, options: SentryAndroidGradlePluginOptions = {}): any { | ||
| const version = options.sentryAndroidGradlePluginVersion; | ||
| const includeProguardMapping = options.includeProguardMapping ?? true; | ||
| const dexguardEnabled = options.dexguardEnabled ?? false; | ||
| const autoUploadProguardMapping = options.autoUploadProguardMapping ?? true; | ||
| const uploadNativeSymbols = options.uploadNativeSymbols ?? true; | ||
| const autoUploadNativeSymbols = options.autoUploadNativeSymbols ?? true; | ||
| const includeNativeSources = options.includeNativeSources ?? true; | ||
| const includeSourceContext = options.includeSourceContext ?? false; | ||
| const autoInstallationEnabled = options.autoInstallationEnabled ?? false; |
There was a problem hiding this comment.
We can set the defaults when deconstructing the options argument, it makes it easier to read and removes the names repetition.
Suggested change
| export function withSentryAndroidGradlePlugin(config: any, options: SentryAndroidGradlePluginOptions = {}): any { | |
| const version = options.sentryAndroidGradlePluginVersion; | |
| const includeProguardMapping = options.includeProguardMapping ?? true; | |
| const dexguardEnabled = options.dexguardEnabled ?? false; | |
| const autoUploadProguardMapping = options.autoUploadProguardMapping ?? true; | |
| const uploadNativeSymbols = options.uploadNativeSymbols ?? true; | |
| const autoUploadNativeSymbols = options.autoUploadNativeSymbols ?? true; | |
| const includeNativeSources = options.includeNativeSources ?? true; | |
| const includeSourceContext = options.includeSourceContext ?? false; | |
| const autoInstallationEnabled = options.autoInstallationEnabled ?? false; | |
| export function withSentryAndroidGradlePlugin( | |
| config: any, | |
| { | |
| sentryAndroidGradlePluginVersion, | |
| includeProguardMapping = true, | |
| dexguardEnabled = false, | |
| autoUploadProguardMapping = true, | |
| uploadNativeSymbols = true, | |
| autoUploadNativeSymbols = true, | |
| includeNativeSources = true, | |
| includeSourceContext = false, | |
| autoInstallationEnabled = false | |
| }: SentryAndroidGradlePluginOptions = {} | |
| ): any { | |
| const version = sentryAndroidGradlePluginVersion; |
There was a problem hiding this comment.
Good idea 🙇
Updated with 59e54ce
| const withSentryAppBuildGradle = (config: any): any => { | ||
| return withAppBuildGradle(config, (config: any) => { | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access | ||
| if (config.modResults.language === 'groovy') { |
There was a problem hiding this comment.
Let's invert this and use it as a guard !== groovy.
There was a problem hiding this comment.
Makes sense 👍 Updated with 7b053f5
| // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access | ||
| config.modResults.contents = contents; | ||
| } else { | ||
| throw new Error('Cannot configure Sentry in android/app/build.gradle because it is not in Groovy.'); |
There was a problem hiding this comment.
Let's use the warnOnce helper instead of throw. It will make the message more readable for the users.
There was a problem hiding this comment.
Good idea 👍
Updated with b1230bd
|
|
||
| // Modify android/build.gradle | ||
| const withSentryProjectBuildGradle = (config: any): any => { | ||
| return withProjectBuildGradle(config, (projectBuildGradle: any) => { |
There was a problem hiding this comment.
Should we also check if the lang is groovy?
There was a problem hiding this comment.
Good point 👍 Added a check with 9d6d9d2
Comment on lines
43
to
46
| projectBuildGradle.modResults.contents = projectBuildGradle.modResults.contents.replace( | ||
| /dependencies\s*{/, | ||
| `dependencies {\n ${dependency}`, | ||
| ); |
There was a problem hiding this comment.
Let's also add a warning if we could not add the dependency.
|
When this is merged and release we should also update the docs. Maybe a sub page to https://docs.sentry.io/platforms/react-native/manual-setup/expo/ (not to interrupt the default setup flow with the gradle options). |
Makes sense 👍 I followed up with a docs PR for this. |
6 tasks
# Conflicts: # CHANGELOG.md
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📢 Type of change
📜 Description
Adds SAGP as an experimental expo plugin when the
enableAndroidGradlePluginis enabled in theexperimental_androidsection of the@sentry/react-native/expoplugin.The supported options are the following:
autoUploadProguardMappingincludeProguardMappingdexguardEnableduploadNativeSymbolsautoUploadNativeSymbolsincludeNativeSourcesincludeSourceContext💡 Motivation and Context
Fixes #4400
💚 How did you test it?
CI, Manual testing:
expo prebuildandroid/build.gradleandandroid/app/build.gradleare modified according to the docs📝 Checklist
sendDefaultPIIis enabled🔮 Next steps
See getsentry/sentry-android-gradle-plugin#809