refactor: use native spotlight integrations on Flutter Android, iOS, …

…macOS (#2285) * update * update changelog * fix tests * fix analyze * update * rm local.properties from git * update naming * update test to use mock platform linux
getsentry · Sep 25, 2024 · 8a111a9 · 8a111a9
1 parent eddc70e
commit 8a111a9
Show file tree

Hide file tree

Showing 11 changed files with 114 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@
 
 ### Enhancements
 
+- Use native spotlight integrations on Flutter Android, iOS, macOS ([#2285](https://github.com/getsentry/sentry-dart/pull/2285))
 - Improve app start integration ([#2266](https://github.com/getsentry/sentry-dart/pull/2266))
   - Fixes ([#2103](https://github.com/getsentry/sentry-dart/issues/2103))
   - Fixes ([#2233](https://github.com/getsentry/sentry-dart/issues/2233))

diff --git a/dart/lib/src/sentry_client.dart b/dart/lib/src/sentry_client.dart
@@ -63,7 +63,14 @@ class SentryClient {
       final rateLimiter = RateLimiter(options);
       options.transport = HttpTransport(options, rateLimiter);
     }
-    if (options.spotlight.enabled) {
+    // TODO: Web might change soon to use the JS SDK so we can remove it here later on
+    final enableFlutterSpotlight = (options.spotlight.enabled &&
+        (options.platformChecker.isWeb ||
+            options.platformChecker.platform.isLinux ||
+            options.platformChecker.platform.isWindows));
+    // Spotlight in the Flutter layer is only enabled for Web, Linux and Windows
+    // Other platforms use spotlight through their native SDKs
+    if (enableFlutterSpotlight) {
       options.transport = SpotlightHttpTransport(options, options.transport);
     }
     return SentryClient._(options);

diff --git a/dart/lib/src/spotlight.dart b/dart/lib/src/spotlight.dart
@@ -1,5 +1,3 @@
-import 'platform_checker.dart';
-
 /// Spotlight configuration class.
 class Spotlight {
   /// Whether to enable Spotlight for local development.
@@ -8,14 +6,7 @@ class Spotlight {
   /// The Spotlight Sidecar URL.
   /// Defaults to http://10.0.2.2:8969/stream due to Emulator on Android.
   /// Otherwise defaults to http://localhost:8969/stream.
-  String url;
-
-  Spotlight({required this.enabled, String? url})
-      : url = url ?? _defaultSpotlightUrl();
-}
+  String? url;
 
-String _defaultSpotlightUrl() {
-  return (PlatformChecker().platform.isAndroid
-      ? 'http://10.0.2.2:8969/stream'
-      : 'http://localhost:8969/stream');
+  Spotlight({required this.enabled, this.url});
 }
diff --git a/dart/lib/src/transport/spotlight_http_transport.dart b/dart/lib/src/transport/spotlight_http_transport.dart
@@ -8,6 +8,8 @@ import '../http_client/client_provider.dart'
     if (dart.library.io) '../http_client/io_client_provider.dart';
 
 /// Spotlight HTTP transport decorator that sends Sentry envelopes to both Sentry and Spotlight.
+/// This will be used on platforms that do not have native SDK support.
+/// Platforms with native SDK support will configure spotlight directly in the native SDK options.
 class SpotlightHttpTransport extends Transport {
   final SentryOptions _options;
   final Transport _transport;
@@ -21,8 +23,8 @@ class SpotlightHttpTransport extends Transport {
   }
 
   SpotlightHttpTransport._(this._options, this._transport)
-      : _requestHandler = HttpTransportRequestHandler(
-            _options, Uri.parse(_options.spotlight.url));
+      : _requestHandler = HttpTransportRequestHandler(_options,
+            Uri.parse(_options.spotlight.url ?? _defaultSpotlightUrl()));
 
   @override
   Future<SentryId?> send(SentryEnvelope envelope) async {
@@ -51,3 +53,7 @@ class SpotlightHttpTransport extends Transport {
         target: 'Spotlight');
   }
 }
+
+String _defaultSpotlightUrl() {
+  return 'http://localhost:8969/stream';
+}
diff --git a/dart/test/mocks/mock_platform.dart b/dart/test/mocks/mock_platform.dart
@@ -21,6 +21,10 @@ class MockPlatform extends Platform with NoSuchMethodProvider {
     return MockPlatform(os: 'linux');
   }
 
+  factory MockPlatform.windows() {
+    return MockPlatform(os: 'windows');
+  }
+
   @override
   String operatingSystem;
 }
diff --git a/dart/test/sentry_client_test.dart b/dart/test/sentry_client_test.dart
@@ -1779,12 +1779,72 @@ void main() {
       expect(capturedEnvelope.header.dsn, fixture.options.dsn);
     });
 
-    test('Spotlight enabled should set transport to SpotlightHttpTransport',
+    test(
+        'Spotlight enabled should not set transport to SpotlightHttpTransport on iOS',
+        () async {
+      fixture.options.platformChecker = MockPlatformChecker(
+        platform: MockPlatform.iOS(),
+      );
+      fixture.options.spotlight = Spotlight(enabled: true);
+      fixture.getSut();
+
+      expect(fixture.options.transport is SpotlightHttpTransport, isFalse);
+    });
+
+    test(
+        'Spotlight enabled should not set transport to SpotlightHttpTransport on macOS',
+        () async {
+      fixture.options.platformChecker = MockPlatformChecker(
+        platform: MockPlatform.macOS(),
+      );
+      fixture.options.spotlight = Spotlight(enabled: true);
+      fixture.getSut();
+
+      expect(fixture.options.transport is SpotlightHttpTransport, isFalse);
+    });
+
+    test(
+        'Spotlight enabled should not set transport to SpotlightHttpTransport on Android',
+        () async {
+      fixture.options.platformChecker = MockPlatformChecker(
+        platform: MockPlatform.android(),
+      );
+      fixture.options.spotlight = Spotlight(enabled: true);
+      fixture.getSut();
+
+      expect(fixture.options.transport is SpotlightHttpTransport, isFalse);
+    });
+
+    test(
+        'Spotlight enabled should set transport to SpotlightHttpTransport on Web',
+        () async {
+      fixture.options.platformChecker = MockPlatformChecker(isWebValue: true);
+      fixture.options.spotlight = Spotlight(enabled: true);
+      fixture.getSut();
+
+      expect(fixture.options.transport is SpotlightHttpTransport, isTrue);
+    });
+
+    test(
+        'Spotlight enabled should set transport to SpotlightHttpTransport on Linux',
+        () async {
+      fixture.options.platformChecker =
+          MockPlatformChecker(platform: MockPlatform.linux());
+      fixture.options.spotlight = Spotlight(enabled: true);
+      fixture.getSut();
+
+      expect(fixture.options.transport is SpotlightHttpTransport, isTrue);
+    });
+
+    test(
+        'Spotlight enabled should set transport to SpotlightHttpTransport on Windows',
         () async {
+      fixture.options.platformChecker =
+          MockPlatformChecker(platform: MockPlatform.windows());
       fixture.options.spotlight = Spotlight(enabled: true);
       fixture.getSut();
 
-      expect(fixture.options.transport is SpotlightHttpTransport, true);
+      expect(fixture.options.transport is SpotlightHttpTransport, isTrue);
     });
   });
 

diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutter.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutter.kt
@@ -81,6 +81,12 @@ class SentryFlutter(
     data.getIfNotNull<String>("proguardUuid") {
       options.proguardUuid = it
     }
+    data.getIfNotNull<Boolean>("enableSpotlight") {
+      options.isEnableSpotlight = it
+    }
+    data.getIfNotNull<String>("spotlightUrl") {
+      options.spotlightConnectionUrl = it
+    }
 
     val nativeCrashHandling = (data["enableNativeCrashHandling"] as? Boolean) ?: true
     // nativeCrashHandling has priority over anrEnabled

diff --git a/flutter/example/android/app/src/main/res/xml/network.xml b/flutter/example/android/app/src/main/res/xml/network.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
+<network-security-config>
+    <domain-config cleartextTrafficPermitted="true">
+        <!-- Allow cleartext traffic from the emulator to the host machine -->
+        <!-- See https://developer.android.com/studio/run/emulator-networking for more details -->
+        <domain includeSubdomains="true">10.0.2.2</domain>
+    </domain-config>
+</network-security-config>
diff --git a/flutter/ios/Classes/SentryFlutter.swift b/flutter/ios/Classes/SentryFlutter.swift
@@ -70,6 +70,12 @@ public final class SentryFlutter {
         if let appHangTimeoutIntervalMillis = data["appHangTimeoutIntervalMillis"] as? NSNumber {
             options.appHangTimeoutInterval = appHangTimeoutIntervalMillis.doubleValue / 1000
         }
+        if let spotlightUrl = data["spotlightUrl"] as? String {
+            options.spotlightUrl = spotlightUrl
+        }
+        if let enableSpotlight = data["enableSpotlight"] as? Bool {
+            options.enableSpotlight = enableSpotlight
+        }
         if let proxy = data["proxy"] as? [String: Any] {
             guard let host = proxy["host"] as? String,
                   let port = proxy["port"] as? Int,

diff --git a/flutter/lib/src/native/sentry_native_channel.dart b/flutter/lib/src/native/sentry_native_channel.dart
@@ -71,6 +71,8 @@ class SentryNativeChannel
         'sessionSampleRate': options.experimental.replay.sessionSampleRate,
         'onErrorSampleRate': options.experimental.replay.onErrorSampleRate,
       },
+      'enableSpotlight': options.spotlight.enabled,
+      'spotlightUrl': options.spotlight.url,
     });
   }
 

diff --git a/flutter/test/integrations/init_native_sdk_test.dart b/flutter/test/integrations/init_native_sdk_test.dart
@@ -69,6 +69,8 @@ void main() {
         'sessionSampleRate': null,
         'onErrorSampleRate': null,
       },
+      'enableSpotlight': false,
+      'spotlightUrl': null,
     });
   });
 
@@ -118,7 +120,9 @@ void main() {
         pass: '0000',
       )
       ..experimental.replay.sessionSampleRate = 0.1
-      ..experimental.replay.onErrorSampleRate = 0.2;
+      ..experimental.replay.onErrorSampleRate = 0.2
+      ..spotlight =
+          Spotlight(enabled: true, url: 'http://localhost:8969/stream');
 
     fixture.options.sdk.addIntegration('foo');
     fixture.options.sdk.addPackage('bar', '1');
@@ -174,6 +178,8 @@ void main() {
         'sessionSampleRate': 0.1,
         'onErrorSampleRate': 0.2,
       },
+      'enableSpotlight': true,
+      'spotlightUrl': 'http://localhost:8969/stream',
     });
   });
 }