Skip to content

Commit 3e83702

Browse files
authored
fix: span ids not re-generating for headers created from scope (#3051)
* Rotate span ids for TwP * Update * Update * Update dio tests * Fix compilation * update * Remove file * Update CHANGELOG * Fix assertion
1 parent 575ebaa commit 3e83702

14 files changed

+312
-58
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
### Fixes
66

7+
- Span ids not re-generating for headers created from scope ([#3051](https://github.com/getsentry/sentry-dart/pull/3051))
78
- `ScreenshotIntegration` not being added for web ([#3055](https://github.com/getsentry/sentry-dart/pull/3055))
89

910
### Enhancements

dart/lib/sentry.dart

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,6 @@ export 'src/type_check_hint.dart';
5353
// ignore: invalid_export_of_internal_element
5454
export 'src/utils.dart';
5555
// ignore: invalid_export_of_internal_element
56-
export 'src/utils/add_tracing_headers_to_http_request.dart';
57-
// ignore: invalid_export_of_internal_element
5856
export 'src/utils/http_header_utils.dart';
5957
// ignore: invalid_export_of_internal_element
6058
export 'src/utils/http_sanitizer.dart';

dart/lib/src/http_client/tracing_client.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import '../hub_adapter.dart';
55
import '../protocol.dart';
66
import '../sentry_trace_origins.dart';
77
import '../tracing.dart';
8-
import '../utils/add_tracing_headers_to_http_request.dart';
98
import '../utils/http_sanitizer.dart';
109
import '../utils/tracing_utils.dart';
1110

dart/lib/src/propagation_context.dart

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,12 @@
11
import 'package:meta/meta.dart';
22

3-
import 'protocol.dart';
4-
import 'sentry_baggage.dart';
3+
import '../sentry.dart';
54

65
@internal
76
class PropagationContext {
87
/// Either represents the incoming `traceId` or the `traceId` generated by the current SDK, if there was no incoming trace.
98
SentryId traceId = SentryId.newId();
109

11-
/// 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.
12-
/// If this value is undefined on the propagation context, the SDK will generate a random span ID for `trace` contexts and trace propagation.
13-
final SpanId spanId = SpanId.newId();
14-
1510
/// The dynamic sampling context.
1611
SentryBaggage? baggage;
1712

@@ -20,5 +15,6 @@ class PropagationContext {
2015
baggage != null ? SentryBaggageHeader.fromBaggage(baggage!) : null;
2116

2217
/// Sentry trace header to attach to http headers.
23-
SentryTraceHeader toSentryTrace() => SentryTraceHeader(traceId, spanId);
18+
SentryTraceHeader toSentryTrace() =>
19+
generateSentryTraceHeader(traceId: traceId);
2420
}

dart/lib/src/protocol/sentry_span.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,9 @@ class SentrySpan extends ISentrySpan {
213213
Map<String, dynamic> get data => _data;
214214

215215
@override
216-
SentryTraceHeader toSentryTrace() => SentryTraceHeader(
217-
_context.traceId,
218-
_context.spanId,
216+
SentryTraceHeader toSentryTrace() => generateSentryTraceHeader(
217+
traceId: _context.traceId,
218+
spanId: _context.spanId,
219219
sampled: samplingDecision?.sampled,
220220
);
221221

dart/lib/src/protocol/sentry_trace_context.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ class SentryTraceContext {
119119
PropagationContext propagationContext) {
120120
return SentryTraceContext(
121121
traceId: propagationContext.traceId,
122-
spanId: propagationContext.spanId,
122+
spanId: SpanId.newId(),
123123
operation: 'default',
124124
replayId: propagationContext.baggage?.getReplayId());
125125
}

dart/lib/src/utils/add_tracing_headers_to_http_request.dart

Lines changed: 0 additions & 27 deletions
This file was deleted.

dart/lib/src/utils/tracing_utils.dart

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,33 @@
11
import '../../sentry.dart';
22

3+
SentryTraceHeader generateSentryTraceHeader(
4+
{SentryId? traceId, SpanId? spanId, bool? sampled}) {
5+
traceId ??= SentryId.newId();
6+
spanId ??= SpanId.newId();
7+
return SentryTraceHeader(traceId, spanId, sampled: sampled);
8+
}
9+
10+
void addTracingHeadersToHttpHeader(Map<String, dynamic> headers, Hub hub,
11+
{ISentrySpan? span}) {
12+
if (span != null) {
13+
addSentryTraceHeaderFromSpan(span, headers);
14+
addBaggageHeaderFromSpan(
15+
span,
16+
headers,
17+
log: hub.options.log,
18+
);
19+
} else {
20+
addSentryTraceHeaderFromScope(hub.scope, headers);
21+
addBaggageHeaderFromScope(hub.scope, headers, log: hub.options.log);
22+
}
23+
}
24+
25+
void addSentryTraceHeaderFromScope(Scope scope, Map<String, dynamic> headers) {
26+
final propagationContext = scope.propagationContext;
27+
final traceHeader = propagationContext.toSentryTrace();
28+
headers[traceHeader.name] = traceHeader.value;
29+
}
30+
331
void addSentryTraceHeaderFromSpan(
432
ISentrySpan span, Map<String, dynamic> headers) {
533
final traceHeader = span.toSentryTrace();
@@ -11,6 +39,17 @@ void addSentryTraceHeader(
1139
headers[traceHeader.name] = traceHeader.value;
1240
}
1341

42+
void addBaggageHeaderFromScope(
43+
Scope scope,
44+
Map<String, dynamic> headers, {
45+
SdkLogCallback? log,
46+
}) {
47+
final baggageHeader = scope.propagationContext.toBaggageHeader();
48+
if (baggageHeader != null) {
49+
addBaggageHeader(baggageHeader, headers, log: log);
50+
}
51+
}
52+
1453
void addBaggageHeaderFromSpan(
1554
ISentrySpan span,
1655
Map<String, dynamic> headers, {

dart/test/http_client/tracing_client_test.dart

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,13 +160,38 @@ void main() {
160160
final response = await sut.get(requestUri);
161161

162162
final baggageHeader = propagationContext.toBaggageHeader();
163-
final sentryTraceHeader = propagationContext.toSentryTrace();
164163

165164
expect(propagationContext.toBaggageHeader(), isNotNull);
166165
expect(
167166
response.request!.headers[baggageHeader!.name], baggageHeader.value);
168-
expect(response.request!.headers[sentryTraceHeader.name],
169-
sentryTraceHeader.value);
167+
168+
final traceHeader = SentryTraceHeader.fromTraceHeader(
169+
response.request!.headers['sentry-trace'] as String,
170+
);
171+
expect(traceHeader.traceId, propagationContext.traceId);
172+
// can't check span id as it is always generated new
173+
});
174+
175+
test(
176+
'tracing header from propagation context should generate new span ids for new events',
177+
() async {
178+
fixture._hub.options.tracesSampleRate = null;
179+
fixture._hub.options.tracesSampler = null;
180+
final sut = fixture.getSut(
181+
client: fixture.getClient(statusCode: 200, reason: 'OK'),
182+
);
183+
final propagationContext = fixture._hub.scope.propagationContext;
184+
propagationContext.baggage = SentryBaggage({'foo': 'bar'});
185+
186+
final response1 = await sut.get(requestUri);
187+
final response2 = await sut.get(requestUri);
188+
189+
final traceHeader1 = SentryTraceHeader.fromTraceHeader(
190+
response1.request!.headers['sentry-trace']!);
191+
final traceHeader2 = SentryTraceHeader.fromTraceHeader(
192+
response2.request!.headers['sentry-trace']!);
193+
194+
expect(traceHeader1.spanId, isNot(traceHeader2.spanId));
170195
});
171196

172197
test(

dart/test/scope_test.dart

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -463,13 +463,34 @@ void main() {
463463
true);
464464
});
465465

466-
test('apply trace context to event', () async {
466+
test('apply trace context to event with active span', () async {
467467
final event = SentryEvent();
468468
final scope = Scope(defaultTestOptions())..span = fixture.sentryTracer;
469469

470470
final updatedEvent = await scope.applyToEvent(event, Hint());
471471

472-
expect(updatedEvent?.contexts['trace'] is SentryTraceContext, true);
472+
expect(updatedEvent?.contexts['trace'] is SentryTraceContext, isTrue);
473+
});
474+
475+
test('apply trace context to event with propagation context', () async {
476+
final event = SentryEvent();
477+
final event2 = SentryEvent();
478+
final scope = Scope(defaultTestOptions());
479+
480+
final updatedEvent = await scope.applyToEvent(event, Hint());
481+
482+
final traceContext =
483+
updatedEvent?.contexts['trace'] as SentryTraceContext;
484+
final spanId1 = traceContext.spanId;
485+
expect(traceContext.traceId, scope.propagationContext.traceId);
486+
487+
final updatedEvent2 = await scope.applyToEvent(event2, Hint());
488+
final traceContext2 =
489+
updatedEvent2?.contexts['trace'] as SentryTraceContext;
490+
final spanId2 = traceContext2.spanId;
491+
492+
// trace contexts from the scope should always re-generate span ids
493+
expect(spanId1, isNot(spanId2));
473494
});
474495

475496
test('should not apply the scope properties when event already has it ',

0 commit comments

Comments
 (0)