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: move all native calls to SentryNativeBinding #2097

Merged
merged 9 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,30 @@ import 'dart:async';

import '../../sentry_flutter.dart';
import '../integrations/integrations.dart';
import '../native/sentry_native.dart';

// ignore: implementation_imports
import 'package:sentry/src/sentry_tracer.dart';

/// EventProcessor that enriches [SentryTransaction] objects with app start
/// measurement.
class NativeAppStartEventProcessor implements EventProcessor {
final SentryNative _native;
final Hub _hub;

NativeAppStartEventProcessor(
this._native, {
Hub? hub,
}) : _hub = hub ?? HubAdapter();
NativeAppStartEventProcessor({Hub? hub}) : _hub = hub ?? HubAdapter();

@override
Future<SentryEvent?> apply(SentryEvent event, Hint hint) async {
final options = _hub.options;
if (_native.didAddAppStartMeasurement ||
if (NativeAppStartIntegration.didAddAppStartMeasurement ||
event is! SentryTransaction ||
options is! SentryFlutterOptions) {
return event;
}

final appStartInfo = await NativeAppStartIntegration.getAppStartInfo();

final appStartEnd = _native.appStartEnd;
if (!options.autoAppStart) {
final appStartEnd = NativeAppStartIntegration.appStartEnd;
if (appStartEnd != null) {
appStartInfo?.end = appStartEnd;
} else {
Expand All @@ -44,7 +39,7 @@ class NativeAppStartEventProcessor implements EventProcessor {
final measurement = appStartInfo?.toMeasurement();
if (measurement != null) {
event.measurements[measurement.name] = measurement;
_native.didAddAppStartMeasurement = true;
NativeAppStartIntegration.didAddAppStartMeasurement = true;
}

if (appStartInfo != null) {
Expand Down
11 changes: 6 additions & 5 deletions flutter/lib/src/file_system_transport.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,21 @@ import 'dart:typed_data';
import 'package:flutter/services.dart';
import 'package:sentry/sentry.dart';

import 'native/sentry_native_binding.dart';

class FileSystemTransport implements Transport {
FileSystemTransport(this._channel, this._options);
FileSystemTransport(this._native, this._options);

final MethodChannel _channel;
final SentryNativeBinding _native;
final SentryOptions _options;

@override
Future<SentryId?> send(SentryEnvelope envelope) async {
final envelopeData = <int>[];
await envelope.envelopeStream(_options).forEach(envelopeData.addAll);
// https://flutter.dev/docs/development/platform-integration/platform-channels#codec
final args = [Uint8List.fromList(envelopeData)];
try {
await _channel.invokeMethod('captureEnvelope', args);
// TODO avoid copy
await _native.captureEnvelope(Uint8List.fromList(envelopeData));
} catch (exception, stackTrace) {
_options.logger(
SentryLevel.error,
Expand Down
18 changes: 8 additions & 10 deletions flutter/lib/src/integrations/load_contexts_integration.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import 'dart:async';

import 'package:flutter/services.dart';
import 'package:sentry/sentry.dart';
import '../native/sentry_native_binding.dart';
import '../sentry_flutter_options.dart';

/// Load Device's Contexts from the iOS & Android SDKs.
Expand All @@ -15,32 +15,30 @@ import '../sentry_flutter_options.dart';
///
/// This integration is only executed on iOS, macOS & Android Apps.
class LoadContextsIntegration extends Integration<SentryFlutterOptions> {
final MethodChannel _channel;
final SentryNativeBinding _native;

LoadContextsIntegration(this._channel);
LoadContextsIntegration(this._native);

@override
void call(Hub hub, SentryFlutterOptions options) {
options.addEventProcessor(
_LoadContextsIntegrationEventProcessor(_channel, options),
_LoadContextsIntegrationEventProcessor(_native, options),
);
options.sdk.addIntegration('loadContextsIntegration');
}
}

class _LoadContextsIntegrationEventProcessor implements EventProcessor {
_LoadContextsIntegrationEventProcessor(this._channel, this._options);
_LoadContextsIntegrationEventProcessor(this._native, this._options);

final MethodChannel _channel;
final SentryNativeBinding _native;
final SentryFlutterOptions _options;

@override
Future<SentryEvent?> apply(SentryEvent event, Hint hint) async {
// TODO don't copy everything (i.e. avoid unnecessary Map.from())
try {
final loadContexts = await _channel.invokeMethod('loadContexts');

final infos =
Map<String, dynamic>.from(loadContexts is Map ? loadContexts : {});
final infos = await _native.loadContexts() ?? {};
final contextsMap = infos['contexts'] as Map?;
if (contextsMap != null && contextsMap.isNotEmpty) {
final contexts = Contexts.fromJson(
Expand Down
52 changes: 11 additions & 41 deletions flutter/lib/src/integrations/load_image_list_integration.dart
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import 'dart:async';

import 'package:flutter/services.dart';
import 'package:sentry/sentry.dart';
import '../native/sentry_native_binding.dart';
import '../sentry_flutter_options.dart';

/// Loads the native debug image list for stack trace symbolication.
class LoadImageListIntegration extends Integration<SentryFlutterOptions> {
final MethodChannel _channel;
final SentryNativeBinding _native;

LoadImageListIntegration(this._channel);
LoadImageListIntegration(this._native);

@override
void call(Hub hub, SentryFlutterOptions options) {
options.addEventProcessor(
_LoadImageListIntegrationEventProcessor(_channel, options),
_LoadImageListIntegrationEventProcessor(_native),
);

options.sdk.addIntegration('loadImageListIntegration');
Expand All @@ -32,64 +32,34 @@ extension _NeedsSymbolication on SentryEvent {
return frames.any((frame) => 'native' == frame?.platform);
}

List<SentryStackFrame?>? _getStacktraceFrames() {
Iterable<SentryStackFrame?>? _getStacktraceFrames() {
if (exceptions?.isNotEmpty == true) {
return exceptions?.first.stackTrace?.frames;
}
if (threads?.isNotEmpty == true) {
var stacktraces = threads?.map((e) => e.stacktrace);
return stacktraces
?.where((element) => element != null)
.expand((element) => element!.frames)
.toList();
.expand((element) => element!.frames);
}
return null;
}
}

class _LoadImageListIntegrationEventProcessor implements EventProcessor {
_LoadImageListIntegrationEventProcessor(this._channel, this._options);
_LoadImageListIntegrationEventProcessor(this._native);

final MethodChannel _channel;
final SentryFlutterOptions _options;
final SentryNativeBinding _native;

@override
Future<SentryEvent?> apply(SentryEvent event, Hint hint) async {
if (event.needsSymbolication()) {
try {
// we call on every event because the loaded image list is cached
// and it could be changed on the Native side.
final loadImageList = await _channel.invokeMethod('loadImageList');
final imageList = List<Map<dynamic, dynamic>>.from(
loadImageList is List ? loadImageList : [],
);
return copyWithDebugImages(event, imageList);
} catch (exception, stackTrace) {
_options.logger(
SentryLevel.error,
'loadImageList failed',
exception: exception,
stackTrace: stackTrace,
);
final images = await _native.loadDebugImages();
if (images != null) {
return event.copyWith(debugMeta: DebugMeta(images: images));
}
}

return event;
}

static SentryEvent copyWithDebugImages(
SentryEvent event, List<Object?> imageList) {
if (imageList.isEmpty) {
return event;
}

final newDebugImages = <DebugImage>[];
for (final obj in imageList) {
final jsonMap = Map<String, dynamic>.from(obj as Map<dynamic, dynamic>);
final image = DebugImage.fromJson(jsonMap);
newDebugImages.add(image);
}

return event.copyWith(debugMeta: DebugMeta(images: newDebugImages));
}
}
38 changes: 29 additions & 9 deletions flutter/lib/src/integrations/native_app_start_integration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import '../../sentry_flutter.dart';
import '../frame_callback_handler.dart';
import '../native/sentry_native.dart';
import '../native/sentry_native_binding.dart';
import '../event_processor/native_app_start_event_processor.dart';

/// Integration which handles communication with native frameworks in order to
Expand All @@ -16,10 +16,23 @@
{Hub? hub})
: _hub = hub ?? HubAdapter();

final SentryNative _native;
final SentryNativeBinding _native;
final FrameCallbackHandler _frameCallbackHandler;
final Hub _hub;

/// This timestamp marks the end of app startup. Either set automatically when
/// [SentryFlutterOptions.autoAppStart] is true, or by calling
/// [SentryFlutter.setAppStartEnd]
@internal
static DateTime? appStartEnd;

/// Flag indicating if app start was already fetched.
static bool _didFetchAppStart = false;

/// Flag indicating if app start measurement was added to the first transaction.
@internal
static bool didAddAppStartMeasurement = false;

/// Timeout duration to wait for the app start info to be fetched.
static const _timeoutDuration = Duration(seconds: 30);

Expand Down Expand Up @@ -55,6 +68,14 @@
static void clearAppStartInfo() {
_appStartInfo = null;
_appStartCompleter = Completer<AppStartInfo?>();
didAddAppStartMeasurement = false;
}

/// Reset state
@visibleForTesting
static void reset() {
appStartEnd = null;
_didFetchAppStart = false;
}

@override
Expand All @@ -73,11 +94,12 @@
return;
}

if (_native.didFetchAppStart) {
if (_didFetchAppStart) {
return;
}

_frameCallbackHandler.addPostFrameCallback((timeStamp) async {
_didFetchAppStart = true;
final nativeAppStart = await _native.fetchNativeAppStart();
if (nativeAppStart == null) {
setAppStartInfo(null);
Expand All @@ -94,14 +116,12 @@
nativeAppStart.appStartTime.toInt());
final pluginRegistrationDateTime = DateTime.fromMillisecondsSinceEpoch(
nativeAppStart.pluginRegistrationTime);
DateTime? appStartEndDateTime;

if (options.autoAppStart) {
// We only assign the current time if it's not already set - this is useful in tests
_native.appStartEnd ??= options.clock();
appStartEndDateTime = _native.appStartEnd;
appStartEnd ??= options.clock();

Check warning on line 122 in flutter/lib/src/integrations/native_app_start_integration.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/integrations/native_app_start_integration.dart#L122

Added line #L122 was not covered by tests

final duration = appStartEndDateTime?.difference(appStartDateTime);
final duration = appStartEnd?.difference(appStartDateTime);

// We filter out app start more than 60s.
// This could be due to many different reasons.
Expand Down Expand Up @@ -143,7 +163,7 @@
final appStartInfo = AppStartInfo(
nativeAppStart.isColdStart ? AppStartType.cold : AppStartType.warm,
start: appStartDateTime,
end: appStartEndDateTime,
end: appStartEnd,
pluginRegistration: pluginRegistrationDateTime,
sentrySetupStart: sentrySetupStartDateTime,
nativeSpanTimes: nativeSpanTimes);
Expand Down Expand Up @@ -174,7 +194,7 @@
}
});

options.addEventProcessor(NativeAppStartEventProcessor(_native, hub: hub));
options.addEventProcessor(NativeAppStartEventProcessor(hub: hub));

options.sdk.addIntegration('nativeAppStartIntegration');
}
Expand Down
4 changes: 2 additions & 2 deletions flutter/lib/src/integrations/native_sdk_integration.dart
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import 'dart:async';

import 'package:sentry/sentry.dart';
import '../native/sentry_native.dart';
import '../native/sentry_native_binding.dart';
import '../sentry_flutter_options.dart';

/// Enables Sentry's native SDKs (Android and iOS) with options.
class NativeSdkIntegration implements Integration<SentryFlutterOptions> {
NativeSdkIntegration(this._native);

SentryFlutterOptions? _options;
final SentryNative _native;
final SentryNativeBinding _native;

@override
Future<void> call(Hub hub, SentryFlutterOptions options) async {
Expand Down
16 changes: 8 additions & 8 deletions flutter/lib/src/native/cocoa/sentry_native_cocoa.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@
class SentryNativeCocoa extends SentryNativeChannel {
late final _lib = cocoa.SentryCocoa(DynamicLibrary.process());

SentryNativeCocoa(super.channel);
SentryNativeCocoa(super.options, super.channel);

@override
int? startProfiler(SentryId traceId) {
final cSentryId = cocoa.SentryId1.alloc(_lib)
..initWithUUIDString_(cocoa.NSString(_lib, traceId.toString()));
final startTime =
cocoa.PrivateSentrySDKOnly.startProfilerForTrace_(_lib, cSentryId);
return startTime;
}
int? startProfiler(SentryId traceId) => tryCatchSync('startProfiler', () {
final cSentryId = cocoa.SentryId1.alloc(_lib)
..initWithUUIDString_(cocoa.NSString(_lib, traceId.toString()));

Check warning on line 18 in flutter/lib/src/native/cocoa/sentry_native_cocoa.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/native/cocoa/sentry_native_cocoa.dart#L18

Added line #L18 was not covered by tests
final startTime =
cocoa.PrivateSentrySDKOnly.startProfilerForTrace_(_lib, cSentryId);

Check warning on line 20 in flutter/lib/src/native/cocoa/sentry_native_cocoa.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/native/cocoa/sentry_native_cocoa.dart#L20

Added line #L20 was not covered by tests
return startTime;
});
}
14 changes: 8 additions & 6 deletions flutter/lib/src/native/factory_real.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
import 'sentry_native_binding.dart';
import 'sentry_native_channel.dart';

SentryNativeBinding createBinding(PlatformChecker pc, MethodChannel channel) {
if (pc.platform.isIOS || pc.platform.isMacOS) {
return SentryNativeCocoa(channel);
} else if (pc.platform.isAndroid) {
return SentryNativeJava(channel);
SentryNativeBinding createBinding(SentryFlutterOptions options,
{MethodChannel channel = const MethodChannel('sentry_flutter')}) {
final platform = options.platformChecker.platform;
if (platform.isIOS || platform.isMacOS) {
return SentryNativeCocoa(options, channel);
} else if (platform.isAndroid) {
return SentryNativeJava(options, channel);
} else {
return SentryNativeChannel(channel);
return SentryNativeChannel(options, channel);

Check warning on line 17 in flutter/lib/src/native/factory_real.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/native/factory_real.dart#L17

Added line #L17 was not covered by tests
}
}
3 changes: 2 additions & 1 deletion flutter/lib/src/native/factory_web.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import '../../sentry_flutter.dart';
import 'sentry_native_binding.dart';

// This isn't actually called, see SentryFlutter.init()
SentryNativeBinding createBinding(PlatformChecker _, MethodChannel __) {
SentryNativeBinding createBinding(SentryFlutterOptions options,
{MethodChannel channel = const MethodChannel('sentry_flutter')}) {
throw UnsupportedError("Native binding is not supported on this platform.");
}
2 changes: 1 addition & 1 deletion flutter/lib/src/native/java/sentry_native_java.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ import '../sentry_native_channel.dart';
// generated JNI bindings. See https://github.com/getsentry/sentry-dart/issues/1444
@internal
class SentryNativeJava extends SentryNativeChannel {
SentryNativeJava(super.channel);
SentryNativeJava(super.options, super.channel);
}
Loading
Loading