From 5c65d805513d1a3db7e78e6ca3ed73e6abeb797d Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Mon, 28 Oct 2024 19:06:37 +0200 Subject: [PATCH 01/10] Passes spotlight initialisation option to the native SDKs --- .../RNSentryCocoaTesterTests/RNSentryTests.mm | 30 +++++++++++++++++++ .../io/sentry/react/RNSentryModuleImpl.java | 3 ++ packages/core/ios/RNSentry.mm | 5 ++++ 3 files changed, 38 insertions(+) diff --git a/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.mm b/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.mm index 615deb45c4..d0bc232877 100644 --- a/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.mm +++ b/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.mm @@ -122,6 +122,36 @@ - (void)testCreateOptionsWithDictionaryAutoPerformanceTracingDisabled XCTAssertEqual(actualOptions.enableAutoPerformanceTracing, false, @"Did not disable Auto Performance Tracing"); } +- (void)testCreateOptionsWithDictionarySpotlightEnabled +{ + RNSentry * rnSentry = [[RNSentry alloc] init]; + NSError* error = nil; + + NSDictionary *_Nonnull mockedReactNativeDictionary = @{ + @"dsn": @"https://abcd@efgh.ingest.sentry.io/123456", + @"spotlight": @YES, + }; + SentryOptions* actualOptions = [rnSentry createOptionsWithDictionary:mockedReactNativeDictionary error:&error]; + XCTAssertNotNil(actualOptions, @"Did not create sentry options"); + XCTAssertNil(error, @"Should not pass no error"); + XCTAssertTrue(actualOptions.enableSpotlight , @"Did not enable spotlight"); +} + +- (void)testCreateOptionsWithDictionarySpotlightDisabled +{ + RNSentry * rnSentry = [[RNSentry alloc] init]; + NSError* error = nil; + + NSDictionary *_Nonnull mockedReactNativeDictionary = @{ + @"dsn": @"https://abcd@efgh.ingest.sentry.io/123456", + @"spotlight": @NO, + }; + SentryOptions* actualOptions = [rnSentry createOptionsWithDictionary:mockedReactNativeDictionary error:&error]; + XCTAssertNotNil(actualOptions, @"Did not create sentry options"); + XCTAssertNil(error, @"Should not pass no error"); + XCTAssertFalse(actualOptions.enableSpotlight, @"Did not disable spotlight"); +} + - (void)testPassesErrorOnWrongDsn { RNSentry * rnSentry = [[RNSentry alloc] init]; diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index ee24c634c2..2f4f15fd37 100644 --- a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -254,6 +254,9 @@ public void initNativeSdk(final ReadableMap rnOptions, Promise promise) { if (rnOptions.hasKey("enableNdk")) { options.setEnableNdk(rnOptions.getBoolean("enableNdk")); } + if (rnOptions.hasKey("spotlight")) { + options.setEnableSpotlight(rnOptions.getBoolean("spotlight")); + } if (rnOptions.hasKey("_experiments")) { options.getExperimental().setSessionReplay(getReplayOptions(rnOptions)); options diff --git a/packages/core/ios/RNSentry.mm b/packages/core/ios/RNSentry.mm index a04733e7b5..547930827e 100644 --- a/packages/core/ios/RNSentry.mm +++ b/packages/core/ios/RNSentry.mm @@ -157,6 +157,11 @@ - (SentryOptions *_Nullable)createOptionsWithDictionary:(NSDictionary *_Nonnull) sentryOptions.integrations = integrations; } } + + // Set spotlight option + if ([mutableOptions valueForKey:@"spotlight"] != nil) { + sentryOptions.enableSpotlight = [mutableOptions[@"spotlight"] boolValue]; + } // Enable the App start and Frames tracking measurements if ([mutableOptions valueForKey:@"enableAutoPerformanceTracing"] != nil) { From 00f43e6e6ac12ffc48f548ec843a892b4ec42bf6 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Mon, 28 Oct 2024 19:13:00 +0200 Subject: [PATCH 02/10] Adds changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 765c58f9c6..9895348c8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ ## Unreleased +### Changes + +- Enables Spotlight in Android and iOS SDKs ([#4211](https://github.com/getsentry/sentry-react-native/pull/4211)) + ### Dependencies - Bump JavaScript SDK from v8.34.0 to v8.35.0 ([#4196](https://github.com/getsentry/sentry-react-native/pull/4196)) From e8cf604b7773b733cc9d1cb801257f7c6cf70504 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Mon, 28 Oct 2024 19:57:51 +0200 Subject: [PATCH 03/10] Extracts getSentryAndroidOptions method and adds test --- .../io/sentry/react/RNSentryModuleImplTest.kt | 8 + .../io/sentry/react/RNSentryModuleImpl.java | 249 +++++++++--------- 2 files changed, 132 insertions(+), 125 deletions(-) diff --git a/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt index 5a6585ab80..39efc3c140 100644 --- a/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt +++ b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt @@ -3,6 +3,7 @@ package io.sentry.react import android.content.pm.PackageInfo import android.content.pm.PackageManager import com.facebook.react.bridge.Arguments +import com.facebook.react.bridge.JavaOnlyMap import com.facebook.react.bridge.Promise import com.facebook.react.bridge.ReactApplicationContext import com.facebook.react.bridge.WritableMap @@ -93,4 +94,11 @@ class RNSentryModuleImplTest { val capturedMap = writableMapCaptor.value assertEquals(false, capturedMap.getBoolean("has_fetched")) } + + @Test + fun `when the spotlight option is enabled, the spotlight SentryAndroidOption is set to true`() { + val options = JavaOnlyMap.of("spotlight", true) + val actualOptions = module.getSentryAndroidOptions(options, logger) + assert(actualOptions.isEnableSpotlight) + } } diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index 2f4f15fd37..122d0a9271 100644 --- a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -173,139 +173,138 @@ public void initNativeReactNavigationNewFrameTracking(Promise promise) { public void initNativeSdk(final ReadableMap rnOptions, Promise promise) { SentryAndroid.init( - this.getReactApplicationContext(), - options -> { - @Nullable SdkVersion sdkVersion = options.getSdkVersion(); - if (sdkVersion == null) { - sdkVersion = new SdkVersion(ANDROID_SDK_NAME, BuildConfig.VERSION_NAME); - } else { - sdkVersion.setName(ANDROID_SDK_NAME); - } - - options.setSentryClientName(sdkVersion.getName() + "/" + sdkVersion.getVersion()); - options.setNativeSdkName(NATIVE_SDK_NAME); - options.setSdkVersion(sdkVersion); + this.getReactApplicationContext(), options -> getSentryAndroidOptions(rnOptions, logger)); - if (rnOptions.hasKey("debug") && rnOptions.getBoolean("debug")) { - options.setDebug(true); - } - if (rnOptions.hasKey("dsn") && rnOptions.getString("dsn") != null) { - String dsn = rnOptions.getString("dsn"); - logger.log(SentryLevel.INFO, String.format("Starting with DSN: '%s'", dsn)); - options.setDsn(dsn); - } else { - // SentryAndroid needs an empty string fallback for the dsn. - options.setDsn(""); - } - if (rnOptions.hasKey("sampleRate")) { - options.setSampleRate(rnOptions.getDouble("sampleRate")); - } - if (rnOptions.hasKey("sendClientReports")) { - options.setSendClientReports(rnOptions.getBoolean("sendClientReports")); - } - if (rnOptions.hasKey("maxBreadcrumbs")) { - options.setMaxBreadcrumbs(rnOptions.getInt("maxBreadcrumbs")); - } - if (rnOptions.hasKey("maxCacheItems")) { - options.setMaxCacheItems(rnOptions.getInt("maxCacheItems")); - } - if (rnOptions.hasKey("environment") && rnOptions.getString("environment") != null) { - options.setEnvironment(rnOptions.getString("environment")); - } - if (rnOptions.hasKey("release") && rnOptions.getString("release") != null) { - options.setRelease(rnOptions.getString("release")); - } - if (rnOptions.hasKey("dist") && rnOptions.getString("dist") != null) { - options.setDist(rnOptions.getString("dist")); - } - if (rnOptions.hasKey("enableAutoSessionTracking")) { - options.setEnableAutoSessionTracking(rnOptions.getBoolean("enableAutoSessionTracking")); - } - if (rnOptions.hasKey("sessionTrackingIntervalMillis")) { - options.setSessionTrackingIntervalMillis( - rnOptions.getInt("sessionTrackingIntervalMillis")); - } - if (rnOptions.hasKey("shutdownTimeout")) { - options.setShutdownTimeoutMillis(rnOptions.getInt("shutdownTimeout")); - } - if (rnOptions.hasKey("enableNdkScopeSync")) { - options.setEnableScopeSync(rnOptions.getBoolean("enableNdkScopeSync")); - } - if (rnOptions.hasKey("attachStacktrace")) { - options.setAttachStacktrace(rnOptions.getBoolean("attachStacktrace")); - } - if (rnOptions.hasKey("attachThreads")) { - // JS use top level stacktrace and android attaches Threads which hides them so - // by default we hide. - options.setAttachThreads(rnOptions.getBoolean("attachThreads")); - } - if (rnOptions.hasKey("attachScreenshot")) { - options.setAttachScreenshot(rnOptions.getBoolean("attachScreenshot")); - } - if (rnOptions.hasKey("attachViewHierarchy")) { - options.setAttachViewHierarchy(rnOptions.getBoolean("attachViewHierarchy")); - } - if (rnOptions.hasKey("sendDefaultPii")) { - options.setSendDefaultPii(rnOptions.getBoolean("sendDefaultPii")); - } - if (rnOptions.hasKey("maxQueueSize")) { - options.setMaxQueueSize(rnOptions.getInt("maxQueueSize")); - } - if (rnOptions.hasKey("enableNdk")) { - options.setEnableNdk(rnOptions.getBoolean("enableNdk")); - } - if (rnOptions.hasKey("spotlight")) { - options.setEnableSpotlight(rnOptions.getBoolean("spotlight")); - } - if (rnOptions.hasKey("_experiments")) { - options.getExperimental().setSessionReplay(getReplayOptions(rnOptions)); - options - .getReplayController() - .setBreadcrumbConverter(new RNSentryReplayBreadcrumbConverter()); - } - options.setBeforeSend( - (event, hint) -> { - // React native internally throws a JavascriptException - // Since we catch it before that, we don't want to send this one - // because we would send it twice - try { - SentryException ex = event.getExceptions().get(0); - if (null != ex && ex.getType().contains("JavascriptException")) { - return null; - } - } catch (Throwable ignored) { // NOPMD - We don't want to crash in any case - // We do nothing - } + promise.resolve(true); + } - setEventOriginTag(event); - addPackages(event, options.getSdkVersion()); + protected SentryAndroidOptions getSentryAndroidOptions(@NotNull ReadableMap rnOptions, ILogger logger) { + final SentryAndroidOptions options = new SentryAndroidOptions(); + @Nullable SdkVersion sdkVersion = options.getSdkVersion(); + if (sdkVersion == null) { + sdkVersion = new SdkVersion(ANDROID_SDK_NAME, BuildConfig.VERSION_NAME); + } else { + sdkVersion.setName(ANDROID_SDK_NAME); + } - return event; - }); + options.setSentryClientName(sdkVersion.getName() + "/" + sdkVersion.getVersion()); + options.setNativeSdkName(NATIVE_SDK_NAME); + options.setSdkVersion(sdkVersion); - if (rnOptions.hasKey("enableNativeCrashHandling") - && !rnOptions.getBoolean("enableNativeCrashHandling")) { - final List integrations = options.getIntegrations(); - for (final Integration integration : integrations) { - if (integration instanceof UncaughtExceptionHandlerIntegration - || integration instanceof AnrIntegration - || integration instanceof NdkIntegration) { - integrations.remove(integration); - } + if (rnOptions.hasKey("debug") && rnOptions.getBoolean("debug")) { + options.setDebug(true); + } + if (rnOptions.hasKey("dsn") && rnOptions.getString("dsn") != null) { + String dsn = rnOptions.getString("dsn"); + logger.log(SentryLevel.INFO, String.format("Starting with DSN: '%s'", dsn)); + options.setDsn(dsn); + } else { + // SentryAndroid needs an empty string fallback for the dsn. + options.setDsn(""); + } + if (rnOptions.hasKey("sampleRate")) { + options.setSampleRate(rnOptions.getDouble("sampleRate")); + } + if (rnOptions.hasKey("sendClientReports")) { + options.setSendClientReports(rnOptions.getBoolean("sendClientReports")); + } + if (rnOptions.hasKey("maxBreadcrumbs")) { + options.setMaxBreadcrumbs(rnOptions.getInt("maxBreadcrumbs")); + } + if (rnOptions.hasKey("maxCacheItems")) { + options.setMaxCacheItems(rnOptions.getInt("maxCacheItems")); + } + if (rnOptions.hasKey("environment") && rnOptions.getString("environment") != null) { + options.setEnvironment(rnOptions.getString("environment")); + } + if (rnOptions.hasKey("release") && rnOptions.getString("release") != null) { + options.setRelease(rnOptions.getString("release")); + } + if (rnOptions.hasKey("dist") && rnOptions.getString("dist") != null) { + options.setDist(rnOptions.getString("dist")); + } + if (rnOptions.hasKey("enableAutoSessionTracking")) { + options.setEnableAutoSessionTracking(rnOptions.getBoolean("enableAutoSessionTracking")); + } + if (rnOptions.hasKey("sessionTrackingIntervalMillis")) { + options.setSessionTrackingIntervalMillis(rnOptions.getInt("sessionTrackingIntervalMillis")); + } + if (rnOptions.hasKey("shutdownTimeout")) { + options.setShutdownTimeoutMillis(rnOptions.getInt("shutdownTimeout")); + } + if (rnOptions.hasKey("enableNdkScopeSync")) { + options.setEnableScopeSync(rnOptions.getBoolean("enableNdkScopeSync")); + } + if (rnOptions.hasKey("attachStacktrace")) { + options.setAttachStacktrace(rnOptions.getBoolean("attachStacktrace")); + } + if (rnOptions.hasKey("attachThreads")) { + // JS use top level stacktrace and android attaches Threads which hides them so + // by default we hide. + options.setAttachThreads(rnOptions.getBoolean("attachThreads")); + } + if (rnOptions.hasKey("attachScreenshot")) { + options.setAttachScreenshot(rnOptions.getBoolean("attachScreenshot")); + } + if (rnOptions.hasKey("attachViewHierarchy")) { + options.setAttachViewHierarchy(rnOptions.getBoolean("attachViewHierarchy")); + } + if (rnOptions.hasKey("sendDefaultPii")) { + options.setSendDefaultPii(rnOptions.getBoolean("sendDefaultPii")); + } + if (rnOptions.hasKey("maxQueueSize")) { + options.setMaxQueueSize(rnOptions.getInt("maxQueueSize")); + } + if (rnOptions.hasKey("enableNdk")) { + options.setEnableNdk(rnOptions.getBoolean("enableNdk")); + } + if (rnOptions.hasKey("spotlight")) { + options.setEnableSpotlight(rnOptions.getBoolean("spotlight")); + } + if (rnOptions.hasKey("_experiments")) { + options.getExperimental().setSessionReplay(getReplayOptions(rnOptions)); + options.getReplayController().setBreadcrumbConverter(new RNSentryReplayBreadcrumbConverter()); + } + options.setBeforeSend( + (event, hint) -> { + // React native internally throws a JavascriptException + // Since we catch it before that, we don't want to send this one + // because we would send it twice + try { + SentryException ex = event.getExceptions().get(0); + if (null != ex && ex.getType().contains("JavascriptException")) { + return null; } + } catch (Throwable ignored) { // NOPMD - We don't want to crash in any case + // We do nothing } - logger.log( - SentryLevel.INFO, - String.format("Native Integrations '%s'", options.getIntegrations())); - - final CurrentActivityHolder currentActivityHolder = CurrentActivityHolder.getInstance(); - final Activity currentActivity = getCurrentActivity(); - if (currentActivity != null) { - currentActivityHolder.setActivity(currentActivity); - } + + setEventOriginTag(event); + addPackages(event, options.getSdkVersion()); + + return event; }); - promise.resolve(true); + if (rnOptions.hasKey("enableNativeCrashHandling") + && !rnOptions.getBoolean("enableNativeCrashHandling")) { + final List integrations = options.getIntegrations(); + for (final Integration integration : integrations) { + if (integration instanceof UncaughtExceptionHandlerIntegration + || integration instanceof AnrIntegration + || integration instanceof NdkIntegration) { + integrations.remove(integration); + } + } + } + logger.log( + SentryLevel.INFO, String.format("Native Integrations '%s'", options.getIntegrations())); + + final CurrentActivityHolder currentActivityHolder = CurrentActivityHolder.getInstance(); + final Activity currentActivity = getCurrentActivity(); + if (currentActivity != null) { + currentActivityHolder.setActivity(currentActivity); + } + return options; } private SentryReplayOptions getReplayOptions(@NotNull ReadableMap rnOptions) { From 11c48ad15cf85e629c5b8a3185a0611b7e5c7b0f Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Mon, 28 Oct 2024 20:03:36 +0200 Subject: [PATCH 04/10] Fixes lint issue --- .../src/main/java/io/sentry/react/RNSentryModuleImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index 122d0a9271..0acdecd095 100644 --- a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -178,7 +178,8 @@ public void initNativeSdk(final ReadableMap rnOptions, Promise promise) { promise.resolve(true); } - protected SentryAndroidOptions getSentryAndroidOptions(@NotNull ReadableMap rnOptions, ILogger logger) { + protected SentryAndroidOptions getSentryAndroidOptions( + @NotNull ReadableMap rnOptions, ILogger logger) { final SentryAndroidOptions options = new SentryAndroidOptions(); @Nullable SdkVersion sdkVersion = options.getSdkVersion(); if (sdkVersion == null) { From cc5383f61b71bb09f7797f666e2c58e8e0009273 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Tue, 29 Oct 2024 17:57:02 +0200 Subject: [PATCH 05/10] Handles spotlight url parameters in cocoa --- .../RNSentryCocoaTesterTests/RNSentryTests.mm | 46 +++++++++++++++++++ packages/core/ios/RNSentry.mm | 9 +++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.mm b/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.mm index d0bc232877..e966dfdf74 100644 --- a/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.mm +++ b/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.mm @@ -137,6 +137,37 @@ - (void)testCreateOptionsWithDictionarySpotlightEnabled XCTAssertTrue(actualOptions.enableSpotlight , @"Did not enable spotlight"); } +- (void)testCreateOptionsWithDictionarySpotlightOne +{ + RNSentry * rnSentry = [[RNSentry alloc] init]; + NSError* error = nil; + + NSDictionary *_Nonnull mockedReactNativeDictionary = @{ + @"dsn": @"https://abcd@efgh.ingest.sentry.io/123456", + @"spotlight": @1, + }; + SentryOptions* actualOptions = [rnSentry createOptionsWithDictionary:mockedReactNativeDictionary error:&error]; + XCTAssertNotNil(actualOptions, @"Did not create sentry options"); + XCTAssertNil(error, @"Should not pass no error"); + XCTAssertTrue(actualOptions.enableSpotlight , @"Did not enable spotlight"); +} + +- (void)testCreateOptionsWithDictionarySpotlightUrl +{ + RNSentry * rnSentry = [[RNSentry alloc] init]; + NSError* error = nil; + + NSDictionary *_Nonnull mockedReactNativeDictionary = @{ + @"dsn": @"https://abcd@efgh.ingest.sentry.io/123456", + @"spotlight": @"http://localhost:8969/teststream", + }; + SentryOptions* actualOptions = [rnSentry createOptionsWithDictionary:mockedReactNativeDictionary error:&error]; + XCTAssertNotNil(actualOptions, @"Did not create sentry options"); + XCTAssertNil(error, @"Should not pass no error"); + XCTAssertTrue(actualOptions.enableSpotlight , @"Did not enable spotlight"); + XCTAssertEqual(actualOptions.spotlightUrl , @"http://localhost:8969/teststream"); +} + - (void)testCreateOptionsWithDictionarySpotlightDisabled { RNSentry * rnSentry = [[RNSentry alloc] init]; @@ -152,6 +183,21 @@ - (void)testCreateOptionsWithDictionarySpotlightDisabled XCTAssertFalse(actualOptions.enableSpotlight, @"Did not disable spotlight"); } +- (void)testCreateOptionsWithDictionarySpotlightZero +{ + RNSentry * rnSentry = [[RNSentry alloc] init]; + NSError* error = nil; + + NSDictionary *_Nonnull mockedReactNativeDictionary = @{ + @"dsn": @"https://abcd@efgh.ingest.sentry.io/123456", + @"spotlight": @0, + }; + SentryOptions* actualOptions = [rnSentry createOptionsWithDictionary:mockedReactNativeDictionary error:&error]; + XCTAssertNotNil(actualOptions, @"Did not create sentry options"); + XCTAssertNil(error, @"Should not pass no error"); + XCTAssertFalse(actualOptions.enableSpotlight, @"Did not disable spotlight"); +} + - (void)testPassesErrorOnWrongDsn { RNSentry * rnSentry = [[RNSentry alloc] init]; diff --git a/packages/core/ios/RNSentry.mm b/packages/core/ios/RNSentry.mm index 547930827e..986f069305 100644 --- a/packages/core/ios/RNSentry.mm +++ b/packages/core/ios/RNSentry.mm @@ -160,7 +160,14 @@ - (SentryOptions *_Nullable)createOptionsWithDictionary:(NSDictionary *_Nonnull) // Set spotlight option if ([mutableOptions valueForKey:@"spotlight"] != nil) { - sentryOptions.enableSpotlight = [mutableOptions[@"spotlight"] boolValue]; + id spotlightValue = [mutableOptions valueForKey:@"spotlight"]; + if ([spotlightValue isKindOfClass:[NSString class]]) { + NSLog(@"The value is a string: %@", spotlightValue); + sentryOptions.enableSpotlight = true; + sentryOptions.spotlightUrl = spotlightValue; + } else if ([spotlightValue isKindOfClass:[NSNumber class]]) { + sentryOptions.enableSpotlight = [spotlightValue boolValue]; + } } // Enable the App start and Frames tracking measurements From 563588582cbc6bea454b97428a9c342fb06dfaa9 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Tue, 29 Oct 2024 18:19:23 +0200 Subject: [PATCH 06/10] Handles spotlight url parameters in java --- .../io/sentry/react/RNSentryModuleImplTest.kt | 16 ++++++++++++++++ .../java/io/sentry/react/RNSentryModuleImpl.java | 8 +++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt index 39efc3c140..fe5d144e07 100644 --- a/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt +++ b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt @@ -11,6 +11,7 @@ import io.sentry.ILogger import io.sentry.SentryLevel import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -101,4 +102,19 @@ class RNSentryModuleImplTest { val actualOptions = module.getSentryAndroidOptions(options, logger) assert(actualOptions.isEnableSpotlight) } + + @Test + fun `when the spotlight url is passed, the spotlight is enabled for the given url`() { + val options = JavaOnlyMap.of("spotlight", "http://localhost:8969/teststream") + val actualOptions = module.getSentryAndroidOptions(options, logger) + assert(actualOptions.isEnableSpotlight) + assertEquals("http://localhost:8969/teststream", actualOptions.spotlightConnectionUrl) + } + + @Test + fun `when the spotlight option is disabled, the spotlight SentryAndroidOption is set to false`() { + val options = JavaOnlyMap.of("spotlight", false) + val actualOptions = module.getSentryAndroidOptions(options, logger) + assertFalse(actualOptions.isEnableSpotlight) + } } diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index 0acdecd095..8efd14482f 100644 --- a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -20,6 +20,7 @@ import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReadableMap; import com.facebook.react.bridge.ReadableMapKeySetIterator; +import com.facebook.react.bridge.ReadableType; import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.bridge.WritableArray; import com.facebook.react.bridge.WritableMap; @@ -260,7 +261,12 @@ protected SentryAndroidOptions getSentryAndroidOptions( options.setEnableNdk(rnOptions.getBoolean("enableNdk")); } if (rnOptions.hasKey("spotlight")) { - options.setEnableSpotlight(rnOptions.getBoolean("spotlight")); + if (rnOptions.getType("spotlight") == ReadableType.Boolean) { + options.setEnableSpotlight(rnOptions.getBoolean("spotlight")); + } else if (rnOptions.getType("spotlight") == ReadableType.String) { + options.setEnableSpotlight(true); + options.setSpotlightConnectionUrl(rnOptions.getString("spotlight")); + } } if (rnOptions.hasKey("_experiments")) { options.getExperimental().setSessionReplay(getReplayOptions(rnOptions)); From 1fe5435c15cecf2ec276a409c128482509a0c3f5 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Wed, 30 Oct 2024 18:11:14 +0200 Subject: [PATCH 07/10] Pass the RN JS defaultSidecarUrl to the platform layers when Spotlight is enabled --- .../io/sentry/react/RNSentryModuleImplTest.kt | 8 +++-- .../RNSentryCocoaTesterTests/RNSentryTests.mm | 4 +++ .../io/sentry/react/RNSentryModuleImpl.java | 1 + packages/core/ios/RNSentry.mm | 4 +++ packages/core/src/js/client.ts | 2 ++ .../core/src/js/integrations/spotlight.ts | 5 +++- packages/core/src/js/wrapper.ts | 2 ++ packages/core/test/wrapper.test.ts | 30 ++++++++++++++++--- 8 files changed, 49 insertions(+), 7 deletions(-) diff --git a/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt index fe5d144e07..0f2b948974 100644 --- a/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt +++ b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt @@ -97,10 +97,14 @@ class RNSentryModuleImplTest { } @Test - fun `when the spotlight option is enabled, the spotlight SentryAndroidOption is set to true`() { - val options = JavaOnlyMap.of("spotlight", true) + fun `when the spotlight option is enabled, the spotlight SentryAndroidOption is set to true and the default url is used`() { + val options = JavaOnlyMap.of( + "spotlight", true, + "defaultSidecarUrl", "http://localhost:8969/teststream" + ) val actualOptions = module.getSentryAndroidOptions(options, logger) assert(actualOptions.isEnableSpotlight) + assertEquals("http://localhost:8969/teststream", actualOptions.spotlightConnectionUrl) } @Test diff --git a/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.mm b/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.mm index e966dfdf74..1ef91af466 100644 --- a/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.mm +++ b/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.mm @@ -130,11 +130,13 @@ - (void)testCreateOptionsWithDictionarySpotlightEnabled NSDictionary *_Nonnull mockedReactNativeDictionary = @{ @"dsn": @"https://abcd@efgh.ingest.sentry.io/123456", @"spotlight": @YES, + @"defaultSidecarUrl": @"http://localhost:8969/teststream", }; SentryOptions* actualOptions = [rnSentry createOptionsWithDictionary:mockedReactNativeDictionary error:&error]; XCTAssertNotNil(actualOptions, @"Did not create sentry options"); XCTAssertNil(error, @"Should not pass no error"); XCTAssertTrue(actualOptions.enableSpotlight , @"Did not enable spotlight"); + XCTAssertEqual(actualOptions.spotlightUrl , @"http://localhost:8969/teststream"); } - (void)testCreateOptionsWithDictionarySpotlightOne @@ -145,11 +147,13 @@ - (void)testCreateOptionsWithDictionarySpotlightOne NSDictionary *_Nonnull mockedReactNativeDictionary = @{ @"dsn": @"https://abcd@efgh.ingest.sentry.io/123456", @"spotlight": @1, + @"defaultSidecarUrl": @"http://localhost:8969/teststream", }; SentryOptions* actualOptions = [rnSentry createOptionsWithDictionary:mockedReactNativeDictionary error:&error]; XCTAssertNotNil(actualOptions, @"Did not create sentry options"); XCTAssertNil(error, @"Should not pass no error"); XCTAssertTrue(actualOptions.enableSpotlight , @"Did not enable spotlight"); + XCTAssertEqual(actualOptions.spotlightUrl , @"http://localhost:8969/teststream"); } - (void)testCreateOptionsWithDictionarySpotlightUrl diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index 8efd14482f..ad737cc0be 100644 --- a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -263,6 +263,7 @@ protected SentryAndroidOptions getSentryAndroidOptions( if (rnOptions.hasKey("spotlight")) { if (rnOptions.getType("spotlight") == ReadableType.Boolean) { options.setEnableSpotlight(rnOptions.getBoolean("spotlight")); + options.setSpotlightConnectionUrl(rnOptions.getString("defaultSidecarUrl")); } else if (rnOptions.getType("spotlight") == ReadableType.String) { options.setEnableSpotlight(true); options.setSpotlightConnectionUrl(rnOptions.getString("spotlight")); diff --git a/packages/core/ios/RNSentry.mm b/packages/core/ios/RNSentry.mm index 986f069305..200fdd5b68 100644 --- a/packages/core/ios/RNSentry.mm +++ b/packages/core/ios/RNSentry.mm @@ -167,6 +167,10 @@ - (SentryOptions *_Nullable)createOptionsWithDictionary:(NSDictionary *_Nonnull) sentryOptions.spotlightUrl = spotlightValue; } else if ([spotlightValue isKindOfClass:[NSNumber class]]) { sentryOptions.enableSpotlight = [spotlightValue boolValue]; + id defaultSpotlightUrl = [mutableOptions valueForKey:@"defaultSidecarUrl"]; + if (defaultSpotlightUrl != nil) { + sentryOptions.spotlightUrl = defaultSpotlightUrl; + } } } diff --git a/packages/core/src/js/client.ts b/packages/core/src/js/client.ts index f01f6d98b0..67e5ba6c5d 100644 --- a/packages/core/src/js/client.ts +++ b/packages/core/src/js/client.ts @@ -15,6 +15,7 @@ import { dateTimestampInSeconds, logger, SentryError } from '@sentry/utils'; import { Alert } from 'react-native'; import { defaultSdkInfo } from './integrations/sdkinfo'; +import { getDefaultSidecarUrl } from './integrations/spotlight'; import type { ReactNativeClientOptions } from './options'; import type { mobileReplayIntegration } from './replay/mobilereplay'; import { MOBILE_REPLAY_INTEGRATION_NAME } from './replay/mobilereplay'; @@ -143,6 +144,7 @@ export class ReactNativeClient extends BaseClient { private _initNativeSdk(): void { NATIVE.initNativeSdk({ ...this._options, + defaultSidecarUrl: getDefaultSidecarUrl(), mobileReplayOptions: this._integrations[MOBILE_REPLAY_INTEGRATION_NAME] && 'options' in this._integrations[MOBILE_REPLAY_INTEGRATION_NAME] diff --git a/packages/core/src/js/integrations/spotlight.ts b/packages/core/src/js/integrations/spotlight.ts index 2116dd0cf8..9707e95026 100644 --- a/packages/core/src/js/integrations/spotlight.ts +++ b/packages/core/src/js/integrations/spotlight.ts @@ -83,7 +83,10 @@ function sendEnvelopesToSidecar(client: Client, sidecarUrl: string): void { }); } -function getDefaultSidecarUrl(): string { +/** + * Gets the default Spotlight sidecar URL. + */ +export function getDefaultSidecarUrl(): string { try { const { url } = ReactNativeLibraries.Devtools?.getDevServer(); return `http://${getHostnameFromString(url)}:8969/stream`; diff --git a/packages/core/src/js/wrapper.ts b/packages/core/src/js/wrapper.ts index d901524793..a7ae24fb2c 100644 --- a/packages/core/src/js/wrapper.ts +++ b/packages/core/src/js/wrapper.ts @@ -50,6 +50,8 @@ export interface Screenshot { } export type NativeSdkOptions = Partial & { + defaultSidecarUrl: string | undefined; +} & { mobileReplayOptions: MobileReplayOptions | undefined; }; diff --git a/packages/core/test/wrapper.test.ts b/packages/core/test/wrapper.test.ts index 656c8f2b9a..ff54667211 100644 --- a/packages/core/test/wrapper.test.ts +++ b/packages/core/test/wrapper.test.ts @@ -96,7 +96,12 @@ describe('Tests Native Wrapper', () => { describe('startWithOptions', () => { test('calls native module', async () => { - await NATIVE.initNativeSdk({ dsn: 'test', enableNative: true, mobileReplayOptions: undefined }); + await NATIVE.initNativeSdk({ + dsn: 'test', + enableNative: true, + defaultSidecarUrl: undefined, + mobileReplayOptions: undefined, + }); expect(RNSentry.initNativeSdk).toBeCalled(); }); @@ -104,7 +109,7 @@ describe('Tests Native Wrapper', () => { test('warns if there is no dsn', async () => { logger.warn = jest.fn(); - await NATIVE.initNativeSdk({ enableNative: true, mobileReplayOptions: undefined }); + await NATIVE.initNativeSdk({ enableNative: true, defaultSidecarUrl: undefined, mobileReplayOptions: undefined }); expect(RNSentry.initNativeSdk).not.toBeCalled(); expect(logger.warn).toHaveBeenLastCalledWith( @@ -119,6 +124,7 @@ describe('Tests Native Wrapper', () => { dsn: 'test', enableNative: false, enableNativeNagger: true, + defaultSidecarUrl: undefined, mobileReplayOptions: undefined, }); @@ -133,6 +139,7 @@ describe('Tests Native Wrapper', () => { enableNative: true, autoInitializeNativeSdk: true, beforeSend: jest.fn(), + defaultSidecarUrl: undefined, mobileReplayOptions: undefined, }); @@ -149,6 +156,7 @@ describe('Tests Native Wrapper', () => { enableNative: true, autoInitializeNativeSdk: true, beforeBreadcrumb: jest.fn(), + defaultSidecarUrl: undefined, mobileReplayOptions: undefined, }); @@ -165,6 +173,7 @@ describe('Tests Native Wrapper', () => { enableNative: true, autoInitializeNativeSdk: true, beforeSendTransaction: jest.fn(), + defaultSidecarUrl: undefined, mobileReplayOptions: undefined, }); @@ -181,6 +190,7 @@ describe('Tests Native Wrapper', () => { enableNative: true, autoInitializeNativeSdk: true, integrations: [], + defaultSidecarUrl: undefined, mobileReplayOptions: undefined, }); @@ -199,6 +209,7 @@ describe('Tests Native Wrapper', () => { dsn: 'test', enableNative: true, autoInitializeNativeSdk: false, + defaultSidecarUrl: undefined, mobileReplayOptions: undefined, }); @@ -238,6 +249,7 @@ describe('Tests Native Wrapper', () => { logger.warn = jest.fn(); await NATIVE.initNativeSdk({ + defaultSidecarUrl: undefined, mobileReplayOptions: undefined, dsn: 'test', enableNative: false, @@ -320,7 +332,12 @@ describe('Tests Native Wrapper', () => { }); test('does not call RNSentry at all if enableNative is false', async () => { try { - await NATIVE.initNativeSdk({ dsn: 'test-dsn', enableNative: false, mobileReplayOptions: undefined }); + await NATIVE.initNativeSdk({ + dsn: 'test-dsn', + enableNative: false, + defaultSidecarUrl: undefined, + mobileReplayOptions: undefined, + }); // @ts-expect-error for testing, does not accept an empty class. await NATIVE.sendEnvelope({}); @@ -512,7 +529,12 @@ describe('Tests Native Wrapper', () => { expect(RNSentry.crash).toBeCalled(); }); test('does not call crash if enableNative is false', async () => { - await NATIVE.initNativeSdk({ dsn: 'test-dsn', enableNative: false, mobileReplayOptions: undefined }); + await NATIVE.initNativeSdk({ + dsn: 'test-dsn', + enableNative: false, + defaultSidecarUrl: undefined, + mobileReplayOptions: undefined, + }); NATIVE.nativeCrash(); expect(RNSentry.crash).not.toBeCalled(); From 93f7992b24348791b6ba228e523fd74efc265c11 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Wed, 30 Oct 2024 19:32:01 +0200 Subject: [PATCH 08/10] Fixes Android initialisation issue --- .../java/io/sentry/react/RNSentryModuleImplTest.kt | 10 +++++++--- .../main/java/io/sentry/react/RNSentryModuleImpl.java | 9 ++++----- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt index 0f2b948974..f699f0d530 100644 --- a/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt +++ b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt @@ -9,6 +9,7 @@ import com.facebook.react.bridge.ReactApplicationContext import com.facebook.react.bridge.WritableMap import io.sentry.ILogger import io.sentry.SentryLevel +import io.sentry.android.core.SentryAndroidOptions import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -102,7 +103,8 @@ class RNSentryModuleImplTest { "spotlight", true, "defaultSidecarUrl", "http://localhost:8969/teststream" ) - val actualOptions = module.getSentryAndroidOptions(options, logger) + val actualOptions = SentryAndroidOptions() + module.getSentryAndroidOptions(actualOptions, options, logger) assert(actualOptions.isEnableSpotlight) assertEquals("http://localhost:8969/teststream", actualOptions.spotlightConnectionUrl) } @@ -110,7 +112,8 @@ class RNSentryModuleImplTest { @Test fun `when the spotlight url is passed, the spotlight is enabled for the given url`() { val options = JavaOnlyMap.of("spotlight", "http://localhost:8969/teststream") - val actualOptions = module.getSentryAndroidOptions(options, logger) + val actualOptions = SentryAndroidOptions() + module.getSentryAndroidOptions(actualOptions, options, logger) assert(actualOptions.isEnableSpotlight) assertEquals("http://localhost:8969/teststream", actualOptions.spotlightConnectionUrl) } @@ -118,7 +121,8 @@ class RNSentryModuleImplTest { @Test fun `when the spotlight option is disabled, the spotlight SentryAndroidOption is set to false`() { val options = JavaOnlyMap.of("spotlight", false) - val actualOptions = module.getSentryAndroidOptions(options, logger) + val actualOptions = SentryAndroidOptions() + module.getSentryAndroidOptions(actualOptions, options, logger) assertFalse(actualOptions.isEnableSpotlight) } } diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index ad737cc0be..a9418bbf97 100644 --- a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -174,14 +174,14 @@ public void initNativeReactNavigationNewFrameTracking(Promise promise) { public void initNativeSdk(final ReadableMap rnOptions, Promise promise) { SentryAndroid.init( - this.getReactApplicationContext(), options -> getSentryAndroidOptions(rnOptions, logger)); + this.getReactApplicationContext(), + options -> getSentryAndroidOptions(options, rnOptions, logger)); promise.resolve(true); } - protected SentryAndroidOptions getSentryAndroidOptions( - @NotNull ReadableMap rnOptions, ILogger logger) { - final SentryAndroidOptions options = new SentryAndroidOptions(); + protected void getSentryAndroidOptions( + @NotNull SentryAndroidOptions options, @NotNull ReadableMap rnOptions, ILogger logger) { @Nullable SdkVersion sdkVersion = options.getSdkVersion(); if (sdkVersion == null) { sdkVersion = new SdkVersion(ANDROID_SDK_NAME, BuildConfig.VERSION_NAME); @@ -312,7 +312,6 @@ protected SentryAndroidOptions getSentryAndroidOptions( if (currentActivity != null) { currentActivityHolder.setActivity(currentActivity); } - return options; } private SentryReplayOptions getReplayOptions(@NotNull ReadableMap rnOptions) { From 9e58c9bdc721b742848c1d3b7bca611f78186992 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 31 Oct 2024 16:26:04 +0200 Subject: [PATCH 09/10] Update CHANGELOG using the features section --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fbf474f19b..ef2aade826 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ - Enhanced accuracy of time-to-display spans. ([#4189](https://github.com/getsentry/sentry-react-native/pull/4189)) -### Changes +### Features - Enables Spotlight in Android and iOS SDKs ([#4211](https://github.com/getsentry/sentry-react-native/pull/4211)) From 5ca9da0cf92423fc5f635ae57638b8a4f95380a3 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 31 Oct 2024 16:28:11 +0200 Subject: [PATCH 10/10] Update log message to communicate the custom spotlight url used clearly Co-authored-by: LucasZF --- packages/core/ios/RNSentry.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/ios/RNSentry.mm b/packages/core/ios/RNSentry.mm index f7c04771f0..98bb3e5ae8 100644 --- a/packages/core/ios/RNSentry.mm +++ b/packages/core/ios/RNSentry.mm @@ -166,7 +166,7 @@ - (SentryOptions *_Nullable)createOptionsWithDictionary:(NSDictionary *_Nonnull) if ([mutableOptions valueForKey:@"spotlight"] != nil) { id spotlightValue = [mutableOptions valueForKey:@"spotlight"]; if ([spotlightValue isKindOfClass:[NSString class]]) { - NSLog(@"The value is a string: %@", spotlightValue); + NSLog(@"Using Spotlight on address: %@", spotlightValue); sentryOptions.enableSpotlight = true; sentryOptions.spotlightUrl = spotlightValue; } else if ([spotlightValue isKindOfClass:[NSNumber class]]) {