diff --git a/CHANGELOG.md b/CHANGELOG.md index 458e511cd5..9a7b04ba0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - Method `hint.addAll(Map keysAndValues)` now takes `Map` instead of `Map` - Method `set(String key, dynamic value)` now takes value of `dynamic` instead of `Object` - Method `hint.get(String key)` now returns `dynamic` instead of `Object?` +- Add `blocked_main_thread` to sync file spans ([#1801](https://github.com/getsentry/sentry-dart/pull/1801)) ## 7.15.0 @@ -27,7 +28,7 @@ - Starting with Flutter 3.16, Sentry adds the [`appFlavor`](https://api.flutter.dev/flutter/services/appFlavor-constant.html) to the `flutter_context` ([#1799](https://github.com/getsentry/sentry-dart/pull/1799)) - Add beforeScreenshotCallback to SentryFlutterOptions ([#1805](https://github.com/getsentry/sentry-dart/pull/1805)) - Add support for `readTransaction` in `sqflite` ([#1819](https://github.com/getsentry/sentry-dart/pull/1819)) - + ### Dependencies - Bump Android SDK from v7.0.0 to v7.2.0 ([#1788](https://github.com/getsentry/sentry-dart/pull/1788), [#1815](https://github.com/getsentry/sentry-dart/pull/1815)) diff --git a/dart/lib/src/sentry_options.dart b/dart/lib/src/sentry_options.dart index 918a5d5758..7ac94babf7 100644 --- a/dart/lib/src/sentry_options.dart +++ b/dart/lib/src/sentry_options.dart @@ -1,5 +1,6 @@ import 'dart:async'; import 'dart:developer'; +import 'dart:isolate'; import 'package:meta/meta.dart'; import 'package:http/http.dart'; @@ -446,6 +447,11 @@ class SentryOptions { late SentryStackTraceFactory stackTraceFactory = SentryStackTraceFactory(this); + @internal + IsMainIsolateCallback isMainIsolate = () { + return Isolate.current.debugName == 'main'; + }; + void _debugLogger( SentryLevel level, String message, { @@ -527,3 +533,6 @@ void dartLogger( stackTrace: stackTrace, ); } + +/// A callback used to check if we are on the main isolate. +typedef IsMainIsolateCallback = bool Function(); diff --git a/file/lib/src/sentry_file.dart b/file/lib/src/sentry_file.dart index de0004c938..b02cd777f0 100644 --- a/file/lib/src/sentry_file.dart +++ b/file/lib/src/sentry_file.dart @@ -490,6 +490,9 @@ class SentryFile implements File { span?.origin = SentryTraceOrigins.autoFile; span?.setData('file.async', false); + final isMainIsolate = _hub.options.isMainIsolate(); + span?.setData('blocked_main_thread', isMainIsolate); + final Map breadcrumbData = {}; breadcrumbData['file.async'] = false; diff --git a/file/test/sentry_file_test.dart b/file/test/sentry_file_test.dart index 3f7135de54..a4413732d6 100644 --- a/file/test/sentry_file_test.dart +++ b/file/test/sentry_file_test.dart @@ -657,6 +657,286 @@ void main() { ); }); }); + + group('$SentryFile span sets `blocked_main_thread`', () { + late Fixture fixture; + + setUp(() { + fixture = Fixture(); + }); + + tearDown(() {}); + + void _assertSpan(bool isMainIsolate) { + final call = fixture.client.captureTransactionCalls.first; + final span = call.transaction.spans.first; + expect(span.data['blocked_main_thread'], isMainIsolate); + } + + test('to `true` when calling `copySync` on main', () async { + final file = File('test_resources/testfile.txt'); + + final sut = fixture.getSut( + file, + sendDefaultPii: true, + tracesSampleRate: 1.0, + isMainIsolate: true, + ); + + final tr = fixture.hub.startTransaction('name', 'op', bindToScope: true); + sut.copySync('test_resources/testfile_copy.txt'); + await tr.finish(); + + _assertSpan(true); + }); + + test('to `false` when not calling `copySync` on main', () async { + final file = File('test_resources/testfile.txt'); + + final sut = fixture.getSut( + file, + sendDefaultPii: true, + tracesSampleRate: 1.0, + isMainIsolate: false, + ); + + final tr = fixture.hub.startTransaction('name', 'op', bindToScope: true); + final newFile = sut.copySync('test_resources/testfile_copy.txt'); + await tr.finish(); + + _assertSpan(false); + + newFile.deleteSync(); + }); + + test('to `true` when calling `createSync` on main', () async { + final file = File('test_resources/testfile.txt'); + + final sut = fixture.getSut( + file, + sendDefaultPii: true, + tracesSampleRate: 1.0, + isMainIsolate: true, + ); + + final tr = fixture.hub.startTransaction('name', 'op', bindToScope: true); + sut.createSync(); + await tr.finish(); + + _assertSpan(true); + }); + + test('to `false` when not calling `createSync` on main', () async { + final file = File('test_resources/testfile.txt'); + + final sut = fixture.getSut( + file, + sendDefaultPii: true, + tracesSampleRate: 1.0, + isMainIsolate: false, + ); + + final tr = fixture.hub.startTransaction('name', 'op', bindToScope: true); + sut.createSync(); + await tr.finish(); + + _assertSpan(false); + }); + + // deleteSync + + test('to `true` when calling `deleteSync` on main', () async { + final file = File('test_resources/testfile_delete.txt'); + file.createSync(); + expect(file.existsSync(), true); + + final sut = fixture.getSut( + file, + sendDefaultPii: true, + tracesSampleRate: 1.0, + isMainIsolate: true, + ); + + final tr = fixture.hub.startTransaction('name', 'op', bindToScope: true); + sut.deleteSync(); + await tr.finish(); + + _assertSpan(true); + }); + + test('to `false` when not calling `deleteSync` on main', () async { + final file = File('test_resources/testfile_delete.txt'); + file.createSync(); + expect(file.existsSync(), true); + + final sut = fixture.getSut( + file, + sendDefaultPii: true, + tracesSampleRate: 1.0, + isMainIsolate: false, + ); + + final tr = fixture.hub.startTransaction('name', 'op', bindToScope: true); + sut.deleteSync(); + await tr.finish(); + + _assertSpan(false); + }); + + // readAsBytesSync + + test('to `true` when calling `readAsBytesSync` on main', () async { + final file = File('test_resources/sentry.png'); + + final sut = fixture.getSut( + file, + sendDefaultPii: true, + tracesSampleRate: 1.0, + isMainIsolate: true, + ); + + final tr = fixture.hub.startTransaction('name', 'op', bindToScope: true); + sut.readAsBytesSync(); + await tr.finish(); + + _assertSpan(true); + }); + + test('to `false` when not calling `readAsBytesSync` on main', () async { + final file = File('test_resources/testfile.txt'); + + final sut = fixture.getSut( + file, + sendDefaultPii: true, + tracesSampleRate: 1.0, + isMainIsolate: false, + ); + + final tr = fixture.hub.startTransaction('name', 'op', bindToScope: true); + sut.readAsBytesSync(); + await tr.finish(); + + _assertSpan(false); + }); + + // readAsLinesSync + + test('to `true` when calling `readAsLinesSync` on main', () async { + final file = File('test_resources/testfile.txt'); + + final sut = fixture.getSut( + file, + sendDefaultPii: true, + tracesSampleRate: 1.0, + isMainIsolate: true, + ); + + final tr = fixture.hub.startTransaction('name', 'op', bindToScope: true); + + sut.readAsLinesSync(); + + await tr.finish(); + + _assertSpan(true); + }); + + test('to `false` when not calling `readAsLinesSync` on main', () async { + final file = File('test_resources/testfile.txt'); + + final sut = fixture.getSut( + file, + sendDefaultPii: true, + tracesSampleRate: 1.0, + isMainIsolate: false, + ); + + final tr = fixture.hub.startTransaction('name', 'op', bindToScope: true); + sut.readAsLinesSync(); + await tr.finish(); + + _assertSpan(false); + }); + + // readAsStringSync + + test('to `true` when calling `readAsStringSync` on main', () async { + final file = File('test_resources/testfile.txt'); + + final sut = fixture.getSut( + file, + sendDefaultPii: true, + tracesSampleRate: 1.0, + isMainIsolate: true, + ); + + final tr = fixture.hub.startTransaction('name', 'op', bindToScope: true); + + sut.readAsStringSync(); + + await tr.finish(); + + _assertSpan(true); + }); + + test('to `false` when not calling `readAsStringSync` on main', () async { + final file = File('test_resources/testfile.txt'); + + final sut = fixture.getSut( + file, + sendDefaultPii: true, + tracesSampleRate: 1.0, + isMainIsolate: false, + ); + + final tr = fixture.hub.startTransaction('name', 'op', bindToScope: true); + sut.readAsStringSync(); + await tr.finish(); + + _assertSpan(false); + }); + + // renameSync + + test('to `true` when calling `renameSync` on main', () async { + final file = File('test_resources/old_name.txt'); + file.createSync(); + + final sut = fixture.getSut( + file, + sendDefaultPii: true, + tracesSampleRate: 1.0, + isMainIsolate: true, + ); + + final tr = fixture.hub.startTransaction('name', 'op', bindToScope: true); + final newFile = sut.renameSync('test_resources/testfile_copy.txt'); + await tr.finish(); + + _assertSpan(true); + + newFile.deleteSync(); + }); + + test('to `false` when not calling `renameSync` on main', () async { + final file = File('test_resources/old_name.txt'); + file.createSync(); + + final sut = fixture.getSut( + file, + sendDefaultPii: true, + tracesSampleRate: 1.0, + isMainIsolate: false, + ); + + final tr = fixture.hub.startTransaction('name', 'op', bindToScope: true); + final newFile = sut.renameSync('test_resources/testfile_copy.txt'); + await tr.finish(); + + _assertSpan(false); + + newFile.deleteSync(); + }); + }); } class Fixture { @@ -667,10 +947,14 @@ class Fixture { SentryFile getSut( File file, { bool sendDefaultPii = false, + bool isMainIsolate = false, double? tracesSampleRate, }) { options.sendDefaultPii = sendDefaultPii; options.tracesSampleRate = tracesSampleRate; + options.isMainIsolate = () { + return isMainIsolate; + }; hub = Hub(options); hub.bindClient(client); diff --git a/flutter/example/integration_test/integration_test.dart b/flutter/example/integration_test/integration_test.dart index c4c71edb41..e26c35b771 100644 --- a/flutter/example/integration_test/integration_test.dart +++ b/flutter/example/integration_test/integration_test.dart @@ -2,9 +2,13 @@ import 'dart:async'; import 'dart:convert'; +import 'dart:io'; +import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:path_provider/path_provider.dart'; +import 'package:sentry_file/sentry_file.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry_flutter_example/main.dart'; import 'package:http/http.dart'; @@ -29,7 +33,7 @@ void main() { await tester.pumpWidget(SentryScreenshotWidget( child: DefaultAssetBundle( bundle: SentryAssetBundle(enableStructuredDataTracing: true), - child: const MyApp(), + child: const MyApp(withNavigatorObserver: false), ))); }, dsn ?? fakeDsn, @@ -141,6 +145,65 @@ void main() { await transaction.finish(); }); + testWidgets( + 'sync call in sentry file sets blocked_main_thread to true on main isolate', + (tester) async { + await setupSentryAndApp(tester); + final documentsDir = await getApplicationDocumentsDirectory(); + final file = File('${documentsDir.path}/testfile.txt').sentryTrace(); + + // Start a transaction if there's no active transaction + Sentry.startTransaction( + 'File', + 'file', + bindToScope: true, + ); + + file.createSync(); + + var span = Sentry.getSpan() as dynamic; + var fileSpan = span.children.first; + expect(fileSpan?.data['blocked_main_thread'], true); + }); + + testWidgets( + 'sync call in sentry file sets blocked_main_thread to false on background isolate', + (tester) async { + // Only setup app without sentry. + await tester.pumpWidget(SentryScreenshotWidget( + child: DefaultAssetBundle( + bundle: SentryAssetBundle(enableStructuredDataTracing: true), + child: const MyApp(withNavigatorObserver: false), + ))); + + final documentsDir = await getApplicationDocumentsDirectory(); + final file = File('${documentsDir.path}/testfile.txt').sentryTrace(); + + final blockedMainThread = await compute((path) async { + // Isolates do not share state. Init Sentry within isolate. + await Sentry.init((options) { + options.dsn = 'https://abc@def.ingest.sentry.io/1234567'; + options.tracesSampleRate = 1.0; + }); + + // Start a transaction if there's no active transaction + Sentry.startTransaction( + 'File', + 'file', + bindToScope: true, + ); + + final sentryFile = file.sentryTrace(); + sentryFile.createSync(); + + var span = Sentry.getSpan() as dynamic; + var fileSpan = span.children.first; + return fileSpan?.data['blocked_main_thread']; + }, file); + + expect(blockedMainThread, false); + }); + // group('e2e', () { // var output = find.byKey(const Key('output')); // late Fixture fixture; diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index 26bbdce6d8..3cb0e39979 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -91,13 +91,23 @@ Future setupSentry(AppRunner appRunner, String dsn, } class MyApp extends StatefulWidget { - const MyApp({Key? key}) : super(key: key); + const MyApp({Key? key, this.withNavigatorObserver = true}) : super(key: key); + + final bool withNavigatorObserver; @override _MyAppState createState() => _MyAppState(); } class _MyAppState extends State { + late bool withNavigatorObserver; + + @override + void initState() { + super.initState(); + withNavigatorObserver = widget.withNavigatorObserver; + } + @override Widget build(BuildContext context) { return feedback.BetterFeedback( @@ -107,7 +117,7 @@ class _MyAppState extends State { builder: (context) => MaterialApp( navigatorKey: navigatorKey, navigatorObservers: [ - SentryNavigatorObserver(), + if (withNavigatorObserver) SentryNavigatorObserver() ], theme: Provider.of(context).theme, home: const MainScaffold(),