feat(expo): Automatically detect Release name and version for Expo Web#4910
feat(expo): Automatically detect Release name and version for Expo Web#4910antonis wants to merge 6 commits into
Conversation
|
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| e12044e | 453.78 ms | 437.58 ms | -16.20 ms |
| 7d3c3cb | 444.85 ms | 456.65 ms | 11.81 ms |
| 69721ae | 424.85 ms | 415.28 ms | -9.57 ms |
| 1f1c420 | 403.32 ms | 411.98 ms | 8.66 ms |
| f2c6fa5 | 445.15 ms | 449.13 ms | 3.98 ms |
| b4d6bde | 425.51 ms | 417.37 ms | -8.14 ms |
| 52d9c3f | 481.48 ms | 468.53 ms | -12.95 ms |
| 6e8a851 | 425.59 ms | 433.51 ms | 7.92 ms |
| a8a0930 | 397.23 ms | 383.78 ms | -13.45 ms |
| 940bd65 | 466.31 ms | 458.52 ms | -7.79 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| e12044e | 17.75 MiB | 20.15 MiB | 2.40 MiB |
| 7d3c3cb | 17.75 MiB | 20.15 MiB | 2.40 MiB |
| 69721ae | 17.75 MiB | 20.15 MiB | 2.40 MiB |
| 1f1c420 | 17.75 MiB | 20.15 MiB | 2.40 MiB |
| f2c6fa5 | 17.75 MiB | 20.15 MiB | 2.40 MiB |
| b4d6bde | 17.75 MiB | 20.15 MiB | 2.40 MiB |
| 52d9c3f | 17.75 MiB | 20.15 MiB | 2.40 MiB |
| 6e8a851 | 17.75 MiB | 20.15 MiB | 2.40 MiB |
| a8a0930 | 17.75 MiB | 20.15 MiB | 2.40 MiB |
| 940bd65 | 17.75 MiB | 20.15 MiB | 2.40 MiB |
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 940bd65+dirty | 408.45 ms | 419.75 ms | 11.30 ms |
| 4e2cbd2+dirty | 371.98 ms | 392.00 ms | 20.02 ms |
| 69721ae+dirty | 423.63 ms | 417.34 ms | -6.29 ms |
| 6e8a851+dirty | 403.44 ms | 430.87 ms | 27.43 ms |
| 1f1c420+dirty | 383.31 ms | 386.98 ms | 3.67 ms |
| 866f143+dirty | 378.91 ms | 368.73 ms | -10.19 ms |
| ec2a485+dirty | 397.67 ms | 390.91 ms | -6.76 ms |
| b4d6bde+dirty | 390.51 ms | 385.60 ms | -4.91 ms |
| 0e42017+dirty | 387.33 ms | 399.30 ms | 11.97 ms |
| 398e5d0+dirty | 422.88 ms | 444.98 ms | 22.10 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 940bd65+dirty | 7.15 MiB | 8.42 MiB | 1.26 MiB |
| 4e2cbd2+dirty | 7.15 MiB | 8.42 MiB | 1.26 MiB |
| 69721ae+dirty | 7.15 MiB | 8.42 MiB | 1.26 MiB |
| 6e8a851+dirty | 7.15 MiB | 8.42 MiB | 1.26 MiB |
| 1f1c420+dirty | 7.15 MiB | 8.42 MiB | 1.26 MiB |
| 866f143+dirty | 7.15 MiB | 8.42 MiB | 1.26 MiB |
| ec2a485+dirty | 7.15 MiB | 8.42 MiB | 1.26 MiB |
| b4d6bde+dirty | 7.15 MiB | 8.42 MiB | 1.26 MiB |
| 0e42017+dirty | 7.15 MiB | 8.42 MiB | 1.26 MiB |
| 398e5d0+dirty | 7.15 MiB | 8.42 MiB | 1.26 MiB |
Previous results on branch: antonis/expo-web-release-version
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f6bb454+dirty | 402.96 ms | 396.39 ms | -6.57 ms |
| d07e72f+dirty | 382.49 ms | 370.06 ms | -12.43 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f6bb454+dirty | 7.15 MiB | 8.42 MiB | 1.26 MiB |
| d07e72f+dirty | 7.15 MiB | 8.42 MiB | 1.26 MiB |
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a8a0930+dirty | 1212.11 ms | 1229.45 ms | 17.34 ms |
| 9e3030a+dirty | 1217.82 ms | 1205.60 ms | -12.21 ms |
| f2c6fa5+dirty | 1223.47 ms | 1227.37 ms | 3.90 ms |
| 7d3c3cb+dirty | 1214.56 ms | 1234.53 ms | 19.97 ms |
| ec2a485+dirty | 1209.65 ms | 1229.18 ms | 19.53 ms |
| 69721ae+dirty | 1251.53 ms | 1253.69 ms | 2.16 ms |
| b4d6bde+dirty | 1218.73 ms | 1223.26 ms | 4.53 ms |
| df5da5d+dirty | 1226.82 ms | 1234.88 ms | 8.06 ms |
| 6e8a851+dirty | 1222.57 ms | 1223.67 ms | 1.10 ms |
| 398e5d0+dirty | 1226.17 ms | 1232.82 ms | 6.65 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a8a0930+dirty | 3.19 MiB | 4.35 MiB | 1.17 MiB |
| 9e3030a+dirty | 3.19 MiB | 4.35 MiB | 1.17 MiB |
| f2c6fa5+dirty | 3.19 MiB | 4.35 MiB | 1.17 MiB |
| 7d3c3cb+dirty | 3.19 MiB | 4.35 MiB | 1.16 MiB |
| ec2a485+dirty | 3.19 MiB | 4.35 MiB | 1.17 MiB |
| 69721ae+dirty | 3.19 MiB | 4.35 MiB | 1.17 MiB |
| b4d6bde+dirty | 3.19 MiB | 4.34 MiB | 1.16 MiB |
| df5da5d+dirty | 3.19 MiB | 4.35 MiB | 1.17 MiB |
| 6e8a851+dirty | 3.19 MiB | 4.35 MiB | 1.17 MiB |
| 398e5d0+dirty | 3.19 MiB | 4.35 MiB | 1.17 MiB |
Previous results on branch: antonis/expo-web-release-version
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f6bb454+dirty | 1216.37 ms | 1239.54 ms | 23.17 ms |
| d07e72f+dirty | 1214.44 ms | 1232.20 ms | 17.77 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f6bb454+dirty | 3.19 MiB | 4.36 MiB | 1.18 MiB |
| d07e72f+dirty | 3.19 MiB | 4.35 MiB | 1.17 MiB |
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a8a0930+dirty | 1215.90 ms | 1227.82 ms | 11.92 ms |
| 1f1c420+dirty | 1216.77 ms | 1214.48 ms | -2.29 ms |
| 9e3030a+dirty | 1215.35 ms | 1234.80 ms | 19.45 ms |
| b4d6bde+dirty | 1223.22 ms | 1243.56 ms | 20.34 ms |
| 940bd65+dirty | 1216.88 ms | 1225.23 ms | 8.35 ms |
| 8bd8033+dirty | 1213.33 ms | 1220.88 ms | 7.55 ms |
| 69721ae+dirty | 1229.82 ms | 1226.00 ms | -3.82 ms |
| 398e5d0+dirty | 1225.30 ms | 1219.94 ms | -5.36 ms |
| ec2a485+dirty | 1219.72 ms | 1224.66 ms | 4.94 ms |
| bdb324a+dirty | 1229.33 ms | 1236.61 ms | 7.29 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a8a0930+dirty | 2.63 MiB | 3.79 MiB | 1.15 MiB |
| 1f1c420+dirty | 2.63 MiB | 3.77 MiB | 1.14 MiB |
| 9e3030a+dirty | 2.63 MiB | 3.79 MiB | 1.15 MiB |
| b4d6bde+dirty | 2.63 MiB | 3.77 MiB | 1.14 MiB |
| 940bd65+dirty | 2.63 MiB | 3.78 MiB | 1.15 MiB |
| 8bd8033+dirty | 2.63 MiB | 3.78 MiB | 1.15 MiB |
| 69721ae+dirty | 2.63 MiB | 3.79 MiB | 1.15 MiB |
| 398e5d0+dirty | 2.63 MiB | 3.79 MiB | 1.15 MiB |
| ec2a485+dirty | 2.63 MiB | 3.78 MiB | 1.15 MiB |
| bdb324a+dirty | 2.63 MiB | 3.79 MiB | 1.15 MiB |
Previous results on branch: antonis/expo-web-release-version
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f6bb454+dirty | 1217.15 ms | 1236.38 ms | 19.23 ms |
| d07e72f+dirty | 1226.68 ms | 1224.77 ms | -1.91 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f6bb454+dirty | 2.63 MiB | 3.80 MiB | 1.17 MiB |
| d07e72f+dirty | 2.63 MiB | 3.78 MiB | 1.15 MiB |
|
|
||
| ### Features | ||
|
|
||
| - Automatically detect Release name and version for Expo Web ([#4910](https://github.com/getsentry/sentry-react-native/pull/4910)) |
There was a problem hiding this comment.
i: We should mention that it only works in development builds.
If you try it on a release build you'll not get the release number
https://sentry-sdks.sentry.io/issues/6673524305/events/04e3a3759a7c4cc5bd4a63038d0c2d9b/?project=5428561
There was a problem hiding this comment.
Good point Lucas 👍
I think this is something we should tackle for the enhancement to make sense.
There was a problem hiding this comment.
If the application name and version are not available in production, I would not added it at all, to avoid confusing users with differences between dev and prod.
If Expo doesn't include any of the information in production on web, we could include the release name during the build (bundling process), utilizing our Sentry Metro Plugin.
There was a problem hiding this comment.
Hey @lucas-zimerman 👋
I've updated the PR with f2648ba to handle the release build case (example). The downside is that it depends on expo-constants and will fail if it is not available. I've tested other approaches like bundle injection, but either didn't work or they were overcomplicated for the task at hand.
I think this approach is simple and would work on most cases. As a fallback the user's can still set the release manually on the web as they do now. Let me know wdyt and I can clean up the PR and prepare it for review 🙇
There was a problem hiding this comment.
Hi @antonis and @lucas-zimerman,
I've taken a look at this again and I think the SDK should not depend on expo-constants, to keep compatibility with non-expo apps. I would also avoid the ENVs approach since it doesn't work both prod and dev.
I would propose to use a custom module to load a global variable with the release information. This will work for both prod and dev.
Similar to how Sentry JS Webpack Plugin is doing it, link.
This is, how it could be implemented: v7...krystofwoldrich:sentry-react-native:@krystofwoldrich/inject-expo-app-release#diff-de4a1259c31e8b44b35788d85ff5eb3d27eef51e73325b3ee41532d9095722cd
Let me know, if this would work for you, I can open a PR.
There was a problem hiding this comment.
Thank you for taking the time to look at this @krystofwoldrich. Your feedback is always welcome :)
I've taken a look at this again and I think the SDK should not depend on
expo-constants, to keep compatibility with non-expo apps.
I agree. This makes sense only as a best effort approach for expo web.
I would propose to use a custom module to load a global variable with the release information. This will work for both prod and dev.
Similar to how Sentry JS Webpack Plugin is doing it, link.
This is, how it could be implemented: v7...krystofwoldrich:sentry-react-native:@krystofwoldrich/inject-expo-app-release#diff-de4a1259c31e8b44b35788d85ff5eb3d27eef51e73325b3ee41532d9095722cd
Your approach looks great and of course a PR is more than welcome 🤗
There was a problem hiding this comment.
Let me know, if anything else is needed.
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
|
Closing in favor of #4967 |
3 participants
📢 Type of change
📜 Description
Automatically detect Release name and version for Expo Web
💡 Motivation and Context
Fixes #4894
💚 How did you test it?
Manual, CI
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps