Skip to content
Merged
Show file tree
Hide file tree
Changes from 23 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
12 changes: 7 additions & 5 deletions dart/lib/src/hub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
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,10 +453,7 @@
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 {
} else if (_options.isTracingEnabled()) {
final item = _peek();

// if transactionContext has no sampled decision, run the traces sampler
Expand All @@ -470,6 +466,7 @@
}

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 @@
return NoOpSentrySpan();
}

@internal

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

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/hub.dart#L497

Added line #L497 was not covered by tests
void generateNewTraceId() {
scope.propagationContext.traceId = SentryId.newId();

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

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/hub.dart#L499

Added line #L499 was not covered by tests
}

/// 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 @@
customSamplingContext: customSamplingContext,
);

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

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

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/hub_adapter.dart#L162-L163

Added lines #L162 - L163 were not covered by tests

@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 @@
}) =>
NoOpSentrySpan();

@override

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
void generateNewTraceId() {}

@override
ISentrySpan? getSpan() => null;

Expand Down
13 changes: 11 additions & 2 deletions dart/lib/src/propagation_context.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,20 @@ 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;

/// Baggage header to attach to http headers.
SentryBaggageHeader? toBaggageHeader() =>
baggage != null ? SentryBaggageHeader.fromBaggage(baggage!) : null;

/// 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 Expand Up @@ -1340,6 +1340,9 @@ class Fixture {
AdditionalInfoExtractor? additionalInfoProvider,
List<String>? ignoreRoutes,
}) {
if (hub is MockHub) {
when(hub.generateNewTraceId()).thenAnswer((_) => {});
}
final frameCallbackHandler = FakeFrameCallbackHandler(
postFrameCallbackDelay: Duration(milliseconds: 10));
timeToInitialDisplayTracker = TimeToInitialDisplayTracker(
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