Skip to content

Commit

Permalink
feat: profiling for iOS/macOS (#1611)
Browse files Browse the repository at this point in the history
Co-authored-by: Bruno Garcia <[email protected]>
Co-authored-by: Denis Andrašec <[email protected]>
Co-authored-by: Stefano <[email protected]>
  • Loading branch information
4 people committed Oct 27, 2023
1 parent 677b331 commit dc54bfc
Show file tree
Hide file tree
Showing 45 changed files with 1,454 additions and 563 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/analyze.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ jobs:
timeout-minutes: 20
steps:
- uses: actions/checkout@v3
- name: Apply dependency override
if: ${{ inputs.package == 'flutter' }}
working-directory: ${{ inputs.package }}
run: |
sed -i.bak 's|sentry:.*|sentry:\n path: /github/workspace/dart|g' pubspec.yaml
- uses: axel-op/dart-package-analyzer@7a6c3c66bce78d82b729a1ffef2d9458fde6c8d2 # pin@v3
id: analysis
with:
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/flutter.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ jobs:
with:
path: "./flutter/coverage/lcov.info"
min_coverage: 90
exclude: "lib/src/native/cocoa/binding.dart"

- name: Build ${{ matrix.target }}
run: |
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/flutter_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ jobs:
avd-name: macOS-avd-x86_64-31
emulator-options: -no-snapshot-save -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none
disable-animations: true
script: flutter test integration_test --verbose
script: flutter test integration_test/all.dart --verbose

cocoa:
name: "${{ matrix.target }} | ${{ matrix.sdk }}"
Expand Down Expand Up @@ -155,7 +155,7 @@ jobs:
- name: run integration test
# Disable flutter integration tests for iOS for now (https://github.com/getsentry/sentry-dart/issues/1605#issuecomment-1695809346)
if: ${{ matrix.target != 'ios' }}
run: flutter test -d "${{ steps.device.outputs.name }}" integration_test --verbose
run: flutter test -d "${{ steps.device.outputs.name }}" integration_test/all.dart --verbose

- name: run native test
# We only have the native unit test package in the iOS xcodeproj at the moment.
Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Features

- Initial (alpha) support for profiling on iOS and macOS ([#1611](https://github.com/getsentry/sentry-dart/pull/1611))

## 7.11.0

### Fixes
Expand Down
1 change: 1 addition & 0 deletions dart/analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ include: package:lints/recommended.yaml
analyzer:
exclude:
- example/** # the example has its own 'analysis_options.yaml'
- test/*.mocks.dart
errors:
# treat missing required parameters as a warning (not a hint)
missing_required_param: error
Expand Down
26 changes: 21 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 'propagation_context.dart';
import 'transport/data_category.dart';

Expand Down Expand Up @@ -435,12 +436,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 @@ -451,6 +452,12 @@ class Hub {
);
}

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

final tracer = SentryTracer(
transactionContext,
this,
Expand All @@ -459,6 +466,7 @@ class Hub {
autoFinishAfter: autoFinishAfter,
trimEnd: trimEnd ?? false,
onFinish: onFinish,
profiler: profiler,
);
if (bindToScope ?? false) {
item.scope.span = tracer;
Expand Down Expand Up @@ -554,6 +562,14 @@ class Hub {
) =>
_throwableToSpan.add(throwable, span, transaction);

@internal
SentryProfilerFactory? get profilerFactory => _profilerFactory;

@internal
set profilerFactory(SentryProfilerFactory? value) => _profilerFactory = value;

SentryProfilerFactory? _profilerFactory;

SentryEvent _assignTraceContext(SentryEvent event) {
// assign trace context
if (event.throwable != null && event.contexts.trace == null) {
Expand Down
11 changes: 11 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 'scope.dart';
import 'sentry.dart';
Expand Down Expand Up @@ -168,6 +169,16 @@ class HubAdapter implements Hub {
) =>
Sentry.currentHub.setSpanContext(throwable, span, transaction);

@internal
@override
set profilerFactory(SentryProfilerFactory? value) =>
Sentry.currentHub.profilerFactory = value;

@internal
@override
SentryProfilerFactory? get profilerFactory =>
Sentry.currentHub.profilerFactory;

@override
Scope get scope => Sentry.currentHub.scope;
}
9 changes: 9 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 'scope.dart';
import 'sentry_client.dart';
Expand Down Expand Up @@ -120,6 +121,14 @@ class NoOpHub implements Hub {
@override
void setSpanContext(throwable, ISentrySpan span, String transaction) {}

@internal
@override
set profilerFactory(SentryProfilerFactory? value) {}

@internal
@override
SentryProfilerFactory? get profilerFactory => null;

@override
Scope get scope => Scope(_options);
}
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 SentryProfilerFactory {
SentryProfiler? startProfiler(SentryTransactionContext context);
}

@internal
abstract class SentryProfiler {
Future<SentryProfileInfo?> finishFor(SentryTransaction transaction);
void dispose();
}

// See https://develop.sentry.dev/sdk/profiles/
@internal
abstract class SentryProfileInfo {
SentryEnvelopeItem asEnvelopeItem();
}
2 changes: 1 addition & 1 deletion dart/lib/src/protocol/sentry_event.dart
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class SentryEvent with SentryEventLike<SentryEvent> {
/// The ID Sentry.io assigned to the submitted event for future reference.
final SentryId eventId;

/// A timestamp representing when the breadcrumb occurred.
/// A timestamp representing when the event occurred.
final DateTime? timestamp;

/// A string representing the platform the SDK is submitting from. This will be used by the Sentry interface to customize various components in the interface.
Expand Down
29 changes: 15 additions & 14 deletions dart/lib/src/protocol/sentry_transaction.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ class SentryTransaction extends SentryEvent {
late final DateTime startTimestamp;
static const String _type = 'transaction';
late final List<SentrySpan> spans;
final SentryTracer _tracer;
@internal
final SentryTracer tracer;
late final Map<String, SentryMeasurement> measurements;
late final SentryTransactionInfo? transactionInfo;

SentryTransaction(
this._tracer, {
this.tracer, {
SentryId? eventId,
DateTime? timestamp,
String? platform,
Expand All @@ -39,37 +40,37 @@ class SentryTransaction extends SentryEvent {
SentryTransactionInfo? transactionInfo,
}) : super(
eventId: eventId,
timestamp: timestamp ?? _tracer.endTimestamp,
timestamp: timestamp ?? tracer.endTimestamp,
platform: platform,
serverName: serverName,
release: release,
dist: dist,
environment: environment,
transaction: transaction ?? _tracer.name,
throwable: throwable ?? _tracer.throwable,
tags: tags ?? _tracer.tags,
transaction: transaction ?? tracer.name,
throwable: throwable ?? tracer.throwable,
tags: tags ?? tracer.tags,
// ignore: deprecated_member_use_from_same_package
extra: extra ?? _tracer.data,
extra: extra ?? tracer.data,
user: user,
contexts: contexts,
breadcrumbs: breadcrumbs,
sdk: sdk,
request: request,
type: _type,
) {
startTimestamp = _tracer.startTimestamp;
startTimestamp = tracer.startTimestamp;

final spanContext = _tracer.context;
spans = _tracer.children;
final spanContext = tracer.context;
spans = tracer.children;
this.measurements = measurements ?? {};

this.contexts.trace = spanContext.toTraceContext(
sampled: _tracer.samplingDecision?.sampled,
status: _tracer.status,
sampled: tracer.samplingDecision?.sampled,
status: tracer.status,
);

this.transactionInfo = transactionInfo ??
SentryTransactionInfo(_tracer.transactionNameSource.name);
SentryTransactionInfo(tracer.transactionNameSource.name);
}

@override
Expand Down Expand Up @@ -136,7 +137,7 @@ class SentryTransaction extends SentryEvent {
SentryTransactionInfo? transactionInfo,
}) =>
SentryTransaction(
_tracer,
tracer,
eventId: eventId ?? this.eventId,
timestamp: timestamp ?? this.timestamp,
platform: platform ?? this.platform,
Expand Down
5 changes: 5 additions & 0 deletions dart/lib/src/sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,11 @@ class SentryClient {
traceContext: traceContext,
attachments: attachments,
);

final profileInfo = preparedTransaction.tracer.profileInfo;
if (profileInfo != null) {
envelope.items.add(profileInfo.asEnvelopeItem());
}
final id = await captureEnvelope(envelope);

return id ?? SentryId.empty();
Expand Down
2 changes: 2 additions & 0 deletions dart/lib/src/sentry_envelope_item.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'dart:async';
import 'dart:convert';

import 'client_reports/client_report.dart';
import 'protocol.dart';
import 'utils.dart';
Expand Down Expand Up @@ -97,6 +98,7 @@ class SentryEnvelopeItem {

final newLine = utf8.encode('\n');
final data = await dataFactory();
// TODO the data copy could be avoided - this would be most significant with attachments.
return [...itemHeader, ...newLine, ...data];
} catch (e) {
return [];
Expand Down
1 change: 1 addition & 0 deletions dart/lib/src/sentry_item_type.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ class SentryItemType {
static const String attachment = 'attachment';
static const String transaction = 'transaction';
static const String clientReport = 'client_report';
static const String profile = 'profile';
static const String unknown = '__unknown__';
}
11 changes: 11 additions & 0 deletions dart/lib/src/sentry_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,17 @@ class SentryOptions {
/// to be sent to Sentry.
TracesSamplerCallback? tracesSampler;

double? _profilesSampleRate;

@internal // Only exposed by SentryFlutterOptions at the moment.
double? get profilesSampleRate => _profilesSampleRate;

@internal // Only exposed by SentryFlutterOptions at the moment.
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
25 changes: 23 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,13 @@ class SentryTracer extends ISentrySpan {

SentryTraceContextHeader? _sentryTraceContextHeader;

// Profiler attached to this tracer.
late final SentryProfiler? profiler;

// Resulting profile, after it has been collected. This is later used by
// SentryClient to attach as an envelope item when sending the transaction.
SentryProfileInfo? profileInfo;

/// If [waitForChildren] is true, this transaction will not finish until all
/// its children are finished.
///
Expand All @@ -52,6 +60,7 @@ class SentryTracer extends ISentrySpan {
Duration? autoFinishAfter,
bool trimEnd = false,
OnTransactionFinish? onFinish,
this.profiler,
}) {
_rootSpan = SentrySpan(
this,
Expand All @@ -77,8 +86,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 +145,17 @@ class SentryTracer extends ISentrySpan {

final transaction = SentryTransaction(this);
transaction.measurements.addAll(_measurements);

profileInfo = (status == null || status == SpanStatus.ok())
? await profiler?.finishFor(transaction)
: null;

await _hub.captureTransaction(
transaction,
traceContext: traceContext(),
);
} finally {
profiler?.dispose();
}
}

Expand Down
Loading

0 comments on commit dc54bfc

Please sign in to comment.