From c107ffb6bc232e08a498e9c882ab3463b07769e0 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 12 Sep 2024 17:10:00 +0200 Subject: [PATCH 1/8] update --- dart/lib/src/sentry_client.dart | 7 ++++++- dart/lib/src/spotlight.dart | 11 ++--------- dart/lib/src/transport/spotlight_http_transport.dart | 10 ++++++++-- .../main/kotlin/io/sentry/flutter/SentryFlutter.kt | 6 ++++++ flutter/example/android/app/local.properties | 8 ++++++++ .../example/android/app/src/main/AndroidManifest.xml | 3 ++- .../example/android/app/src/main/res/xml/network.xml | 8 ++++++++ flutter/example/ios/Runner/AppDelegate.swift | 2 +- flutter/example/lib/main.dart | 4 ++-- flutter/ios/Classes/SentryFlutter.swift | 6 ++++++ flutter/lib/src/native/sentry_native_channel.dart | 2 ++ flutter/test/integrations/init_native_sdk_test.dart | 8 +++++++- 12 files changed, 58 insertions(+), 17 deletions(-) create mode 100644 flutter/example/android/app/local.properties create mode 100644 flutter/example/android/app/src/main/res/xml/network.xml diff --git a/dart/lib/src/sentry_client.dart b/dart/lib/src/sentry_client.dart index e3809568da..f567508f24 100644 --- a/dart/lib/src/sentry_client.dart +++ b/dart/lib/src/sentry_client.dart @@ -63,7 +63,12 @@ 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 enableSpotlight = (options.spotlight.enabled && + (options.platformChecker.isWeb || + options.platformChecker.platform.isLinux || + options.platformChecker.platform.isWindows)); + if (enableSpotlight) { options.transport = SpotlightHttpTransport(options, options.transport); } return SentryClient._(options); diff --git a/dart/lib/src/spotlight.dart b/dart/lib/src/spotlight.dart index b106ed3547..9a914cab20 100644 --- a/dart/lib/src/spotlight.dart +++ b/dart/lib/src/spotlight.dart @@ -8,14 +8,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; + String? url; - Spotlight({required this.enabled, String? url}) - : url = url ?? _defaultSpotlightUrl(); -} - -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 index 1889bb7339..e44cae44b9 100644 --- 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 send(SentryEnvelope envelope) async { @@ -48,3 +50,7 @@ class SpotlightHttpTransport extends Transport { target: 'Spotlight'); } } + +String _defaultSpotlightUrl() { + return 'http://localhost:8969/stream'; +} diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutter.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutter.kt index d38872f6ee..8d4e40948d 100644 --- 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("proguardUuid") { options.proguardUuid = it } + data.getIfNotNull("enableSpotlight") { + options.isEnableSpotlight = it + } + data.getIfNotNull("spotlightUrl") { + options.spotlightConnectionUrl = it + } val nativeCrashHandling = (data["enableNativeCrashHandling"] as? Boolean) ?: true // nativeCrashHandling has priority over anrEnabled diff --git a/flutter/example/android/app/local.properties b/flutter/example/android/app/local.properties new file mode 100644 index 0000000000..127723a8f4 --- /dev/null +++ b/flutter/example/android/app/local.properties @@ -0,0 +1,8 @@ +## This file must *NOT* be checked into Version Control Systems, +# as it contains information specific to your local configuration. +# +# Location of the SDK. This is only used by Gradle. +# For customization when using a Version Control System, please read the +# header note. +#Tue Sep 10 17:20:30 CEST 2024 +sdk.dir=/Users/giancarlobuenaflor/Library/Android/sdk diff --git a/flutter/example/android/app/src/main/AndroidManifest.xml b/flutter/example/android/app/src/main/AndroidManifest.xml index 1b2b2012cf..800fb96d4c 100644 --- a/flutter/example/android/app/src/main/AndroidManifest.xml +++ b/flutter/example/android/app/src/main/AndroidManifest.xml @@ -8,7 +8,8 @@ android:allowBackup="false" android:label="sentry_flutter_example" android:icon="@mipmap/ic_launcher" - android:extractNativeLibs="true"> + android:extractNativeLibs="true" + android:networkSecurityConfig="@xml/network"> + + + + + 10.0.2.2 + + diff --git a/flutter/example/ios/Runner/AppDelegate.swift b/flutter/example/ios/Runner/AppDelegate.swift index a231cc9c60..c24cacbbb2 100644 --- a/flutter/example/ios/Runner/AppDelegate.swift +++ b/flutter/example/ios/Runner/AppDelegate.swift @@ -2,7 +2,7 @@ import UIKit import Flutter import Sentry -@UIApplicationMain +@main @objc class AppDelegate: FlutterAppDelegate { private let _channel = "example.flutter.sentry.io" diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index dd870eb587..3254562687 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -90,8 +90,8 @@ Future setupSentry( options.maxResponseBodySize = MaxResponseBodySize.always; options.navigatorKey = navigatorKey; - options.experimental.replay.sessionSampleRate = 1.0; - options.experimental.replay.onErrorSampleRate = 1.0; + // options.experimental.replay.sessionSampleRate = 1.0; + // options.experimental.replay.onErrorSampleRate = 1.0; _isIntegrationTest = isIntegrationTest; if (_isIntegrationTest) { diff --git a/flutter/ios/Classes/SentryFlutter.swift b/flutter/ios/Classes/SentryFlutter.swift index 987528987c..02beabb5df 100644 --- 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 index 360290230d..ce5f063d37 100644 --- 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 index bfce621eb3..46b64a545b 100644 --- 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', }); }); } From 3666a21a4059b594f710b66a3b529449e8bcf5d9 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 12 Sep 2024 20:42:01 +0200 Subject: [PATCH 2/8] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 387633d6e3..cd8fb02919 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- Use native spotlight integrations on Flutter Android, iOS, macOS ([#2285](https://github.com/getsentry/sentry-dart/pull/2285)) + ## 8.9.0 ### Features From 3f83aca2f78ced61d18a323a874e9f6f6c80bb5c Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 12 Sep 2024 20:49:40 +0200 Subject: [PATCH 3/8] fix tests --- dart/test/mocks/mock_platform.dart | 4 ++ dart/test/sentry_client_test.dart | 63 +++++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/dart/test/mocks/mock_platform.dart b/dart/test/mocks/mock_platform.dart index 9f8f391c88..75025a4a15 100644 --- 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 index 07d5aab834..e75c83967e 100644 --- a/dart/test/sentry_client_test.dart +++ b/dart/test/sentry_client_test.dart @@ -1767,12 +1767,71 @@ 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(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 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); }); }); From 4b62e33d3f11aab292c163a751ae581d09564827 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 12 Sep 2024 20:50:07 +0200 Subject: [PATCH 4/8] fix analyze --- dart/lib/src/spotlight.dart | 2 -- 1 file changed, 2 deletions(-) diff --git a/dart/lib/src/spotlight.dart b/dart/lib/src/spotlight.dart index 9a914cab20..e4c387a30f 100644 --- 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. From 19a2287ae59ae5d4bad855023aec44421935f9b9 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 12 Sep 2024 21:15:40 +0200 Subject: [PATCH 5/8] update --- flutter/example/android/app/src/main/AndroidManifest.xml | 3 +-- flutter/example/lib/main.dart | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/flutter/example/android/app/src/main/AndroidManifest.xml b/flutter/example/android/app/src/main/AndroidManifest.xml index 800fb96d4c..1b2b2012cf 100644 --- a/flutter/example/android/app/src/main/AndroidManifest.xml +++ b/flutter/example/android/app/src/main/AndroidManifest.xml @@ -8,8 +8,7 @@ android:allowBackup="false" android:label="sentry_flutter_example" android:icon="@mipmap/ic_launcher" - android:extractNativeLibs="true" - android:networkSecurityConfig="@xml/network"> + android:extractNativeLibs="true"> setupSentry( options.maxResponseBodySize = MaxResponseBodySize.always; options.navigatorKey = navigatorKey; - // options.experimental.replay.sessionSampleRate = 1.0; - // options.experimental.replay.onErrorSampleRate = 1.0; + options.experimental.replay.sessionSampleRate = 1.0; + options.experimental.replay.onErrorSampleRate = 1.0; _isIntegrationTest = isIntegrationTest; if (_isIntegrationTest) { From 66a1c4cfbf3ae6c5a0a108670565a20a417ee87b Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 12 Sep 2024 21:16:39 +0200 Subject: [PATCH 6/8] rm local.properties from git --- flutter/example/android/app/local.properties | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 flutter/example/android/app/local.properties diff --git a/flutter/example/android/app/local.properties b/flutter/example/android/app/local.properties deleted file mode 100644 index 127723a8f4..0000000000 --- a/flutter/example/android/app/local.properties +++ /dev/null @@ -1,8 +0,0 @@ -## This file must *NOT* be checked into Version Control Systems, -# as it contains information specific to your local configuration. -# -# Location of the SDK. This is only used by Gradle. -# For customization when using a Version Control System, please read the -# header note. -#Tue Sep 10 17:20:30 CEST 2024 -sdk.dir=/Users/giancarlobuenaflor/Library/Android/sdk From 002c9f775d0cd07898e95a9d78dc75bfad4482fd Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 12 Sep 2024 21:20:11 +0200 Subject: [PATCH 7/8] update naming --- dart/lib/src/sentry_client.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dart/lib/src/sentry_client.dart b/dart/lib/src/sentry_client.dart index f567508f24..74bb690eb6 100644 --- a/dart/lib/src/sentry_client.dart +++ b/dart/lib/src/sentry_client.dart @@ -64,11 +64,13 @@ class SentryClient { options.transport = HttpTransport(options, rateLimiter); } // TODO: Web might change soon to use the JS SDK so we can remove it here later on - final enableSpotlight = (options.spotlight.enabled && + final enableFlutterSpotlight = (options.spotlight.enabled && (options.platformChecker.isWeb || options.platformChecker.platform.isLinux || options.platformChecker.platform.isWindows)); - if (enableSpotlight) { + // 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); From f11d0417aef10daab8cba7d1ee7ccb31afaabd4c Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 16 Sep 2024 15:50:28 +0200 Subject: [PATCH 8/8] update test to use mock platform linux --- dart/test/sentry_client_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dart/test/sentry_client_test.dart b/dart/test/sentry_client_test.dart index e75c83967e..282aed74fa 100644 --- a/dart/test/sentry_client_test.dart +++ b/dart/test/sentry_client_test.dart @@ -1816,7 +1816,8 @@ void main() { test( 'Spotlight enabled should set transport to SpotlightHttpTransport on Linux', () async { - fixture.options.platformChecker = MockPlatformChecker(isWebValue: true); + fixture.options.platformChecker = + MockPlatformChecker(platform: MockPlatform.linux()); fixture.options.spotlight = Spotlight(enabled: true); fixture.getSut();