Skip to content

Commit 23d0dbf

Browse files
buenaflorcursoragent
buenaflor
and
cursoragent
authored
feat: add flag to SentryNavigatorObserver to enable/disable starting new traces on navigation (#3096)
* Implement trace sampling propagation across transactions Co-authored-by: giancarlo.buenaflor <[email protected]> * Update hub.dart * Update * Update * Update hub_test.dart * Update hub_test.dart * Update hub.dart * Update hub.dart * Update * Update * Update CHANGELOG * Fix analyze * Update * Update * Update * Update * Update CHANGELOG * Update docs * Update test --------- Co-authored-by: Cursor Agent <[email protected]>
1 parent 6f47800 commit 23d0dbf

File tree

3 files changed

+135
-72
lines changed

3 files changed

+135
-72
lines changed

‎CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@
88
- Search for feature flags that are prefixed with `flutter:*`
99
- This works on Flutter builds that include [this PR](https://github.com/flutter/flutter/pull/171545)
1010
- Add `LoggingIntegration` support for `SentryLog` ([#3050](https://github.com/getsentry/sentry-dart/pull/3050))
11+
- Add `enableNewTraceOnNavigation` flag to `SentryNavigatorObserver` ([#3096](https://github.com/getsentry/sentry-dart/pull/3096))
12+
- **Default:** `true`
13+
- **Disable** by passing `false`, e.g.:
14+
```dart
15+
SentryNavigatorObserver(enableNewTraceOnNavigation: false)
16+
```
17+
- _Note: traces differ from transactions/spans — see tracing concepts [here](https://docs.sentry.io/concepts/key-terms/tracing/)_
1118
1219
### Fixes
1320

‎flutter/lib/src/navigation/sentry_navigator_observer.dart

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ typedef AdditionalInfoExtractor = Map<String, dynamic>? Function(
6262
/// if those happen to take longer. The transaction will be set to [Scope.span]
6363
/// if the latter is empty.
6464
///
65+
/// If [enableNewTraceOnNavigation] is true (default), a
66+
/// fresh trace is generated before each push, pop, or replace event.
67+
///
6568
/// Enabling the [setRouteNameAsTransaction] option overrides the current
6669
/// [Scope.transaction] which will also override the name of the current
6770
/// [Scope.span]. So be careful when this is used together with performance
@@ -74,13 +77,15 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
7477
SentryNavigatorObserver({
7578
Hub? hub,
7679
bool enableAutoTransactions = true,
80+
bool enableNewTraceOnNavigation = true,
7781
Duration autoFinishAfter = const Duration(seconds: 3),
7882
bool setRouteNameAsTransaction = false,
7983
RouteNameExtractor? routeNameExtractor,
8084
AdditionalInfoExtractor? additionalInfoProvider,
8185
List<String>? ignoreRoutes,
8286
}) : _hub = hub ?? HubAdapter(),
8387
_enableAutoTransactions = enableAutoTransactions,
88+
_enableNewTraceOnNavigation = enableNewTraceOnNavigation,
8489
_autoFinishAfter = autoFinishAfter,
8590
_setRouteNameAsTransaction = setRouteNameAsTransaction,
8691
_routeNameExtractor = routeNameExtractor,
@@ -110,6 +115,7 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
110115

111116
final Hub _hub;
112117
final bool _enableAutoTransactions;
118+
final bool _enableNewTraceOnNavigation;
113119
final Duration _autoFinishAfter;
114120
final bool _setRouteNameAsTransaction;
115121
final RouteNameExtractor? _routeNameExtractor;
@@ -143,7 +149,7 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
143149
return;
144150
}
145151

146-
_hub.generateNewTrace();
152+
_startNewTraceIfEnabled();
147153
_setCurrentRouteName(route);
148154
_setCurrentRouteNameAsTransaction(route);
149155

@@ -174,7 +180,7 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
174180
return;
175181
}
176182

177-
_hub.generateNewTrace();
183+
_startNewTraceIfEnabled();
178184
_setCurrentRouteName(newRoute);
179185
_setCurrentRouteNameAsTransaction(newRoute);
180186

@@ -196,7 +202,7 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
196202
return;
197203
}
198204

199-
_hub.generateNewTrace();
205+
_startNewTraceIfEnabled();
200206
_setCurrentRouteName(previousRoute);
201207
_setCurrentRouteNameAsTransaction(previousRoute);
202208

@@ -212,6 +218,12 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
212218
_finishTransaction(endTimestamp: timestamp);
213219
}
214220

221+
void _startNewTraceIfEnabled() {
222+
if (_enableNewTraceOnNavigation) {
223+
_hub.generateNewTrace();
224+
}
225+
}
226+
215227
void _addWebSessions({Route<dynamic>? from, Route<dynamic>? to}) async {
216228
final fromName = from != null ? _getRouteName(from) : null;
217229
final toName = to != null ? _getRouteName(to) : null;

‎flutter/test/navigation/sentry_navigator_observer_traces_test.dart

Lines changed: 113 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -15,82 +15,120 @@ void main() {
1515
fixture = Fixture();
1616
});
1717

18-
test('didPush generates a new trace', () {
19-
final fromRoute = _route(RouteSettings(name: 'From Route'));
20-
final toRoute = _route(RouteSettings(name: 'To Route'));
21-
final oldTraceId = fixture.hub.scope.propagationContext.traceId;
18+
group('SentryNavigatorObserver', () {
19+
group('when starting traces on navigation is enabled (default)', () {
20+
test('didPush should start a new trace', () {
21+
final from = _route(RouteSettings(name: 'From Route'));
22+
final to = _route(RouteSettings(name: 'To Route'));
23+
final before = fixture.hub.scope.propagationContext.traceId;
24+
25+
fixture.getSut().didPush(to, from);
26+
27+
final after = fixture.hub.scope.propagationContext.traceId;
28+
expect(after, isNot(before));
29+
});
30+
31+
test('didPop should start a new trace', () {
32+
final from = _route(RouteSettings(name: 'From Route'));
33+
final to = _route(RouteSettings(name: 'To Route'));
34+
final before = fixture.hub.scope.propagationContext.traceId;
35+
36+
fixture.getSut().didPop(to, from);
37+
38+
final after = fixture.hub.scope.propagationContext.traceId;
39+
expect(after, isNot(before));
40+
});
41+
42+
test('didReplace should start a new trace', () {
43+
final from = _route(RouteSettings(name: 'From Route'));
44+
final to = _route(RouteSettings(name: 'To Route'));
45+
final before = fixture.hub.scope.propagationContext.traceId;
46+
47+
fixture.getSut().didReplace(newRoute: to, oldRoute: from);
48+
49+
final after = fixture.hub.scope.propagationContext.traceId;
50+
expect(after, isNot(before));
51+
});
52+
53+
group('execution order', () {
54+
void _stubHub() {
55+
when(fixture.mockHub.generateNewTrace()).thenReturn(null);
56+
when(fixture.mockHub.configureScope(any))
57+
.thenAnswer((_) => Future.value());
58+
when(fixture.mockHub.startTransactionWithContext(
59+
any,
60+
bindToScope: anyNamed('bindToScope'),
61+
waitForChildren: anyNamed('waitForChildren'),
62+
autoFinishAfter: anyNamed('autoFinishAfter'),
63+
trimEnd: anyNamed('trimEnd'),
64+
onFinish: anyNamed('onFinish'),
65+
customSamplingContext: anyNamed('customSamplingContext'),
66+
startTimestamp: anyNamed('startTimestamp'),
67+
)).thenReturn(NoOpSentrySpan());
68+
}
69+
70+
test(
71+
'didPush should call generateNewTrace before starting the transaction',
72+
() {
73+
final from = _route(RouteSettings(name: 'From Route'));
74+
final to = _route(RouteSettings(name: 'To Route'));
75+
76+
_stubHub();
77+
final sut = fixture.getSut(hub: fixture.mockHub);
78+
sut.didPush(to, from);
79+
80+
verifyInOrder([
81+
fixture.mockHub.generateNewTrace(),
82+
fixture.mockHub.startTransactionWithContext(
83+
any,
84+
bindToScope: anyNamed('bindToScope'),
85+
waitForChildren: anyNamed('waitForChildren'),
86+
autoFinishAfter: anyNamed('autoFinishAfter'),
87+
trimEnd: anyNamed('trimEnd'),
88+
onFinish: anyNamed('onFinish'),
89+
customSamplingContext: anyNamed('customSamplingContext'),
90+
startTimestamp: anyNamed('startTimestamp'),
91+
),
92+
]);
93+
});
94+
});
95+
});
2296

23-
final sut = fixture.getSut();
24-
sut.didPush(toRoute, fromRoute);
97+
group('when starting traces on navigation is disabled', () {
98+
test('didPush should not start a new trace', () {
99+
final from = _route(RouteSettings(name: 'From Route'));
100+
final to = _route(RouteSettings(name: 'To Route'));
101+
final before = fixture.hub.scope.propagationContext.traceId;
25102

26-
final newTraceId = fixture.hub.scope.propagationContext.traceId;
27-
expect(oldTraceId, isNot(newTraceId));
28-
});
103+
fixture.getSut(enableNewTraceOnNavigation: false).didPush(to, from);
29104

30-
test('didPop generates a new trace', () {
31-
final fromRoute = _route(RouteSettings(name: 'From Route'));
32-
final toRoute = _route(RouteSettings(name: 'To Route'));
33-
final oldTraceId = fixture.hub.scope.propagationContext.traceId;
105+
final after = fixture.hub.scope.propagationContext.traceId;
106+
expect(after, equals(before));
107+
});
34108

35-
final sut = fixture.getSut();
36-
sut.didPop(toRoute, fromRoute);
109+
test('didPop should not start a new trace', () {
110+
final from = _route(RouteSettings(name: 'From Route'));
111+
final to = _route(RouteSettings(name: 'To Route'));
112+
final before = fixture.hub.scope.propagationContext.traceId;
37113

38-
final newTraceId = fixture.hub.scope.propagationContext.traceId;
39-
expect(oldTraceId, isNot(newTraceId));
40-
});
114+
fixture.getSut(enableNewTraceOnNavigation: false).didPop(to, from);
41115

42-
test('didReplace generates a new trace', () {
43-
final fromRoute = _route(RouteSettings(name: 'From Route'));
44-
final toRoute = _route(RouteSettings(name: 'To Route'));
45-
final oldTraceId = fixture.hub.scope.propagationContext.traceId;
116+
final after = fixture.hub.scope.propagationContext.traceId;
117+
expect(after, equals(before));
118+
});
46119

47-
final sut = fixture.getSut();
48-
sut.didReplace(newRoute: toRoute, oldRoute: fromRoute);
120+
test('didReplace should not start a new trace', () {
121+
final from = _route(RouteSettings(name: 'From Route'));
122+
final to = _route(RouteSettings(name: 'To Route'));
123+
final before = fixture.hub.scope.propagationContext.traceId;
49124

50-
final newTraceId = fixture.hub.scope.propagationContext.traceId;
51-
expect(oldTraceId, isNot(newTraceId));
52-
});
125+
fixture
126+
.getSut(enableNewTraceOnNavigation: false)
127+
.didReplace(newRoute: to, oldRoute: from);
53128

54-
group('execution order', () {
55-
/// Prepares mocks, we don't care about what they exactly do.
56-
/// We only test the order of execution in this group.
57-
void _prepareMocks() {
58-
when(fixture.mockHub.generateNewTrace()).thenAnswer((_) => {});
59-
when(fixture.mockHub.configureScope(any))
60-
.thenAnswer((_) => Future.value());
61-
when(fixture.mockHub.startTransactionWithContext(
62-
any,
63-
bindToScope: anyNamed('bindToScope'),
64-
waitForChildren: anyNamed('waitForChildren'),
65-
autoFinishAfter: anyNamed('autoFinishAfter'),
66-
trimEnd: anyNamed('trimEnd'),
67-
onFinish: anyNamed('onFinish'),
68-
customSamplingContext: anyNamed('customSamplingContext'),
69-
startTimestamp: anyNamed('startTimestamp'),
70-
)).thenReturn(NoOpSentrySpan());
71-
}
72-
73-
test('didPush generates a new trace before creating transaction spans', () {
74-
final fromRoute = _route(RouteSettings(name: 'From Route'));
75-
final toRoute = _route(RouteSettings(name: 'To Route'));
76-
77-
_prepareMocks();
78-
79-
final sut = fixture.getSut(hub: fixture.mockHub);
80-
sut.didPush(toRoute, fromRoute);
81-
verifyInOrder([
82-
fixture.mockHub.generateNewTrace(),
83-
fixture.mockHub.startTransactionWithContext(
84-
any,
85-
bindToScope: anyNamed('bindToScope'),
86-
waitForChildren: anyNamed('waitForChildren'),
87-
autoFinishAfter: anyNamed('autoFinishAfter'),
88-
trimEnd: anyNamed('trimEnd'),
89-
onFinish: anyNamed('onFinish'),
90-
customSamplingContext: anyNamed('customSamplingContext'),
91-
startTimestamp: anyNamed('startTimestamp'),
92-
),
93-
]);
129+
final after = fixture.hub.scope.propagationContext.traceId;
130+
expect(after, equals(before));
131+
});
94132
});
95133
});
96134
}
@@ -105,11 +143,17 @@ class Fixture {
105143
late final mockHub = MockHub();
106144
late final hub = Hub(options);
107145

108-
SentryNavigatorObserver getSut({Hub? hub}) {
146+
SentryNavigatorObserver getSut({
147+
Hub? hub,
148+
bool enableNewTraceOnNavigation = true,
149+
}) {
109150
hub ??= this.hub;
110151
if (hub == mockHub) {
111152
when(mockHub.options).thenReturn(options);
112153
}
113-
return SentryNavigatorObserver(hub: hub);
154+
return SentryNavigatorObserver(
155+
hub: hub,
156+
enableNewTraceOnNavigation: enableNewTraceOnNavigation,
157+
);
114158
}
115159
}

0 commit comments

Comments
 (0)