Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: profiling for iOS/macOS #1611

Merged
merged 36 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
696ba61
todo
vaind Aug 17, 2023
4f85144
feat: expose cocoa profiling via method channels
vaind Aug 18, 2023
de6f244
feat: prepare profiler interfaces and hub integration
vaind Aug 22, 2023
7c73348
fix CI
vaind Aug 23, 2023
42af467
integrate dart & cocoa profiling
vaind Aug 24, 2023
162c2ba
fix API breakage
vaind Aug 26, 2023
31a92e5
fix tests
vaind Aug 28, 2023
b12978e
dart format
vaind Aug 28, 2023
3582adf
ci: fix pana
vaind Aug 29, 2023
1d74bac
update tests
vaind Aug 29, 2023
8d23d3f
analysis issues
vaind Aug 29, 2023
c9be10c
update to the latest cocoa SDK API
vaind Sep 5, 2023
32c6e5d
linter issue
vaind Sep 7, 2023
0ae79d4
fix import
vaind Sep 12, 2023
df9fb5b
refactor: SentryNative integration
vaind Sep 12, 2023
0fb649c
update cocoa native binding to use ffi startProfiler()
vaind Sep 12, 2023
dccb853
tmp: findPrimeNumber in example
vaind Sep 12, 2023
6119812
fix: make FFI dependency conditional (web/vm)
vaind Sep 18, 2023
5218efa
exclude generated binding code from code coverage
vaind Sep 18, 2023
4c8f2bf
test: profiler integration test
vaind Sep 18, 2023
e0bb3ab
workaround for the integration test issue
vaind Sep 18, 2023
8a4fed7
chore: formatting
vaind Sep 19, 2023
2d29cb6
Merge branch 'main' into feat/profiling
vaind Sep 19, 2023
4748a00
Merge branch 'main' into feat/profiling
vaind Sep 20, 2023
39d1187
chore: remove obsolete code
vaind Sep 25, 2023
50994d4
Update flutter/example/lib/main.dart
vaind Oct 3, 2023
6e599fb
renames
vaind Oct 3, 2023
e7582d7
Breadcrumbs for file I/O operations (#1649)
denrase Sep 25, 2023
36b052e
ci: don't run CI on markdown updates (#1651)
vaind Sep 25, 2023
813b947
fixup mock names after renames
vaind Oct 3, 2023
ce59509
Merge branch 'main' into feat/profiling
stefanosiano Oct 4, 2023
e13e7de
more renames (Sentry prefix)
vaind Oct 4, 2023
8e6bd13
chore: update changelog
vaind Oct 4, 2023
227b701
Merge branch 'main' into feat/profiling
vaind Oct 26, 2023
defdfa6
don't inline findPrimeNumber profiler-test function
vaind Oct 27, 2023
4074f01
fixup changelog
vaind Oct 27, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: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 @@
} 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 @@
);
}

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

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

@internal
SentryProfilerFactory? get profilerFactory => _profilerFactory;

Check warning on line 566 in dart/lib/src/hub.dart

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/hub.dart#L565-L566

Added lines #L565 - L566 were not covered by tests

@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 '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 @@
) =>
Sentry.currentHub.setSpanContext(throwable, span, transaction);

@internal

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

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/hub_adapter.dart#L172

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

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

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/hub_adapter.dart#L175

Added line #L175 was not covered by tests

@internal

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

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/hub_adapter.dart#L177

Added line #L177 was not covered by tests
@override
SentryProfilerFactory? get profilerFactory =>
Sentry.currentHub.profilerFactory;

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

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/hub_adapter.dart#L180

Added line #L180 was not covered by tests

@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 '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 @@
@override
void setSpanContext(throwable, ISentrySpan span, String transaction) {}

@internal

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

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/noop_hub.dart#L124

Added line #L124 was not covered by tests
@override
set profilerFactory(SentryProfilerFactory? value) {}

@internal

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

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/noop_hub.dart#L128

Added line #L128 was not covered by tests
@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 @@
traceContext: traceContext,
attachments: attachments,
);

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

Check warning on line 351 in dart/lib/src/sentry_client.dart

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/sentry_client.dart#L351

Added line #L351 was not covered by tests
}
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.
vaind marked this conversation as resolved.
Show resolved Hide resolved
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have a profilesSampler option in the other SDKs, which, if set, is prioritized over profilesSampleRate
I'm fine adding it in another PR, if we decide so


@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
Loading