Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

fix(ci): Consider build environment variables in expo sample app configuration #4583

Merged
merged 13 commits into from
Feb 25, 2025
Merged

fix(ci): Consider build environment variables in expo sample app configuration #4583

merged 13 commits into from
Feb 25, 2025

Conversation

antonis
Copy link
Collaborator

@antonis antonis commented Feb 24, 2025 •
edited
Loading

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Consider the SENTRY_DISABLE_AUTO_UPLOAD and SENTRY_DISABLE_NATIVE_DEBUG_UPLOAD environment variables in the Sentry Android Gradle Plugin Expo plugin configuration.

This is done by using shouldSentryAutoUpload in the generated gradle script similar to the gradle script of the react native sample.

💡 Motivation and Context

Fixes dependabot CI failure #4582 (comment)

💚 How did you test it?

CI, Manual

📝 Checklist

  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

🔮 Next steps

dependabot bot and others added 2 commits February 24, 2025 07:43
@dependabot
chore(deps): bump github/codeql-action from 3.28.9 to 3.28.10
16714ad 
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.28.9 to 3.28.10.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@9e8d078...b56ba49)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
@antonis
Use dynamic expo config for the sample
004e4fc
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 24, 2025 •
edited
Loading

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 427.15 ms 430.88 ms 3.73 ms
Size 17.75 MiB 20.12 MiB 2.37 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b8ff156 438.80 ms 454.14 ms 15.34 ms
cdc3945 422.77 ms 438.19 ms 15.42 ms
c991c90 418.78 ms 404.16 ms -14.62 ms
9f0f6c8 433.91 ms 429.16 ms -4.75 ms
9cab16b 463.30 ms 455.06 ms -8.24 ms
8e80789 430.76 ms 431.45 ms 0.69 ms
60ad48c 430.73 ms 432.85 ms 2.12 ms
8b86336 424.98 ms 413.06 ms -11.92 ms
30189be 440.46 ms 435.60 ms -4.85 ms
686b3bc 469.59 ms 474.41 ms 4.82 ms

App size

Revision Plain With Sentry Diff
b8ff156 17.74 MiB 20.09 MiB 2.35 MiB
cdc3945 17.75 MiB 20.12 MiB 2.37 MiB
c991c90 17.75 MiB 20.11 MiB 2.36 MiB
9f0f6c8 17.74 MiB 20.08 MiB 2.35 MiB
9cab16b 17.74 MiB 20.08 MiB 2.34 MiB
8e80789 17.74 MiB 20.10 MiB 2.36 MiB
60ad48c 17.75 MiB 20.12 MiB 2.37 MiB
8b86336 17.75 MiB 20.11 MiB 2.36 MiB
30189be 17.75 MiB 20.11 MiB 2.36 MiB
686b3bc 17.75 MiB 20.11 MiB 2.37 MiB

Previous results on branch: antonis/expo-sample-dynamic-config

Startup times

Revision Plain With Sentry Diff
fe41768 393.79 ms 407.44 ms 13.65 ms
15fca99 418.10 ms 444.15 ms 26.05 ms
26d9070 422.06 ms 420.02 ms -2.04 ms
1508db3 404.95 ms 399.95 ms -5.00 ms

App size

Revision Plain With Sentry Diff
fe41768 17.75 MiB 20.12 MiB 2.37 MiB
15fca99 17.75 MiB 20.12 MiB 2.37 MiB
26d9070 17.75 MiB 20.12 MiB 2.37 MiB
1508db3 17.75 MiB 20.12 MiB 2.37 MiB

@antonis antonis changed the title fix(ci): Use dynamic expo config for the sample fix(ci): Consider SENTRY_DISABLE_AUTO_UPLOAD in expo sample app configuration Feb 24, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 24, 2025 •
edited
Loading

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 371.16 ms 353.08 ms -18.08 ms
Size 7.15 MiB 8.39 MiB 1.23 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
4cc5c27+dirty 398.36 ms 452.50 ms 54.14 ms
bab7feb+dirty 377.47 ms 415.60 ms 38.13 ms
8de2810+dirty 368.43 ms 412.20 ms 43.77 ms
80e955a+dirty 399.50 ms 453.16 ms 53.66 ms
e2b64fe+dirty 258.82 ms 304.26 ms 45.44 ms
9f0f6c8+dirty 365.53 ms 419.16 ms 53.63 ms
488c9c5+dirty 448.98 ms 531.62 ms 82.64 ms
21a0abb+dirty 383.90 ms 442.02 ms 58.12 ms
d8992c6+dirty 376.55 ms 393.98 ms 17.43 ms
de59d3a+dirty 439.90 ms 454.20 ms 14.30 ms

App size

Revision Plain With Sentry Diff
4cc5c27+dirty 7.15 MiB 8.22 MiB 1.07 MiB
bab7feb+dirty 7.15 MiB 8.38 MiB 1.23 MiB
8de2810+dirty 7.15 MiB 8.35 MiB 1.20 MiB
80e955a+dirty 7.15 MiB 8.38 MiB 1.23 MiB
e2b64fe+dirty 7.15 MiB 8.07 MiB 947.16 KiB
9f0f6c8+dirty 7.15 MiB 8.37 MiB 1.22 MiB
488c9c5+dirty 7.15 MiB 8.38 MiB 1.23 MiB
21a0abb+dirty 7.15 MiB 8.38 MiB 1.23 MiB
d8992c6+dirty 7.15 MiB 8.39 MiB 1.23 MiB
de59d3a+dirty 7.15 MiB 8.38 MiB 1.23 MiB

Previous results on branch: antonis/expo-sample-dynamic-config

Startup times

Revision Plain With Sentry Diff
fe41768+dirty 411.71 ms 395.98 ms -15.73 ms
15fca99+dirty 381.84 ms 371.82 ms -10.02 ms
26d9070+dirty 418.23 ms 411.46 ms -6.77 ms
1508db3+dirty 479.88 ms 474.36 ms -5.52 ms

App size

Revision Plain With Sentry Diff
fe41768+dirty 7.15 MiB 8.39 MiB 1.23 MiB
15fca99+dirty 7.15 MiB 8.39 MiB 1.23 MiB
26d9070+dirty 7.15 MiB 8.39 MiB 1.23 MiB
1508db3+dirty 7.15 MiB 8.39 MiB 1.23 MiB

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 24, 2025 •
edited
Loading

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1216.76 ms 1222.27 ms 5.51 ms
Size 2.63 MiB 3.75 MiB 1.12 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8e80789+dirty 1228.04 ms 1237.20 ms 9.16 ms
c2a4e9b+dirty 1240.10 ms 1239.22 ms -0.88 ms
5fe8312+dirty 1222.06 ms 1220.56 ms -1.51 ms
d361d38+dirty 1246.04 ms 1267.12 ms 21.08 ms
a18210a+dirty 1237.15 ms 1224.85 ms -12.29 ms
1faf8e3+dirty 1214.87 ms 1222.83 ms 7.97 ms
cdf2bdf+dirty 1230.10 ms 1238.08 ms 7.98 ms
77680ec+dirty 1231.81 ms 1238.35 ms 6.54 ms
b95b8af+dirty 1221.39 ms 1228.52 ms 7.13 ms
86d6d2c+dirty 1267.55 ms 1286.21 ms 18.66 ms

App size

Revision Plain With Sentry Diff
8e80789+dirty 2.36 MiB 3.10 MiB 759.43 KiB
c2a4e9b+dirty 2.36 MiB 3.08 MiB 734.00 KiB
5fe8312+dirty 2.63 MiB 3.70 MiB 1.06 MiB
d361d38+dirty 2.36 MiB 2.85 MiB 499.84 KiB
a18210a+dirty 2.63 MiB 3.70 MiB 1.06 MiB
1faf8e3+dirty 2.36 MiB 3.08 MiB 736.75 KiB
cdf2bdf+dirty 2.36 MiB 3.12 MiB 779.40 KiB
77680ec+dirty 2.36 MiB 3.10 MiB 753.42 KiB
b95b8af+dirty 2.36 MiB 3.14 MiB 793.32 KiB
86d6d2c+dirty 2.36 MiB 2.82 MiB 462.82 KiB

Previous results on branch: antonis/expo-sample-dynamic-config

Startup times

Revision Plain With Sentry Diff
fe41768+dirty 1211.07 ms 1218.04 ms 6.98 ms
1508db3+dirty 1210.41 ms 1214.87 ms 4.46 ms
15fca99+dirty 1223.69 ms 1228.02 ms 4.33 ms
26d9070+dirty 1222.27 ms 1240.60 ms 18.33 ms

App size

Revision Plain With Sentry Diff
fe41768+dirty 2.63 MiB 3.75 MiB 1.12 MiB
1508db3+dirty 2.63 MiB 3.75 MiB 1.12 MiB
15fca99+dirty 2.63 MiB 3.75 MiB 1.12 MiB
26d9070+dirty 2.63 MiB 3.75 MiB 1.12 MiB

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 24, 2025 •
edited
Loading

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1230.02 ms 1234.51 ms 4.49 ms
Size 3.19 MiB 4.32 MiB 1.13 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8e80789+dirty 1235.66 ms 1223.78 ms -11.88 ms
c2a4e9b+dirty 1247.39 ms 1243.04 ms -4.35 ms
5fe8312+dirty 1235.07 ms 1220.41 ms -14.66 ms
d361d38+dirty 1272.96 ms 1291.70 ms 18.74 ms
a18210a+dirty 1218.06 ms 1215.49 ms -2.57 ms
1faf8e3+dirty 1223.38 ms 1220.56 ms -2.82 ms
cdf2bdf+dirty 1237.04 ms 1237.86 ms 0.82 ms
77680ec+dirty 1237.65 ms 1236.24 ms -1.41 ms
b95b8af+dirty 1235.60 ms 1242.06 ms 6.46 ms
86d6d2c+dirty 1291.62 ms 1296.80 ms 5.18 ms

App size

Revision Plain With Sentry Diff
8e80789+dirty 2.92 MiB 3.67 MiB 772.00 KiB
c2a4e9b+dirty 2.92 MiB 3.64 MiB 739.91 KiB
5fe8312+dirty 3.19 MiB 4.26 MiB 1.08 MiB
d361d38+dirty 2.92 MiB 3.41 MiB 503.57 KiB
a18210a+dirty 3.19 MiB 4.26 MiB 1.08 MiB
1faf8e3+dirty 2.92 MiB 3.64 MiB 742.61 KiB
cdf2bdf+dirty 2.92 MiB 3.69 MiB 790.57 KiB
77680ec+dirty 2.92 MiB 3.66 MiB 758.54 KiB
b95b8af+dirty 2.92 MiB 3.69 MiB 794.16 KiB
86d6d2c+dirty 2.92 MiB 3.37 MiB 464.31 KiB

Previous results on branch: antonis/expo-sample-dynamic-config

Startup times

Revision Plain With Sentry Diff
fe41768+dirty 1224.94 ms 1230.76 ms 5.82 ms
1508db3+dirty 1213.76 ms 1217.40 ms 3.64 ms
15fca99+dirty 1209.66 ms 1224.28 ms 14.62 ms
26d9070+dirty 1212.40 ms 1216.11 ms 3.70 ms

App size

Revision Plain With Sentry Diff
fe41768+dirty 3.19 MiB 4.32 MiB 1.13 MiB
1508db3+dirty 3.19 MiB 4.32 MiB 1.13 MiB
15fca99+dirty 3.19 MiB 4.32 MiB 1.13 MiB
26d9070+dirty 3.19 MiB 4.32 MiB 1.13 MiB

@antonis antonis changed the title fix(ci): Consider SENTRY_DISABLE_AUTO_UPLOAD in expo sample app configuration fix(ci): Consider build environment variables in expo sample app configuration Feb 24, 2025
antonis
antonis commented Feb 24, 2025
View reviewed changes
@antonis antonis changed the base branch from dependabot/github_actions/github/codeql-action-3.28.10 to main February 24, 2025 11:46
@antonis antonis marked this pull request as ready for review February 24, 2025 12:04
@antonis antonis mentioned this pull request Feb 24, 2025
Closed
6 tasks
@krystofwoldrich
Copy link
Member

Adding shouldSentryAutoUpload definitely makes sense, but I think we should not read the variable in the Expo settings, but we should add it to the generated code.

The difference is that Expo settings apply during the native project generation and not during the build.

@antonis antonis marked this pull request as draft February 24, 2025 12:39
@antonis antonis marked this pull request as ready for review February 24, 2025 14:03
@antonis
Copy link
Collaborator Author

antonis commented Feb 24, 2025

Thank you for your feedback @krystofwoldrich 🙇

The difference is that Expo settings apply during the native project generation and not during the build.

I've updated the PR changing the generated code.

@antonis antonis mentioned this pull request Feb 25, 2025
Merged
krystofwoldrich
krystofwoldrich approved these changes Feb 25, 2025
View reviewed changes
Copy link
Member

@krystofwoldrich krystofwoldrich left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Thank you for the fix.

This is a good catch we missed in the initial implementation.

@antonis antonis merged commit c81e67f into main Feb 25, 2025
71 checks passed
@antonis antonis deleted the antonis/expo-sample-dynamic-config branch February 25, 2025 14:16
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@krystofwoldrich krystofwoldrich krystofwoldrich approved these changes

@lucas-zimerman lucas-zimerman Awaiting requested review from lucas-zimerman lucas-zimerman is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@antonis @krystofwoldrich