From 5c874775eebe7aec2fa685f91c45b0b0da397135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Wed, 24 Nov 2021 16:43:48 +0100 Subject: [PATCH 01/38] didPush starts transaction --- .../navigation/sentry_navigator_observer.dart | 14 ++ flutter/pubspec.yaml | 1 + flutter/test/mocks.mocks.dart | 125 +++++++++++++----- .../test/sentry_navigator_observer_test.dart | 30 ++++- 4 files changed, 130 insertions(+), 40 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 2193ab568d..270f058bb5 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -44,6 +44,7 @@ class SentryNavigatorObserver extends RouteObserver> { final Hub _hub; final bool _setRouteNameAsTransaction; + ISentrySpan? _transaction; @override void didPush(Route route, Route? previousRoute) { @@ -54,6 +55,7 @@ class SentryNavigatorObserver extends RouteObserver> { from: previousRoute?.settings, to: route.settings, ); + _startTransaction(route.settings.name, route.settings.arguments); } @override @@ -100,6 +102,18 @@ class SentryNavigatorObserver extends RouteObserver> { }); } } + + void _startTransaction(String? name, Object? arguments) { + _transaction = _hub.startTransaction( + name ?? 'unknown', + 'ui.load', + bindToScope: true, + ); + if (arguments != null) { + _transaction?.setData('route_settings_arguments', arguments); + } + } + } /// This class makes it easier to record breadcrumbs for events of Flutters diff --git a/flutter/pubspec.yaml b/flutter/pubspec.yaml index d1c3006510..dcf1b6a385 100644 --- a/flutter/pubspec.yaml +++ b/flutter/pubspec.yaml @@ -17,6 +17,7 @@ dependencies: package_info_plus: ^1.0.0 dev_dependencies: + build_runner: ^2.1.5 flutter_test: sdk: flutter mockito: ^5.0.16 diff --git a/flutter/test/mocks.mocks.dart b/flutter/test/mocks.mocks.dart index 2ee6b6f00e..5f87f720cc 100644 --- a/flutter/test/mocks.mocks.dart +++ b/flutter/test/mocks.mocks.dart @@ -1,29 +1,32 @@ -// Mocks generated by Mockito 5.0.7 from annotations -// in sentry_flutter/example/ios/.symlinks/plugins/sentry_flutter/test/mocks.dart. +// Mocks generated by Mockito 5.0.16 from annotations +// in sentry_flutter/test/mocks.dart. // Do not manually edit this file. -import 'dart:async' as _i4; +import 'dart:async' as _i5; import 'package:mockito/mockito.dart' as _i1; import 'package:sentry/src/hub.dart' as _i3; -import 'package:sentry/src/protocol/breadcrumb.dart' as _i7; -import 'package:sentry/src/protocol/sentry_event.dart' as _i5; -import 'package:sentry/src/protocol/sentry_id.dart' as _i2; -import 'package:sentry/src/protocol/sentry_level.dart' as _i6; -import 'package:sentry/src/sentry_client.dart' as _i8; -import 'package:sentry/src/sentry_envelope.dart' as _i10; -import 'package:sentry/src/transport/transport.dart' as _i9; +import 'package:sentry/src/protocol.dart' as _i2; +import 'package:sentry/src/sentry_client.dart' as _i7; +import 'package:sentry/src/sentry_envelope.dart' as _i9; +import 'package:sentry/src/sentry_user_feedback.dart' as _i6; +import 'package:sentry/src/tracing.dart' as _i4; +import 'package:sentry/src/transport/transport.dart' as _i8; +// ignore_for_file: avoid_redundant_argument_values +// ignore_for_file: avoid_setters_without_getters // ignore_for_file: comment_references -// ignore_for_file: unnecessary_parenthesis - +// ignore_for_file: implementation_imports +// ignore_for_file: invalid_use_of_visible_for_testing_member // ignore_for_file: prefer_const_constructors +// ignore_for_file: unnecessary_parenthesis +// ignore_for_file: camel_case_types -// ignore_for_file: avoid_redundant_argument_values +class _FakeSentryId_0 extends _i1.Fake implements _i2.SentryId {} -class _FakeSentryId extends _i1.Fake implements _i2.SentryId {} +class _FakeHub_1 extends _i1.Fake implements _i3.Hub {} -class _FakeHub extends _i1.Fake implements _i3.Hub {} +class _FakeISentrySpan_2 extends _i1.Fake implements _i4.ISentrySpan {} /// A class which mocks [Hub]. /// @@ -40,9 +43,9 @@ class MockHub extends _i1.Mock implements _i3.Hub { @override _i2.SentryId get lastEventId => (super.noSuchMethod(Invocation.getter(#lastEventId), - returnValue: _FakeSentryId()) as _i2.SentryId); + returnValue: _FakeSentryId_0()) as _i2.SentryId); @override - _i4.Future<_i2.SentryId> captureEvent(_i5.SentryEvent? event, + _i5.Future<_i2.SentryId> captureEvent(_i2.SentryEvent? event, {dynamic stackTrace, dynamic hint, _i3.ScopeCallback? withScope}) => (super.noSuchMethod( Invocation.method(#captureEvent, [ @@ -52,10 +55,10 @@ class MockHub extends _i1.Mock implements _i3.Hub { #hint: hint, #withScope: withScope }), - returnValue: Future<_i2.SentryId>.value(_FakeSentryId())) - as _i4.Future<_i2.SentryId>); + returnValue: Future<_i2.SentryId>.value(_FakeSentryId_0())) + as _i5.Future<_i2.SentryId>); @override - _i4.Future<_i2.SentryId> captureException(dynamic throwable, + _i5.Future<_i2.SentryId> captureException(dynamic throwable, {dynamic stackTrace, dynamic hint, _i3.ScopeCallback? withScope}) => (super.noSuchMethod( Invocation.method(#captureException, [ @@ -65,11 +68,11 @@ class MockHub extends _i1.Mock implements _i3.Hub { #hint: hint, #withScope: withScope }), - returnValue: Future<_i2.SentryId>.value(_FakeSentryId())) - as _i4.Future<_i2.SentryId>); + returnValue: Future<_i2.SentryId>.value(_FakeSentryId_0())) + as _i5.Future<_i2.SentryId>); @override - _i4.Future<_i2.SentryId> captureMessage(String? message, - {_i6.SentryLevel? level, + _i5.Future<_i2.SentryId> captureMessage(String? message, + {_i2.SentryLevel? level, String? template, List? params, dynamic hint, @@ -84,40 +87,90 @@ class MockHub extends _i1.Mock implements _i3.Hub { #hint: hint, #withScope: withScope }), - returnValue: Future<_i2.SentryId>.value(_FakeSentryId())) - as _i4.Future<_i2.SentryId>); + returnValue: Future<_i2.SentryId>.value(_FakeSentryId_0())) + as _i5.Future<_i2.SentryId>); + @override + _i5.Future captureUserFeedback(_i6.SentryUserFeedback? userFeedback) => + (super.noSuchMethod( + Invocation.method(#captureUserFeedback, [userFeedback]), + returnValue: Future.value(), + returnValueForMissingStub: Future.value()) as _i5.Future); @override - void addBreadcrumb(_i7.Breadcrumb? crumb, {dynamic hint}) => super + void addBreadcrumb(_i2.Breadcrumb? crumb, {dynamic hint}) => super .noSuchMethod(Invocation.method(#addBreadcrumb, [crumb], {#hint: hint}), returnValueForMissingStub: null); @override - void bindClient(_i8.SentryClient? client) => + void bindClient(_i7.SentryClient? client) => super.noSuchMethod(Invocation.method(#bindClient, [client]), returnValueForMissingStub: null); @override _i3.Hub clone() => (super.noSuchMethod(Invocation.method(#clone, []), - returnValue: _FakeHub()) as _i3.Hub); + returnValue: _FakeHub_1()) as _i3.Hub); @override - _i4.Future close() => (super.noSuchMethod(Invocation.method(#close, []), - returnValue: Future.value(null), - returnValueForMissingStub: Future.value()) as _i4.Future); + _i5.Future close() => (super.noSuchMethod(Invocation.method(#close, []), + returnValue: Future.value(), + returnValueForMissingStub: Future.value()) as _i5.Future); @override void configureScope(_i3.ScopeCallback? callback) => super.noSuchMethod(Invocation.method(#configureScope, [callback]), returnValueForMissingStub: null); + @override + _i4.ISentrySpan startTransaction(String? name, String? operation, + {String? description, + bool? bindToScope, + Map? customSamplingContext}) => + (super.noSuchMethod( + Invocation.method(#startTransaction, [ + name, + operation + ], { + #description: description, + #bindToScope: bindToScope, + #customSamplingContext: customSamplingContext + }), + returnValue: _FakeISentrySpan_2()) as _i4.ISentrySpan); + @override + _i4.ISentrySpan startTransactionWithContext( + _i4.SentryTransactionContext? transactionContext, + {Map? customSamplingContext, + bool? bindToScope}) => + (super.noSuchMethod( + Invocation.method(#startTransactionWithContext, [ + transactionContext + ], { + #customSamplingContext: customSamplingContext, + #bindToScope: bindToScope + }), + returnValue: _FakeISentrySpan_2()) as _i4.ISentrySpan); + @override + _i5.Future<_i2.SentryId> captureTransaction( + _i2.SentryTransaction? transaction) => + (super.noSuchMethod(Invocation.method(#captureTransaction, [transaction]), + returnValue: Future<_i2.SentryId>.value(_FakeSentryId_0())) + as _i5.Future<_i2.SentryId>); + @override + void setSpanContext( + dynamic throwable, _i4.ISentrySpan? span, String? transaction) => + super.noSuchMethod( + Invocation.method(#setSpanContext, [throwable, span, transaction]), + returnValueForMissingStub: null); + @override + String toString() => super.toString(); } /// A class which mocks [Transport]. /// /// See the documentation for Mockito's code generation for more information. -class MockTransport extends _i1.Mock implements _i9.Transport { +class MockTransport extends _i1.Mock implements _i8.Transport { MockTransport() { _i1.throwOnMissingStub(this); } @override - _i4.Future<_i2.SentryId> send(_i10.SentryEnvelope? envelope) => + _i5.Future<_i2.SentryId?> send(_i9.SentryEnvelope? envelope) => (super.noSuchMethod(Invocation.method(#send, [envelope]), - returnValue: Future<_i2.SentryId>.value(_FakeSentryId())) - as _i4.Future<_i2.SentryId>); + returnValue: Future<_i2.SentryId?>.value()) + as _i5.Future<_i2.SentryId?>); + @override + String toString() => super.toString(); } diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 0ca5496d09..f493b679ff 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -1,3 +1,4 @@ +import 'package:flutter/material.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; @@ -6,13 +7,37 @@ import 'package:sentry_flutter/sentry_flutter.dart'; import 'mocks.dart'; import 'mocks.mocks.dart'; + void main() { late Fixture fixture; + PageRoute route(RouteSettings? settings) => PageRouteBuilder( + pageBuilder: (_, __, ___) => Container(), + settings: settings, + ); + setUp(() { fixture = Fixture(); }); + group('RouteObserverTransaction', () { + test('didPush starts transaction', () { + final currentRoute = route(RouteSettings(name: 'Current Route')); + + final hub = MockHub(); + when(hub.startTransaction('Current Route', 'ui.load', description: null, bindToScope: true, customSamplingContext: null)).thenReturn(NoOpSentrySpan()); + final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + + sut.didPush(currentRoute, null); + + verify(hub.startTransaction('Current Route', 'ui.load', description: null, bindToScope: true, customSamplingContext: null)); + }); + + test('didPush finishes previous transaction', () { + + }); + }); + group('RouteObserverBreadcrumb', () { test('happy path with string route agrument', () { const fromRouteSettings = RouteSettings( @@ -142,10 +167,7 @@ void main() { }); group('SentryNavigatorObserver', () { - PageRoute route(RouteSettings? settings) => PageRouteBuilder( - pageBuilder: (_, __, ___) => Container(), - settings: settings, - ); + RouteSettings routeSettings(String? name, [Object? arguments]) => RouteSettings(name: name, arguments: arguments); From e72ccc651a60375eda5c67cb5597249d77f1c67f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Wed, 24 Nov 2021 17:06:37 +0100 Subject: [PATCH 02/38] didPush finishes previous transaction --- .../navigation/sentry_navigator_observer.dart | 5 ++ flutter/test/mocks.dart | 2 +- flutter/test/mocks.mocks.dart | 72 +++++++++++++++++++ .../test/sentry_navigator_observer_test.dart | 12 ++++ 4 files changed, 90 insertions(+), 1 deletion(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 270f058bb5..f05a259ef7 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -55,6 +55,7 @@ class SentryNavigatorObserver extends RouteObserver> { from: previousRoute?.settings, to: route.settings, ); + _finishTransaction(); _startTransaction(route.settings.name, route.settings.arguments); } @@ -114,6 +115,10 @@ class SentryNavigatorObserver extends RouteObserver> { } } + Future _finishTransaction() async { + //_transaction?.status ??= SpanStatus.ok(); + return await _transaction?.finish(); + } } /// This class makes it easier to record breadcrumbs for events of Flutters diff --git a/flutter/test/mocks.dart b/flutter/test/mocks.dart index 3e7ae0e655..79a026a902 100644 --- a/flutter/test/mocks.dart +++ b/flutter/test/mocks.dart @@ -6,7 +6,7 @@ import 'package:sentry/src/sentry_user_feedback.dart'; const fakeDsn = 'https://abc@def.ingest.sentry.io/1234567'; -@GenerateMocks([Hub, Transport]) +@GenerateMocks([Hub, Transport, NoOpSentrySpan]) void main() {} class MockPlatform implements Platform { diff --git a/flutter/test/mocks.mocks.dart b/flutter/test/mocks.mocks.dart index 5f87f720cc..0dc037d7d2 100644 --- a/flutter/test/mocks.mocks.dart +++ b/flutter/test/mocks.mocks.dart @@ -6,6 +6,7 @@ import 'dart:async' as _i5; import 'package:mockito/mockito.dart' as _i1; import 'package:sentry/src/hub.dart' as _i3; +import 'package:sentry/src/noop_sentry_span.dart' as _i10; import 'package:sentry/src/protocol.dart' as _i2; import 'package:sentry/src/sentry_client.dart' as _i7; import 'package:sentry/src/sentry_envelope.dart' as _i9; @@ -28,6 +29,14 @@ class _FakeHub_1 extends _i1.Fake implements _i3.Hub {} class _FakeISentrySpan_2 extends _i1.Fake implements _i4.ISentrySpan {} +class _FakeSentrySpanContext_3 extends _i1.Fake + implements _i4.SentrySpanContext {} + +class _FakeDateTime_4 extends _i1.Fake implements DateTime {} + +class _FakeSentryTraceHeader_5 extends _i1.Fake + implements _i2.SentryTraceHeader {} + /// A class which mocks [Hub]. /// /// See the documentation for Mockito's code generation for more information. @@ -174,3 +183,66 @@ class MockTransport extends _i1.Mock implements _i8.Transport { @override String toString() => super.toString(); } + +/// A class which mocks [NoOpSentrySpan]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockNoOpSentrySpan extends _i1.Mock implements _i10.NoOpSentrySpan { + MockNoOpSentrySpan() { + _i1.throwOnMissingStub(this); + } + + @override + _i4.SentrySpanContext get context => + (super.noSuchMethod(Invocation.getter(#context), + returnValue: _FakeSentrySpanContext_3()) as _i4.SentrySpanContext); + @override + DateTime get startTimestamp => + (super.noSuchMethod(Invocation.getter(#startTimestamp), + returnValue: _FakeDateTime_4()) as DateTime); + @override + bool get finished => + (super.noSuchMethod(Invocation.getter(#finished), returnValue: false) + as bool); + @override + set throwable(dynamic throwable) => + super.noSuchMethod(Invocation.setter(#throwable, throwable), + returnValueForMissingStub: null); + @override + set status(_i2.SpanStatus? status) => + super.noSuchMethod(Invocation.setter(#status, status), + returnValueForMissingStub: null); + @override + _i5.Future finish({_i2.SpanStatus? status}) => + (super.noSuchMethod(Invocation.method(#finish, [], {#status: status}), + returnValue: Future.value(), + returnValueForMissingStub: Future.value()) as _i5.Future); + @override + void removeData(String? key) => + super.noSuchMethod(Invocation.method(#removeData, [key]), + returnValueForMissingStub: null); + @override + void removeTag(String? key) => + super.noSuchMethod(Invocation.method(#removeTag, [key]), + returnValueForMissingStub: null); + @override + void setData(String? key, dynamic value) => + super.noSuchMethod(Invocation.method(#setData, [key, value]), + returnValueForMissingStub: null); + @override + void setTag(String? key, String? value) => + super.noSuchMethod(Invocation.method(#setTag, [key, value]), + returnValueForMissingStub: null); + @override + _i4.ISentrySpan startChild(String? operation, {String? description}) => + (super.noSuchMethod( + Invocation.method( + #startChild, [operation], {#description: description}), + returnValue: _FakeISentrySpan_2()) as _i4.ISentrySpan); + @override + _i2.SentryTraceHeader toSentryTrace() => + (super.noSuchMethod(Invocation.method(#toSentryTrace, []), + returnValue: _FakeSentryTraceHeader_5()) as _i2.SentryTraceHeader); + @override + String toString() => super.toString(); +} diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index f493b679ff..a823d6ea70 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -34,7 +34,19 @@ void main() { }); test('didPush finishes previous transaction', () { + final firstRoute = route(RouteSettings(name: 'First Route')); + final secondRoute = route(RouteSettings(name: 'Second Route')); + final hub = MockHub(); + final span = MockNoOpSentrySpan(); + when(hub.startTransaction('First Route', 'ui.load', description: null, bindToScope: true, customSamplingContext: null)).thenReturn(span); + when(hub.startTransaction('Second Route', 'ui.load', description: null, bindToScope: true, customSamplingContext: null)).thenReturn(NoOpSentrySpan()); + final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + + sut.didPush(firstRoute, null); + sut.didPush(secondRoute, firstRoute); + + verify(span.finish()); }); }); From ff0896eb92808407cfded73beb92035fbc702db3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Thu, 25 Nov 2021 13:39:44 +0100 Subject: [PATCH 03/38] fix mocking --- flutter/test/mocks.dart | 9 +- flutter/test/mocks.mocks.dart | 287 +++++++++--------- .../test/sentry_navigator_observer_test.dart | 26 +- 3 files changed, 176 insertions(+), 146 deletions(-) diff --git a/flutter/test/mocks.dart b/flutter/test/mocks.dart index 79a026a902..412c048068 100644 --- a/flutter/test/mocks.dart +++ b/flutter/test/mocks.dart @@ -4,9 +4,16 @@ import 'package:sentry/src/platform/platform.dart'; import 'package:sentry/src/platform_checker.dart'; import 'package:sentry/src/sentry_user_feedback.dart'; +import 'mocks.mocks.dart'; + const fakeDsn = 'https://abc@def.ingest.sentry.io/1234567'; -@GenerateMocks([Hub, Transport, NoOpSentrySpan]) +// https://github.com/dart-lang/mockito/blob/master/NULL_SAFETY_README.md#fallback-generators +ISentrySpan startTransactionShim(String? name, String? operation, {String? description, bool? bindToScope, Map? customSamplingContext,}) { + return MockNoOpSentrySpan(); +} + +@GenerateMocks([Transport, NoOpSentrySpan], customMocks: [MockSpec(fallbackGenerators: {#startTransaction: startTransactionShim})]) void main() {} class MockPlatform implements Platform { diff --git a/flutter/test/mocks.mocks.dart b/flutter/test/mocks.mocks.dart index 0dc037d7d2..c5d3e4f350 100644 --- a/flutter/test/mocks.mocks.dart +++ b/flutter/test/mocks.mocks.dart @@ -2,17 +2,18 @@ // in sentry_flutter/test/mocks.dart. // Do not manually edit this file. -import 'dart:async' as _i5; +import 'dart:async' as _i6; import 'package:mockito/mockito.dart' as _i1; -import 'package:sentry/src/hub.dart' as _i3; -import 'package:sentry/src/noop_sentry_span.dart' as _i10; -import 'package:sentry/src/protocol.dart' as _i2; -import 'package:sentry/src/sentry_client.dart' as _i7; -import 'package:sentry/src/sentry_envelope.dart' as _i9; -import 'package:sentry/src/sentry_user_feedback.dart' as _i6; -import 'package:sentry/src/tracing.dart' as _i4; -import 'package:sentry/src/transport/transport.dart' as _i8; +import 'package:sentry/src/hub.dart' as _i4; +import 'package:sentry/src/protocol.dart' as _i3; +import 'package:sentry/src/sentry_client.dart' as _i9; +import 'package:sentry/src/sentry_envelope.dart' as _i7; +import 'package:sentry/src/sentry_user_feedback.dart' as _i8; +import 'package:sentry/src/tracing.dart' as _i2; +import 'package:sentry/src/transport/transport.dart' as _i5; + +import 'mocks.dart' as _i10; // ignore_for_file: avoid_redundant_argument_values // ignore_for_file: avoid_setters_without_getters @@ -23,24 +24,104 @@ import 'package:sentry/src/transport/transport.dart' as _i8; // ignore_for_file: unnecessary_parenthesis // ignore_for_file: camel_case_types -class _FakeSentryId_0 extends _i1.Fake implements _i2.SentryId {} +class _FakeSentrySpanContext_0 extends _i1.Fake + implements _i2.SentrySpanContext {} + +class _FakeDateTime_1 extends _i1.Fake implements DateTime {} + +class _FakeISentrySpan_2 extends _i1.Fake implements _i2.ISentrySpan {} -class _FakeHub_1 extends _i1.Fake implements _i3.Hub {} +class _FakeSentryTraceHeader_3 extends _i1.Fake + implements _i3.SentryTraceHeader {} -class _FakeISentrySpan_2 extends _i1.Fake implements _i4.ISentrySpan {} +class _FakeSentryId_4 extends _i1.Fake implements _i3.SentryId {} -class _FakeSentrySpanContext_3 extends _i1.Fake - implements _i4.SentrySpanContext {} +class _FakeHub_5 extends _i1.Fake implements _i4.Hub {} -class _FakeDateTime_4 extends _i1.Fake implements DateTime {} +/// A class which mocks [Transport]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockTransport extends _i1.Mock implements _i5.Transport { + MockTransport() { + _i1.throwOnMissingStub(this); + } + + @override + _i6.Future<_i3.SentryId?> send(_i7.SentryEnvelope? envelope) => + (super.noSuchMethod(Invocation.method(#send, [envelope]), + returnValue: Future<_i3.SentryId?>.value()) + as _i6.Future<_i3.SentryId?>); + @override + String toString() => super.toString(); +} + +/// A class which mocks [NoOpSentrySpan]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockNoOpSentrySpan extends _i1.Mock implements _i2.NoOpSentrySpan { + MockNoOpSentrySpan() { + _i1.throwOnMissingStub(this); + } -class _FakeSentryTraceHeader_5 extends _i1.Fake - implements _i2.SentryTraceHeader {} + @override + _i2.SentrySpanContext get context => + (super.noSuchMethod(Invocation.getter(#context), + returnValue: _FakeSentrySpanContext_0()) as _i2.SentrySpanContext); + @override + DateTime get startTimestamp => + (super.noSuchMethod(Invocation.getter(#startTimestamp), + returnValue: _FakeDateTime_1()) as DateTime); + @override + bool get finished => + (super.noSuchMethod(Invocation.getter(#finished), returnValue: false) + as bool); + @override + set throwable(dynamic throwable) => + super.noSuchMethod(Invocation.setter(#throwable, throwable), + returnValueForMissingStub: null); + @override + set status(_i3.SpanStatus? status) => + super.noSuchMethod(Invocation.setter(#status, status), + returnValueForMissingStub: null); + @override + _i6.Future finish({_i3.SpanStatus? status}) => + (super.noSuchMethod(Invocation.method(#finish, [], {#status: status}), + returnValue: Future.value(), + returnValueForMissingStub: Future.value()) as _i6.Future); + @override + void removeData(String? key) => + super.noSuchMethod(Invocation.method(#removeData, [key]), + returnValueForMissingStub: null); + @override + void removeTag(String? key) => + super.noSuchMethod(Invocation.method(#removeTag, [key]), + returnValueForMissingStub: null); + @override + void setData(String? key, dynamic value) => + super.noSuchMethod(Invocation.method(#setData, [key, value]), + returnValueForMissingStub: null); + @override + void setTag(String? key, String? value) => + super.noSuchMethod(Invocation.method(#setTag, [key, value]), + returnValueForMissingStub: null); + @override + _i2.ISentrySpan startChild(String? operation, {String? description}) => + (super.noSuchMethod( + Invocation.method( + #startChild, [operation], {#description: description}), + returnValue: _FakeISentrySpan_2()) as _i2.ISentrySpan); + @override + _i3.SentryTraceHeader toSentryTrace() => + (super.noSuchMethod(Invocation.method(#toSentryTrace, []), + returnValue: _FakeSentryTraceHeader_3()) as _i3.SentryTraceHeader); + @override + String toString() => super.toString(); +} /// A class which mocks [Hub]. /// /// See the documentation for Mockito's code generation for more information. -class MockHub extends _i1.Mock implements _i3.Hub { +class MockHub extends _i1.Mock implements _i4.Hub { MockHub() { _i1.throwOnMissingStub(this); } @@ -50,12 +131,12 @@ class MockHub extends _i1.Mock implements _i3.Hub { (super.noSuchMethod(Invocation.getter(#isEnabled), returnValue: false) as bool); @override - _i2.SentryId get lastEventId => + _i3.SentryId get lastEventId => (super.noSuchMethod(Invocation.getter(#lastEventId), - returnValue: _FakeSentryId_0()) as _i2.SentryId); + returnValue: _FakeSentryId_4()) as _i3.SentryId); @override - _i5.Future<_i2.SentryId> captureEvent(_i2.SentryEvent? event, - {dynamic stackTrace, dynamic hint, _i3.ScopeCallback? withScope}) => + _i6.Future<_i3.SentryId> captureEvent(_i3.SentryEvent? event, + {dynamic stackTrace, dynamic hint, _i4.ScopeCallback? withScope}) => (super.noSuchMethod( Invocation.method(#captureEvent, [ event @@ -64,11 +145,11 @@ class MockHub extends _i1.Mock implements _i3.Hub { #hint: hint, #withScope: withScope }), - returnValue: Future<_i2.SentryId>.value(_FakeSentryId_0())) - as _i5.Future<_i2.SentryId>); + returnValue: Future<_i3.SentryId>.value(_FakeSentryId_4())) + as _i6.Future<_i3.SentryId>); @override - _i5.Future<_i2.SentryId> captureException(dynamic throwable, - {dynamic stackTrace, dynamic hint, _i3.ScopeCallback? withScope}) => + _i6.Future<_i3.SentryId> captureException(dynamic throwable, + {dynamic stackTrace, dynamic hint, _i4.ScopeCallback? withScope}) => (super.noSuchMethod( Invocation.method(#captureException, [ throwable @@ -77,15 +158,15 @@ class MockHub extends _i1.Mock implements _i3.Hub { #hint: hint, #withScope: withScope }), - returnValue: Future<_i2.SentryId>.value(_FakeSentryId_0())) - as _i5.Future<_i2.SentryId>); + returnValue: Future<_i3.SentryId>.value(_FakeSentryId_4())) + as _i6.Future<_i3.SentryId>); @override - _i5.Future<_i2.SentryId> captureMessage(String? message, - {_i2.SentryLevel? level, + _i6.Future<_i3.SentryId> captureMessage(String? message, + {_i3.SentryLevel? level, String? template, List? params, dynamic hint, - _i3.ScopeCallback? withScope}) => + _i4.ScopeCallback? withScope}) => (super.noSuchMethod( Invocation.method(#captureMessage, [ message @@ -96,51 +177,55 @@ class MockHub extends _i1.Mock implements _i3.Hub { #hint: hint, #withScope: withScope }), - returnValue: Future<_i2.SentryId>.value(_FakeSentryId_0())) - as _i5.Future<_i2.SentryId>); + returnValue: Future<_i3.SentryId>.value(_FakeSentryId_4())) + as _i6.Future<_i3.SentryId>); @override - _i5.Future captureUserFeedback(_i6.SentryUserFeedback? userFeedback) => + _i6.Future captureUserFeedback(_i8.SentryUserFeedback? userFeedback) => (super.noSuchMethod( Invocation.method(#captureUserFeedback, [userFeedback]), returnValue: Future.value(), - returnValueForMissingStub: Future.value()) as _i5.Future); + returnValueForMissingStub: Future.value()) as _i6.Future); @override - void addBreadcrumb(_i2.Breadcrumb? crumb, {dynamic hint}) => super + void addBreadcrumb(_i3.Breadcrumb? crumb, {dynamic hint}) => super .noSuchMethod(Invocation.method(#addBreadcrumb, [crumb], {#hint: hint}), returnValueForMissingStub: null); @override - void bindClient(_i7.SentryClient? client) => + void bindClient(_i9.SentryClient? client) => super.noSuchMethod(Invocation.method(#bindClient, [client]), returnValueForMissingStub: null); @override - _i3.Hub clone() => (super.noSuchMethod(Invocation.method(#clone, []), - returnValue: _FakeHub_1()) as _i3.Hub); + _i4.Hub clone() => (super.noSuchMethod(Invocation.method(#clone, []), + returnValue: _FakeHub_5()) as _i4.Hub); @override - _i5.Future close() => (super.noSuchMethod(Invocation.method(#close, []), + _i6.Future close() => (super.noSuchMethod(Invocation.method(#close, []), returnValue: Future.value(), - returnValueForMissingStub: Future.value()) as _i5.Future); + returnValueForMissingStub: Future.value()) as _i6.Future); @override - void configureScope(_i3.ScopeCallback? callback) => + void configureScope(_i4.ScopeCallback? callback) => super.noSuchMethod(Invocation.method(#configureScope, [callback]), returnValueForMissingStub: null); @override - _i4.ISentrySpan startTransaction(String? name, String? operation, + _i2.ISentrySpan startTransaction(String? name, String? operation, {String? description, bool? bindToScope, Map? customSamplingContext}) => (super.noSuchMethod( - Invocation.method(#startTransaction, [ - name, - operation - ], { - #description: description, - #bindToScope: bindToScope, - #customSamplingContext: customSamplingContext - }), - returnValue: _FakeISentrySpan_2()) as _i4.ISentrySpan); - @override - _i4.ISentrySpan startTransactionWithContext( - _i4.SentryTransactionContext? transactionContext, + Invocation.method(#startTransaction, [ + name, + operation + ], { + #description: description, + #bindToScope: bindToScope, + #customSamplingContext: customSamplingContext + }), + returnValue: _i10.startTransactionShim(name, operation, + description: description, + bindToScope: bindToScope, + customSamplingContext: customSamplingContext)) + as _i2.ISentrySpan); + @override + _i2.ISentrySpan startTransactionWithContext( + _i2.SentryTransactionContext? transactionContext, {Map? customSamplingContext, bool? bindToScope}) => (super.noSuchMethod( @@ -150,99 +235,19 @@ class MockHub extends _i1.Mock implements _i3.Hub { #customSamplingContext: customSamplingContext, #bindToScope: bindToScope }), - returnValue: _FakeISentrySpan_2()) as _i4.ISentrySpan); + returnValue: _FakeISentrySpan_2()) as _i2.ISentrySpan); @override - _i5.Future<_i2.SentryId> captureTransaction( - _i2.SentryTransaction? transaction) => + _i6.Future<_i3.SentryId> captureTransaction( + _i3.SentryTransaction? transaction) => (super.noSuchMethod(Invocation.method(#captureTransaction, [transaction]), - returnValue: Future<_i2.SentryId>.value(_FakeSentryId_0())) - as _i5.Future<_i2.SentryId>); + returnValue: Future<_i3.SentryId>.value(_FakeSentryId_4())) + as _i6.Future<_i3.SentryId>); @override void setSpanContext( - dynamic throwable, _i4.ISentrySpan? span, String? transaction) => + dynamic throwable, _i2.ISentrySpan? span, String? transaction) => super.noSuchMethod( Invocation.method(#setSpanContext, [throwable, span, transaction]), returnValueForMissingStub: null); @override String toString() => super.toString(); } - -/// A class which mocks [Transport]. -/// -/// See the documentation for Mockito's code generation for more information. -class MockTransport extends _i1.Mock implements _i8.Transport { - MockTransport() { - _i1.throwOnMissingStub(this); - } - - @override - _i5.Future<_i2.SentryId?> send(_i9.SentryEnvelope? envelope) => - (super.noSuchMethod(Invocation.method(#send, [envelope]), - returnValue: Future<_i2.SentryId?>.value()) - as _i5.Future<_i2.SentryId?>); - @override - String toString() => super.toString(); -} - -/// A class which mocks [NoOpSentrySpan]. -/// -/// See the documentation for Mockito's code generation for more information. -class MockNoOpSentrySpan extends _i1.Mock implements _i10.NoOpSentrySpan { - MockNoOpSentrySpan() { - _i1.throwOnMissingStub(this); - } - - @override - _i4.SentrySpanContext get context => - (super.noSuchMethod(Invocation.getter(#context), - returnValue: _FakeSentrySpanContext_3()) as _i4.SentrySpanContext); - @override - DateTime get startTimestamp => - (super.noSuchMethod(Invocation.getter(#startTimestamp), - returnValue: _FakeDateTime_4()) as DateTime); - @override - bool get finished => - (super.noSuchMethod(Invocation.getter(#finished), returnValue: false) - as bool); - @override - set throwable(dynamic throwable) => - super.noSuchMethod(Invocation.setter(#throwable, throwable), - returnValueForMissingStub: null); - @override - set status(_i2.SpanStatus? status) => - super.noSuchMethod(Invocation.setter(#status, status), - returnValueForMissingStub: null); - @override - _i5.Future finish({_i2.SpanStatus? status}) => - (super.noSuchMethod(Invocation.method(#finish, [], {#status: status}), - returnValue: Future.value(), - returnValueForMissingStub: Future.value()) as _i5.Future); - @override - void removeData(String? key) => - super.noSuchMethod(Invocation.method(#removeData, [key]), - returnValueForMissingStub: null); - @override - void removeTag(String? key) => - super.noSuchMethod(Invocation.method(#removeTag, [key]), - returnValueForMissingStub: null); - @override - void setData(String? key, dynamic value) => - super.noSuchMethod(Invocation.method(#setData, [key, value]), - returnValueForMissingStub: null); - @override - void setTag(String? key, String? value) => - super.noSuchMethod(Invocation.method(#setTag, [key, value]), - returnValueForMissingStub: null); - @override - _i4.ISentrySpan startChild(String? operation, {String? description}) => - (super.noSuchMethod( - Invocation.method( - #startChild, [operation], {#description: description}), - returnValue: _FakeISentrySpan_2()) as _i4.ISentrySpan); - @override - _i2.SentryTraceHeader toSentryTrace() => - (super.noSuchMethod(Invocation.method(#toSentryTrace, []), - returnValue: _FakeSentryTraceHeader_5()) as _i2.SentryTraceHeader); - @override - String toString() => super.toString(); -} diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index a823d6ea70..726a9c8bb8 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -16,6 +16,17 @@ void main() { settings: settings, ); + void _whenAnyStart(MockHub mockHub, ISentrySpan thenReturnSpan) { + when( + mockHub.startTransaction( + any, any, + description: anyNamed('description'), + bindToScope: anyNamed('bindToScope'), + customSamplingContext: anyNamed('customSamplingContext'), + ) + ).thenReturn(thenReturnSpan); + } + setUp(() { fixture = Fixture(); }); @@ -25,7 +36,7 @@ void main() { final currentRoute = route(RouteSettings(name: 'Current Route')); final hub = MockHub(); - when(hub.startTransaction('Current Route', 'ui.load', description: null, bindToScope: true, customSamplingContext: null)).thenReturn(NoOpSentrySpan()); + _whenAnyStart(hub, NoOpSentrySpan()); final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); sut.didPush(currentRoute, null); @@ -39,8 +50,7 @@ void main() { final hub = MockHub(); final span = MockNoOpSentrySpan(); - when(hub.startTransaction('First Route', 'ui.load', description: null, bindToScope: true, customSamplingContext: null)).thenReturn(span); - when(hub.startTransaction('Second Route', 'ui.load', description: null, bindToScope: true, customSamplingContext: null)).thenReturn(NoOpSentrySpan()); + _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); sut.didPush(firstRoute, null); @@ -48,6 +58,8 @@ void main() { verify(span.finish()); }); + + }); group('RouteObserverBreadcrumb', () { @@ -180,12 +192,12 @@ void main() { group('SentryNavigatorObserver', () { - RouteSettings routeSettings(String? name, [Object? arguments]) => RouteSettings(name: name, arguments: arguments); test('Test recording of Breadcrumbs', () { final hub = MockHub(); + _whenAnyStart(hub, NoOpSentrySpan()); final observer = fixture.getSut(hub: hub); final to = routeSettings('to', 'foobar'); @@ -207,6 +219,7 @@ void main() { test('No arguments', () { final hub = MockHub(); + _whenAnyStart(hub, NoOpSentrySpan()); final observer = fixture.getSut(hub: hub); final to = routeSettings('to'); @@ -228,6 +241,7 @@ void main() { test('No arguments & no name', () { final hub = MockHub(); + _whenAnyStart(hub, NoOpSentrySpan()); final observer = fixture.getSut(hub: hub); final to = route(null); @@ -270,6 +284,7 @@ void main() { test('route name as transaction', () { final hub = _MockHub(); + _whenAnyStart(hub, NoOpSentrySpan()); final observer = fixture.getSut( hub: hub, setRouteNameAsTransaction: true, @@ -290,6 +305,7 @@ void main() { test('route name does nothing if null', () { final hub = _MockHub(); + _whenAnyStart(hub, NoOpSentrySpan()); final observer = fixture.getSut( hub: hub, setRouteNameAsTransaction: true, @@ -306,6 +322,7 @@ void main() { test('disabled route as transaction', () { final hub = _MockHub(); + _whenAnyStart(hub, NoOpSentrySpan()); final observer = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); @@ -325,6 +342,7 @@ void main() { } class Fixture { + SentryNavigatorObserver getSut({ required Hub hub, bool setRouteNameAsTransaction = false, From adde75d15770a6994baf1c14d2e090c1f094b0fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Thu, 25 Nov 2021 13:54:17 +0100 Subject: [PATCH 04/38] test finish with idle timer --- .../navigation/sentry_navigator_observer.dart | 8 ++++ .../test/sentry_navigator_observer_test.dart | 38 ++++++++++++++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index f05a259ef7..52f83bcc04 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:flutter/widgets.dart'; import 'package:sentry/sentry.dart'; import '../../sentry_flutter.dart'; @@ -44,7 +46,9 @@ class SentryNavigatorObserver extends RouteObserver> { final Hub _hub; final bool _setRouteNameAsTransaction; + ISentrySpan? _transaction; + Timer? _idleTimer; @override void didPush(Route route, Route? previousRoute) { @@ -113,9 +117,13 @@ class SentryNavigatorObserver extends RouteObserver> { if (arguments != null) { _transaction?.setData('route_settings_arguments', arguments); } + _idleTimer = Timer(Duration(seconds: 3), () async { + await _finishTransaction(); + }); } Future _finishTransaction() async { + _idleTimer?.cancel(); //_transaction?.status ??= SpanStatus.ok(); return await _transaction?.finish(); } diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 726a9c8bb8..f50184169f 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -16,10 +16,11 @@ void main() { settings: settings, ); - void _whenAnyStart(MockHub mockHub, ISentrySpan thenReturnSpan) { + void _whenAnyStart(MockHub mockHub, ISentrySpan thenReturnSpan, {String? name}) { when( mockHub.startTransaction( - any, any, + name ?? any, + any, description: anyNamed('description'), bindToScope: anyNamed('bindToScope'), customSamplingContext: anyNamed('customSamplingContext'), @@ -59,7 +60,40 @@ void main() { verify(span.finish()); }); + test('didPush finishes transaction after timeout', () async { + final currentRoute = route(RouteSettings(name: 'Current Route')); + + final hub = MockHub(); + final span = MockNoOpSentrySpan(); + _whenAnyStart(hub, span); + final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + + sut.didPush(currentRoute, null); + + await Future.delayed(Duration(milliseconds: 3100)); + + verify(span.finish()); + }); + test('didPush finish not called multiple times', () async { + final firstRoute = route(RouteSettings(name: 'First Route')); + final secondRoute = route(RouteSettings(name: 'Second Route')); + + final hub = MockHub(); + final firstSpan = MockNoOpSentrySpan(); + final secondSpan = MockNoOpSentrySpan(); + _whenAnyStart(hub, firstSpan, name: 'First Route'); + _whenAnyStart(hub, secondSpan, name: 'Second Route'); + final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + + sut.didPush(firstRoute, null); + sut.didPush(secondRoute, firstRoute); + + await Future.delayed(Duration(milliseconds: 3100)); + + verify(firstSpan.finish()).called(1); + verify(secondSpan.finish()).called(1); + }); }); group('RouteObserverBreadcrumb', () { From 7a0c96b3893b2375967a21e7dfc9e4cb63042568 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Thu, 25 Nov 2021 14:38:20 +0100 Subject: [PATCH 05/38] finish transaction on pop --- .../navigation/sentry_navigator_observer.dart | 1 + flutter/test/mocks.dart | 15 +++++- .../test/sentry_navigator_observer_test.dart | 52 ++++++++++++------- 3 files changed, 47 insertions(+), 21 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 52f83bcc04..42b35a0578 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -83,6 +83,7 @@ class SentryNavigatorObserver extends RouteObserver> { from: route.settings, to: previousRoute?.settings, ); + _finishTransaction(); } void _addBreadcrumb({ diff --git a/flutter/test/mocks.dart b/flutter/test/mocks.dart index 412c048068..ec2ae524d8 100644 --- a/flutter/test/mocks.dart +++ b/flutter/test/mocks.dart @@ -9,11 +9,22 @@ import 'mocks.mocks.dart'; const fakeDsn = 'https://abc@def.ingest.sentry.io/1234567'; // https://github.com/dart-lang/mockito/blob/master/NULL_SAFETY_README.md#fallback-generators -ISentrySpan startTransactionShim(String? name, String? operation, {String? description, bool? bindToScope, Map? customSamplingContext,}) { +ISentrySpan startTransactionShim( + String? name, + String? operation, { + String? description, + bool? bindToScope, + Map? customSamplingContext, +}) { return MockNoOpSentrySpan(); } -@GenerateMocks([Transport, NoOpSentrySpan], customMocks: [MockSpec(fallbackGenerators: {#startTransaction: startTransactionShim})]) +@GenerateMocks([ + Transport, + NoOpSentrySpan +], customMocks: [ + MockSpec(fallbackGenerators: {#startTransaction: startTransactionShim}) +]) void main() {} class MockPlatform implements Platform { diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index f50184169f..bf312018d5 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -7,25 +7,23 @@ import 'package:sentry_flutter/sentry_flutter.dart'; import 'mocks.dart'; import 'mocks.mocks.dart'; - void main() { late Fixture fixture; PageRoute route(RouteSettings? settings) => PageRouteBuilder( - pageBuilder: (_, __, ___) => Container(), - settings: settings, - ); - - void _whenAnyStart(MockHub mockHub, ISentrySpan thenReturnSpan, {String? name}) { - when( - mockHub.startTransaction( - name ?? any, - any, - description: anyNamed('description'), - bindToScope: anyNamed('bindToScope'), - customSamplingContext: anyNamed('customSamplingContext'), - ) - ).thenReturn(thenReturnSpan); + pageBuilder: (_, __, ___) => Container(), + settings: settings, + ); + + void _whenAnyStart(MockHub mockHub, ISentrySpan thenReturnSpan, + {String? name}) { + when(mockHub.startTransaction( + name ?? any, + any, + description: anyNamed('description'), + bindToScope: anyNamed('bindToScope'), + customSamplingContext: anyNamed('customSamplingContext'), + )).thenReturn(thenReturnSpan); } setUp(() { @@ -42,7 +40,8 @@ void main() { sut.didPush(currentRoute, null); - verify(hub.startTransaction('Current Route', 'ui.load', description: null, bindToScope: true, customSamplingContext: null)); + verify(hub.startTransaction('Current Route', 'ui.load', + description: null, bindToScope: true, customSamplingContext: null)); }); test('didPush finishes previous transaction', () { @@ -75,7 +74,24 @@ void main() { verify(span.finish()); }); - test('didPush finish not called multiple times', () async { + test('didPop finishes transaction', () async { + final currentRoute = route(RouteSettings(name: 'Current Route')); + + final hub = MockHub(); + final span = MockNoOpSentrySpan(); + _whenAnyStart(hub, span); + + final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + + sut.didPush(currentRoute, null); + sut.didPop(currentRoute, null); + + await Future.delayed(Duration(milliseconds: 3100)); + + verify(span.finish()).called(1); + }); + + test('didPush push multiple finishes transactions', () async { final firstRoute = route(RouteSettings(name: 'First Route')); final secondRoute = route(RouteSettings(name: 'Second Route')); @@ -225,7 +241,6 @@ void main() { }); group('SentryNavigatorObserver', () { - RouteSettings routeSettings(String? name, [Object? arguments]) => RouteSettings(name: name, arguments: arguments); @@ -376,7 +391,6 @@ void main() { } class Fixture { - SentryNavigatorObserver getSut({ required Hub hub, bool setRouteNameAsTransaction = false, From 0f79e44ab0ac9dd58fe7508a04d3f390c652f215 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Thu, 25 Nov 2021 15:00:28 +0100 Subject: [PATCH 06/38] check if span status is set --- .../lib/src/navigation/sentry_navigator_observer.dart | 2 +- flutter/test/sentry_navigator_observer_test.dart | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 42b35a0578..f72b58d351 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -125,7 +125,7 @@ class SentryNavigatorObserver extends RouteObserver> { Future _finishTransaction() async { _idleTimer?.cancel(); - //_transaction?.status ??= SpanStatus.ok(); + _transaction?.status ??= SpanStatus.ok(); return await _transaction?.finish(); } } diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index bf312018d5..a2f05a3b4d 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -50,12 +50,14 @@ void main() { final hub = MockHub(); final span = MockNoOpSentrySpan(); + when(span.status).thenReturn(null); _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); sut.didPush(firstRoute, null); sut.didPush(secondRoute, firstRoute); + verify(span.status = SpanStatus.ok()); verify(span.finish()); }); @@ -64,6 +66,7 @@ void main() { final hub = MockHub(); final span = MockNoOpSentrySpan(); + when(span.status).thenReturn(null); _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); @@ -71,6 +74,7 @@ void main() { await Future.delayed(Duration(milliseconds: 3100)); + verify(span.status = SpanStatus.ok()); verify(span.finish()); }); @@ -79,6 +83,7 @@ void main() { final hub = MockHub(); final span = MockNoOpSentrySpan(); + when(span.status).thenReturn(null); _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); @@ -88,6 +93,7 @@ void main() { await Future.delayed(Duration(milliseconds: 3100)); + verify(span.status = SpanStatus.ok()); verify(span.finish()).called(1); }); @@ -97,7 +103,9 @@ void main() { final hub = MockHub(); final firstSpan = MockNoOpSentrySpan(); + when(firstSpan.status).thenReturn(null); final secondSpan = MockNoOpSentrySpan(); + when(secondSpan.status).thenReturn(null); _whenAnyStart(hub, firstSpan, name: 'First Route'); _whenAnyStart(hub, secondSpan, name: 'Second Route'); final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); @@ -107,7 +115,9 @@ void main() { await Future.delayed(Duration(milliseconds: 3100)); + verify(firstSpan.status = SpanStatus.ok()); verify(firstSpan.finish()).called(1); + verify(secondSpan.status = SpanStatus.ok()); verify(secondSpan.finish()).called(1); }); }); From 0bbce7a6f3c99982cf6413ee25f5cacb6610d6bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Thu, 25 Nov 2021 15:06:31 +0100 Subject: [PATCH 07/38] test setting of arguments --- .../test/sentry_navigator_observer_test.dart | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index a2f05a3b4d..df2261543b 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -120,6 +120,27 @@ void main() { verify(secondSpan.status = SpanStatus.ok()); verify(secondSpan.finish()).called(1); }); + + test('route arguments are set on transaction', () { + final arguments = {'foo': 'bar'}; + final currentRoute = route( + RouteSettings( + name: 'Current Route', + arguments: arguments, + ) + ); + + final hub = MockHub(); + final span = MockNoOpSentrySpan(); + when(span.status).thenReturn(null); + _whenAnyStart(hub, span); + + final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + + sut.didPush(currentRoute, null); + + verify(span.setData('route_settings_arguments', arguments)); + }); }); group('RouteObserverBreadcrumb', () { From 45186fe10f64031684331766eca4928e1eef3578 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Thu, 25 Nov 2021 15:06:48 +0100 Subject: [PATCH 08/38] format --- flutter/test/sentry_navigator_observer_test.dart | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index df2261543b..d1fd1e44d8 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -121,14 +121,12 @@ void main() { verify(secondSpan.finish()).called(1); }); - test('route arguments are set on transaction', () { + test('route arguments are set on transaction', () { final arguments = {'foo': 'bar'}; - final currentRoute = route( - RouteSettings( - name: 'Current Route', - arguments: arguments, - ) - ); + final currentRoute = route(RouteSettings( + name: 'Current Route', + arguments: arguments, + )); final hub = MockHub(); final span = MockNoOpSentrySpan(); From deef4a92c2228c9082c8af39390e682db0039c94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Thu, 25 Nov 2021 16:29:42 +0100 Subject: [PATCH 09/38] re-start previous --- .../navigation/sentry_navigator_observer.dart | 2 ++ .../test/sentry_navigator_observer_test.dart | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index f72b58d351..2090b5f085 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -84,6 +84,8 @@ class SentryNavigatorObserver extends RouteObserver> { to: previousRoute?.settings, ); _finishTransaction(); + _startTransaction( + previousRoute?.settings.name, previousRoute?.settings.arguments); } void _addBreadcrumb({ diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index d1fd1e44d8..928599f4cb 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -97,6 +97,23 @@ void main() { verify(span.finish()).called(1); }); + test('didPop re-starts previous', () { + final previousRoute = route(RouteSettings(name: 'Previous Route')); + final currentRoute = route(RouteSettings(name: 'Current Route')); + + final hub = MockHub(); + final previousSpan = MockNoOpSentrySpan(); + when(previousSpan.status).thenReturn(null); + _whenAnyStart(hub, previousSpan, name: 'Previous Route'); + + final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + + sut.didPop(currentRoute, previousRoute); + + verify(hub.startTransaction('Previous Route', 'ui.load', + description: null, bindToScope: true, customSamplingContext: null)); + }); + test('didPush push multiple finishes transactions', () async { final firstRoute = route(RouteSettings(name: 'First Route')); final secondRoute = route(RouteSettings(name: 'Second Route')); From 1e204c0405e49e595dade3d7614b46605811b155 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Thu, 25 Nov 2021 17:02:41 +0100 Subject: [PATCH 10/38] dont start transaction with empty name --- dart/lib/src/hub.dart | 2 ++ .../src/navigation/sentry_navigator_observer.dart | 9 ++++++++- flutter/test/sentry_navigator_observer_test.dart | 13 +++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/dart/lib/src/hub.dart b/dart/lib/src/hub.dart index e533ef3501..c85bb0f0b5 100644 --- a/dart/lib/src/hub.dart +++ b/dart/lib/src/hub.dart @@ -34,6 +34,8 @@ class Hub { final _WeakMap _throwableToSpan = _WeakMap(); + SentryOptions get options => _options; + factory Hub(SentryOptions options) { _validateOptions(options); diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 2090b5f085..aae1912a62 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -112,8 +112,15 @@ class SentryNavigatorObserver extends RouteObserver> { } void _startTransaction(String? name, Object? arguments) { + if (name == null) { + _hub.options.logger( + SentryLevel.info, + 'Auto transaction not started because of missing RouteSettings', + ); + return; + } _transaction = _hub.startTransaction( - name ?? 'unknown', + name, 'ui.load', bindToScope: true, ); diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 928599f4cb..0a1346c5cc 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -44,6 +44,19 @@ void main() { description: null, bindToScope: true, customSamplingContext: null)); }); + test('route with empty name does not start transaction', () { + final currentRoute = route(null); + + final hub = MockHub(); + _whenAnyStart(hub, NoOpSentrySpan()); + final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + + sut.didPush(currentRoute, null); + + verifyNever(hub.startTransaction('Current Route', 'ui.load', + description: null, bindToScope: true, customSamplingContext: null)); + }); + test('didPush finishes previous transaction', () { final firstRoute = route(RouteSettings(name: 'First Route')); final secondRoute = route(RouteSettings(name: 'Second Route')); From 0dd396cf07968cb44268a6f5531e2f5735ec8548 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Thu, 25 Nov 2021 17:11:06 +0100 Subject: [PATCH 11/38] dont expose options --- dart/lib/src/hub.dart | 2 -- dart/test/default_integrations_test.dart | 1 + flutter/lib/src/navigation/sentry_navigator_observer.dart | 4 ---- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/dart/lib/src/hub.dart b/dart/lib/src/hub.dart index c85bb0f0b5..e533ef3501 100644 --- a/dart/lib/src/hub.dart +++ b/dart/lib/src/hub.dart @@ -34,8 +34,6 @@ class Hub { final _WeakMap _throwableToSpan = _WeakMap(); - SentryOptions get options => _options; - factory Hub(SentryOptions options) { _validateOptions(options); diff --git a/dart/test/default_integrations_test.dart b/dart/test/default_integrations_test.dart index 3c9e865503..5a0d12b8e4 100644 --- a/dart/test/default_integrations_test.dart +++ b/dart/test/default_integrations_test.dart @@ -191,4 +191,5 @@ class PrintRecursionMockHub extends MockHub { bool? bindToScope}) { return NoOpSentrySpan(); } + } diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index aae1912a62..6a7cb7f7d3 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -113,10 +113,6 @@ class SentryNavigatorObserver extends RouteObserver> { void _startTransaction(String? name, Object? arguments) { if (name == null) { - _hub.options.logger( - SentryLevel.info, - 'Auto transaction not started because of missing RouteSettings', - ); return; } _transaction = _hub.startTransaction( From 998ee182c2eb749f1d314bcdaa1861545e02bd27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Thu, 25 Nov 2021 17:15:26 +0100 Subject: [PATCH 12/38] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 415a645cd6..edf93eb554 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +* Feat: Auto Transactions (#643) + # 6.1.2 * Fix: Remove is Enum check to support older Dart versions (#635) From c3c7de8ef2f368f6eeb37313bd29505b1cc29533 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Fri, 26 Nov 2021 10:03:02 +0100 Subject: [PATCH 13/38] provide opt-out of auto transactions --- .../navigation/sentry_navigator_observer.dart | 10 +++++- .../test/sentry_navigator_observer_test.dart | 31 ++++++++++++++----- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 6a7cb7f7d3..e2efb1052c 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -40,11 +40,16 @@ const _navigationKey = 'navigation'; /// - [RouteObserver](https://api.flutter.dev/flutter/widgets/RouteObserver-class.html) /// - [Navigating with arguments](https://flutter.dev/docs/cookbook/navigation/navigate-with-arguments) class SentryNavigatorObserver extends RouteObserver> { - SentryNavigatorObserver({Hub? hub, bool setRouteNameAsTransaction = false}) + SentryNavigatorObserver( + {Hub? hub, + bool enableAutoTransactions = true, + bool setRouteNameAsTransaction = false}) : _hub = hub ?? HubAdapter(), + _enableAutoTransactions = enableAutoTransactions, _setRouteNameAsTransaction = setRouteNameAsTransaction; final Hub _hub; + final bool _enableAutoTransactions; final bool _setRouteNameAsTransaction; ISentrySpan? _transaction; @@ -112,6 +117,9 @@ class SentryNavigatorObserver extends RouteObserver> { } void _startTransaction(String? name, Object? arguments) { + if (!_enableAutoTransactions) { + return; + } if (name == null) { return; } diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 0a1346c5cc..2c05121662 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -36,7 +36,7 @@ void main() { final hub = MockHub(); _whenAnyStart(hub, NoOpSentrySpan()); - final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + final sut = fixture.getSut(hub: hub); sut.didPush(currentRoute, null); @@ -49,7 +49,20 @@ void main() { final hub = MockHub(); _whenAnyStart(hub, NoOpSentrySpan()); - final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + final sut = fixture.getSut(hub: hub); + + sut.didPush(currentRoute, null); + + verifyNever(hub.startTransaction('Current Route', 'ui.load', + description: null, bindToScope: true, customSamplingContext: null)); + }); + + test('no transaction on opt-out', () { + final currentRoute = route(RouteSettings(name: 'Current Route')); + + final hub = MockHub(); + _whenAnyStart(hub, NoOpSentrySpan()); + final sut = fixture.getSut(hub: hub, enableAutoTransactions: false); sut.didPush(currentRoute, null); @@ -65,7 +78,7 @@ void main() { final span = MockNoOpSentrySpan(); when(span.status).thenReturn(null); _whenAnyStart(hub, span); - final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + final sut = fixture.getSut(hub: hub); sut.didPush(firstRoute, null); sut.didPush(secondRoute, firstRoute); @@ -81,7 +94,7 @@ void main() { final span = MockNoOpSentrySpan(); when(span.status).thenReturn(null); _whenAnyStart(hub, span); - final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + final sut = fixture.getSut(hub: hub); sut.didPush(currentRoute, null); @@ -99,7 +112,7 @@ void main() { when(span.status).thenReturn(null); _whenAnyStart(hub, span); - final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + final sut = fixture.getSut(hub: hub); sut.didPush(currentRoute, null); sut.didPop(currentRoute, null); @@ -119,7 +132,7 @@ void main() { when(previousSpan.status).thenReturn(null); _whenAnyStart(hub, previousSpan, name: 'Previous Route'); - final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + final sut = fixture.getSut(hub: hub); sut.didPop(currentRoute, previousRoute); @@ -138,7 +151,7 @@ void main() { when(secondSpan.status).thenReturn(null); _whenAnyStart(hub, firstSpan, name: 'First Route'); _whenAnyStart(hub, secondSpan, name: 'Second Route'); - final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + final sut = fixture.getSut(hub: hub); sut.didPush(firstRoute, null); sut.didPush(secondRoute, firstRoute); @@ -163,7 +176,7 @@ void main() { when(span.status).thenReturn(null); _whenAnyStart(hub, span); - final sut = fixture.getSut(hub: hub, setRouteNameAsTransaction: false); + final sut = fixture.getSut(hub: hub); sut.didPush(currentRoute, null); @@ -452,10 +465,12 @@ void main() { class Fixture { SentryNavigatorObserver getSut({ required Hub hub, + bool enableAutoTransactions = true, bool setRouteNameAsTransaction = false, }) { return SentryNavigatorObserver( hub: hub, + enableAutoTransactions: enableAutoTransactions, setRouteNameAsTransaction: setRouteNameAsTransaction, ); } From 5406ad482407952a645a5699a1a59eb565e7ed3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Fri, 26 Nov 2021 10:28:05 +0100 Subject: [PATCH 14/38] run format --- dart/test/default_integrations_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/dart/test/default_integrations_test.dart b/dart/test/default_integrations_test.dart index 5a0d12b8e4..3c9e865503 100644 --- a/dart/test/default_integrations_test.dart +++ b/dart/test/default_integrations_test.dart @@ -191,5 +191,4 @@ class PrintRecursionMockHub extends MockHub { bool? bindToScope}) { return NoOpSentrySpan(); } - } From 75fa0434da7128be8ef3bb3cda9afda7268695c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Tue, 30 Nov 2021 15:32:17 +0100 Subject: [PATCH 15/38] move idle timer to tracer, only start transaction if scope is not set already --- dart/lib/src/hub.dart | 11 ++- dart/lib/src/hub_adapter.dart | 2 + dart/lib/src/noop_hub.dart | 2 + dart/lib/src/sentry_tracer.dart | 14 ++++ dart/test/default_integrations_test.dart | 8 ++- dart/test/mocks/mock_hub.dart | 2 + dart/test/sentry_tracer_test.dart | 9 +++ .../navigation/sentry_navigator_observer.dart | 28 ++++---- flutter/test/mocks.dart | 3 + flutter/test/mocks.mocks.dart | 9 ++- .../test/sentry_navigator_observer_test.dart | 68 ++++++++----------- 11 files changed, 98 insertions(+), 58 deletions(-) diff --git a/dart/lib/src/hub.dart b/dart/lib/src/hub.dart index e533ef3501..8aff4e2c7a 100644 --- a/dart/lib/src/hub.dart +++ b/dart/lib/src/hub.dart @@ -340,6 +340,7 @@ class Hub { String operation, { String? description, bool? bindToScope, + Duration? idleFinishDuration, Map? customSamplingContext, }) => startTransactionWithContext( @@ -357,6 +358,7 @@ class Hub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, + Duration? idleFinishDuration, }) { if (!_isEnabled) { _options.logger( @@ -380,8 +382,13 @@ class Hub { transactionContext = transactionContext.copyWith(sampled: sampled); } - final tracer = SentryTracer(transactionContext, this); - + final tracer = SentryTracer( + transactionContext, + this, + ); + if (idleFinishDuration != null) { + tracer.finishAfterIdleTime(idleFinishDuration); + } if (bindToScope ?? false) { item.scope.span = tracer; } diff --git a/dart/lib/src/hub_adapter.dart b/dart/lib/src/hub_adapter.dart index 11eb8044bd..458d5fbaab 100644 --- a/dart/lib/src/hub_adapter.dart +++ b/dart/lib/src/hub_adapter.dart @@ -102,6 +102,7 @@ class HubAdapter implements Hub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, + Duration? idleFinishDuration, }) => Sentry.startTransactionWithContext( transactionContext, @@ -115,6 +116,7 @@ class HubAdapter implements Hub { String operation, { String? description, bool? bindToScope, + Duration? idleFinishDuration, Map? customSamplingContext, }) => Sentry.startTransaction( diff --git a/dart/lib/src/noop_hub.dart b/dart/lib/src/noop_hub.dart index 06163bac20..d5846547f3 100644 --- a/dart/lib/src/noop_hub.dart +++ b/dart/lib/src/noop_hub.dart @@ -79,6 +79,7 @@ class NoOpHub implements Hub { String operation, { String? description, bool? bindToScope, + Duration? idleFinishDuration, Map? customSamplingContext, }) => NoOpSentrySpan(); @@ -88,6 +89,7 @@ class NoOpHub implements Hub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, + Duration? idleFinishDuration, }) => NoOpSentrySpan(); diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index 964f2097a7..a089011cba 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:meta/meta.dart'; import '../sentry.dart'; @@ -12,6 +14,7 @@ class SentryTracer extends ISentrySpan { late final SentrySpan _rootSpan; final List _children = []; final Map _extra = {}; + Timer? _idleTimer; SentryTracer(SentryTransactionContext transactionContext, this._hub) { _rootSpan = SentrySpan( @@ -23,8 +26,19 @@ class SentryTracer extends ISentrySpan { name = transactionContext.name; } + void finishAfterIdleTime(Duration time) { + _idleTimer = Timer(time, () async { + SpanStatus? status; + if (_rootSpan.status == null) { + status = SpanStatus.ok(); + } + await finish(status: status); + }); + } + @override Future finish({SpanStatus? status}) async { + _idleTimer?.cancel(); await _rootSpan.finish(status: status); // finish unfinished spans otherwise transaction gets dropped diff --git a/dart/test/default_integrations_test.dart b/dart/test/default_integrations_test.dart index 3c9e865503..42dc9f8dab 100644 --- a/dart/test/default_integrations_test.dart +++ b/dart/test/default_integrations_test.dart @@ -186,9 +186,11 @@ class PrintRecursionMockHub extends MockHub { @override ISentrySpan startTransactionWithContext( - SentryTransactionContext transactionContext, - {Map? customSamplingContext, - bool? bindToScope}) { + SentryTransactionContext transactionContext, { + Map? customSamplingContext, + bool? bindToScope, + Duration? idleFinishDuration, + }) { return NoOpSentrySpan(); } } diff --git a/dart/test/mocks/mock_hub.dart b/dart/test/mocks/mock_hub.dart index b0e672abbd..daf07c6f2b 100644 --- a/dart/test/mocks/mock_hub.dart +++ b/dart/test/mocks/mock_hub.dart @@ -127,6 +127,7 @@ class MockHub implements Hub { String operation, { String? description, bool? bindToScope, + Duration? idleFinishDuration, Map? customSamplingContext, }) { return NoOpSentrySpan(); @@ -137,6 +138,7 @@ class MockHub implements Hub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, + Duration? idleFinishDuration, }) { return NoOpSentrySpan(); } diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index 640a249fa8..94f83db85d 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -112,6 +112,15 @@ void main() { expect(sut.toSentryTrace().value, '${sut.context.traceId}-${sut.context.spanId}-1'); }); + + test('tracer finishes after idle time', () async { + final sut = fixture.getSut(); + sut.finishAfterIdleTime(Duration(milliseconds: 200)); + + expect(sut.finished, false); + await Future.delayed(Duration(milliseconds: 210)); + expect(sut.finished, true); + }); } class Fixture { diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index e2efb1052c..75a3ab5931 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -53,7 +53,6 @@ class SentryNavigatorObserver extends RouteObserver> { final bool _setRouteNameAsTransaction; ISentrySpan? _transaction; - Timer? _idleTimer; @override void didPush(Route route, Route? previousRoute) { @@ -90,7 +89,9 @@ class SentryNavigatorObserver extends RouteObserver> { ); _finishTransaction(); _startTransaction( - previousRoute?.settings.name, previousRoute?.settings.arguments); + previousRoute?.settings.name, + previousRoute?.settings.arguments, + ); } void _addBreadcrumb({ @@ -123,21 +124,22 @@ class SentryNavigatorObserver extends RouteObserver> { if (name == null) { return; } - _transaction = _hub.startTransaction( - name, - 'ui.load', - bindToScope: true, - ); - if (arguments != null) { - _transaction?.setData('route_settings_arguments', arguments); - } - _idleTimer = Timer(Duration(seconds: 3), () async { - await _finishTransaction(); + _hub.configureScope((scope) { + if (scope.span == null) { + _transaction = _hub.startTransaction( + name, + 'ui.load', + bindToScope: true, + idleFinishDuration: Duration(seconds: 3), + ); + if (arguments != null) { + _transaction?.setData('route_settings_arguments', arguments); + } + } }); } Future _finishTransaction() async { - _idleTimer?.cancel(); _transaction?.status ??= SpanStatus.ok(); return await _transaction?.finish(); } diff --git a/flutter/test/mocks.dart b/flutter/test/mocks.dart index ec2ae524d8..66cb581ed3 100644 --- a/flutter/test/mocks.dart +++ b/flutter/test/mocks.dart @@ -14,6 +14,7 @@ ISentrySpan startTransactionShim( String? operation, { String? description, bool? bindToScope, + Duration? idleFinishDuration, Map? customSamplingContext, }) { return MockNoOpSentrySpan(); @@ -187,6 +188,7 @@ class NoOpHub implements Hub { String operation, { String? description, bool? bindToScope, + Duration? idleFinishDuration, Map? customSamplingContext, }) { return NoOpSentrySpan(); @@ -196,6 +198,7 @@ class NoOpHub implements Hub { ISentrySpan startTransactionWithContext( SentryTransactionContext transactionContext, { Map? customSamplingContext, + Duration? idleFinishDuration, bool? bindToScope, }) { return NoOpSentrySpan(); diff --git a/flutter/test/mocks.mocks.dart b/flutter/test/mocks.mocks.dart index c5d3e4f350..bc0aa217e2 100644 --- a/flutter/test/mocks.mocks.dart +++ b/flutter/test/mocks.mocks.dart @@ -208,6 +208,7 @@ class MockHub extends _i1.Mock implements _i4.Hub { _i2.ISentrySpan startTransaction(String? name, String? operation, {String? description, bool? bindToScope, + Duration? idleFinishDuration, Map? customSamplingContext}) => (super.noSuchMethod( Invocation.method(#startTransaction, [ @@ -216,24 +217,28 @@ class MockHub extends _i1.Mock implements _i4.Hub { ], { #description: description, #bindToScope: bindToScope, + #idleFinishDuration: idleFinishDuration, #customSamplingContext: customSamplingContext }), returnValue: _i10.startTransactionShim(name, operation, description: description, bindToScope: bindToScope, + idleFinishDuration: idleFinishDuration, customSamplingContext: customSamplingContext)) as _i2.ISentrySpan); @override _i2.ISentrySpan startTransactionWithContext( _i2.SentryTransactionContext? transactionContext, {Map? customSamplingContext, - bool? bindToScope}) => + bool? bindToScope, + Duration? idleFinishDuration}) => (super.noSuchMethod( Invocation.method(#startTransactionWithContext, [ transactionContext ], { #customSamplingContext: customSamplingContext, - #bindToScope: bindToScope + #bindToScope: bindToScope, + #idleFinishDuration: idleFinishDuration }), returnValue: _FakeISentrySpan_2()) as _i2.ISentrySpan); @override diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 2c05121662..2d7346cf2d 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -22,6 +22,7 @@ void main() { any, description: anyNamed('description'), bindToScope: anyNamed('bindToScope'), + idleFinishDuration: anyNamed('idleFinishDuration'), customSamplingContext: anyNamed('customSamplingContext'), )).thenReturn(thenReturnSpan); } @@ -34,71 +35,68 @@ void main() { test('didPush starts transaction', () { final currentRoute = route(RouteSettings(name: 'Current Route')); - final hub = MockHub(); + final hub = _MockHub(); _whenAnyStart(hub, NoOpSentrySpan()); final sut = fixture.getSut(hub: hub); sut.didPush(currentRoute, null); verify(hub.startTransaction('Current Route', 'ui.load', - description: null, bindToScope: true, customSamplingContext: null)); + bindToScope: true, idleFinishDuration: Duration(seconds: 3))); }); test('route with empty name does not start transaction', () { final currentRoute = route(null); - final hub = MockHub(); + final hub = _MockHub(); _whenAnyStart(hub, NoOpSentrySpan()); final sut = fixture.getSut(hub: hub); sut.didPush(currentRoute, null); verifyNever(hub.startTransaction('Current Route', 'ui.load', - description: null, bindToScope: true, customSamplingContext: null)); + bindToScope: true, idleFinishDuration: Duration(seconds: 3))); }); test('no transaction on opt-out', () { final currentRoute = route(RouteSettings(name: 'Current Route')); - final hub = MockHub(); + final hub = _MockHub(); _whenAnyStart(hub, NoOpSentrySpan()); final sut = fixture.getSut(hub: hub, enableAutoTransactions: false); sut.didPush(currentRoute, null); verifyNever(hub.startTransaction('Current Route', 'ui.load', - description: null, bindToScope: true, customSamplingContext: null)); + bindToScope: true, idleFinishDuration: Duration(seconds: 3))); }); - test('didPush finishes previous transaction', () { - final firstRoute = route(RouteSettings(name: 'First Route')); - final secondRoute = route(RouteSettings(name: 'Second Route')); + test('no transaction when scope already has one', () { + final currentRoute = route(RouteSettings(name: 'Current Route')); - final hub = MockHub(); - final span = MockNoOpSentrySpan(); - when(span.status).thenReturn(null); - _whenAnyStart(hub, span); - final sut = fixture.getSut(hub: hub); + final hub = _MockHub(); + hub.scope.span = NoOpSentrySpan(); + _whenAnyStart(hub, NoOpSentrySpan()); + final sut = fixture.getSut(hub: hub, enableAutoTransactions: false); - sut.didPush(firstRoute, null); - sut.didPush(secondRoute, firstRoute); + sut.didPush(currentRoute, null); - verify(span.status = SpanStatus.ok()); - verify(span.finish()); + verifyNever(hub.startTransaction('Current Route', 'ui.load', + bindToScope: true, idleFinishDuration: Duration(seconds: 3))); }); - test('didPush finishes transaction after timeout', () async { - final currentRoute = route(RouteSettings(name: 'Current Route')); + test('didPush finishes previous transaction', () { + final firstRoute = route(RouteSettings(name: 'First Route')); + final secondRoute = route(RouteSettings(name: 'Second Route')); - final hub = MockHub(); + final hub = _MockHub(); final span = MockNoOpSentrySpan(); when(span.status).thenReturn(null); _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub); - sut.didPush(currentRoute, null); - - await Future.delayed(Duration(milliseconds: 3100)); + sut.didPush(firstRoute, null); + sut.didPush(secondRoute, firstRoute); verify(span.status = SpanStatus.ok()); verify(span.finish()); @@ -107,7 +105,7 @@ void main() { test('didPop finishes transaction', () async { final currentRoute = route(RouteSettings(name: 'Current Route')); - final hub = MockHub(); + final hub = _MockHub(); final span = MockNoOpSentrySpan(); when(span.status).thenReturn(null); _whenAnyStart(hub, span); @@ -117,17 +115,15 @@ void main() { sut.didPush(currentRoute, null); sut.didPop(currentRoute, null); - await Future.delayed(Duration(milliseconds: 3100)); - verify(span.status = SpanStatus.ok()); - verify(span.finish()).called(1); + verify(span.finish()); }); test('didPop re-starts previous', () { final previousRoute = route(RouteSettings(name: 'Previous Route')); final currentRoute = route(RouteSettings(name: 'Current Route')); - final hub = MockHub(); + final hub = _MockHub(); final previousSpan = MockNoOpSentrySpan(); when(previousSpan.status).thenReturn(null); _whenAnyStart(hub, previousSpan, name: 'Previous Route'); @@ -137,14 +133,14 @@ void main() { sut.didPop(currentRoute, previousRoute); verify(hub.startTransaction('Previous Route', 'ui.load', - description: null, bindToScope: true, customSamplingContext: null)); + bindToScope: true, idleFinishDuration: Duration(seconds: 3))); }); - test('didPush push multiple finishes transactions', () async { + test('didPush push multiple finishes previous', () async { final firstRoute = route(RouteSettings(name: 'First Route')); final secondRoute = route(RouteSettings(name: 'Second Route')); - final hub = MockHub(); + final hub = _MockHub(); final firstSpan = MockNoOpSentrySpan(); when(firstSpan.status).thenReturn(null); final secondSpan = MockNoOpSentrySpan(); @@ -156,12 +152,8 @@ void main() { sut.didPush(firstRoute, null); sut.didPush(secondRoute, firstRoute); - await Future.delayed(Duration(milliseconds: 3100)); - verify(firstSpan.status = SpanStatus.ok()); - verify(firstSpan.finish()).called(1); - verify(secondSpan.status = SpanStatus.ok()); - verify(secondSpan.finish()).called(1); + verify(firstSpan.finish()); }); test('route arguments are set on transaction', () { @@ -171,7 +163,7 @@ void main() { arguments: arguments, )); - final hub = MockHub(); + final hub = _MockHub(); final span = MockNoOpSentrySpan(); when(span.status).thenReturn(null); _whenAnyStart(hub, span); From f7b841727ea41779695a74dcbb50ec2a028137b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Tue, 30 Nov 2021 17:46:38 +0100 Subject: [PATCH 16/38] implement waitForChildren --- dart/lib/src/noop_sentry_span.dart | 4 +- dart/lib/src/protocol/sentry_span.dart | 1 + dart/lib/src/sentry_span_interface.dart | 9 ++- dart/lib/src/sentry_tracer.dart | 74 ++++++++++++++++++------- dart/test/sentry_tracer_test.dart | 51 +++++++++++++++-- 5 files changed, 112 insertions(+), 27 deletions(-) diff --git a/dart/lib/src/noop_sentry_span.dart b/dart/lib/src/noop_sentry_span.dart index d1a98c4d87..a56e8942f2 100644 --- a/dart/lib/src/noop_sentry_span.dart +++ b/dart/lib/src/noop_sentry_span.dart @@ -26,7 +26,9 @@ class NoOpSentrySpan extends ISentrySpan { } @override - Future finish({SpanStatus? status}) async {} + Future finish({SpanStatus? status}) async { + await super.finish(status: status); + } @override void removeData(String key) {} diff --git a/dart/lib/src/protocol/sentry_span.dart b/dart/lib/src/protocol/sentry_span.dart index 8d60f6bdd4..0048f1ebbd 100644 --- a/dart/lib/src/protocol/sentry_span.dart +++ b/dart/lib/src/protocol/sentry_span.dart @@ -41,6 +41,7 @@ class SentrySpan extends ISentrySpan { if (_throwable != null) { _hub.setSpanContext(_throwable, this, _tracer.name); } + await super.finish(status: status); } @override diff --git a/dart/lib/src/sentry_span_interface.dart b/dart/lib/src/sentry_span_interface.dart index 4942fb736f..f2f04c7208 100644 --- a/dart/lib/src/sentry_span_interface.dart +++ b/dart/lib/src/sentry_span_interface.dart @@ -24,9 +24,9 @@ abstract class ISentrySpan { void removeData(String key); /// Sets span timestamp marking this span as finished. - Future finish({ - SpanStatus? status, - }); + Future finish({SpanStatus? status}) async { + finishedCallback?.call(); + } /// Gets span status. SpanStatus? get status; @@ -55,6 +55,9 @@ abstract class ISentrySpan { @internal bool? get sampled; + @internal + void Function()? finishedCallback; + /// Returns the trace information that could be sent as a sentry-trace header. SentryTraceHeader toSentryTrace(); } diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index a089011cba..47b1ffcbcf 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -7,22 +7,24 @@ import '../sentry.dart'; @internal class SentryTracer extends ISentrySpan { final Hub _hub; + late bool _waitForChildren; late String name; - // missing waitForChildren - late final SentrySpan _rootSpan; final List _children = []; final Map _extra = {}; Timer? _idleTimer; + var _finishStatus = SentryTracerFinishStatus.notFinishing(); - SentryTracer(SentryTransactionContext transactionContext, this._hub) { + SentryTracer(SentryTransactionContext transactionContext, this._hub, + {bool waitForChildren = false}) { _rootSpan = SentrySpan( this, transactionContext, _hub, sampled: transactionContext.sampled, ); + _waitForChildren = waitForChildren; name = transactionContext.name; } @@ -39,24 +41,28 @@ class SentryTracer extends ISentrySpan { @override Future finish({SpanStatus? status}) async { _idleTimer?.cancel(); - await _rootSpan.finish(status: status); - - // finish unfinished spans otherwise transaction gets dropped - for (final span in _children) { - if (!span.finished) { - await span.finish(status: SpanStatus.deadlineExceeded()); + _finishStatus = SentryTracerFinishStatus.finishing(status); + if (!_rootSpan.finished && + (!_waitForChildren || _haveAllChildrenFinished())) { + await _rootSpan.finish(status: status); + + // finish unfinished spans otherwise transaction gets dropped + for (final span in _children) { + if (!span.finished) { + await span.finish(status: SpanStatus.deadlineExceeded()); + } } - } - // remove from scope - _hub.configureScope((scope) { - if (scope.span == this) { - scope.span = null; - } - }); + // remove from scope + _hub.configureScope((scope) { + if (scope.span == this) { + scope.span = null; + } + }); - final transaction = SentryTransaction(this); - await _hub.captureTransaction(transaction); + final transaction = SentryTransaction(this); + await _hub.captureTransaction(transaction); + } } @override @@ -84,10 +90,17 @@ class SentryTracer extends ISentrySpan { String operation, { String? description, }) { - return _rootSpan.startChild( + final child = _rootSpan.startChild( operation, description: description, ); + child.finishedCallback = () { + final finishStatus = _finishStatus; + if (finishStatus.finishing) { + finish(status: finishStatus.status); + } + }; + return child; } ISentrySpan startChildWithParentSpanId( @@ -149,4 +162,27 @@ class SentryTracer extends ISentrySpan { @override SentryTraceHeader toSentryTrace() => _rootSpan.toSentryTrace(); + + bool _haveAllChildrenFinished() { + for (final child in children) { + if (!child.finished) { + return false; + } + } + return true; + } +} + +@internal +class SentryTracerFinishStatus { + final bool finishing; + final SpanStatus? status; + + SentryTracerFinishStatus.finishing(SpanStatus? status) + : finishing = true, + status = status; + + SentryTracerFinishStatus.notFinishing() + : finishing = false, + status = null; } diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index 94f83db85d..fc585fcff0 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -121,6 +121,51 @@ void main() { await Future.delayed(Duration(milliseconds: 210)); expect(sut.finished, true); }); + + test('tracer finish needs child to finish', () async { + final sut = fixture.getSut(waitForChildren: true); + + final child = sut.startChild('operation', description: 'description'); + + await sut.finish(); + expect(sut.finished, false); + + await child.finish(); + expect(sut.finished, true); + }); + + test('tracer finish needs all children to finish', () async { + final sut = fixture.getSut(waitForChildren: true); + + final childA = sut.startChild('operation-a', description: 'description'); + final childB = sut.startChild('operation-b', description: 'description'); + + await sut.finish(); + expect(sut.finished, false); + + await childA.finish(); + expect(sut.finished, false); + + await childB.finish(); + expect(sut.finished, true); + }); + + test('tracer without finish will not be finished when children are finished', + () async { + final sut = fixture.getSut(waitForChildren: true); + + final childA = sut.startChild('operation-a', description: 'description'); + final childB = sut.startChild('operation-b', description: 'description'); + + await childA.finish(); + expect(sut.finished, false); + + await childB.finish(); + expect(sut.finished, false); + + await sut.finish(); + expect(sut.finished, true); + }); } class Fixture { @@ -128,15 +173,13 @@ class Fixture { SentryTracer getSut({ bool? sampled = true, + bool waitForChildren = false, }) { final context = SentryTransactionContext( 'name', 'op', sampled: sampled, ); - return SentryTracer( - context, - hub, - ); + return SentryTracer(context, hub, waitForChildren: waitForChildren); } } From e5276c071e64644a381196b6412f245b53166529 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Tue, 30 Nov 2021 18:02:17 +0100 Subject: [PATCH 17/38] move finishAfter to ISentrySpan --- dart/lib/src/hub.dart | 6 +-- dart/lib/src/hub_adapter.dart | 2 - dart/lib/src/noop_hub.dart | 2 - dart/lib/src/sentry_span_interface.dart | 5 +++ dart/lib/src/sentry_tracer.dart | 11 ++--- dart/test/default_integrations_test.dart | 1 - dart/test/mocks/mock_hub.dart | 2 - dart/test/sentry_tracer_test.dart | 3 +- .../navigation/sentry_navigator_observer.dart | 3 +- flutter/test/mocks.dart | 3 -- flutter/test/mocks.mocks.dart | 18 ++++---- .../test/sentry_navigator_observer_test.dart | 42 ++++++++++++------- 12 files changed, 52 insertions(+), 46 deletions(-) diff --git a/dart/lib/src/hub.dart b/dart/lib/src/hub.dart index 8aff4e2c7a..c48563a8a5 100644 --- a/dart/lib/src/hub.dart +++ b/dart/lib/src/hub.dart @@ -340,7 +340,6 @@ class Hub { String operation, { String? description, bool? bindToScope, - Duration? idleFinishDuration, Map? customSamplingContext, }) => startTransactionWithContext( @@ -358,7 +357,6 @@ class Hub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, - Duration? idleFinishDuration, }) { if (!_isEnabled) { _options.logger( @@ -385,10 +383,8 @@ class Hub { final tracer = SentryTracer( transactionContext, this, + waitForChildren: true ); - if (idleFinishDuration != null) { - tracer.finishAfterIdleTime(idleFinishDuration); - } if (bindToScope ?? false) { item.scope.span = tracer; } diff --git a/dart/lib/src/hub_adapter.dart b/dart/lib/src/hub_adapter.dart index 458d5fbaab..11eb8044bd 100644 --- a/dart/lib/src/hub_adapter.dart +++ b/dart/lib/src/hub_adapter.dart @@ -102,7 +102,6 @@ class HubAdapter implements Hub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, - Duration? idleFinishDuration, }) => Sentry.startTransactionWithContext( transactionContext, @@ -116,7 +115,6 @@ class HubAdapter implements Hub { String operation, { String? description, bool? bindToScope, - Duration? idleFinishDuration, Map? customSamplingContext, }) => Sentry.startTransaction( diff --git a/dart/lib/src/noop_hub.dart b/dart/lib/src/noop_hub.dart index d5846547f3..06163bac20 100644 --- a/dart/lib/src/noop_hub.dart +++ b/dart/lib/src/noop_hub.dart @@ -79,7 +79,6 @@ class NoOpHub implements Hub { String operation, { String? description, bool? bindToScope, - Duration? idleFinishDuration, Map? customSamplingContext, }) => NoOpSentrySpan(); @@ -89,7 +88,6 @@ class NoOpHub implements Hub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, - Duration? idleFinishDuration, }) => NoOpSentrySpan(); diff --git a/dart/lib/src/sentry_span_interface.dart b/dart/lib/src/sentry_span_interface.dart index f2f04c7208..d0e4549a3b 100644 --- a/dart/lib/src/sentry_span_interface.dart +++ b/dart/lib/src/sentry_span_interface.dart @@ -28,6 +28,11 @@ abstract class ISentrySpan { finishedCallback?.call(); } + @internal + void finishAfter(Duration duration, {SpanStatus? status}) { + // Stub + } + /// Gets span status. SpanStatus? get status; diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index 47b1ffcbcf..d739e14ee3 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -28,13 +28,10 @@ class SentryTracer extends ISentrySpan { name = transactionContext.name; } - void finishAfterIdleTime(Duration time) { - _idleTimer = Timer(time, () async { - SpanStatus? status; - if (_rootSpan.status == null) { - status = SpanStatus.ok(); - } - await finish(status: status); + @override + void finishAfter(Duration duration, {SpanStatus? status}) { + _idleTimer = Timer(duration, () async { + await finish(status: _rootSpan.status ?? status); }); } diff --git a/dart/test/default_integrations_test.dart b/dart/test/default_integrations_test.dart index 42dc9f8dab..02b9c2f9ca 100644 --- a/dart/test/default_integrations_test.dart +++ b/dart/test/default_integrations_test.dart @@ -189,7 +189,6 @@ class PrintRecursionMockHub extends MockHub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, - Duration? idleFinishDuration, }) { return NoOpSentrySpan(); } diff --git a/dart/test/mocks/mock_hub.dart b/dart/test/mocks/mock_hub.dart index daf07c6f2b..b0e672abbd 100644 --- a/dart/test/mocks/mock_hub.dart +++ b/dart/test/mocks/mock_hub.dart @@ -127,7 +127,6 @@ class MockHub implements Hub { String operation, { String? description, bool? bindToScope, - Duration? idleFinishDuration, Map? customSamplingContext, }) { return NoOpSentrySpan(); @@ -138,7 +137,6 @@ class MockHub implements Hub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, - Duration? idleFinishDuration, }) { return NoOpSentrySpan(); } diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index fc585fcff0..80340709a0 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -115,10 +115,11 @@ void main() { test('tracer finishes after idle time', () async { final sut = fixture.getSut(); - sut.finishAfterIdleTime(Duration(milliseconds: 200)); + sut.finishAfter(Duration(milliseconds: 200), status: SpanStatus.ok()); expect(sut.finished, false); await Future.delayed(Duration(milliseconds: 210)); + expect(sut.status, SpanStatus.ok()); expect(sut.finished, true); }); diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 75a3ab5931..a639921631 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -130,11 +130,12 @@ class SentryNavigatorObserver extends RouteObserver> { name, 'ui.load', bindToScope: true, - idleFinishDuration: Duration(seconds: 3), ); if (arguments != null) { _transaction?.setData('route_settings_arguments', arguments); } + _transaction?.finishAfter(Duration(seconds: 3), + status: SpanStatus.ok()); } }); } diff --git a/flutter/test/mocks.dart b/flutter/test/mocks.dart index 66cb581ed3..ec2ae524d8 100644 --- a/flutter/test/mocks.dart +++ b/flutter/test/mocks.dart @@ -14,7 +14,6 @@ ISentrySpan startTransactionShim( String? operation, { String? description, bool? bindToScope, - Duration? idleFinishDuration, Map? customSamplingContext, }) { return MockNoOpSentrySpan(); @@ -188,7 +187,6 @@ class NoOpHub implements Hub { String operation, { String? description, bool? bindToScope, - Duration? idleFinishDuration, Map? customSamplingContext, }) { return NoOpSentrySpan(); @@ -198,7 +196,6 @@ class NoOpHub implements Hub { ISentrySpan startTransactionWithContext( SentryTransactionContext transactionContext, { Map? customSamplingContext, - Duration? idleFinishDuration, bool? bindToScope, }) { return NoOpSentrySpan(); diff --git a/flutter/test/mocks.mocks.dart b/flutter/test/mocks.mocks.dart index bc0aa217e2..e356d73994 100644 --- a/flutter/test/mocks.mocks.dart +++ b/flutter/test/mocks.mocks.dart @@ -84,6 +84,10 @@ class MockNoOpSentrySpan extends _i1.Mock implements _i2.NoOpSentrySpan { super.noSuchMethod(Invocation.setter(#status, status), returnValueForMissingStub: null); @override + set finishedCallback(void Function()? _finishedCallback) => super + .noSuchMethod(Invocation.setter(#finishedCallback, _finishedCallback), + returnValueForMissingStub: null); + @override _i6.Future finish({_i3.SpanStatus? status}) => (super.noSuchMethod(Invocation.method(#finish, [], {#status: status}), returnValue: Future.value(), @@ -115,6 +119,11 @@ class MockNoOpSentrySpan extends _i1.Mock implements _i2.NoOpSentrySpan { (super.noSuchMethod(Invocation.method(#toSentryTrace, []), returnValue: _FakeSentryTraceHeader_3()) as _i3.SentryTraceHeader); @override + void finishAfter(Duration? duration, {_i3.SpanStatus? status}) => + super.noSuchMethod( + Invocation.method(#finishAfter, [duration], {#status: status}), + returnValueForMissingStub: null); + @override String toString() => super.toString(); } @@ -208,7 +217,6 @@ class MockHub extends _i1.Mock implements _i4.Hub { _i2.ISentrySpan startTransaction(String? name, String? operation, {String? description, bool? bindToScope, - Duration? idleFinishDuration, Map? customSamplingContext}) => (super.noSuchMethod( Invocation.method(#startTransaction, [ @@ -217,28 +225,24 @@ class MockHub extends _i1.Mock implements _i4.Hub { ], { #description: description, #bindToScope: bindToScope, - #idleFinishDuration: idleFinishDuration, #customSamplingContext: customSamplingContext }), returnValue: _i10.startTransactionShim(name, operation, description: description, bindToScope: bindToScope, - idleFinishDuration: idleFinishDuration, customSamplingContext: customSamplingContext)) as _i2.ISentrySpan); @override _i2.ISentrySpan startTransactionWithContext( _i2.SentryTransactionContext? transactionContext, {Map? customSamplingContext, - bool? bindToScope, - Duration? idleFinishDuration}) => + bool? bindToScope}) => (super.noSuchMethod( Invocation.method(#startTransactionWithContext, [ transactionContext ], { #customSamplingContext: customSamplingContext, - #bindToScope: bindToScope, - #idleFinishDuration: idleFinishDuration + #bindToScope: bindToScope }), returnValue: _FakeISentrySpan_2()) as _i2.ISentrySpan); @override diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 2d7346cf2d..435a4909a8 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -22,7 +22,6 @@ void main() { any, description: anyNamed('description'), bindToScope: anyNamed('bindToScope'), - idleFinishDuration: anyNamed('idleFinishDuration'), customSamplingContext: anyNamed('customSamplingContext'), )).thenReturn(thenReturnSpan); } @@ -36,39 +35,47 @@ void main() { final currentRoute = route(RouteSettings(name: 'Current Route')); final hub = _MockHub(); - _whenAnyStart(hub, NoOpSentrySpan()); + final span = MockNoOpSentrySpan(); + _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub); sut.didPush(currentRoute, null); - verify(hub.startTransaction('Current Route', 'ui.load', - bindToScope: true, idleFinishDuration: Duration(seconds: 3))); + verify( + hub.startTransaction('Current Route', 'ui.load', bindToScope: true)); + verify(span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); }); test('route with empty name does not start transaction', () { final currentRoute = route(null); final hub = _MockHub(); - _whenAnyStart(hub, NoOpSentrySpan()); + final span = MockNoOpSentrySpan(); + _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub); sut.didPush(currentRoute, null); - verifyNever(hub.startTransaction('Current Route', 'ui.load', - bindToScope: true, idleFinishDuration: Duration(seconds: 3))); + verifyNever( + hub.startTransaction('Current Route', 'ui.load', bindToScope: true)); + verifyNever( + span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); }); test('no transaction on opt-out', () { final currentRoute = route(RouteSettings(name: 'Current Route')); final hub = _MockHub(); - _whenAnyStart(hub, NoOpSentrySpan()); + final span = MockNoOpSentrySpan(); + _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub, enableAutoTransactions: false); sut.didPush(currentRoute, null); - verifyNever(hub.startTransaction('Current Route', 'ui.load', - bindToScope: true, idleFinishDuration: Duration(seconds: 3))); + verifyNever( + hub.startTransaction('Current Route', 'ui.load', bindToScope: true)); + verifyNever( + span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); }); test('no transaction when scope already has one', () { @@ -76,13 +83,16 @@ void main() { final hub = _MockHub(); hub.scope.span = NoOpSentrySpan(); - _whenAnyStart(hub, NoOpSentrySpan()); + final span = MockNoOpSentrySpan(); + _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub, enableAutoTransactions: false); sut.didPush(currentRoute, null); - verifyNever(hub.startTransaction('Current Route', 'ui.load', - bindToScope: true, idleFinishDuration: Duration(seconds: 3))); + verifyNever( + hub.startTransaction('Current Route', 'ui.load', bindToScope: true)); + verifyNever( + span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); }); test('didPush finishes previous transaction', () { @@ -132,8 +142,10 @@ void main() { sut.didPop(currentRoute, previousRoute); - verify(hub.startTransaction('Previous Route', 'ui.load', - bindToScope: true, idleFinishDuration: Duration(seconds: 3))); + verify( + hub.startTransaction('Previous Route', 'ui.load', bindToScope: true)); + verify(previousSpan.finishAfter(Duration(seconds: 3), + status: SpanStatus.ok())); }); test('didPush push multiple finishes previous', () async { From 97533fc0f71f0cba7266c403e45c9b5c994fff07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Wed, 1 Dec 2021 13:40:07 +0100 Subject: [PATCH 18/38] expose waitForChildren api --- dart/lib/src/hub.dart | 5 ++++- dart/lib/src/hub_adapter.dart | 2 ++ dart/lib/src/noop_hub.dart | 2 ++ dart/lib/src/noop_sentry_span.dart | 1 + dart/lib/src/protocol/sentry_span.dart | 1 + dart/lib/src/sentry.dart | 4 ++++ dart/lib/src/sentry_span_interface.dart | 10 +++------ dart/lib/src/sentry_tracer.dart | 16 +++++++------- dart/test/default_integrations_test.dart | 1 + dart/test/mocks/mock_hub.dart | 2 ++ .../navigation/sentry_navigator_observer.dart | 1 + flutter/test/mocks.dart | 3 +++ flutter/test/mocks.mocks.dart | 9 ++++++-- .../test/sentry_navigator_observer_test.dart | 21 ++++++++++--------- 14 files changed, 51 insertions(+), 27 deletions(-) diff --git a/dart/lib/src/hub.dart b/dart/lib/src/hub.dart index c48563a8a5..68ff4bf4ed 100644 --- a/dart/lib/src/hub.dart +++ b/dart/lib/src/hub.dart @@ -340,6 +340,7 @@ class Hub { String operation, { String? description, bool? bindToScope, + bool? waitForChildren, Map? customSamplingContext, }) => startTransactionWithContext( @@ -349,6 +350,7 @@ class Hub { description: description, ), bindToScope: bindToScope, + waitForChildren: waitForChildren, customSamplingContext: customSamplingContext, ); @@ -357,6 +359,7 @@ class Hub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, + bool? waitForChildren, }) { if (!_isEnabled) { _options.logger( @@ -383,7 +386,7 @@ class Hub { final tracer = SentryTracer( transactionContext, this, - waitForChildren: true + waitForChildren: waitForChildren ?? false ); if (bindToScope ?? false) { item.scope.span = tracer; diff --git a/dart/lib/src/hub_adapter.dart b/dart/lib/src/hub_adapter.dart index 11eb8044bd..547c296428 100644 --- a/dart/lib/src/hub_adapter.dart +++ b/dart/lib/src/hub_adapter.dart @@ -102,6 +102,7 @@ class HubAdapter implements Hub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, + bool? waitForChildren, }) => Sentry.startTransactionWithContext( transactionContext, @@ -115,6 +116,7 @@ class HubAdapter implements Hub { String operation, { String? description, bool? bindToScope, + bool? waitForChildren, Map? customSamplingContext, }) => Sentry.startTransaction( diff --git a/dart/lib/src/noop_hub.dart b/dart/lib/src/noop_hub.dart index 06163bac20..59d051ed23 100644 --- a/dart/lib/src/noop_hub.dart +++ b/dart/lib/src/noop_hub.dart @@ -79,6 +79,7 @@ class NoOpHub implements Hub { String operation, { String? description, bool? bindToScope, + bool? waitForChildren, Map? customSamplingContext, }) => NoOpSentrySpan(); @@ -88,6 +89,7 @@ class NoOpHub implements Hub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, + bool? waitForChildren, }) => NoOpSentrySpan(); diff --git a/dart/lib/src/noop_sentry_span.dart b/dart/lib/src/noop_sentry_span.dart index a56e8942f2..65fe34c51f 100644 --- a/dart/lib/src/noop_sentry_span.dart +++ b/dart/lib/src/noop_sentry_span.dart @@ -27,6 +27,7 @@ class NoOpSentrySpan extends ISentrySpan { @override Future finish({SpanStatus? status}) async { + finishedCallback?.call(); await super.finish(status: status); } diff --git a/dart/lib/src/protocol/sentry_span.dart b/dart/lib/src/protocol/sentry_span.dart index 0048f1ebbd..76996a1c09 100644 --- a/dart/lib/src/protocol/sentry_span.dart +++ b/dart/lib/src/protocol/sentry_span.dart @@ -41,6 +41,7 @@ class SentrySpan extends ISentrySpan { if (_throwable != null) { _hub.setSpanContext(_throwable, this, _tracer.name); } + finishedCallback?.call(); await super.finish(status: status); } diff --git a/dart/lib/src/sentry.dart b/dart/lib/src/sentry.dart index 4ded2d49ce..90d5580229 100644 --- a/dart/lib/src/sentry.dart +++ b/dart/lib/src/sentry.dart @@ -229,6 +229,7 @@ class Sentry { String operation, { String? description, bool? bindToScope, + bool? waitForChildren, Map? customSamplingContext, }) => _hub.startTransaction( @@ -236,6 +237,7 @@ class Sentry { operation, description: description, bindToScope: bindToScope, + waitForChildren: waitForChildren, customSamplingContext: customSamplingContext, ); @@ -244,11 +246,13 @@ class Sentry { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, + bool? waitForChildren, }) => _hub.startTransactionWithContext( transactionContext, customSamplingContext: customSamplingContext, bindToScope: bindToScope, + waitForChildren: waitForChildren, ); /// Gets the current active transaction or span. diff --git a/dart/lib/src/sentry_span_interface.dart b/dart/lib/src/sentry_span_interface.dart index d0e4549a3b..141055616f 100644 --- a/dart/lib/src/sentry_span_interface.dart +++ b/dart/lib/src/sentry_span_interface.dart @@ -24,14 +24,10 @@ abstract class ISentrySpan { void removeData(String key); /// Sets span timestamp marking this span as finished. - Future finish({SpanStatus? status}) async { - finishedCallback?.call(); - } + Future finish({SpanStatus? status}) async {} - @internal - void finishAfter(Duration duration, {SpanStatus? status}) { - // Stub - } + /// Calls finish after the provided duration with the status parameter. + void finishAfter(Duration duration, {SpanStatus? status}) {} /// Gets span status. SpanStatus? get status; diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index d739e14ee3..dad05ba05c 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -28,13 +28,6 @@ class SentryTracer extends ISentrySpan { name = transactionContext.name; } - @override - void finishAfter(Duration duration, {SpanStatus? status}) { - _idleTimer = Timer(duration, () async { - await finish(status: _rootSpan.status ?? status); - }); - } - @override Future finish({SpanStatus? status}) async { _idleTimer?.cancel(); @@ -59,9 +52,18 @@ class SentryTracer extends ISentrySpan { final transaction = SentryTransaction(this); await _hub.captureTransaction(transaction); + + finishedCallback?.call(); } } + @override + void finishAfter(Duration duration, {SpanStatus? status}) { + _idleTimer = Timer(duration, () async { + await finish(status: _rootSpan.status ?? status); + }); + } + @override void removeData(String key) { _extra.remove(key); diff --git a/dart/test/default_integrations_test.dart b/dart/test/default_integrations_test.dart index 02b9c2f9ca..10b520fc5a 100644 --- a/dart/test/default_integrations_test.dart +++ b/dart/test/default_integrations_test.dart @@ -189,6 +189,7 @@ class PrintRecursionMockHub extends MockHub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, + bool? waitForChildren, }) { return NoOpSentrySpan(); } diff --git a/dart/test/mocks/mock_hub.dart b/dart/test/mocks/mock_hub.dart index b0e672abbd..74e83df85e 100644 --- a/dart/test/mocks/mock_hub.dart +++ b/dart/test/mocks/mock_hub.dart @@ -127,6 +127,7 @@ class MockHub implements Hub { String operation, { String? description, bool? bindToScope, + bool? waitForChildren, Map? customSamplingContext, }) { return NoOpSentrySpan(); @@ -137,6 +138,7 @@ class MockHub implements Hub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, + bool? waitForChildren, }) { return NoOpSentrySpan(); } diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index a639921631..b9080a89ab 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -130,6 +130,7 @@ class SentryNavigatorObserver extends RouteObserver> { name, 'ui.load', bindToScope: true, + waitForChildren: true, ); if (arguments != null) { _transaction?.setData('route_settings_arguments', arguments); diff --git a/flutter/test/mocks.dart b/flutter/test/mocks.dart index ec2ae524d8..8470563875 100644 --- a/flutter/test/mocks.dart +++ b/flutter/test/mocks.dart @@ -14,6 +14,7 @@ ISentrySpan startTransactionShim( String? operation, { String? description, bool? bindToScope, + bool? waitForChildren, Map? customSamplingContext, }) { return MockNoOpSentrySpan(); @@ -187,6 +188,7 @@ class NoOpHub implements Hub { String operation, { String? description, bool? bindToScope, + bool? waitForChildren, Map? customSamplingContext, }) { return NoOpSentrySpan(); @@ -197,6 +199,7 @@ class NoOpHub implements Hub { SentryTransactionContext transactionContext, { Map? customSamplingContext, bool? bindToScope, + bool? waitForChildren, }) { return NoOpSentrySpan(); } diff --git a/flutter/test/mocks.mocks.dart b/flutter/test/mocks.mocks.dart index e356d73994..8e28a2f087 100644 --- a/flutter/test/mocks.mocks.dart +++ b/flutter/test/mocks.mocks.dart @@ -217,6 +217,7 @@ class MockHub extends _i1.Mock implements _i4.Hub { _i2.ISentrySpan startTransaction(String? name, String? operation, {String? description, bool? bindToScope, + bool? waitForChildren, Map? customSamplingContext}) => (super.noSuchMethod( Invocation.method(#startTransaction, [ @@ -225,24 +226,28 @@ class MockHub extends _i1.Mock implements _i4.Hub { ], { #description: description, #bindToScope: bindToScope, + #waitForChildren: waitForChildren, #customSamplingContext: customSamplingContext }), returnValue: _i10.startTransactionShim(name, operation, description: description, bindToScope: bindToScope, + waitForChildren: waitForChildren, customSamplingContext: customSamplingContext)) as _i2.ISentrySpan); @override _i2.ISentrySpan startTransactionWithContext( _i2.SentryTransactionContext? transactionContext, {Map? customSamplingContext, - bool? bindToScope}) => + bool? bindToScope, + bool? waitForChildren}) => (super.noSuchMethod( Invocation.method(#startTransactionWithContext, [ transactionContext ], { #customSamplingContext: customSamplingContext, - #bindToScope: bindToScope + #bindToScope: bindToScope, + #waitForChildren: waitForChildren }), returnValue: _FakeISentrySpan_2()) as _i2.ISentrySpan); @override diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 435a4909a8..dc211b59a2 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -22,6 +22,7 @@ void main() { any, description: anyNamed('description'), bindToScope: anyNamed('bindToScope'), + waitForChildren: anyNamed('waitForChildren'), customSamplingContext: anyNamed('customSamplingContext'), )).thenReturn(thenReturnSpan); } @@ -41,8 +42,8 @@ void main() { sut.didPush(currentRoute, null); - verify( - hub.startTransaction('Current Route', 'ui.load', bindToScope: true)); + verify(hub.startTransaction('Current Route', 'ui.load', + bindToScope: true, waitForChildren: true)); verify(span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); }); @@ -56,8 +57,8 @@ void main() { sut.didPush(currentRoute, null); - verifyNever( - hub.startTransaction('Current Route', 'ui.load', bindToScope: true)); + verifyNever(hub.startTransaction('Current Route', 'ui.load', + bindToScope: true, waitForChildren: true)); verifyNever( span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); }); @@ -72,8 +73,8 @@ void main() { sut.didPush(currentRoute, null); - verifyNever( - hub.startTransaction('Current Route', 'ui.load', bindToScope: true)); + verifyNever(hub.startTransaction('Current Route', 'ui.load', + bindToScope: true, waitForChildren: true)); verifyNever( span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); }); @@ -89,8 +90,8 @@ void main() { sut.didPush(currentRoute, null); - verifyNever( - hub.startTransaction('Current Route', 'ui.load', bindToScope: true)); + verifyNever(hub.startTransaction('Current Route', 'ui.load', + bindToScope: true, waitForChildren: true)); verifyNever( span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); }); @@ -142,8 +143,8 @@ void main() { sut.didPop(currentRoute, previousRoute); - verify( - hub.startTransaction('Previous Route', 'ui.load', bindToScope: true)); + verify(hub.startTransaction('Previous Route', 'ui.load', + bindToScope: true, waitForChildren: true)); verify(previousSpan.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); }); From 6161cb05b83a61f4b000346f1bc9950927f5ea88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Wed, 1 Dec 2021 17:24:47 +0100 Subject: [PATCH 19/38] move finishAfter implementation out of abstract class --- dart/lib/src/protocol/sentry_span.dart | 11 +++++++++++ dart/lib/src/sentry_tracer.dart | 11 ++++++----- dart/test/sentry_span_test.dart | 10 ++++++++++ dart/test/sentry_tracer_test.dart | 2 +- 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/dart/lib/src/protocol/sentry_span.dart b/dart/lib/src/protocol/sentry_span.dart index 76996a1c09..f40d292b49 100644 --- a/dart/lib/src/protocol/sentry_span.dart +++ b/dart/lib/src/protocol/sentry_span.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import '../hub.dart'; import '../protocol.dart'; @@ -17,6 +19,7 @@ class SentrySpan extends ISentrySpan { SpanStatus? _status; final Map _tags = {}; + Timer? _finishAfterTimer; @override bool? sampled; @@ -32,6 +35,7 @@ class SentrySpan extends ISentrySpan { @override Future finish({SpanStatus? status}) async { + _finishAfterTimer?.cancel(); if (status != null) { _status = status; } @@ -45,6 +49,13 @@ class SentrySpan extends ISentrySpan { await super.finish(status: status); } + @override + void finishAfter(Duration duration, {SpanStatus? status}) { + _finishAfterTimer = Timer(duration, () async { + await finish(status: status); + }); + } + @override void removeData(String key) { _data.remove(key); diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index dad05ba05c..0e27c49d75 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -13,7 +13,7 @@ class SentryTracer extends ISentrySpan { late final SentrySpan _rootSpan; final List _children = []; final Map _extra = {}; - Timer? _idleTimer; + Timer? _finishAfterTimer; var _finishStatus = SentryTracerFinishStatus.notFinishing(); SentryTracer(SentryTransactionContext transactionContext, this._hub, @@ -30,11 +30,12 @@ class SentryTracer extends ISentrySpan { @override Future finish({SpanStatus? status}) async { - _idleTimer?.cancel(); + _finishAfterTimer?.cancel(); _finishStatus = SentryTracerFinishStatus.finishing(status); if (!_rootSpan.finished && (!_waitForChildren || _haveAllChildrenFinished())) { - await _rootSpan.finish(status: status); + _rootSpan.status ??= status; + await _rootSpan.finish(); // finish unfinished spans otherwise transaction gets dropped for (final span in _children) { @@ -59,8 +60,8 @@ class SentryTracer extends ISentrySpan { @override void finishAfter(Duration duration, {SpanStatus? status}) { - _idleTimer = Timer(duration, () async { - await finish(status: _rootSpan.status ?? status); + _finishAfterTimer = Timer(duration, () async { + await finish(status: status); }); } diff --git a/dart/test/sentry_span_test.dart b/dart/test/sentry_span_test.dart index 92a7baaf4c..1bbb0b36a1 100644 --- a/dart/test/sentry_span_test.dart +++ b/dart/test/sentry_span_test.dart @@ -113,6 +113,16 @@ void main() { expect(sut.toSentryTrace().value, '${sut.context.traceId}-${sut.context.spanId}-1'); }); + + test('finishes after duration', () async { + final sut = fixture.getSut(); + sut.finishAfter(Duration(milliseconds: 200), status: SpanStatus.ok()); + + expect(sut.finished, false); + await Future.delayed(Duration(milliseconds: 210)); + expect(sut.status, SpanStatus.ok()); + expect(sut.finished, true); + }); } class Fixture { diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index 80340709a0..bc59edad63 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -113,7 +113,7 @@ void main() { '${sut.context.traceId}-${sut.context.spanId}-1'); }); - test('tracer finishes after idle time', () async { + test('tracer finishes after duration', () async { final sut = fixture.getSut(); sut.finishAfter(Duration(milliseconds: 200), status: SpanStatus.ok()); From c003afc1e830a7305063ec0213efc10d9c862d6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Wed, 1 Dec 2021 17:30:06 +0100 Subject: [PATCH 20/38] add missing tests for callback on finish --- dart/test/sentry_span_test.dart | 11 +++++++++++ dart/test/sentry_tracer_test.dart | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/dart/test/sentry_span_test.dart b/dart/test/sentry_span_test.dart index 1bbb0b36a1..07bd119b31 100644 --- a/dart/test/sentry_span_test.dart +++ b/dart/test/sentry_span_test.dart @@ -123,6 +123,17 @@ void main() { expect(sut.status, SpanStatus.ok()); expect(sut.finished, true); }); + + test('callback called on finish', () async { + final sut = fixture.getSut(); + var numberOfCallbackCalls = 0; + sut.finishedCallback = () { + numberOfCallbackCalls += 1; + }; + await sut.finish(); + + expect(numberOfCallbackCalls, 1); + }); } class Fixture { diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index bc59edad63..145f26b33a 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -167,6 +167,17 @@ void main() { await sut.finish(); expect(sut.finished, true); }); + + test('callback called on finish', () async { + final sut = fixture.getSut(); + var numberOfCallbackCalls = 0; + sut.finishedCallback = () { + numberOfCallbackCalls += 1; + }; + await sut.finish(); + + expect(numberOfCallbackCalls, 1); + }); } class Fixture { From 5656aa6aa7b321d8ebd839209d22d752b86eb0a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Wed, 1 Dec 2021 17:36:52 +0100 Subject: [PATCH 21/38] move SentryTracerFinishStatus to own file --- dart/lib/src/sentry_tracer.dart | 15 +-------------- dart/lib/src/sentry_tracer_finish_status.dart | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 14 deletions(-) create mode 100644 dart/lib/src/sentry_tracer_finish_status.dart diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index 0e27c49d75..a43db09e36 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -3,6 +3,7 @@ import 'dart:async'; import 'package:meta/meta.dart'; import '../sentry.dart'; +import 'sentry_tracer_finish_status.dart'; @internal class SentryTracer extends ISentrySpan { @@ -172,17 +173,3 @@ class SentryTracer extends ISentrySpan { return true; } } - -@internal -class SentryTracerFinishStatus { - final bool finishing; - final SpanStatus? status; - - SentryTracerFinishStatus.finishing(SpanStatus? status) - : finishing = true, - status = status; - - SentryTracerFinishStatus.notFinishing() - : finishing = false, - status = null; -} diff --git a/dart/lib/src/sentry_tracer_finish_status.dart b/dart/lib/src/sentry_tracer_finish_status.dart new file mode 100644 index 0000000000..a5eca97948 --- /dev/null +++ b/dart/lib/src/sentry_tracer_finish_status.dart @@ -0,0 +1,17 @@ +import 'package:meta/meta.dart'; + +import '../sentry.dart'; + +@internal +class SentryTracerFinishStatus { + final bool finishing; + final SpanStatus? status; + + SentryTracerFinishStatus.finishing(SpanStatus? status) + : finishing = true, + status = status; + + SentryTracerFinishStatus.notFinishing() + : finishing = false, + status = null; +} From 941d85467930caf21d8e5fa53285e100f9c91577 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Wed, 1 Dec 2021 17:39:29 +0100 Subject: [PATCH 22/38] run format --- dart/lib/src/hub.dart | 7 ++----- dart/test/sentry_span_test.dart | 2 +- dart/test/sentry_tracer_test.dart | 2 +- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/dart/lib/src/hub.dart b/dart/lib/src/hub.dart index 68ff4bf4ed..535e2d0291 100644 --- a/dart/lib/src/hub.dart +++ b/dart/lib/src/hub.dart @@ -383,11 +383,8 @@ class Hub { transactionContext = transactionContext.copyWith(sampled: sampled); } - final tracer = SentryTracer( - transactionContext, - this, - waitForChildren: waitForChildren ?? false - ); + final tracer = SentryTracer(transactionContext, this, + waitForChildren: waitForChildren ?? false); if (bindToScope ?? false) { item.scope.span = tracer; } diff --git a/dart/test/sentry_span_test.dart b/dart/test/sentry_span_test.dart index 07bd119b31..e27c8f32fa 100644 --- a/dart/test/sentry_span_test.dart +++ b/dart/test/sentry_span_test.dart @@ -124,7 +124,7 @@ void main() { expect(sut.finished, true); }); - test('callback called on finish', () async { + test('callback called on finish', () async { final sut = fixture.getSut(); var numberOfCallbackCalls = 0; sut.finishedCallback = () { diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index 145f26b33a..0b3062c20f 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -168,7 +168,7 @@ void main() { expect(sut.finished, true); }); - test('callback called on finish', () async { + test('callback called on finish', () async { final sut = fixture.getSut(); var numberOfCallbackCalls = 0; sut.finishedCallback = () { From 34569112c7d01287a4c1f132a6e941b3208a3767 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Wed, 1 Dec 2021 18:15:22 +0100 Subject: [PATCH 23/38] propagate waitForChildren --- dart/lib/src/hub_adapter.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dart/lib/src/hub_adapter.dart b/dart/lib/src/hub_adapter.dart index 547c296428..c38246c56a 100644 --- a/dart/lib/src/hub_adapter.dart +++ b/dart/lib/src/hub_adapter.dart @@ -108,6 +108,7 @@ class HubAdapter implements Hub { transactionContext, customSamplingContext: customSamplingContext, bindToScope: bindToScope, + waitForChildren: waitForChildren, ); @override @@ -124,6 +125,7 @@ class HubAdapter implements Hub { operation, description: description, bindToScope: bindToScope, + waitForChildren: waitForChildren, customSamplingContext: customSamplingContext, ); From c32a413f98ba210bd98e8a51e61dead61d511b77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Fri, 3 Dec 2021 14:16:00 +0100 Subject: [PATCH 24/38] only bind to scope if scope span is null --- .../navigation/sentry_navigator_observer.dart | 22 +++++++++---------- .../test/sentry_navigator_observer_test.dart | 10 ++++----- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index b9080a89ab..e73f562d5f 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -125,19 +125,17 @@ class SentryNavigatorObserver extends RouteObserver> { return; } _hub.configureScope((scope) { - if (scope.span == null) { - _transaction = _hub.startTransaction( - name, - 'ui.load', - bindToScope: true, - waitForChildren: true, - ); - if (arguments != null) { - _transaction?.setData('route_settings_arguments', arguments); - } - _transaction?.finishAfter(Duration(seconds: 3), - status: SpanStatus.ok()); + _transaction = _hub.startTransaction( + name, + 'ui.load', + bindToScope: scope.span == null, + waitForChildren: true, + ); + if (arguments != null) { + _transaction?.setData('route_settings_arguments', arguments); } + _transaction?.finishAfter(Duration(seconds: 3), + status: SpanStatus.ok()); }); } diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index dc211b59a2..85e558a503 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -79,20 +79,20 @@ void main() { span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); }); - test('no transaction when scope already has one', () { + test('do not bind to scope if already set', () { final currentRoute = route(RouteSettings(name: 'Current Route')); final hub = _MockHub(); hub.scope.span = NoOpSentrySpan(); final span = MockNoOpSentrySpan(); _whenAnyStart(hub, span); - final sut = fixture.getSut(hub: hub, enableAutoTransactions: false); + final sut = fixture.getSut(hub: hub); sut.didPush(currentRoute, null); - verifyNever(hub.startTransaction('Current Route', 'ui.load', - bindToScope: true, waitForChildren: true)); - verifyNever( + verify(hub.startTransaction('Current Route', 'ui.load', + bindToScope: false, waitForChildren: true)); + verify( span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); }); From 5b791ad43b3cd258579240894e88064194fb9182 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Fri, 3 Dec 2021 14:16:36 +0100 Subject: [PATCH 25/38] format --- flutter/lib/src/navigation/sentry_navigator_observer.dart | 3 +-- flutter/test/sentry_navigator_observer_test.dart | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index e73f562d5f..1f76323139 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -134,8 +134,7 @@ class SentryNavigatorObserver extends RouteObserver> { if (arguments != null) { _transaction?.setData('route_settings_arguments', arguments); } - _transaction?.finishAfter(Duration(seconds: 3), - status: SpanStatus.ok()); + _transaction?.finishAfter(Duration(seconds: 3), status: SpanStatus.ok()); }); } diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 85e558a503..46c97297d6 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -92,8 +92,7 @@ void main() { verify(hub.startTransaction('Current Route', 'ui.load', bindToScope: false, waitForChildren: true)); - verify( - span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); + verify(span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); }); test('didPush finishes previous transaction', () { From 22a395f3a95f0212b5a0cb82ee9ea35fc78d8a79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Thu, 9 Dec 2021 11:08:42 +0100 Subject: [PATCH 26/38] move finifhed callback to SentrySpan ctor --- dart/lib/src/noop_sentry_span.dart | 1 - dart/lib/src/protocol/sentry_span.dart | 5 ++++- dart/lib/src/sentry_span_interface.dart | 3 --- dart/lib/src/sentry_tracer.dart | 30 +++++++++---------------- dart/test/sentry_span_test.dart | 9 ++++---- dart/test/sentry_tracer_test.dart | 11 --------- flutter/test/mocks.mocks.dart | 4 ---- 7 files changed, 20 insertions(+), 43 deletions(-) diff --git a/dart/lib/src/noop_sentry_span.dart b/dart/lib/src/noop_sentry_span.dart index 65fe34c51f..a56e8942f2 100644 --- a/dart/lib/src/noop_sentry_span.dart +++ b/dart/lib/src/noop_sentry_span.dart @@ -27,7 +27,6 @@ class NoOpSentrySpan extends ISentrySpan { @override Future finish({SpanStatus? status}) async { - finishedCallback?.call(); await super.finish(status: status); } diff --git a/dart/lib/src/protocol/sentry_span.dart b/dart/lib/src/protocol/sentry_span.dart index f40d292b49..ef28b49b12 100644 --- a/dart/lib/src/protocol/sentry_span.dart +++ b/dart/lib/src/protocol/sentry_span.dart @@ -20,6 +20,7 @@ class SentrySpan extends ISentrySpan { SpanStatus? _status; final Map _tags = {}; Timer? _finishAfterTimer; + void Function()? _finishedCallback; @override bool? sampled; @@ -29,8 +30,10 @@ class SentrySpan extends ISentrySpan { this._context, this._hub, { bool? sampled, + Function()? finishedCallback, }) { this.sampled = sampled; + _finishedCallback = finishedCallback; } @override @@ -45,7 +48,7 @@ class SentrySpan extends ISentrySpan { if (_throwable != null) { _hub.setSpanContext(_throwable, this, _tracer.name); } - finishedCallback?.call(); + _finishedCallback?.call(); await super.finish(status: status); } diff --git a/dart/lib/src/sentry_span_interface.dart b/dart/lib/src/sentry_span_interface.dart index 141055616f..5133b805cd 100644 --- a/dart/lib/src/sentry_span_interface.dart +++ b/dart/lib/src/sentry_span_interface.dart @@ -56,9 +56,6 @@ abstract class ISentrySpan { @internal bool? get sampled; - @internal - void Function()? finishedCallback; - /// Returns the trace information that could be sent as a sentry-trace header. SentryTraceHeader toSentryTrace(); } diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index a43db09e36..6e8e4c0f32 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -54,8 +54,6 @@ class SentryTracer extends ISentrySpan { final transaction = SentryTransaction(this); await _hub.captureTransaction(transaction); - - finishedCallback?.call(); } } @@ -95,12 +93,6 @@ class SentryTracer extends ISentrySpan { operation, description: description, ); - child.finishedCallback = () { - final finishStatus = _finishStatus; - if (finishStatus.finishing) { - finish(status: finishStatus.status); - } - }; return child; } @@ -110,18 +102,18 @@ class SentryTracer extends ISentrySpan { String? description, }) { final context = SentrySpanContext( - traceId: _rootSpan.context.traceId, - parentSpanId: parentSpanId, - operation: operation, - description: description, - ); + traceId: _rootSpan.context.traceId, + parentSpanId: parentSpanId, + operation: operation, + description: description); - final child = SentrySpan( - this, - context, - _hub, - sampled: _rootSpan.sampled, - ); + final child = SentrySpan(this, context, _hub, sampled: _rootSpan.sampled, + finishedCallback: () { + final finishStatus = _finishStatus; + if (finishStatus.finishing) { + finish(status: finishStatus.status); + } + }); _children.add(child); diff --git a/dart/test/sentry_span_test.dart b/dart/test/sentry_span_test.dart index e27c8f32fa..cf0fe186a7 100644 --- a/dart/test/sentry_span_test.dart +++ b/dart/test/sentry_span_test.dart @@ -125,11 +125,11 @@ void main() { }); test('callback called on finish', () async { - final sut = fixture.getSut(); var numberOfCallbackCalls = 0; - sut.finishedCallback = () { + final sut = fixture.getSut(finishedCallback: () { numberOfCallbackCalls += 1; - }; + }); + await sut.finish(); expect(numberOfCallbackCalls, 1); @@ -144,7 +144,7 @@ class Fixture { late SentryTracer tracer; final hub = MockHub(); - SentrySpan getSut({bool? sampled = true}) { + SentrySpan getSut({bool? sampled = true, Function()? finishedCallback}) { tracer = SentryTracer(context, hub); return SentrySpan( @@ -152,6 +152,7 @@ class Fixture { context, hub, sampled: sampled, + finishedCallback: finishedCallback, ); } } diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index 0b3062c20f..bc59edad63 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -167,17 +167,6 @@ void main() { await sut.finish(); expect(sut.finished, true); }); - - test('callback called on finish', () async { - final sut = fixture.getSut(); - var numberOfCallbackCalls = 0; - sut.finishedCallback = () { - numberOfCallbackCalls += 1; - }; - await sut.finish(); - - expect(numberOfCallbackCalls, 1); - }); } class Fixture { diff --git a/flutter/test/mocks.mocks.dart b/flutter/test/mocks.mocks.dart index 8e28a2f087..4383a0c25e 100644 --- a/flutter/test/mocks.mocks.dart +++ b/flutter/test/mocks.mocks.dart @@ -84,10 +84,6 @@ class MockNoOpSentrySpan extends _i1.Mock implements _i2.NoOpSentrySpan { super.noSuchMethod(Invocation.setter(#status, status), returnValueForMissingStub: null); @override - set finishedCallback(void Function()? _finishedCallback) => super - .noSuchMethod(Invocation.setter(#finishedCallback, _finishedCallback), - returnValueForMissingStub: null); - @override _i6.Future finish({_i3.SpanStatus? status}) => (super.noSuchMethod(Invocation.method(#finish, [], {#status: status}), returnValue: Future.value(), From 96b4a5c5d97aace01b8b929e38b908df579b8140 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Thu, 9 Dec 2021 11:23:29 +0100 Subject: [PATCH 27/38] update doc for enableAutoTransactions --- .../lib/src/navigation/sentry_navigator_observer.dart | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 1f76323139..c6fda6e234 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -32,9 +32,16 @@ const _navigationKey = 'navigation'; /// // other parameter ... /// ) /// ``` +/// +/// The option [enableAutoTransactions] is enabled by default. For every new +/// route a transaction is started. It's automatically finished after 3 seconds +/// or when all child spans are finished, if those happen to take longer. The +/// transaction will be set to [Scope.span] if the latter is empty. +/// /// Enabling the [setRouteNameAsTransaction] option overrides the current -/// [Scope.transaction]. So be careful when this is used together with -/// performance monitoring. +/// [Scope.transaction] which will also override the name of the current +/// [Scope.span]. So be careful when this is used together with performance +/// monitoring. /// /// See also: /// - [RouteObserver](https://api.flutter.dev/flutter/widgets/RouteObserver-class.html) From 28bc71153bbb6da04770752638c8bb3530765ac1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Thu, 9 Dec 2021 15:02:13 +0100 Subject: [PATCH 28/38] remove unneccesary imports --- dart/lib/src/hub.dart | 1 - dart/lib/src/noop_hub.dart | 1 - dart/lib/src/protocol/breadcrumb.dart | 1 - dart/lib/src/protocol/sentry_transaction.dart | 1 - dart/lib/src/sentry_envelope.dart | 2 -- dart/lib/src/sentry_envelope_item.dart | 1 - dart/lib/src/sentry_traces_sampler.dart | 1 - dart/test/contexts_test.dart | 1 - dart/test/default_integrations_test.dart | 1 - dart/test/http_client/failed_request_client_test.dart | 1 - dart/test/http_client/sentry_http_client_test.dart | 1 - dart/test/hub_test.dart | 2 -- dart/test/mocks.dart | 1 - dart/test/mocks/mock_hub.dart | 2 -- dart/test/mocks/mock_sentry_client.dart | 1 - dart/test/scope_test.dart | 1 - dart/test/sentry_envelope_item_test.dart | 1 - dart/test/sentry_envelope_test.dart | 2 -- dart/test/sentry_envelope_vm_test.dart | 3 --- dart/test/sentry_event_test.dart | 2 -- dart/test/sentry_span_test.dart | 1 - flutter/lib/src/widgets_binding_observer.dart | 3 --- flutter/test/default_integrations_test.dart | 4 ---- flutter/test/initialization_test.dart | 1 - flutter/test/load_contexts_integrations_test.dart | 1 - flutter/test/mocks.dart | 2 -- flutter/test/not_initialized_widgets_binding_test.dart | 1 - flutter/test/sentry_flutter_test.dart | 2 -- flutter/test/widgets_binding_observer_test.dart | 1 - 29 files changed, 43 deletions(-) diff --git a/dart/lib/src/hub.dart b/dart/lib/src/hub.dart index 535e2d0291..ec28b02658 100644 --- a/dart/lib/src/hub.dart +++ b/dart/lib/src/hub.dart @@ -3,7 +3,6 @@ import 'dart:collection'; import 'package:meta/meta.dart'; -import 'noop_sentry_span.dart'; import 'protocol.dart'; import 'scope.dart'; import 'sentry_client.dart'; diff --git a/dart/lib/src/noop_hub.dart b/dart/lib/src/noop_hub.dart index 59d051ed23..bbcee80b54 100644 --- a/dart/lib/src/noop_hub.dart +++ b/dart/lib/src/noop_hub.dart @@ -4,7 +4,6 @@ import 'protocol.dart'; import 'sentry_client.dart'; import 'sentry_user_feedback.dart'; import 'tracing.dart'; -import 'noop_sentry_span.dart'; class NoOpHub implements Hub { NoOpHub._(); diff --git a/dart/lib/src/protocol/breadcrumb.dart b/dart/lib/src/protocol/breadcrumb.dart index e1ac8cd4fc..198942c2e4 100644 --- a/dart/lib/src/protocol/breadcrumb.dart +++ b/dart/lib/src/protocol/breadcrumb.dart @@ -1,7 +1,6 @@ import 'package:meta/meta.dart'; import '../utils.dart'; -import 'sentry_level.dart'; import '../protocol.dart'; /// Structed data to describe more information pior to the event captured. diff --git a/dart/lib/src/protocol/sentry_transaction.dart b/dart/lib/src/protocol/sentry_transaction.dart index 2d1496198e..2bbdbfc7bb 100644 --- a/dart/lib/src/protocol/sentry_transaction.dart +++ b/dart/lib/src/protocol/sentry_transaction.dart @@ -2,7 +2,6 @@ import 'package:meta/meta.dart'; import '../protocol.dart'; import '../sentry_tracer.dart'; -import 'sentry_span.dart'; import '../utils.dart'; @immutable diff --git a/dart/lib/src/sentry_envelope.dart b/dart/lib/src/sentry_envelope.dart index ac93c54b07..7c38fd63f8 100644 --- a/dart/lib/src/sentry_envelope.dart +++ b/dart/lib/src/sentry_envelope.dart @@ -6,8 +6,6 @@ import 'utils.dart'; import 'sentry_attachment/sentry_attachment.dart'; import 'sentry_envelope_header.dart'; import 'sentry_envelope_item.dart'; -import 'protocol/sentry_event.dart'; -import 'protocol/sdk_version.dart'; import 'sentry_user_feedback.dart'; /// Class representation of `Envelope` file. diff --git a/dart/lib/src/sentry_envelope_item.dart b/dart/lib/src/sentry_envelope_item.dart index 69778046c9..b274334127 100644 --- a/dart/lib/src/sentry_envelope_item.dart +++ b/dart/lib/src/sentry_envelope_item.dart @@ -3,7 +3,6 @@ import 'protocol.dart'; import 'utils.dart'; import 'sentry_attachment/sentry_attachment.dart'; import 'sentry_item_type.dart'; -import 'protocol/sentry_event.dart'; import 'sentry_envelope_item_header.dart'; import 'sentry_user_feedback.dart'; diff --git a/dart/lib/src/sentry_traces_sampler.dart b/dart/lib/src/sentry_traces_sampler.dart index 3501ff64cc..9d981cca04 100644 --- a/dart/lib/src/sentry_traces_sampler.dart +++ b/dart/lib/src/sentry_traces_sampler.dart @@ -3,7 +3,6 @@ import 'dart:math'; import 'package:meta/meta.dart'; import '../sentry.dart'; -import 'tracing.dart'; @internal class SentryTracesSampler { diff --git a/dart/test/contexts_test.dart b/dart/test/contexts_test.dart index 5bd97189d0..137795c716 100644 --- a/dart/test/contexts_test.dart +++ b/dart/test/contexts_test.dart @@ -6,7 +6,6 @@ import 'dart:convert'; import 'package:collection/collection.dart'; import 'package:sentry/sentry.dart'; -import 'package:sentry/src/protocol/sentry_gpu.dart'; import 'package:test/test.dart'; void main() { diff --git a/dart/test/default_integrations_test.dart b/dart/test/default_integrations_test.dart index 9bc26e0794..9e83a37b8b 100644 --- a/dart/test/default_integrations_test.dart +++ b/dart/test/default_integrations_test.dart @@ -1,5 +1,4 @@ import 'package:sentry/sentry.dart'; -import 'package:sentry/src/noop_sentry_span.dart'; import 'package:test/test.dart'; import 'mocks/mock_hub.dart'; diff --git a/dart/test/http_client/failed_request_client_test.dart b/dart/test/http_client/failed_request_client_test.dart index 80fce80fc1..342a7f00f1 100644 --- a/dart/test/http_client/failed_request_client_test.dart +++ b/dart/test/http_client/failed_request_client_test.dart @@ -3,7 +3,6 @@ import 'package:http/testing.dart'; import 'package:mockito/mockito.dart'; import 'package:sentry/sentry.dart'; import 'package:sentry/src/http_client/failed_request_client.dart'; -import 'package:sentry/src/http_client/sentry_http_client.dart'; import 'package:test/test.dart'; import '../mocks.dart'; diff --git a/dart/test/http_client/sentry_http_client_test.dart b/dart/test/http_client/sentry_http_client_test.dart index 390cc98561..697803b89c 100644 --- a/dart/test/http_client/sentry_http_client_test.dart +++ b/dart/test/http_client/sentry_http_client_test.dart @@ -3,7 +3,6 @@ import 'package:http/testing.dart'; import 'package:mockito/mockito.dart'; import 'package:sentry/sentry.dart'; import 'package:sentry/src/http_client/failed_request_client.dart'; -import 'package:sentry/src/http_client/sentry_http_client.dart'; import 'package:test/test.dart'; import '../mocks/mock_hub.dart'; diff --git a/dart/test/hub_test.dart b/dart/test/hub_test.dart index 32bf3ab1d9..2da153bf04 100644 --- a/dart/test/hub_test.dart +++ b/dart/test/hub_test.dart @@ -1,7 +1,5 @@ import 'package:collection/collection.dart'; import 'package:sentry/sentry.dart'; -import 'package:sentry/src/hub.dart'; -import 'package:sentry/src/noop_sentry_span.dart'; import 'package:sentry/src/sentry_tracer.dart'; import 'package:test/test.dart'; diff --git a/dart/test/mocks.dart b/dart/test/mocks.dart index c9bd199512..cb36172d06 100644 --- a/dart/test/mocks.dart +++ b/dart/test/mocks.dart @@ -1,7 +1,6 @@ import 'dart:async'; import 'package:sentry/sentry.dart'; -import 'package:sentry/src/protocol.dart'; import 'package:sentry/src/transport/rate_limiter.dart'; final fakeDsn = 'https://abc@def.ingest.sentry.io/1234567'; diff --git a/dart/test/mocks/mock_hub.dart b/dart/test/mocks/mock_hub.dart index 74e83df85e..0601eeb652 100644 --- a/dart/test/mocks/mock_hub.dart +++ b/dart/test/mocks/mock_hub.dart @@ -1,7 +1,5 @@ import 'package:sentry/sentry.dart'; import 'package:sentry/src/noop_hub.dart'; -import 'package:sentry/src/noop_sentry_span.dart'; -import 'package:sentry/src/sentry_user_feedback.dart'; class MockHub implements Hub { List captureEventCalls = []; diff --git a/dart/test/mocks/mock_sentry_client.dart b/dart/test/mocks/mock_sentry_client.dart index f61e0f2360..8a9dbec36d 100644 --- a/dart/test/mocks/mock_sentry_client.dart +++ b/dart/test/mocks/mock_sentry_client.dart @@ -1,5 +1,4 @@ import 'package:sentry/sentry.dart'; -import 'package:sentry/src/sentry_envelope.dart'; class MockSentryClient implements SentryClient { List captureEventCalls = []; diff --git a/dart/test/scope_test.dart b/dart/test/scope_test.dart index a647ac0a1f..f01b4cf763 100644 --- a/dart/test/scope_test.dart +++ b/dart/test/scope_test.dart @@ -2,7 +2,6 @@ import 'dart:async'; import 'package:collection/collection.dart'; import 'package:sentry/sentry.dart'; -import 'package:sentry/src/noop_sentry_span.dart'; import 'package:sentry/src/sentry_tracer.dart'; import 'package:test/test.dart'; diff --git a/dart/test/sentry_envelope_item_test.dart b/dart/test/sentry_envelope_item_test.dart index 65b7f915f1..35c553c468 100644 --- a/dart/test/sentry_envelope_item_test.dart +++ b/dart/test/sentry_envelope_item_test.dart @@ -4,7 +4,6 @@ import 'package:sentry/sentry.dart'; import 'package:sentry/src/sentry_envelope_item_header.dart'; import 'package:sentry/src/sentry_envelope_item.dart'; import 'package:sentry/src/sentry_item_type.dart'; -import 'package:sentry/src/protocol/sentry_id.dart'; import 'package:sentry/src/sentry_tracer.dart'; import 'package:sentry/src/utils.dart'; import 'package:test/test.dart'; diff --git a/dart/test/sentry_envelope_test.dart b/dart/test/sentry_envelope_test.dart index 5a090e398f..bc28c508de 100644 --- a/dart/test/sentry_envelope_test.dart +++ b/dart/test/sentry_envelope_test.dart @@ -2,12 +2,10 @@ import 'dart:convert'; import 'dart:typed_data'; import 'package:sentry/sentry.dart'; -import 'package:sentry/src/sentry_envelope.dart'; import 'package:sentry/src/sentry_envelope_header.dart'; import 'package:sentry/src/sentry_envelope_item_header.dart'; import 'package:sentry/src/sentry_envelope_item.dart'; import 'package:sentry/src/sentry_item_type.dart'; -import 'package:sentry/src/protocol/sentry_id.dart'; import 'package:sentry/src/sentry_tracer.dart'; import 'package:sentry/src/utils.dart'; import 'package:test/test.dart'; diff --git a/dart/test/sentry_envelope_vm_test.dart b/dart/test/sentry_envelope_vm_test.dart index d3395a1d8a..b1bd880f91 100644 --- a/dart/test/sentry_envelope_vm_test.dart +++ b/dart/test/sentry_envelope_vm_test.dart @@ -1,13 +1,10 @@ @TestOn('vm') import 'dart:io'; -import 'package:sentry/sentry.dart'; import 'package:sentry/sentry_io.dart'; -import 'package:sentry/src/sentry_envelope.dart'; import 'package:sentry/src/sentry_envelope_header.dart'; import 'package:sentry/src/sentry_envelope_item_header.dart'; import 'package:sentry/src/sentry_envelope_item.dart'; -import 'package:sentry/src/protocol/sentry_id.dart'; import 'package:test/test.dart'; void main() { diff --git a/dart/test/sentry_event_test.dart b/dart/test/sentry_event_test.dart index 2d57771254..2417ca5296 100644 --- a/dart/test/sentry_event_test.dart +++ b/dart/test/sentry_event_test.dart @@ -4,8 +4,6 @@ import 'package:collection/collection.dart'; import 'package:sentry/sentry.dart'; -import 'package:sentry/src/protocol/sentry_request.dart'; -import 'package:sentry/src/protocol/sentry_thread.dart'; import 'package:sentry/src/version.dart'; import 'package:sentry/src/utils.dart'; import 'package:test/test.dart'; diff --git a/dart/test/sentry_span_test.dart b/dart/test/sentry_span_test.dart index cf0fe186a7..bb55054229 100644 --- a/dart/test/sentry_span_test.dart +++ b/dart/test/sentry_span_test.dart @@ -1,6 +1,5 @@ import 'package:sentry/sentry.dart'; import 'package:sentry/src/sentry_tracer.dart'; -import 'package:sentry/src/protocol/sentry_span.dart'; import 'package:test/test.dart'; import 'mocks/mock_hub.dart'; diff --git a/flutter/lib/src/widgets_binding_observer.dart b/flutter/lib/src/widgets_binding_observer.dart index 2cfc193d27..4af1789efe 100644 --- a/flutter/lib/src/widgets_binding_observer.dart +++ b/flutter/lib/src/widgets_binding_observer.dart @@ -1,7 +1,4 @@ import 'package:flutter/material.dart'; -import 'package:flutter/services.dart'; -import 'package:flutter/widgets.dart'; -import 'sentry_flutter_options.dart'; import '../sentry_flutter.dart'; /// This is a `WidgetsBindingObserver` which can observe some events of a diff --git a/flutter/test/default_integrations_test.dart b/flutter/test/default_integrations_test.dart index 2f2be6e499..24e74187f5 100644 --- a/flutter/test/default_integrations_test.dart +++ b/flutter/test/default_integrations_test.dart @@ -1,14 +1,10 @@ -import 'dart:async'; - import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; import 'package:package_info_plus/package_info_plus.dart'; -import 'package:sentry/sentry.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; -import 'package:sentry_flutter/src/sentry_flutter_options.dart'; import 'mocks.dart'; import 'mocks.mocks.dart'; diff --git a/flutter/test/initialization_test.dart b/flutter/test/initialization_test.dart index de10456975..11c2324b1b 100644 --- a/flutter/test/initialization_test.dart +++ b/flutter/test/initialization_test.dart @@ -1,7 +1,6 @@ @TestOn('vm') import 'package:flutter_test/flutter_test.dart'; -import 'package:sentry/sentry.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; import 'mocks.dart'; diff --git a/flutter/test/load_contexts_integrations_test.dart b/flutter/test/load_contexts_integrations_test.dart index c034086c39..baca5ff99b 100644 --- a/flutter/test/load_contexts_integrations_test.dart +++ b/flutter/test/load_contexts_integrations_test.dart @@ -1,7 +1,6 @@ import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; -import 'package:sentry_flutter/src/sentry_flutter_options.dart'; import 'mocks.mocks.dart'; diff --git a/flutter/test/mocks.dart b/flutter/test/mocks.dart index 8470563875..39833873d8 100644 --- a/flutter/test/mocks.dart +++ b/flutter/test/mocks.dart @@ -1,8 +1,6 @@ import 'package:mockito/annotations.dart'; import 'package:sentry/sentry.dart'; import 'package:sentry/src/platform/platform.dart'; -import 'package:sentry/src/platform_checker.dart'; -import 'package:sentry/src/sentry_user_feedback.dart'; import 'mocks.mocks.dart'; diff --git a/flutter/test/not_initialized_widgets_binding_test.dart b/flutter/test/not_initialized_widgets_binding_test.dart index 7ffebc982a..3919255506 100644 --- a/flutter/test/not_initialized_widgets_binding_test.dart +++ b/flutter/test/not_initialized_widgets_binding_test.dart @@ -1,6 +1,5 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; -import 'package:sentry_flutter/src/sentry_flutter_options.dart'; import 'mocks.mocks.dart'; diff --git a/flutter/test/sentry_flutter_test.dart b/flutter/test/sentry_flutter_test.dart index 012f439eda..4827451cf6 100644 --- a/flutter/test/sentry_flutter_test.dart +++ b/flutter/test/sentry_flutter_test.dart @@ -1,9 +1,7 @@ import 'package:flutter/foundation.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:package_info_plus/package_info_plus.dart'; -import 'package:sentry/sentry.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; -import 'package:sentry/src/platform_checker.dart'; import 'package:sentry_flutter/src/integrations/debug_print_integration.dart'; import 'package:sentry_flutter/src/version.dart'; import 'mocks.dart'; diff --git a/flutter/test/widgets_binding_observer_test.dart b/flutter/test/widgets_binding_observer_test.dart index 501980701c..d92d0133e8 100644 --- a/flutter/test/widgets_binding_observer_test.dart +++ b/flutter/test/widgets_binding_observer_test.dart @@ -3,7 +3,6 @@ import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; -import 'package:sentry_flutter/src/sentry_flutter_options.dart'; import 'package:sentry_flutter/src/widgets_binding_observer.dart'; import 'mocks.mocks.dart'; From ef633a06674f4a8bb28dd98d0407597ef488fddf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Tue, 14 Dec 2021 14:03:14 +0100 Subject: [PATCH 29/38] dont call super in noop method --- dart/lib/src/noop_sentry_span.dart | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dart/lib/src/noop_sentry_span.dart b/dart/lib/src/noop_sentry_span.dart index a56e8942f2..d1a98c4d87 100644 --- a/dart/lib/src/noop_sentry_span.dart +++ b/dart/lib/src/noop_sentry_span.dart @@ -26,9 +26,7 @@ class NoOpSentrySpan extends ISentrySpan { } @override - Future finish({SpanStatus? status}) async { - await super.finish(status: status); - } + Future finish({SpanStatus? status}) async {} @override void removeData(String key) {} From 7817a285a69007d8064eab2c6f3f40ee5d1f1adb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Tue, 14 Dec 2021 16:55:45 +0100 Subject: [PATCH 30/38] move auto finish after duration to start transactions as param --- dart/lib/src/hub.dart | 11 +++++-- dart/lib/src/hub_adapter.dart | 4 +++ dart/lib/src/noop_hub.dart | 2 ++ dart/lib/src/protocol/sentry_span.dart | 11 +------ dart/lib/src/sentry.dart | 4 +++ dart/lib/src/sentry_span_interface.dart | 3 -- dart/lib/src/sentry_tracer.dart | 18 +++++------- dart/test/default_integrations_test.dart | 1 + dart/test/mocks/mock_hub.dart | 2 ++ dart/test/sentry_span_test.dart | 10 ------- dart/test/sentry_tracer_test.dart | 13 ++++++--- .../navigation/sentry_navigator_observer.dart | 2 +- flutter/test/mocks.dart | 3 ++ flutter/test/mocks.mocks.dart | 21 ++++++++------ .../test/sentry_navigator_observer_test.dart | 29 ++++++++++--------- 15 files changed, 72 insertions(+), 62 deletions(-) diff --git a/dart/lib/src/hub.dart b/dart/lib/src/hub.dart index ec28b02658..b239e83bb7 100644 --- a/dart/lib/src/hub.dart +++ b/dart/lib/src/hub.dart @@ -340,6 +340,7 @@ class Hub { String? description, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, Map? customSamplingContext, }) => startTransactionWithContext( @@ -350,6 +351,7 @@ class Hub { ), bindToScope: bindToScope, waitForChildren: waitForChildren, + autoFinishAfter: autoFinishAfter, customSamplingContext: customSamplingContext, ); @@ -359,6 +361,7 @@ class Hub { Map? customSamplingContext, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, }) { if (!_isEnabled) { _options.logger( @@ -382,8 +385,12 @@ class Hub { transactionContext = transactionContext.copyWith(sampled: sampled); } - final tracer = SentryTracer(transactionContext, this, - waitForChildren: waitForChildren ?? false); + final tracer = SentryTracer( + transactionContext, + this, + waitForChildren: waitForChildren ?? false, + autoFinishAfter: autoFinishAfter, + ); if (bindToScope ?? false) { item.scope.span = tracer; } diff --git a/dart/lib/src/hub_adapter.dart b/dart/lib/src/hub_adapter.dart index c38246c56a..3f7803fc29 100644 --- a/dart/lib/src/hub_adapter.dart +++ b/dart/lib/src/hub_adapter.dart @@ -103,12 +103,14 @@ class HubAdapter implements Hub { Map? customSamplingContext, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, }) => Sentry.startTransactionWithContext( transactionContext, customSamplingContext: customSamplingContext, bindToScope: bindToScope, waitForChildren: waitForChildren, + autoFinishAfter: autoFinishAfter, ); @override @@ -118,6 +120,7 @@ class HubAdapter implements Hub { String? description, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, Map? customSamplingContext, }) => Sentry.startTransaction( @@ -126,6 +129,7 @@ class HubAdapter implements Hub { description: description, bindToScope: bindToScope, waitForChildren: waitForChildren, + autoFinishAfter: autoFinishAfter, customSamplingContext: customSamplingContext, ); diff --git a/dart/lib/src/noop_hub.dart b/dart/lib/src/noop_hub.dart index bbcee80b54..9963771496 100644 --- a/dart/lib/src/noop_hub.dart +++ b/dart/lib/src/noop_hub.dart @@ -79,6 +79,7 @@ class NoOpHub implements Hub { String? description, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, Map? customSamplingContext, }) => NoOpSentrySpan(); @@ -89,6 +90,7 @@ class NoOpHub implements Hub { Map? customSamplingContext, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, }) => NoOpSentrySpan(); diff --git a/dart/lib/src/protocol/sentry_span.dart b/dart/lib/src/protocol/sentry_span.dart index ef28b49b12..b8a620d5bd 100644 --- a/dart/lib/src/protocol/sentry_span.dart +++ b/dart/lib/src/protocol/sentry_span.dart @@ -19,7 +19,6 @@ class SentrySpan extends ISentrySpan { SpanStatus? _status; final Map _tags = {}; - Timer? _finishAfterTimer; void Function()? _finishedCallback; @override @@ -37,8 +36,7 @@ class SentrySpan extends ISentrySpan { } @override - Future finish({SpanStatus? status}) async { - _finishAfterTimer?.cancel(); + Future finish({SpanStatus? status, Duration? autoFinishAfter}) async { if (status != null) { _status = status; } @@ -52,13 +50,6 @@ class SentrySpan extends ISentrySpan { await super.finish(status: status); } - @override - void finishAfter(Duration duration, {SpanStatus? status}) { - _finishAfterTimer = Timer(duration, () async { - await finish(status: status); - }); - } - @override void removeData(String key) { _data.remove(key); diff --git a/dart/lib/src/sentry.dart b/dart/lib/src/sentry.dart index 90d5580229..3d1a74f02b 100644 --- a/dart/lib/src/sentry.dart +++ b/dart/lib/src/sentry.dart @@ -230,6 +230,7 @@ class Sentry { String? description, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, Map? customSamplingContext, }) => _hub.startTransaction( @@ -238,6 +239,7 @@ class Sentry { description: description, bindToScope: bindToScope, waitForChildren: waitForChildren, + autoFinishAfter: autoFinishAfter, customSamplingContext: customSamplingContext, ); @@ -247,12 +249,14 @@ class Sentry { Map? customSamplingContext, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, }) => _hub.startTransactionWithContext( transactionContext, customSamplingContext: customSamplingContext, bindToScope: bindToScope, waitForChildren: waitForChildren, + autoFinishAfter: autoFinishAfter, ); /// Gets the current active transaction or span. diff --git a/dart/lib/src/sentry_span_interface.dart b/dart/lib/src/sentry_span_interface.dart index 5133b805cd..d59de3c0ed 100644 --- a/dart/lib/src/sentry_span_interface.dart +++ b/dart/lib/src/sentry_span_interface.dart @@ -26,9 +26,6 @@ abstract class ISentrySpan { /// Sets span timestamp marking this span as finished. Future finish({SpanStatus? status}) async {} - /// Calls finish after the provided duration with the status parameter. - void finishAfter(Duration duration, {SpanStatus? status}) {} - /// Gets span status. SpanStatus? get status; diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index 6e8e4c0f32..72ab64288b 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -14,11 +14,11 @@ class SentryTracer extends ISentrySpan { late final SentrySpan _rootSpan; final List _children = []; final Map _extra = {}; - Timer? _finishAfterTimer; + Timer? _autoFinishAfterTimer; var _finishStatus = SentryTracerFinishStatus.notFinishing(); SentryTracer(SentryTransactionContext transactionContext, this._hub, - {bool waitForChildren = false}) { + {bool waitForChildren = false, Duration? autoFinishAfter}) { _rootSpan = SentrySpan( this, transactionContext, @@ -26,12 +26,17 @@ class SentryTracer extends ISentrySpan { sampled: transactionContext.sampled, ); _waitForChildren = waitForChildren; + if (autoFinishAfter != null) { + _autoFinishAfterTimer = Timer(autoFinishAfter, () async { + await finish(status: status ?? SpanStatus.ok()); + }); + } name = transactionContext.name; } @override Future finish({SpanStatus? status}) async { - _finishAfterTimer?.cancel(); + _autoFinishAfterTimer?.cancel(); _finishStatus = SentryTracerFinishStatus.finishing(status); if (!_rootSpan.finished && (!_waitForChildren || _haveAllChildrenFinished())) { @@ -57,13 +62,6 @@ class SentryTracer extends ISentrySpan { } } - @override - void finishAfter(Duration duration, {SpanStatus? status}) { - _finishAfterTimer = Timer(duration, () async { - await finish(status: status); - }); - } - @override void removeData(String key) { _extra.remove(key); diff --git a/dart/test/default_integrations_test.dart b/dart/test/default_integrations_test.dart index 9e83a37b8b..026025cb5e 100644 --- a/dart/test/default_integrations_test.dart +++ b/dart/test/default_integrations_test.dart @@ -189,6 +189,7 @@ class PrintRecursionMockHub extends MockHub { Map? customSamplingContext, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, }) { return NoOpSentrySpan(); } diff --git a/dart/test/mocks/mock_hub.dart b/dart/test/mocks/mock_hub.dart index 0601eeb652..5fddcff7f7 100644 --- a/dart/test/mocks/mock_hub.dart +++ b/dart/test/mocks/mock_hub.dart @@ -126,6 +126,7 @@ class MockHub implements Hub { String? description, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, Map? customSamplingContext, }) { return NoOpSentrySpan(); @@ -137,6 +138,7 @@ class MockHub implements Hub { Map? customSamplingContext, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, }) { return NoOpSentrySpan(); } diff --git a/dart/test/sentry_span_test.dart b/dart/test/sentry_span_test.dart index bb55054229..d778579243 100644 --- a/dart/test/sentry_span_test.dart +++ b/dart/test/sentry_span_test.dart @@ -113,16 +113,6 @@ void main() { '${sut.context.traceId}-${sut.context.spanId}-1'); }); - test('finishes after duration', () async { - final sut = fixture.getSut(); - sut.finishAfter(Duration(milliseconds: 200), status: SpanStatus.ok()); - - expect(sut.finished, false); - await Future.delayed(Duration(milliseconds: 210)); - expect(sut.status, SpanStatus.ok()); - expect(sut.finished, true); - }); - test('callback called on finish', () async { var numberOfCallbackCalls = 0; final sut = fixture.getSut(finishedCallback: () { diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index bc59edad63..fa921c3ba7 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -113,9 +113,8 @@ void main() { '${sut.context.traceId}-${sut.context.spanId}-1'); }); - test('tracer finishes after duration', () async { - final sut = fixture.getSut(); - sut.finishAfter(Duration(milliseconds: 200), status: SpanStatus.ok()); + test('tracer finishes after auto finish duration', () async { + final sut = fixture.getSut(autoFinishAfter: Duration(milliseconds: 200)); expect(sut.finished, false); await Future.delayed(Duration(milliseconds: 210)); @@ -175,12 +174,18 @@ class Fixture { SentryTracer getSut({ bool? sampled = true, bool waitForChildren = false, + Duration? autoFinishAfter, }) { final context = SentryTransactionContext( 'name', 'op', sampled: sampled, ); - return SentryTracer(context, hub, waitForChildren: waitForChildren); + return SentryTracer( + context, + hub, + waitForChildren: waitForChildren, + autoFinishAfter: autoFinishAfter, + ); } } diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index c6fda6e234..e301cbf300 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -137,11 +137,11 @@ class SentryNavigatorObserver extends RouteObserver> { 'ui.load', bindToScope: scope.span == null, waitForChildren: true, + autoFinishAfter: Duration(seconds: 3), ); if (arguments != null) { _transaction?.setData('route_settings_arguments', arguments); } - _transaction?.finishAfter(Duration(seconds: 3), status: SpanStatus.ok()); }); } diff --git a/flutter/test/mocks.dart b/flutter/test/mocks.dart index 39833873d8..98aff4d1da 100644 --- a/flutter/test/mocks.dart +++ b/flutter/test/mocks.dart @@ -13,6 +13,7 @@ ISentrySpan startTransactionShim( String? description, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, Map? customSamplingContext, }) { return MockNoOpSentrySpan(); @@ -187,6 +188,7 @@ class NoOpHub implements Hub { String? description, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, Map? customSamplingContext, }) { return NoOpSentrySpan(); @@ -198,6 +200,7 @@ class NoOpHub implements Hub { Map? customSamplingContext, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, }) { return NoOpSentrySpan(); } diff --git a/flutter/test/mocks.mocks.dart b/flutter/test/mocks.mocks.dart index 4383a0c25e..53e0f52cba 100644 --- a/flutter/test/mocks.mocks.dart +++ b/flutter/test/mocks.mocks.dart @@ -84,8 +84,11 @@ class MockNoOpSentrySpan extends _i1.Mock implements _i2.NoOpSentrySpan { super.noSuchMethod(Invocation.setter(#status, status), returnValueForMissingStub: null); @override - _i6.Future finish({_i3.SpanStatus? status}) => - (super.noSuchMethod(Invocation.method(#finish, [], {#status: status}), + _i6.Future finish( + {_i3.SpanStatus? status, Duration? autoFinishAfter}) => + (super.noSuchMethod( + Invocation.method(#finish, [], + {#status: status, #autoFinishAfter: autoFinishAfter}), returnValue: Future.value(), returnValueForMissingStub: Future.value()) as _i6.Future); @override @@ -115,11 +118,6 @@ class MockNoOpSentrySpan extends _i1.Mock implements _i2.NoOpSentrySpan { (super.noSuchMethod(Invocation.method(#toSentryTrace, []), returnValue: _FakeSentryTraceHeader_3()) as _i3.SentryTraceHeader); @override - void finishAfter(Duration? duration, {_i3.SpanStatus? status}) => - super.noSuchMethod( - Invocation.method(#finishAfter, [duration], {#status: status}), - returnValueForMissingStub: null); - @override String toString() => super.toString(); } @@ -214,6 +212,7 @@ class MockHub extends _i1.Mock implements _i4.Hub { {String? description, bool? bindToScope, bool? waitForChildren, + Duration? autoFinishAfter, Map? customSamplingContext}) => (super.noSuchMethod( Invocation.method(#startTransaction, [ @@ -223,12 +222,14 @@ class MockHub extends _i1.Mock implements _i4.Hub { #description: description, #bindToScope: bindToScope, #waitForChildren: waitForChildren, + #autoFinishAfter: autoFinishAfter, #customSamplingContext: customSamplingContext }), returnValue: _i10.startTransactionShim(name, operation, description: description, bindToScope: bindToScope, waitForChildren: waitForChildren, + autoFinishAfter: autoFinishAfter, customSamplingContext: customSamplingContext)) as _i2.ISentrySpan); @override @@ -236,14 +237,16 @@ class MockHub extends _i1.Mock implements _i4.Hub { _i2.SentryTransactionContext? transactionContext, {Map? customSamplingContext, bool? bindToScope, - bool? waitForChildren}) => + bool? waitForChildren, + Duration? autoFinishAfter}) => (super.noSuchMethod( Invocation.method(#startTransactionWithContext, [ transactionContext ], { #customSamplingContext: customSamplingContext, #bindToScope: bindToScope, - #waitForChildren: waitForChildren + #waitForChildren: waitForChildren, + #autoFinishAfter: autoFinishAfter }), returnValue: _FakeISentrySpan_2()) as _i2.ISentrySpan); @override diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 46c97297d6..6436ca4c32 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -23,6 +23,7 @@ void main() { description: anyNamed('description'), bindToScope: anyNamed('bindToScope'), waitForChildren: anyNamed('waitForChildren'), + autoFinishAfter: anyNamed('autoFinishAfter'), customSamplingContext: anyNamed('customSamplingContext'), )).thenReturn(thenReturnSpan); } @@ -43,8 +44,9 @@ void main() { sut.didPush(currentRoute, null); verify(hub.startTransaction('Current Route', 'ui.load', - bindToScope: true, waitForChildren: true)); - verify(span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); + bindToScope: true, + waitForChildren: true, + autoFinishAfter: Duration(seconds: 3))); }); test('route with empty name does not start transaction', () { @@ -58,9 +60,9 @@ void main() { sut.didPush(currentRoute, null); verifyNever(hub.startTransaction('Current Route', 'ui.load', - bindToScope: true, waitForChildren: true)); - verifyNever( - span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); + bindToScope: true, + waitForChildren: true, + autoFinishAfter: Duration(seconds: 3))); }); test('no transaction on opt-out', () { @@ -74,9 +76,9 @@ void main() { sut.didPush(currentRoute, null); verifyNever(hub.startTransaction('Current Route', 'ui.load', - bindToScope: true, waitForChildren: true)); - verifyNever( - span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); + bindToScope: true, + waitForChildren: true, + autoFinishAfter: Duration(seconds: 3))); }); test('do not bind to scope if already set', () { @@ -91,8 +93,9 @@ void main() { sut.didPush(currentRoute, null); verify(hub.startTransaction('Current Route', 'ui.load', - bindToScope: false, waitForChildren: true)); - verify(span.finishAfter(Duration(seconds: 3), status: SpanStatus.ok())); + bindToScope: false, + waitForChildren: true, + autoFinishAfter: Duration(seconds: 3))); }); test('didPush finishes previous transaction', () { @@ -143,9 +146,9 @@ void main() { sut.didPop(currentRoute, previousRoute); verify(hub.startTransaction('Previous Route', 'ui.load', - bindToScope: true, waitForChildren: true)); - verify(previousSpan.finishAfter(Duration(seconds: 3), - status: SpanStatus.ok())); + bindToScope: true, + waitForChildren: true, + autoFinishAfter: Duration(seconds: 3))); }); test('didPush push multiple finishes previous', () async { From 835fbd3c9c02c16da04f3dbbf33264ba9fb0dc9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Wed, 15 Dec 2021 13:29:20 +0100 Subject: [PATCH 31/38] Remove unused param --- dart/lib/src/protocol/sentry_span.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dart/lib/src/protocol/sentry_span.dart b/dart/lib/src/protocol/sentry_span.dart index b8a620d5bd..047797e44f 100644 --- a/dart/lib/src/protocol/sentry_span.dart +++ b/dart/lib/src/protocol/sentry_span.dart @@ -36,7 +36,7 @@ class SentrySpan extends ISentrySpan { } @override - Future finish({SpanStatus? status, Duration? autoFinishAfter}) async { + Future finish({SpanStatus? status}) async { if (status != null) { _status = status; } From 8bb14a45678916d46e9d4bb25142f74545d5f8de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Wed, 15 Dec 2021 13:30:30 +0100 Subject: [PATCH 32/38] return immed. --- dart/lib/src/sentry_tracer.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index 72ab64288b..86ccfc1e0d 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -87,11 +87,10 @@ class SentryTracer extends ISentrySpan { String operation, { String? description, }) { - final child = _rootSpan.startChild( + return _rootSpan.startChild( operation, description: description, ); - return child; } ISentrySpan startChildWithParentSpanId( From 7cdf8540e25e295e6b459182e521f3582278efd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Wed, 15 Dec 2021 15:27:48 +0100 Subject: [PATCH 33/38] no need to double check finished --- dart/lib/src/sentry_tracer.dart | 3 --- 1 file changed, 3 deletions(-) diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index c05aa29b41..88988875e6 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -36,9 +36,6 @@ class SentryTracer extends ISentrySpan { @override Future finish({SpanStatus? status}) async { - if (finished) { - return; - } _autoFinishAfterTimer?.cancel(); _finishStatus = SentryTracerFinishStatus.finishing(status); if (!_rootSpan.finished && From 2251af21d8d44a6405e81f5714d2df9b09c627b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Wed, 15 Dec 2021 15:43:55 +0100 Subject: [PATCH 34/38] use more descriptive changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 635b243c8a..d09937f81c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Unreleased -* Feat: Auto Transactions (#643) +* Feat: Automatically create transactions when navigating between screens (#643) # 6.2.2 * Fix: ConcurrentModificationError in when finishing span (#664) From 2f268a3e45d9b83a7d8f9e79bee07d543fbacf07 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 15 Dec 2021 18:10:53 +0100 Subject: [PATCH 35/38] use current span instead of new transaction for sample --- .../android/app/src/main/AndroidManifest.xml | 3 ++- flutter/example/lib/main.dart | 11 +++++----- .../navigation/sentry_navigator_observer.dart | 21 ++++++++++--------- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/flutter/example/android/app/src/main/AndroidManifest.xml b/flutter/example/android/app/src/main/AndroidManifest.xml index da3c097241..c2029920e9 100644 --- a/flutter/example/android/app/src/main/AndroidManifest.xml +++ b/flutter/example/android/app/src/main/AndroidManifest.xml @@ -12,7 +12,8 @@ android:theme="@style/LaunchTheme" android:configChanges="orientation|keyboardHidden|keyboard|screenSize|smallestScreenSize|locale|layoutDirection|fontScale|screenLayout|density|uiMode" android:hardwareAccelerated="true" - android:windowSoftInputMode="adjustResize"> + android:windowSoftInputMode="adjustResize" + android:exported="true">