Skip to content
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
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
10 changes: 6 additions & 4 deletions dart/lib/src/hub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import 'package:meta/meta.dart';
import '../sentry.dart';
import 'client_reports/discard_reason.dart';
import 'profiling.dart';
import 'propagation_context.dart';
import 'sentry_tracer.dart';
import 'sentry_traces_sampler.dart';
import 'transport/data_category.dart';
Expand Down Expand Up @@ -454,9 +453,6 @@ class Hub {
SentryLevel.warning,
"Instance is disabled and this 'startTransaction' call is a no-op.",
);
} else if (!_options.isTracingEnabled()) {
final item = _peek();
item.scope.propagationContext = PropagationContext();
Comment on lines -457 to -459
Copy link
Copy Markdown
Contributor Author

@buenaflor buenaflor Apr 15, 2025

Choose a reason for hiding this comment

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

this code doesn't make sense, we can remove it. a new start transaction with performance disabled should not be able to create a new trace, it should just no-op

} else {
final item = _peek();

Expand All @@ -470,6 +466,7 @@ class Hub {
}

transactionContext.origin ??= SentryTraceOrigins.manual;
transactionContext.traceId = scope.propagationContext.traceId;
Copy link
Copy Markdown
Contributor Author

@buenaflor buenaflor Apr 15, 2025

Choose a reason for hiding this comment

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

navigator observer will take care of updating the propagation context so new transactions just need to use it and child spans will inherit the trace id from parent


SentryProfiler? profiler;
if (_profilerFactory != null &&
Expand Down Expand Up @@ -497,6 +494,11 @@ class Hub {
return NoOpSentrySpan();
}

@internal
void generateNewTraceId() {
scope.propagationContext.traceId = SentryId.newId();
}

/// Gets the current active transaction or span.
ISentrySpan? getSpan() {
ISentrySpan? span;
Expand Down
3 changes: 3 additions & 0 deletions dart/lib/src/hub_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ class HubAdapter implements Hub {
customSamplingContext: customSamplingContext,
);

@override
void generateNewTraceId() => Sentry.currentHub.generateNewTraceId();

@override
void setSpanContext(
dynamic throwable,
Expand Down
3 changes: 3 additions & 0 deletions dart/lib/src/noop_hub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ class NoOpHub implements Hub {
}) =>
NoOpSentrySpan();

@override
void generateNewTraceId() {}

@override
ISentrySpan? getSpan() => null;

Expand Down
12 changes: 10 additions & 2 deletions dart/lib/src/propagation_context.dart
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
import 'package:meta/meta.dart';

import 'protocol.dart';
import 'sentry_baggage.dart';

@internal
class PropagationContext {
late SentryId traceId = SentryId.newId();
late SpanId spanId = SpanId.newId();
/// Either represents the incoming `traceId` or the `traceId` generated by the current SDK, if there was no incoming trace.
SentryId traceId = SentryId.newId();

/// A span ID that should be used for the `trace` context of various event type, when performance is disabled or when there is no active span.
/// If this value is undefined on the propagation context, the SDK will generate a random span ID for `trace` contexts and trace propagation.
final SpanId spanId = SpanId.newId();

/// The dynamic sampling context.
SentryBaggage? baggage;

/// Sentry trace header to attach to http headers.
SentryTraceHeader toSentryTrace() => SentryTraceHeader(traceId, spanId);
}
12 changes: 12 additions & 0 deletions dart/lib/src/scope.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ class Scope {
/// Returns active transaction or null if there is no active transaction.
ISentrySpan? span;

/// The propagation context for connecting errors and spans to traces.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Love the additional descriptions/documetnations you have added. ❤️

/// There should always be a propagation context available at all times.
///
/// Default behaviour of trace generation in Flutter:
///
/// If `SentryNavigatorObserver` is available:
/// - new trace on navigation for all platforms
///
/// if `SentryNavigatorObserver` is not available:
/// - Mobile: traces will be cycled with the background/foreground hooks, similar to how sessions are defined in mobile
/// - Web: traces will stick until the next refresh (this might change in the future)
@internal
PropagationContext propagationContext = PropagationContext();

Expand Down Expand Up @@ -246,6 +257,7 @@ class Scope {
_extra.clear();
_eventProcessors.clear();
_replayId = null;
propagationContext = PropagationContext();

_clearBreadcrumbsSync();
_setUserSync(null);
Expand Down
4 changes: 2 additions & 2 deletions dart/test/sentry_client_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:collection/collection.dart';
import 'package:sentry/sentry.dart';
import 'package:sentry/src/client_reports/discard_reason.dart';
import 'package:sentry/src/client_reports/noop_client_report_recorder.dart';
import 'package:sentry/src/event_processor/exception/exception_group_event_processor.dart';
import 'package:sentry/src/platform/mock_platform.dart';
import 'package:sentry/src/sentry_item_type.dart';
import 'package:sentry/src/sentry_stack_trace_factory.dart';
Expand All @@ -15,9 +16,8 @@ import 'package:sentry/src/transport/data_category.dart';
import 'package:sentry/src/transport/noop_transport.dart';
import 'package:sentry/src/transport/spotlight_http_transport.dart';
import 'package:sentry/src/utils/iterable_utils.dart';
import 'package:sentry/src/event_processor/exception/exception_group_event_processor.dart';

import 'package:test/test.dart';

import 'mocks.dart';
import 'mocks/mock_client_report_recorder.dart';
import 'mocks/mock_hub.dart';
Expand Down
3 changes: 3 additions & 0 deletions flutter/lib/src/navigation/sentry_navigator_observer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
return;
}

_hub.generateNewTraceId();
_setCurrentRouteName(route);
_setCurrentRouteNameAsTransaction(route);

Expand Down Expand Up @@ -180,6 +181,7 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
return;
}

_hub.generateNewTraceId();
_setCurrentRouteName(newRoute);
_setCurrentRouteNameAsTransaction(newRoute);

Expand All @@ -201,6 +203,7 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
return;
}

_hub.generateNewTraceId();
_setCurrentRouteName(previousRoute);
_setCurrentRouteNameAsTransaction(previousRoute);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import 'package:sentry_flutter/src/navigation/time_to_display_tracker.dart';
import 'package:sentry_flutter/src/navigation/time_to_full_display_tracker.dart';
import 'package:sentry_flutter/src/navigation/time_to_initial_display_tracker.dart';

import 'fake_frame_callback_handler.dart';
import 'mocks.dart';
import 'mocks.mocks.dart';
import '../fake_frame_callback_handler.dart';
import '../mocks.dart';
import '../mocks.mocks.dart';

void main() {
late Fixture fixture;
Expand Down
115 changes: 115 additions & 0 deletions flutter/test/navigation/sentry_navigator_observer_traces_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
// ignore_for_file: invalid_use_of_internal_member

import 'package:flutter/cupertino.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:mockito/mockito.dart';
import 'package:sentry_flutter/sentry_flutter.dart';

import '../mocks.dart';
import '../mocks.mocks.dart';

void main() {
late Fixture fixture;

setUp(() {
fixture = Fixture();
});

test('didPush generates a new trace', () {
final fromRoute = _route(RouteSettings(name: 'From Route'));
final toRoute = _route(RouteSettings(name: 'To Route'));
final oldTraceId = fixture.hub.scope.propagationContext.traceId;

final sut = fixture.getSut();
sut.didPush(toRoute, fromRoute);

final newTraceId = fixture.hub.scope.propagationContext.traceId;
expect(oldTraceId, isNot(newTraceId));
});

test('didPop generates a new trace', () {
final fromRoute = _route(RouteSettings(name: 'From Route'));
final toRoute = _route(RouteSettings(name: 'To Route'));
final oldTraceId = fixture.hub.scope.propagationContext.traceId;

final sut = fixture.getSut();
sut.didPop(toRoute, fromRoute);

final newTraceId = fixture.hub.scope.propagationContext.traceId;
expect(oldTraceId, isNot(newTraceId));
});

test('didReplace generates a new trace', () {
final fromRoute = _route(RouteSettings(name: 'From Route'));
final toRoute = _route(RouteSettings(name: 'To Route'));
final oldTraceId = fixture.hub.scope.propagationContext.traceId;

final sut = fixture.getSut();
sut.didReplace(newRoute: toRoute, oldRoute: fromRoute);

final newTraceId = fixture.hub.scope.propagationContext.traceId;
expect(oldTraceId, isNot(newTraceId));
});

group('execution order', () {
/// Prepares mocks, we don't care about what they exactly do.
/// We only test the order of execution in this group.
void _prepareMocks() {
when(fixture.mockHub.generateNewTraceId()).thenAnswer((_) => {});
when(fixture.mockHub.configureScope(any))
.thenAnswer((_) => Future.value());
when(fixture.mockHub.startTransactionWithContext(
any,
bindToScope: anyNamed('bindToScope'),
waitForChildren: anyNamed('waitForChildren'),
autoFinishAfter: anyNamed('autoFinishAfter'),
trimEnd: anyNamed('trimEnd'),
onFinish: anyNamed('onFinish'),
customSamplingContext: anyNamed('customSamplingContext'),
startTimestamp: anyNamed('startTimestamp'),
)).thenReturn(NoOpSentrySpan());
}

test('didPush generates a new trace before creating transaction spans', () {
final fromRoute = _route(RouteSettings(name: 'From Route'));
final toRoute = _route(RouteSettings(name: 'To Route'));

_prepareMocks();

final sut = fixture.getSut(hub: fixture.mockHub);
sut.didPush(toRoute, fromRoute);
verifyInOrder([
fixture.mockHub.generateNewTraceId(),
fixture.mockHub.startTransactionWithContext(
any,
bindToScope: anyNamed('bindToScope'),
waitForChildren: anyNamed('waitForChildren'),
autoFinishAfter: anyNamed('autoFinishAfter'),
trimEnd: anyNamed('trimEnd'),
onFinish: anyNamed('onFinish'),
customSamplingContext: anyNamed('customSamplingContext'),
startTimestamp: anyNamed('startTimestamp'),
),
]);
});
});
}

PageRoute<dynamic> _route(RouteSettings? settings) => PageRouteBuilder<void>(
pageBuilder: (_, __, ___) => Container(),
settings: settings,
);

class Fixture {
final options = defaultTestOptions();
late final mockHub = MockHub();
late final hub = Hub(options);

SentryNavigatorObserver getSut({Hub? hub}) {
hub ??= this.hub;
if (hub == mockHub) {
when(mockHub.options).thenReturn(options);
}
return SentryNavigatorObserver(hub: hub);
}
}
Loading