Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

refactor: use native spotlight integrations on Flutter Android, iOS, macOS #2285

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
9 changes: 8 additions & 1 deletion dart/lib/src/sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
13 changes: 2 additions & 11 deletions dart/lib/src/spotlight.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import 'platform_checker.dart';

/// Spotlight configuration class.
class Spotlight {
/// Whether to enable Spotlight for local development.
Expand All @@ -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});
}
10 changes: 8 additions & 2 deletions dart/lib/src/transport/spotlight_http_transport.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -51,3 +53,7 @@ class SpotlightHttpTransport extends Transport {
target: 'Spotlight');
}
}

String _defaultSpotlightUrl() {
return 'http://localhost:8969/stream';
}
4 changes: 4 additions & 0 deletions dart/test/mocks/mock_platform.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ class MockPlatform extends Platform with NoSuchMethodProvider {
return MockPlatform(os: 'linux');
}

factory MockPlatform.windows() {
return MockPlatform(os: 'windows');
}

@override
String operatingSystem;
}
64 changes: 62 additions & 2 deletions dart/test/sentry_client_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions flutter/example/android/app/src/main/res/xml/network.xml
Original file line number Diff line number Diff line change
@@ -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>
2 changes: 1 addition & 1 deletion flutter/example/ios/Runner/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import UIKit
import Flutter
import Sentry

@UIApplicationMain
@main
@objc class AppDelegate: FlutterAppDelegate {
private let _channel = "example.flutter.sentry.io"

Expand Down
6 changes: 6 additions & 0 deletions flutter/ios/Classes/SentryFlutter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions flutter/lib/src/native/sentry_native_channel.dart
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ class SentryNativeChannel
'sessionSampleRate': options.experimental.replay.sessionSampleRate,
'onErrorSampleRate': options.experimental.replay.onErrorSampleRate,
},
'enableSpotlight': options.spotlight.enabled,
'spotlightUrl': options.spotlight.url,
});
}

Expand Down
8 changes: 7 additions & 1 deletion flutter/test/integrations/init_native_sdk_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ void main() {
'sessionSampleRate': null,
'onErrorSampleRate': null,
},
'enableSpotlight': false,
'spotlightUrl': null,
});
});

Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -174,6 +178,8 @@ void main() {
'sessionSampleRate': 0.1,
'onErrorSampleRate': 0.2,
},
'enableSpotlight': true,
'spotlightUrl': 'http://localhost:8969/stream',
});
});
}
Expand Down
Loading