Skip to content

Commit

Permalink
feat: prepare profiler interfaces and hub integration
Browse files Browse the repository at this point in the history
  • Loading branch information
vaind committed Aug 22, 2023
1 parent f9fd5d8 commit 9ad58ee
Show file tree
Hide file tree
Showing 12 changed files with 305 additions and 7 deletions.
23 changes: 18 additions & 5 deletions dart/lib/src/hub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}
Expand All @@ -449,6 +450,12 @@ class Hub {
);
}

Profiler? profiler;
if (_profilerFactory != null &&
_tracesSampler.sampleProfiling(samplingDecision)) {
profiler = _profilerFactory?.startProfiling(transactionContext);
}

final tracer = SentryTracer(
transactionContext,
this,
Expand All @@ -457,6 +464,7 @@ class Hub {
autoFinishAfter: autoFinishAfter,
trimEnd: trimEnd ?? false,
onFinish: onFinish,
profiler: profiler,
);
if (bindToScope ?? false) {
item.scope.span = tracer;
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions dart/lib/src/hub_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -166,4 +167,9 @@ class HubAdapter implements Hub {
String transaction,
) =>
Sentry.currentHub.setSpanContext(throwable, span, transaction);

@internal

Check warning on line 171 in dart/lib/src/hub_adapter.dart

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/hub_adapter.dart#L171

Added line #L171 was not covered by tests
@override
set profilerFactory(ProfilerFactory? value) =>
Sentry.currentHub.profilerFactory = value;

Check warning on line 174 in dart/lib/src/hub_adapter.dart

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/hub_adapter.dart#L174

Added line #L174 was not covered by tests
}
5 changes: 5 additions & 0 deletions dart/lib/src/noop_hub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -118,4 +119,8 @@ class NoOpHub implements Hub {

@override
void setSpanContext(throwable, ISentrySpan span, String transaction) {}

@internal

Check warning on line 123 in dart/lib/src/noop_hub.dart

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/noop_hub.dart#L123

Added line #L123 was not covered by tests
@override
set profilerFactory(ProfilerFactory? value) {}
}
22 changes: 22 additions & 0 deletions dart/lib/src/profiling.dart
Original file line number Diff line number Diff line change
@@ -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<ProfileInfo> finishFor(SentryTransaction transaction);
void dispose();
}

// See https://develop.sentry.dev/sdk/profiles/
@internal
abstract class ProfileInfo {
FutureOr<SentryEnvelopeItem> asEnvelopeItem();
}
4 changes: 4 additions & 0 deletions dart/lib/src/protocol/sentry_transaction.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'package:meta/meta.dart';

import '../profiling.dart';
import '../protocol.dart';
import '../sentry_tracer.dart';
import '../utils.dart';
Expand All @@ -14,6 +15,9 @@ class SentryTransaction extends SentryEvent {
late final Map<String, SentryMeasurement> measurements;
late final SentryTransactionInfo? transactionInfo;

@internal
late final ProfileInfo? profileInfo;

SentryTransaction(
this._tracer, {
SentryId? eventId,
Expand Down
17 changes: 17 additions & 0 deletions dart/lib/src/sentry_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
22 changes: 20 additions & 2 deletions dart/lib/src/sentry_tracer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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.
///
Expand All @@ -52,6 +55,7 @@ class SentryTracer extends ISentrySpan {
Duration? autoFinishAfter,
bool trimEnd = false,
OnTransactionFinish? onFinish,
this.profiler,
}) {
_rootSpan = SentrySpan(
this,
Expand All @@ -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
Expand Down Expand Up @@ -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();
}
}

Expand Down
8 changes: 8 additions & 0 deletions dart/lib/src/sentry_traces_sampler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
1 change: 1 addition & 0 deletions dart/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
60 changes: 60 additions & 0 deletions dart/test/hub_test.dart
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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<SentryTracer>());
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', () {
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 9 additions & 0 deletions dart/test/mocks.dart
Original file line number Diff line number Diff line change
@@ -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://[email protected]/1234567';
Expand Down Expand Up @@ -149,3 +151,10 @@ class MockRateLimiter implements RateLimiter {
this.errorCode = errorCode;
}
}

@GenerateMocks([
ProfilerFactory,
Profiler,
ProfileInfo,
])
void main() {}
Loading

0 comments on commit 9ad58ee

Please sign in to comment.