-
-
Notifications
You must be signed in to change notification settings - Fork 274
enh: offload captureEnvelope
to background isolate for iOS and Android
#3232
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
base: main
Are you sure you want to change the base?
Changes from all commits
03f8118
6928f3a
a91cbae
a6bd3cc
b9269c7
6ef9960
a43f2e1
e334269
aa728e7
45cc8c3
a603960
147da01
2b11149
8325952
3dbe751
71ba593
fe7f6df
39a951e
884642f
f5f5401
de232c6
69d5111
79d9ff9
62bb12b
532d2b3
53c6036
06ee227
10d9419
e6771bb
e2ae6a3
ae9b24c
7efb747
4b440d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,4 +21,3 @@ dependency_overrides: | |
isar_flutter_libs: | ||
git: | ||
url: https://github.com/MrLittleWhite/isar_flutter_libs.git | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
import 'dart:developer' as developer; | ||
|
||
import 'package:meta/meta.dart'; | ||
|
||
import '../../sentry_flutter.dart'; | ||
|
||
/// Static logger for Isolates that writes diagnostic messages to `dart:developer.log`. | ||
/// | ||
/// Intended for worker/background isolates where a `SentryOptions` instance | ||
/// or hub may not be available. Because Dart statics are isolate-local, | ||
/// you must call [configure] once per isolate before using [log]. | ||
class IsolateLogger { | ||
IsolateLogger._(); | ||
|
||
static late bool _debug; | ||
static late SentryLevel _level; | ||
static late String _loggerName; | ||
static bool _isConfigured = false; | ||
|
||
/// Configures this logger for the current isolate. | ||
/// | ||
/// Must be called once per isolate before invoking [log]. | ||
/// Throws [StateError] if called more than once without calling [reset] first. | ||
/// | ||
/// - [debug]: when false, suppresses all logs except [SentryLevel.fatal]. | ||
/// - [level]: minimum severity threshold (inclusive) when [debug] is true. | ||
/// - [loggerName]: logger name for the call sites | ||
static void configure( | ||
{required bool debug, | ||
required SentryLevel level, | ||
required String loggerName}) { | ||
if (_isConfigured) { | ||
throw StateError( | ||
'IsolateLogger.configure has already been called. It can only be configured once per isolate.'); | ||
} | ||
Comment on lines
+7
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We gotta use a static logger since we cannot pass There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the alternative would be not to log in isolates at all |
||
_debug = debug; | ||
_level = level; | ||
_loggerName = loggerName; | ||
_isConfigured = true; | ||
} | ||
|
||
/// Resets the logger state to allow reconfiguration. | ||
/// | ||
/// This is intended for testing purposes only. | ||
@visibleForTesting | ||
static void reset() { | ||
_isConfigured = false; | ||
} | ||
|
||
/// Emits a log entry if enabled. | ||
/// | ||
/// Messages are forwarded to [developer.log]. The provided [level] is | ||
/// mapped via [SentryLevel.toDartLogLevel] to a `developer.log` numeric level. | ||
/// If logging is disabled or [level] is below the configured threshold, | ||
/// nothing is emitted. [SentryLevel.fatal] is always emitted. | ||
static void log( | ||
SentryLevel level, | ||
String message, { | ||
String? logger, | ||
Object? exception, | ||
StackTrace? stackTrace, | ||
}) { | ||
assert( | ||
_isConfigured, 'IsolateLogger.configure must be called before logging'); | ||
if (_isEnabled(level)) { | ||
developer.log( | ||
'[${level.name}] $message', | ||
level: level.toDartLogLevel(), | ||
name: logger ?? _loggerName, | ||
time: DateTime.now(), | ||
error: exception, | ||
stackTrace: stackTrace, | ||
); | ||
} | ||
} | ||
|
||
static bool _isEnabled(SentryLevel level) { | ||
return (_debug && level.ordinal >= _level.ordinal) || | ||
level == SentryLevel.fatal; | ||
} | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,177 @@ | ||
import 'dart:async'; | ||
import 'dart:isolate'; | ||
|
||
import '../../sentry_flutter.dart'; | ||
import 'isolate_logger.dart'; | ||
|
||
const _shutdownCommand = '_shutdown_'; | ||
|
||
// ------------------------------------------- | ||
// HOST-SIDE API (runs on the main isolate) | ||
// ------------------------------------------- | ||
|
||
/// Minimal config passed to isolates - extend as needed. | ||
class WorkerConfig { | ||
final bool debug; | ||
final SentryLevel diagnosticLevel; | ||
final String debugName; | ||
final bool automatedTestMode; | ||
|
||
const WorkerConfig({ | ||
required this.debug, | ||
required this.diagnosticLevel, | ||
required this.debugName, | ||
this.automatedTestMode = false, | ||
}); | ||
} | ||
|
||
/// Host-side helper for workers to perform minimal request/response. | ||
/// Adapted from https://dart.dev/language/isolates#robust-ports-example | ||
class Worker { | ||
denrase marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Worker(this._workerPort, this._responses) { | ||
_responses.listen(_handleResponse); | ||
} | ||
|
||
final SendPort _workerPort; | ||
SendPort get port => _workerPort; | ||
final ReceivePort _responses; | ||
final Map<int, Completer<Object?>> _pending = {}; | ||
int _idCounter = 0; | ||
bool _closed = false; | ||
|
||
/// Fire-and-forget send to the worker. | ||
void send(Object? message) { | ||
_workerPort.send(message); | ||
} | ||
|
||
/// Send a request to the worker and await a response. | ||
Future<Object?> request(Object? payload) async { | ||
if (_closed) throw StateError('Worker is closed'); | ||
final id = _idCounter++; | ||
final completer = Completer<Object?>.sync(); | ||
_pending[id] = completer; | ||
_workerPort.send((id, payload)); | ||
return await completer.future; | ||
} | ||
|
||
void close() { | ||
if (_closed) return; | ||
_closed = true; | ||
_workerPort.send(_shutdownCommand); | ||
if (_pending.isEmpty) { | ||
_responses.close(); | ||
} | ||
} | ||
|
||
void _handleResponse(dynamic message) { | ||
final (int id, Object? response) = message as (int, Object?); | ||
final completer = _pending.remove(id); | ||
if (completer == null) return; | ||
|
||
if (response is RemoteError) { | ||
completer.completeError(response); | ||
} else { | ||
completer.complete(response); | ||
} | ||
|
||
if (_closed && _pending.isEmpty) { | ||
_responses.close(); | ||
} | ||
} | ||
} | ||
|
||
/// Worker (isolate) entry-point signature. | ||
typedef WorkerEntry = void Function((SendPort, WorkerConfig)); | ||
|
||
/// Spawn a worker isolate and handshake to obtain its SendPort. | ||
Future<Worker> spawnWorker( | ||
WorkerConfig config, | ||
WorkerEntry entry, | ||
) async { | ||
final initPort = RawReceivePort(); | ||
final connection = Completer<(ReceivePort, SendPort)>.sync(); | ||
initPort.handler = (SendPort commandPort) { | ||
connection.complete(( | ||
ReceivePort.fromRawReceivePort(initPort), | ||
commandPort, | ||
)); | ||
}; | ||
|
||
try { | ||
await Isolate.spawn<(SendPort, WorkerConfig)>( | ||
entry, | ||
(initPort.sendPort, config), | ||
debugName: config.debugName, | ||
); | ||
} on Object { | ||
initPort.close(); | ||
rethrow; | ||
} | ||
|
||
final (ReceivePort receivePort, SendPort sendPort) = await connection.future; | ||
return Worker(sendPort, receivePort); | ||
} | ||
|
||
// ------------------------------------------- | ||
// ISOLATE-SIDE API (runs inside the worker isolate) | ||
// ------------------------------------------- | ||
|
||
/// Message/request handler that runs inside the worker isolate. | ||
/// | ||
/// This does not represent the isolate lifecycle; it only defines how | ||
/// the worker processes incoming messages and optional request/response. | ||
abstract class WorkerHandler { | ||
/// Handle fire-and-forget messages sent from the host. | ||
FutureOr<void> onMessage(Object? message); | ||
|
||
/// Handle request/response payloads sent from the host. | ||
/// Return value is sent back to the host. Default: no-op. | ||
FutureOr<Object?> onRequest(Object? payload) => {}; | ||
} | ||
|
||
/// Runs the Sentry worker loop inside a background isolate. | ||
/// | ||
/// Call this only from the worker isolate entry-point spawned via | ||
/// [spawnWorker]. It configures logging, handshakes with the host, and routes | ||
/// messages | ||
void runWorker( | ||
WorkerConfig config, | ||
SendPort host, | ||
WorkerHandler handler, | ||
) { | ||
IsolateLogger.configure( | ||
debug: config.debug, | ||
level: config.diagnosticLevel, | ||
loggerName: config.debugName, | ||
); | ||
|
||
final inbox = ReceivePort(); | ||
host.send(inbox.sendPort); | ||
|
||
inbox.listen((msg) async { | ||
if (msg == _shutdownCommand) { | ||
IsolateLogger.log(SentryLevel.debug, 'Isolate received shutdown'); | ||
inbox.close(); | ||
IsolateLogger.log(SentryLevel.debug, 'Isolate closed'); | ||
return; | ||
} | ||
|
||
if (msg is (int, Object?)) { | ||
final (id, payload) = msg; | ||
try { | ||
final result = await handler.onRequest(payload); | ||
host.send((id, result)); | ||
} catch (e, st) { | ||
host.send((id, RemoteError(e.toString(), st.toString()))); | ||
} | ||
return; | ||
} | ||
|
||
try { | ||
await handler.onMessage(msg); | ||
} catch (exception, stackTrace) { | ||
IsolateLogger.log(SentryLevel.error, 'Isolate failed to handle message', | ||
exception: exception, stackTrace: stackTrace); | ||
} | ||
}); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
import 'dart:async'; | ||
import 'dart:isolate'; | ||
import 'dart:typed_data'; | ||
|
||
import 'package:meta/meta.dart'; | ||
import 'package:objective_c/objective_c.dart'; | ||
|
||
import '../../../sentry_flutter.dart'; | ||
import '../../isolate/isolate_worker.dart'; | ||
import '../../isolate/isolate_logger.dart'; | ||
import 'binding.dart' as cocoa; | ||
|
||
typedef SpawnWorkerFn = Future<Worker> Function(WorkerConfig, WorkerEntry); | ||
|
||
class CocoaEnvelopeSender { | ||
final SentryFlutterOptions _options; | ||
final WorkerConfig _config; | ||
final SpawnWorkerFn _spawn; | ||
Worker? _worker; | ||
|
||
CocoaEnvelopeSender(this._options, {SpawnWorkerFn? spawn}) | ||
: _config = WorkerConfig( | ||
debugName: 'SentryCocoaEnvelopeSender', | ||
debug: _options.debug, | ||
diagnosticLevel: _options.diagnosticLevel, | ||
automatedTestMode: _options.automatedTestMode, | ||
), | ||
_spawn = spawn ?? spawnWorker; | ||
|
||
@internal | ||
static CocoaEnvelopeSender Function(SentryFlutterOptions) factory = | ||
CocoaEnvelopeSender.new; | ||
|
||
FutureOr<void> start() async { | ||
if (_worker != null) return; | ||
_worker = await _spawn(_config, _entryPoint); | ||
} | ||
|
||
FutureOr<void> close() { | ||
_worker?.close(); | ||
_worker = null; | ||
} | ||
|
||
/// Fire-and-forget send of envelope bytes to the worker. | ||
void captureEnvelope(Uint8List envelopeData) { | ||
final client = _worker; | ||
if (client == null) { | ||
_options.log( | ||
SentryLevel.warning, | ||
'captureEnvelope called before start; dropping', | ||
); | ||
return; | ||
} | ||
client.send(TransferableTypedData.fromList([envelopeData])); | ||
} | ||
|
||
static void _entryPoint((SendPort, WorkerConfig) init) { | ||
final (host, config) = init; | ||
runWorker(config, host, _CocoaEnvelopeHandler(config)); | ||
} | ||
} | ||
|
||
class _CocoaEnvelopeHandler extends WorkerHandler { | ||
final WorkerConfig _config; | ||
|
||
_CocoaEnvelopeHandler(this._config); | ||
|
||
@override | ||
FutureOr<void> onMessage(Object? msg) { | ||
if (msg is TransferableTypedData) { | ||
final data = msg.materialize().asUint8List(); | ||
_captureEnvelope(data); | ||
} else { | ||
IsolateLogger.log(SentryLevel.warning, 'Unexpected message type: $msg'); | ||
} | ||
} | ||
|
||
void _captureEnvelope(Uint8List envelopeData) { | ||
try { | ||
final nsData = envelopeData.toNSData(); | ||
final envelope = cocoa.PrivateSentrySDKOnly.envelopeWithData(nsData); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be great to do some performance tests if using the background isolate here is really bringing more performance improvements than the overhead introduce by isolate communication. Alternatively, if calling the plugin code or a new helper we add from here is possible, we could also do the If possible, this would be way less code, making it more maintainable and we'd have no sending overhead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
the whole There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Isolate communication overhead is essentially 0 since we use https://api.flutter.dev/flutter/dart-isolate/TransferableTypedData-class.html There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. all in all I'd expect there to be almost no difference in speed but less load on the main isolate which is kind of hard to benchmark, I guess the best way would be to compare frame data There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have benchmarked this via a ~36mb envelope and
|
||
if (envelope != null) { | ||
cocoa.PrivateSentrySDKOnly.captureEnvelope(envelope); | ||
} else { | ||
IsolateLogger.log(SentryLevel.error, | ||
'Native Cocoa SDK returned null when capturing envelope'); | ||
} | ||
} catch (exception, stackTrace) { | ||
IsolateLogger.log(SentryLevel.error, 'Failed to capture envelope', | ||
exception: exception, stackTrace: stackTrace); | ||
if (_config.automatedTestMode) { | ||
rethrow; | ||
} | ||
} | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.