diff --git a/dart/lib/src/hub.dart b/dart/lib/src/hub.dart index e1ffe802dd..080956591b 100644 --- a/dart/lib/src/hub.dart +++ b/dart/lib/src/hub.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'dart:collection'; import 'package:meta/meta.dart'; +import 'profiling.dart'; import 'transport/data_category.dart'; import '../sentry.dart'; @@ -433,12 +434,12 @@ class Hub { } else { final item = _peek(); - final samplingContext = SentrySamplingContext( - transactionContext, customSamplingContext ?? {}); - // if transactionContext has no sampled decision, run the traces sampler - if (transactionContext.samplingDecision == null) { - final samplingDecision = _tracesSampler.sample(samplingContext); + var samplingDecision = transactionContext.samplingDecision; + if (samplingDecision == null) { + final samplingContext = SentrySamplingContext( + transactionContext, customSamplingContext ?? {}); + samplingDecision = _tracesSampler.sample(samplingContext); transactionContext = transactionContext.copyWith(samplingDecision: samplingDecision); } @@ -449,6 +450,12 @@ class Hub { ); } + Profiler? profiler; + if (_profilerFactory != null && + _tracesSampler.sampleProfiling(samplingDecision)) { + profiler = _profilerFactory?.startProfiling(transactionContext); + } + final tracer = SentryTracer( transactionContext, this, @@ -457,6 +464,7 @@ class Hub { autoFinishAfter: autoFinishAfter, trimEnd: trimEnd ?? false, onFinish: onFinish, + profiler: profiler, ); if (bindToScope ?? false) { item.scope.span = tracer; @@ -552,6 +560,11 @@ class Hub { ) => _throwableToSpan.add(throwable, span, transaction); + @internal + set profilerFactory(ProfilerFactory? value) => _profilerFactory = value; + + ProfilerFactory? _profilerFactory; + SentryEvent _assignTraceContext(SentryEvent event) { // assign trace context if (event.throwable != null && event.contexts.trace == null) { diff --git a/dart/lib/src/hub_adapter.dart b/dart/lib/src/hub_adapter.dart index 8c1a176df8..aafd186d17 100644 --- a/dart/lib/src/hub_adapter.dart +++ b/dart/lib/src/hub_adapter.dart @@ -4,6 +4,7 @@ import 'package:meta/meta.dart'; import 'hint.dart'; import 'hub.dart'; +import 'profiling.dart'; import 'protocol.dart'; import 'sentry.dart'; import 'sentry_client.dart'; @@ -166,4 +167,9 @@ class HubAdapter implements Hub { String transaction, ) => Sentry.currentHub.setSpanContext(throwable, span, transaction); + + @internal + @override + set profilerFactory(ProfilerFactory? value) => + Sentry.currentHub.profilerFactory = value; } diff --git a/dart/lib/src/noop_hub.dart b/dart/lib/src/noop_hub.dart index ea005184b6..38914b9f5e 100644 --- a/dart/lib/src/noop_hub.dart +++ b/dart/lib/src/noop_hub.dart @@ -4,6 +4,7 @@ import 'package:meta/meta.dart'; import 'hint.dart'; import 'hub.dart'; +import 'profiling.dart'; import 'protocol.dart'; import 'sentry_client.dart'; import 'sentry_options.dart'; @@ -118,4 +119,8 @@ class NoOpHub implements Hub { @override void setSpanContext(throwable, ISentrySpan span, String transaction) {} + + @internal + @override + set profilerFactory(ProfilerFactory? value) {} } diff --git a/dart/lib/src/profiling.dart b/dart/lib/src/profiling.dart new file mode 100644 index 0000000000..e6dd4eb144 --- /dev/null +++ b/dart/lib/src/profiling.dart @@ -0,0 +1,22 @@ +import 'dart:async'; + +import 'package:meta/meta.dart'; + +import '../sentry.dart'; + +@internal +abstract class ProfilerFactory { + Profiler startProfiling(SentryTransactionContext context); +} + +@internal +abstract class Profiler { + Future finishFor(SentryTransaction transaction); + void dispose(); +} + +// See https://develop.sentry.dev/sdk/profiles/ +@internal +abstract class ProfileInfo { + FutureOr asEnvelopeItem(); +} diff --git a/dart/lib/src/protocol/sentry_transaction.dart b/dart/lib/src/protocol/sentry_transaction.dart index 44e6c4298f..606c15e8d9 100644 --- a/dart/lib/src/protocol/sentry_transaction.dart +++ b/dart/lib/src/protocol/sentry_transaction.dart @@ -1,5 +1,6 @@ import 'package:meta/meta.dart'; +import '../profiling.dart'; import '../protocol.dart'; import '../sentry_tracer.dart'; import '../utils.dart'; @@ -14,6 +15,9 @@ class SentryTransaction extends SentryEvent { late final Map measurements; late final SentryTransactionInfo? transactionInfo; + @internal + late final ProfileInfo? profileInfo; + SentryTransaction( this._tracer, { SentryId? eventId, diff --git a/dart/lib/src/sentry_options.dart b/dart/lib/src/sentry_options.dart index 55450be9dd..6003da8841 100644 --- a/dart/lib/src/sentry_options.dart +++ b/dart/lib/src/sentry_options.dart @@ -289,6 +289,23 @@ class SentryOptions { /// to be sent to Sentry. TracesSamplerCallback? tracesSampler; + double? _profilesSampleRate; + + /// The sample rate for profiling traces in the range [0.0, 1.0]. + /// This is relative to tracesSampleRate - it is a ratio of profiled traces out of all sampled traces. + /// At the moment, only apps targeting iOS and macOS are supported. + @experimental + double? get profilesSampleRate => _profilesSampleRate; + + /// The sample rate for profiling traces in the range [0.0, 1.0]. + /// This is relative to tracesSampleRate - it is a ratio of profiled traces out of all sampled traces. + /// At the moment, only apps targeting iOS and macOS are supported. + @experimental + set profilesSampleRate(double? value) { + assert(value == null || (value >= 0 && value <= 1)); + _profilesSampleRate = value; + } + /// Send statistics to sentry when the client drops events. bool sendClientReports = true; diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index 2ca82a91ca..d2a7fe7458 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 'profiling.dart'; import 'sentry_tracer_finish_status.dart'; import 'utils/sample_rate_format.dart'; @@ -31,6 +32,8 @@ class SentryTracer extends ISentrySpan { SentryTraceContextHeader? _sentryTraceContextHeader; + late final Profiler? profiler; + /// If [waitForChildren] is true, this transaction will not finish until all /// its children are finished. /// @@ -52,6 +55,7 @@ class SentryTracer extends ISentrySpan { Duration? autoFinishAfter, bool trimEnd = false, OnTransactionFinish? onFinish, + this.profiler, }) { _rootSpan = SentrySpan( this, @@ -77,8 +81,13 @@ class SentryTracer extends ISentrySpan { final commonEndTimestamp = endTimestamp ?? _hub.options.clock(); _autoFinishAfterTimer?.cancel(); _finishStatus = SentryTracerFinishStatus.finishing(status); - if (!_rootSpan.finished && - (!_waitForChildren || _haveAllChildrenFinished())) { + if (_rootSpan.finished) { + return; + } + if (_waitForChildren && !_haveAllChildrenFinished()) { + return; + } + try { _rootSpan.status ??= status; // remove span where its endTimestamp is before startTimestamp @@ -131,10 +140,19 @@ class SentryTracer extends ISentrySpan { final transaction = SentryTransaction(this); transaction.measurements.addAll(_measurements); + + if (profiler != null) { + if (status == null || status == SpanStatus.ok()) { + transaction.profileInfo = await profiler?.finishFor(transaction); + } + } + await _hub.captureTransaction( transaction, traceContext: traceContext(), ); + } finally { + profiler?.dispose(); } } diff --git a/dart/lib/src/sentry_traces_sampler.dart b/dart/lib/src/sentry_traces_sampler.dart index 62d94dc339..876668b8a6 100644 --- a/dart/lib/src/sentry_traces_sampler.dart +++ b/dart/lib/src/sentry_traces_sampler.dart @@ -67,5 +67,13 @@ class SentryTracesSampler { return SentryTracesSamplingDecision(false); } + bool sampleProfiling(SentryTracesSamplingDecision tracesSamplingDecision) { + double? optionsRate = _options.profilesSampleRate; + if (optionsRate == null || !tracesSamplingDecision.sampled) { + return false; + } + return _sample(optionsRate); + } + bool _sample(double result) => !(result < _random.nextDouble()); } diff --git a/dart/pubspec.yaml b/dart/pubspec.yaml index 330d6084e7..1efa18432b 100644 --- a/dart/pubspec.yaml +++ b/dart/pubspec.yaml @@ -18,6 +18,7 @@ dependencies: uuid: ^3.0.0 dev_dependencies: + build_runner: ^2.4.2 mockito: ^5.1.0 lints: ^2.0.0 test: ^1.21.1 diff --git a/dart/test/hub_test.dart b/dart/test/hub_test.dart index 32e0f1021c..e1d839d922 100644 --- a/dart/test/hub_test.dart +++ b/dart/test/hub_test.dart @@ -1,11 +1,14 @@ import 'package:collection/collection.dart'; +import 'package:mockito/mockito.dart'; import 'package:sentry/sentry.dart'; import 'package:sentry/src/client_reports/discard_reason.dart'; +import 'package:sentry/src/profiling.dart'; import 'package:sentry/src/sentry_tracer.dart'; import 'package:sentry/src/transport/data_category.dart'; import 'package:test/test.dart'; import 'mocks.dart'; +import 'mocks.mocks.dart'; import 'mocks/mock_client_report_recorder.dart'; import 'mocks/mock_sentry_client.dart'; @@ -374,6 +377,61 @@ void main() { expect( fixture.client.captureTransactionCalls.first.traceContext, context); }); + + test('profiler is not started by default', () async { + final hub = fixture.getSut(); + final tr = hub.startTransaction('name', 'op'); + expect(tr, isA()); + expect((tr as SentryTracer).profiler, isNull); + }); + + test('profiler is started according to the sampling rate', () async { + final hub = fixture.getSut(); + final factory = MockProfilerFactory(); + when(factory.startProfiling(fixture._context)).thenReturn(MockProfiler()); + hub.profilerFactory = factory; + + var tr = hub.startTransactionWithContext(fixture._context); + expect((tr as SentryTracer).profiler, isNull); + verifyZeroInteractions(factory); + + hub.options.profilesSampleRate = 1.0; + tr = hub.startTransactionWithContext(fixture._context); + expect((tr as SentryTracer).profiler, isNotNull); + verify(factory.startProfiling(fixture._context)).called(1); + }); + + test('profiler.finish() is called', () async { + final hub = fixture.getSut(); + final factory = MockProfilerFactory(); + final profiler = MockProfiler(); + final expected = MockProfileInfo(); + when(factory.startProfiling(fixture._context)).thenReturn(profiler); + when(profiler.finishFor(any)).thenAnswer((_) async => expected); + + hub.profilerFactory = factory; + hub.options.profilesSampleRate = 1.0; + final tr = hub.startTransactionWithContext(fixture._context); + await tr.finish(); + verify(profiler.finishFor(any)).called(1); + verify(profiler.dispose()).called(1); + }); + + test('profiler.dispose() is called even if not captured', () async { + final hub = fixture.getSut(); + final factory = MockProfilerFactory(); + final profiler = MockProfiler(); + final expected = MockProfileInfo(); + when(factory.startProfiling(fixture._context)).thenReturn(profiler); + when(profiler.finishFor(any)).thenAnswer((_) async => expected); + + hub.profilerFactory = factory; + hub.options.profilesSampleRate = 1.0; + final tr = hub.startTransactionWithContext(fixture._context); + await tr.finish(status: SpanStatus.aborted()); + verify(profiler.dispose()).called(1); + verifyNever(profiler.finishFor(any)); + }); }); group('Hub scope', () { @@ -641,10 +699,12 @@ class Fixture { final hub = Hub(options); + // A fully configured context - won't trigger a copy in startTransaction(). _context = SentryTransactionContext( 'name', 'op', samplingDecision: SentryTracesSamplingDecision(sampled!), + origin: SentryTraceOrigins.manual, ); tracer = SentryTracer(_context, hub); diff --git a/dart/test/mocks.dart b/dart/test/mocks.dart index 82430d96e8..edc3edb211 100644 --- a/dart/test/mocks.dart +++ b/dart/test/mocks.dart @@ -1,4 +1,6 @@ +import 'package:mockito/annotations.dart'; import 'package:sentry/sentry.dart'; +import 'package:sentry/src/profiling.dart'; import 'package:sentry/src/transport/rate_limiter.dart'; final fakeDsn = 'https://abc@def.ingest.sentry.io/1234567'; @@ -149,3 +151,10 @@ class MockRateLimiter implements RateLimiter { this.errorCode = errorCode; } } + +@GenerateMocks([ + ProfilerFactory, + Profiler, + ProfileInfo, +]) +void main() {} diff --git a/dart/test/mocks.mocks.dart b/dart/test/mocks.mocks.dart new file mode 100644 index 0000000000..8f8f67a5ae --- /dev/null +++ b/dart/test/mocks.mocks.dart @@ -0,0 +1,135 @@ +// Mocks generated by Mockito 5.4.2 from annotations +// in sentry/test/mocks.dart. +// Do not manually edit this file. + +// ignore_for_file: no_leading_underscores_for_library_prefixes +import 'dart:async' as _i4; + +import 'package:mockito/mockito.dart' as _i1; +import 'package:sentry/sentry.dart' as _i3; +import 'package:sentry/src/profiling.dart' as _i2; + +// ignore_for_file: type=lint +// ignore_for_file: avoid_redundant_argument_values +// ignore_for_file: avoid_setters_without_getters +// ignore_for_file: comment_references +// 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: subtype_of_sealed_class + +class _FakeProfiler_0 extends _i1.SmartFake implements _i2.Profiler { + _FakeProfiler_0( + Object parent, + Invocation parentInvocation, + ) : super( + parent, + parentInvocation, + ); +} + +class _FakeProfileInfo_1 extends _i1.SmartFake implements _i2.ProfileInfo { + _FakeProfileInfo_1( + Object parent, + Invocation parentInvocation, + ) : super( + parent, + parentInvocation, + ); +} + +class _FakeSentryEnvelopeItem_2 extends _i1.SmartFake + implements _i3.SentryEnvelopeItem { + _FakeSentryEnvelopeItem_2( + Object parent, + Invocation parentInvocation, + ) : super( + parent, + parentInvocation, + ); +} + +/// A class which mocks [ProfilerFactory]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockProfilerFactory extends _i1.Mock implements _i2.ProfilerFactory { + MockProfilerFactory() { + _i1.throwOnMissingStub(this); + } + + @override + _i2.Profiler startProfiling(_i3.SentryTransactionContext? context) => + (super.noSuchMethod( + Invocation.method( + #startProfiling, + [context], + ), + returnValue: _FakeProfiler_0( + this, + Invocation.method( + #startProfiling, + [context], + ), + ), + ) as _i2.Profiler); +} + +/// A class which mocks [Profiler]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockProfiler extends _i1.Mock implements _i2.Profiler { + MockProfiler() { + _i1.throwOnMissingStub(this); + } + + @override + _i4.Future<_i2.ProfileInfo> finishFor(_i3.SentryTransaction? transaction) => + (super.noSuchMethod( + Invocation.method( + #finishFor, + [transaction], + ), + returnValue: _i4.Future<_i2.ProfileInfo>.value(_FakeProfileInfo_1( + this, + Invocation.method( + #finishFor, + [transaction], + ), + )), + ) as _i4.Future<_i2.ProfileInfo>); + @override + void dispose() => super.noSuchMethod( + Invocation.method( + #dispose, + [], + ), + returnValueForMissingStub: null, + ); +} + +/// A class which mocks [ProfileInfo]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockProfileInfo extends _i1.Mock implements _i2.ProfileInfo { + MockProfileInfo() { + _i1.throwOnMissingStub(this); + } + + @override + _i4.FutureOr<_i3.SentryEnvelopeItem> asEnvelopeItem() => (super.noSuchMethod( + Invocation.method( + #asEnvelopeItem, + [], + ), + returnValue: + _i4.Future<_i3.SentryEnvelopeItem>.value(_FakeSentryEnvelopeItem_2( + this, + Invocation.method( + #asEnvelopeItem, + [], + ), + )), + ) as _i4.FutureOr<_i3.SentryEnvelopeItem>); +}