Enable Spotlight in Android and iOS SDKs #4211

antonis · 2024-10-28T17:08:23Z

📢 Type of change

Bugfix
New feature
Enhancement
Refactoring

📜 Description

Passes spotlight initialisation option to the native Android and iOS SDKs. When a url is not passed the RN JS defaultSidecarUrl is used.

When spotlight is enabled on both JS and platform SDKs, JS events will be send twice. The duplication only affects the Developer Info tab and the rest of the UI remains uncluttered.

Tab	RN	RN + Cocoa	RN + Java
SDKs
Errors
Developer Info

💡 Motivation and Context

Fixes #3706

💚 How did you test it?

Manual testing, CI

📝 Checklist

I reviewed submitted code
I added tests to verify changes
No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
All tests passing
No breaking changes

🔮 Next steps

github-actions · 2024-10-28T17:33:26Z

Android (new) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	384.19 ms	422.10 ms	37.91 ms
Size	7.15 MiB	8.35 MiB	1.20 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`f06c879`+dirty	361.27 ms	407.88 ms	46.61 ms
`d43a46b`+dirty	417.65 ms	472.98 ms	55.33 ms
`5446992`+dirty	371.61 ms	390.00 ms	18.39 ms
`fe13591`+dirty	539.51 ms	597.92 ms	58.40 ms
`5a22220`+dirty	384.61 ms	419.06 ms	34.45 ms
`2534337`+dirty	597.14 ms	665.04 ms	67.90 ms
`e73d82f`+dirty	377.67 ms	407.06 ms	29.39 ms
`dadc233`+dirty	363.19 ms	370.37 ms	7.18 ms
`0677344`+dirty	288.40 ms	391.44 ms	103.04 ms
`70caa60`+dirty	308.83 ms	393.06 ms	84.23 ms

App size

Revision	Plain	With Sentry	Diff
`f06c879`+dirty	7.15 MiB	8.12 MiB	997.78 KiB
`d43a46b`+dirty	7.15 MiB	8.34 MiB	1.19 MiB
`5446992`+dirty	7.15 MiB	8.12 MiB	999.45 KiB
`fe13591`+dirty	7.15 MiB	8.35 MiB	1.20 MiB
`5a22220`+dirty	7.15 MiB	8.21 MiB	1.06 MiB
`2534337`+dirty	7.15 MiB	8.11 MiB	988.68 KiB
`e73d82f`+dirty	7.15 MiB	8.34 MiB	1.19 MiB
`dadc233`+dirty	7.15 MiB	8.04 MiB	910.84 KiB
`0677344`+dirty	7.15 MiB	8.07 MiB	949.80 KiB
`70caa60`+dirty	7.15 MiB	8.03 MiB	901.79 KiB

Previous results on branch: antonis/3706-enable-native-spotlight

Startup times

Revision	Plain	With Sentry	Diff
`205310a`+dirty	392.67 ms	453.48 ms	60.81 ms
`c290d35`+dirty	486.24 ms	545.96 ms	59.72 ms

App size

Revision	Plain	With Sentry	Diff
`205310a`+dirty	7.15 MiB	8.35 MiB	1.20 MiB
`c290d35`+dirty	7.15 MiB	8.35 MiB	1.20 MiB

github-actions · 2024-10-28T17:42:25Z

iOS (legacy) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1225.82 ms	1222.12 ms	-3.69 ms
Size	2.36 MiB	3.08 MiB	737.28 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`c2a4e9b`+dirty	1240.10 ms	1239.22 ms	-0.88 ms
`5571a20`+dirty	1203.57 ms	1204.57 ms	1.00 ms
`e2b64fe`+dirty	1232.22 ms	1255.20 ms	22.98 ms
`31fcca2`+dirty	1209.17 ms	1216.21 ms	7.04 ms
`c639edf`+dirty	1236.18 ms	1235.04 ms	-1.14 ms
`0d3e677`+dirty	1214.39 ms	1225.70 ms	11.31 ms
`9433f35`+dirty	1246.94 ms	1271.45 ms	24.52 ms
`b95b8af`+dirty	1221.39 ms	1228.52 ms	7.13 ms
`2ec71da`+dirty	1225.85 ms	1231.57 ms	5.72 ms
`62a750b`+dirty	1216.60 ms	1229.14 ms	12.54 ms

App size

Revision	Plain	With Sentry	Diff
`c2a4e9b`+dirty	2.36 MiB	3.08 MiB	734.00 KiB
`5571a20`+dirty	2.36 MiB	2.92 MiB	569.93 KiB
`e2b64fe`+dirty	2.36 MiB	2.85 MiB	495.80 KiB
`31fcca2`+dirty	2.36 MiB	2.90 MiB	552.95 KiB
`c639edf`+dirty	2.36 MiB	3.08 MiB	736.63 KiB
`0d3e677`+dirty	2.36 MiB	3.10 MiB	753.12 KiB
`9433f35`+dirty	2.36 MiB	2.85 MiB	499.80 KiB
`b95b8af`+dirty	2.36 MiB	3.14 MiB	793.32 KiB
`2ec71da`+dirty	2.36 MiB	3.13 MiB	784.66 KiB
`62a750b`+dirty	2.36 MiB	2.92 MiB	570.00 KiB

Previous results on branch: antonis/3706-enable-native-spotlight

Startup times

Revision	Plain	With Sentry	Diff
`c290d35`+dirty	1212.42 ms	1223.76 ms	11.34 ms
`205310a`+dirty	1218.18 ms	1218.54 ms	0.36 ms

App size

Revision	Plain	With Sentry	Diff
`c290d35`+dirty	2.36 MiB	3.08 MiB	736.00 KiB
`205310a`+dirty	2.36 MiB	3.08 MiB	737.04 KiB

github-actions · 2024-10-28T17:51:44Z

iOS (new) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1229.04 ms	1231.29 ms	2.24 ms
Size	2.92 MiB	3.64 MiB	743.04 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`c2a4e9b`+dirty	1247.39 ms	1243.04 ms	-4.35 ms
`5571a20`+dirty	1228.09 ms	1233.45 ms	5.36 ms
`e2b64fe`+dirty	1285.78 ms	1297.56 ms	11.78 ms
`31fcca2`+dirty	1222.04 ms	1226.51 ms	4.47 ms
`c639edf`+dirty	1223.63 ms	1227.98 ms	4.35 ms
`0d3e677`+dirty	1239.02 ms	1241.22 ms	2.20 ms
`9433f35`+dirty	1232.24 ms	1232.74 ms	0.50 ms
`b95b8af`+dirty	1235.60 ms	1242.06 ms	6.46 ms
`2ec71da`+dirty	1230.29 ms	1239.50 ms	9.21 ms
`62a750b`+dirty	1228.12 ms	1230.53 ms	2.41 ms

App size

Revision	Plain	With Sentry	Diff
`c2a4e9b`+dirty	2.92 MiB	3.64 MiB	739.91 KiB
`5571a20`+dirty	2.92 MiB	3.48 MiB	575.54 KiB
`e2b64fe`+dirty	2.92 MiB	3.41 MiB	499.97 KiB
`31fcca2`+dirty	2.92 MiB	3.46 MiB	557.31 KiB
`c639edf`+dirty	2.92 MiB	3.64 MiB	742.55 KiB
`0d3e677`+dirty	2.92 MiB	3.66 MiB	758.42 KiB
`9433f35`+dirty	2.92 MiB	3.41 MiB	503.55 KiB
`b95b8af`+dirty	2.92 MiB	3.69 MiB	794.16 KiB
`2ec71da`+dirty	2.92 MiB	3.69 MiB	791.06 KiB
`62a750b`+dirty	2.92 MiB	3.48 MiB	575.59 KiB

Previous results on branch: antonis/3706-enable-native-spotlight

Startup times

Revision	Plain	With Sentry	Diff
`c290d35`+dirty	1245.04 ms	1244.04 ms	-1.00 ms
`205310a`+dirty	1222.87 ms	1222.64 ms	-0.23 ms

App size

Revision	Plain	With Sentry	Diff
`c290d35`+dirty	2.92 MiB	3.64 MiB	742.12 KiB
`205310a`+dirty	2.92 MiB	3.64 MiB	743.02 KiB

github-actions · 2024-10-28T18:17:40Z

Android (legacy) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	456.02 ms	438.89 ms	-17.13 ms
Size	17.74 MiB	20.08 MiB	2.34 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`f06c879`	408.41 ms	424.54 ms	16.13 ms
`0ebca77`	414.93 ms	444.49 ms	29.56 ms
`e2b64fe`	316.88 ms	330.23 ms	13.35 ms
`148f924`	492.65 ms	500.28 ms	7.63 ms
`d7401ac`+dirty	375.20 ms	383.51 ms	8.31 ms
`34aba08`	328.10 ms	342.84 ms	14.74 ms
`62a750b`	395.96 ms	423.36 ms	27.41 ms
`1c65324`	426.37 ms	460.36 ms	33.99 ms
`86d6d2c`+dirty	332.90 ms	352.45 ms	19.55 ms
`e73d82f`	475.82 ms	506.55 ms	30.73 ms

App size

Revision	Plain	With Sentry	Diff
`f06c879`	17.73 MiB	19.85 MiB	2.12 MiB
`0ebca77`	17.73 MiB	19.95 MiB	2.21 MiB
`e2b64fe`	17.73 MiB	19.80 MiB	2.07 MiB
`148f924`	17.73 MiB	19.94 MiB	2.21 MiB
`d7401ac`+dirty	17.73 MiB	19.75 MiB	2.02 MiB
`34aba08`	17.73 MiB	19.80 MiB	2.07 MiB
`62a750b`	17.73 MiB	19.93 MiB	2.20 MiB
`1c65324`	17.73 MiB	19.95 MiB	2.21 MiB
`86d6d2c`+dirty	17.73 MiB	20.04 MiB	2.31 MiB
`e73d82f`	17.73 MiB	20.07 MiB	2.33 MiB

Previous results on branch: antonis/3706-enable-native-spotlight

Startup times

Revision	Plain	With Sentry	Diff
`c290d35`	472.70 ms	458.52 ms	-14.18 ms

App size

Revision	Plain	With Sentry	Diff
`c290d35`	17.74 MiB	20.08 MiB	2.34 MiB

lucas-zimerman

PR Looks good to me, Thank you for the contribution!

packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt

packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.mm

krystofwoldrich · 2024-10-29T15:35:32Z

RN Default URL is based on the Metro dev server host, this is slightly different from the platform SDK, which use localhost as default.

Depending on the effort it would be good to use the same default.

sentry-react-native/packages/core/src/js/integrations/spotlight.ts

Line 86 in ddcabff

function getDefaultSidecarUrl(): string {

This should ensure that Dev Builds running on Real devices will be also sending platform events to spotlight.

krystofwoldrich · 2024-10-29T15:38:53Z

When spotlight is enabled on both JS an platform SDKs, JS event will be send twice. This will be super useful for debugging the SDK, but for Spotlight customers this should not result in duplicates in the Spotlight UI (only in Developer Tab).

Maybe you already check this. But let's confirm that it works this way.

antonis · 2024-10-29T16:43:29Z

Thank you for your feedback @krystofwoldrich 🙇

RN Default URL is based on the Metro dev server host, this is slightly different from the platform SDK, which use localhost as default.

iOS uses http://localhost:8969/stream and Android may use http://10.0.2.2:8969/stream as default.

Depending on the effort it would be good to use the same default.

If I understand correctly, we want to use the same default values on the RN side (or vice versa)?

When spotlight is enabled on both JS an platform SDKs, JS event will be send twice. This will be super useful for debugging the SDK, but for Spotlight customers this should not result in duplicates in the Spotlight UI (only in Developer Tab).

I will check this 🙏

krystofwoldrich · 2024-10-30T10:52:34Z

RN Default URL is based on the Metro dev server host, this is slightly different from the platform SDK, which use localhost as default.

iOS uses http://localhost:8969/stream and Android may use http://10.0.2.2:8969/stream as default.

Those are the default of the native SDKs. I was not exact, the same default meant the default set in RN.

In RN JS we take the host of the Metro dev server as host of Spotlight (the assumption is that the dev server and spotlight run on the same computer). We should pass the RN JS default to the platform layers when Spotlight is enabled.

antonis · 2024-10-30T14:58:32Z

When spotlight is enabled on both JS an platform SDKs, JS event will be send twice. This will be super useful for debugging the SDK, but for Spotlight customers this should not result in duplicates in the Spotlight UI (only in Developer Tab).

Maybe you already check this. But let's confirm that it works this way.

Rechecking this I think the behaviour is the expected. The errors are not duplicated and only the events on the Developer Info screen are recorded twice.

Tab	RN	RN + Cocoa	RN + Java
SDKs
Errors
Developer Info

…t is enabled

antonis · 2024-10-30T17:40:44Z

Thanks again for the feedback 🙇
Ready for another pass :)

# Conflicts: # CHANGELOG.md

CHANGELOG.md

packages/core/ios/RNSentry.mm

lucas-zimerman · 2024-10-31T14:24:28Z

This PR is also a reminder to me that I need to do the same on Capacitor

Co-authored-by: LucasZF <[email protected]>

packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java

krystofwoldrich

Awesome, thanks for adding the screens from testing and more platform tests. 🚀

BYK · 2024-10-31T22:29:45Z

Hey @antonis, thanks so much for this! 🥳

github-actions · 2024-11-01T05:00:05Z

	Fails
🚫	Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Enable Spotlight in Android and iOS SDKs ([#4211](https://github.com/getsentry/sentry-react-native/pull/4211))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 5ca9da0

antonis added 2 commits October 28, 2024 19:06

Passes spotlight initialisation option to the native SDKs

5c65d80

Adds changelog

00f43e6

antonis added 2 commits October 28, 2024 19:57

Extracts getSentryAndroidOptions method and adds test

e8cf604

Fixes lint issue

11c48ad

antonis marked this pull request as ready for review October 28, 2024 18:06

antonis requested review from krystofwoldrich and lucas-zimerman as code owners October 28, 2024 18:06

lucas-zimerman approved these changes Oct 28, 2024

View reviewed changes

krystofwoldrich reviewed Oct 29, 2024

View reviewed changes

packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt Show resolved Hide resolved

krystofwoldrich reviewed Oct 29, 2024

View reviewed changes

packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.mm Show resolved Hide resolved

antonis added 2 commits October 29, 2024 17:57

Handles spotlight url parameters in cocoa

cc5383f

Handles spotlight url parameters in java

5635885

antonis added 4 commits October 30, 2024 18:11

Pass the RN JS defaultSidecarUrl to the platform layers when Spotligh…

1fe5435

…t is enabled

Merge branch 'main' into antonis/3706-enable-native-spotlight

a9fbf0c

Fixes Android initialisation issue

93f7992

Merge branch 'main' into antonis/3706-enable-native-spotlight

f615224

antonis requested review from krystofwoldrich and lucas-zimerman October 30, 2024 17:40

Merge branch 'main' into antonis/3706-enable-native-spotlight

674bd98

# Conflicts: # CHANGELOG.md

krystofwoldrich reviewed Oct 31, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

lucas-zimerman reviewed Oct 31, 2024

View reviewed changes

packages/core/ios/RNSentry.mm Outdated Show resolved Hide resolved

antonis and others added 2 commits October 31, 2024 16:26

Update CHANGELOG using the features section

9e58c9b

Update log message to communicate the custom spotlight url used clearly

5ca9da0

Co-authored-by: LucasZF <[email protected]>

krystofwoldrich reviewed Oct 31, 2024

View reviewed changes

packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java Show resolved Hide resolved

krystofwoldrich reviewed Oct 31, 2024

View reviewed changes

packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java Show resolved Hide resolved

krystofwoldrich approved these changes Oct 31, 2024

View reviewed changes

antonis merged commit 9cab16b into main Oct 31, 2024
59 of 60 checks passed

antonis deleted the antonis/3706-enable-native-spotlight branch October 31, 2024 15:31

Enable Spotlight in Android and iOS SDKs #4211

Enable Spotlight in Android and iOS SDKs #4211

Conversation

antonis commented Oct 28, 2024 • edited Loading

📢 Type of change

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

github-actions bot commented Oct 28, 2024 • edited Loading

Android (new) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: antonis/3706-enable-native-spotlight

Startup times

App size

github-actions bot commented Oct 28, 2024 • edited Loading

iOS (legacy) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: antonis/3706-enable-native-spotlight

Startup times

App size

github-actions bot commented Oct 28, 2024 • edited Loading

iOS (new) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: antonis/3706-enable-native-spotlight

Startup times

App size

github-actions bot commented Oct 28, 2024 • edited Loading

Android (legacy) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: antonis/3706-enable-native-spotlight

Startup times

App size

lucas-zimerman left a comment

Choose a reason for hiding this comment

krystofwoldrich commented Oct 29, 2024 • edited Loading

krystofwoldrich commented Oct 29, 2024

antonis commented Oct 29, 2024

krystofwoldrich commented Oct 30, 2024

antonis commented Oct 30, 2024 • edited Loading

antonis commented Oct 30, 2024

lucas-zimerman commented Oct 31, 2024

krystofwoldrich left a comment

Choose a reason for hiding this comment

BYK commented Oct 31, 2024

github-actions bot commented Nov 1, 2024

Instructions and example for changelog

antonis commented Oct 28, 2024 •

edited

Loading

github-actions bot commented Oct 28, 2024 •

edited

Loading

github-actions bot commented Oct 28, 2024 •

edited

Loading

github-actions bot commented Oct 28, 2024 •

edited

Loading

github-actions bot commented Oct 28, 2024 •

edited

Loading

krystofwoldrich commented Oct 29, 2024 •

edited

Loading

antonis commented Oct 30, 2024 •

edited

Loading