Skip to content

Commit 86943da

Browse files
Ensure consistent sampleRand per trace (#3079)
* Implement sampleRand propagation for consistent trace sampling Co-authored-by: giancarlo.buenaflor <[email protected]> * Update * Update CHANGELOG * UpdatE * Update hub.dart * Update propagation_context.dart * Update hub.dart * Update * Add test for resetting sampleRand in new trace * Update --------- Co-authored-by: Cursor Agent <[email protected]>
1 parent f54f7d5 commit 86943da

19 files changed

+128
-44
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@
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))
1111

12+
### Fixes
13+
14+
- Ensure consistent sampling per trace ([#3079](https://github.com/getsentry/sentry-dart/pull/3079))
15+
1216
### Dependencies
1317

1418
- Bump Native SDK from v0.9.0 to v0.9.1 ([#3018](https://github.com/getsentry/sentry-dart/pull/3018))

dart/lib/src/hub.dart

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import 'dart:async';
22
import 'dart:collection';
3+
import 'dart:math';
34

45
import 'package:meta/meta.dart';
56

@@ -490,17 +491,29 @@ class Hub {
490491
} else if (_options.isTracingEnabled()) {
491492
final item = _peek();
492493

493-
// if transactionContext has no sampled decision, run the traces sampler
494+
// if transactionContext has no sampling decision yet, run the traces sampler
494495
var samplingDecision = transactionContext.samplingDecision;
496+
final propagationContext = scope.propagationContext;
497+
// Store the generated/used sampleRand on the propagation context so
498+
// that subsequent transactions in the same trace reuse it.
499+
propagationContext.sampleRand ??= Random().nextDouble();
500+
495501
if (samplingDecision == null) {
496502
final samplingContext = SentrySamplingContext(
497503
transactionContext, customSamplingContext ?? {});
498-
samplingDecision = _tracesSampler.sample(samplingContext);
504+
505+
samplingDecision = _tracesSampler.sample(
506+
samplingContext,
507+
// sampleRand is guaranteed not to be null here
508+
propagationContext.sampleRand!,
509+
);
510+
511+
// Persist the sampling decision within the transaction context
499512
transactionContext.samplingDecision = samplingDecision;
500513
}
501514

502515
transactionContext.origin ??= SentryTraceOrigins.manual;
503-
transactionContext.traceId = scope.propagationContext.traceId;
516+
transactionContext.traceId = propagationContext.traceId;
504517

505518
SentryProfiler? profiler;
506519
if (_profilerFactory != null &&
@@ -529,8 +542,10 @@ class Hub {
529542
}
530543

531544
@internal
532-
void generateNewTraceId() {
545+
void generateNewTrace() {
533546
scope.propagationContext.traceId = SentryId.newId();
547+
// Reset sampleRand so that a new one is generated for the new trace when a new transaction is started
548+
scope.propagationContext.sampleRand = null;
534549
}
535550

536551
/// Gets the current active transaction or span.

dart/lib/src/hub_adapter.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ class HubAdapter implements Hub {
162162
);
163163

164164
@override
165-
void generateNewTraceId() => Sentry.currentHub.generateNewTraceId();
165+
void generateNewTrace() => Sentry.currentHub.generateNewTrace();
166166

167167
@override
168168
void setSpanContext(

dart/lib/src/noop_hub.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ class NoOpHub implements Hub {
126126
NoOpSentrySpan();
127127

128128
@override
129-
void generateNewTraceId() {}
129+
void generateNewTrace() {}
130130

131131
@override
132132
ISentrySpan? getSpan() => null;

dart/lib/src/propagation_context.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,13 @@ class PropagationContext {
1010
/// The dynamic sampling context.
1111
SentryBaggage? baggage;
1212

13+
/// Random number generated for sampling decisions.
14+
///
15+
/// This value must be generated **once per trace** and reused across all
16+
/// child spans and transactions that belong to the same trace. It is reset
17+
/// whenever a new trace is started.
18+
double? sampleRand;
19+
1320
/// Baggage header to attach to http headers.
1421
SentryBaggageHeader? toBaggageHeader() =>
1522
baggage != null ? SentryBaggageHeader.fromBaggage(baggage!) : null;

dart/lib/src/sentry_traces_sampler.dart

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ class SentryTracesSampler {
1919
}
2020
}
2121

22-
SentryTracesSamplingDecision sample(SentrySamplingContext samplingContext) {
22+
SentryTracesSamplingDecision sample(
23+
SentrySamplingContext samplingContext,
24+
double sampleRand,
25+
) {
2326
final samplingDecision =
2427
samplingContext.transactionContext.samplingDecision;
2528
if (samplingDecision != null) {
@@ -31,7 +34,7 @@ class SentryTracesSampler {
3134
try {
3235
final sampleRate = tracesSampler(samplingContext);
3336
if (sampleRate != null) {
34-
return _makeSampleDecision(sampleRate);
37+
return _makeSampleDecision(sampleRate, sampleRand);
3538
}
3639
} catch (exception, stackTrace) {
3740
_options.log(
@@ -54,7 +57,7 @@ class SentryTracesSampler {
5457

5558
double? optionsRate = _options.tracesSampleRate;
5659
if (optionsRate != null) {
57-
return _makeSampleDecision(optionsRate);
60+
return _makeSampleDecision(optionsRate, sampleRand);
5861
}
5962

6063
return SentryTracesSamplingDecision(false);
@@ -68,8 +71,10 @@ class SentryTracesSampler {
6871
return _isSampled(optionsRate);
6972
}
7073

71-
SentryTracesSamplingDecision _makeSampleDecision(double sampleRate) {
72-
final sampleRand = _random.nextDouble();
74+
SentryTracesSamplingDecision _makeSampleDecision(
75+
double sampleRate,
76+
double sampleRand,
77+
) {
7378
final sampled = _isSampled(sampleRate, sampleRand: sampleRand);
7479
return SentryTracesSamplingDecision(sampled,
7580
sampleRate: sampleRate, sampleRand: sampleRand);

dart/test/hub_test.dart

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -581,14 +581,23 @@ void main() {
581581
});
582582
});
583583

584-
test('generateNewTraceId creates new trace id in propagation context', () {
584+
test('generateNewTrace creates new trace id in propagation context', () {
585585
final oldTraceId = hub.scope.propagationContext.traceId;
586586

587-
hub.generateNewTraceId();
587+
hub.generateNewTrace();
588588

589589
final newTraceId = hub.scope.propagationContext.traceId;
590590
expect(oldTraceId, isNot(newTraceId));
591591
});
592+
593+
test('generateNewTrace resets sampleRand in propagation context', () {
594+
hub.scope.propagationContext.sampleRand = 1.0;
595+
596+
hub.generateNewTrace();
597+
598+
final newSampleRand = hub.scope.propagationContext.sampleRand;
599+
expect(newSampleRand, isNull);
600+
});
592601
});
593602

594603
group('Hub scope callback', () {
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import 'package:sentry/sentry.dart';
2+
import 'package:sentry/src/sentry_tracer.dart';
3+
import 'package:test/test.dart';
4+
5+
import 'test_utils.dart';
6+
7+
void main() {
8+
group('sampleRand', () {
9+
test('is reused for transactions within the same trace', () {
10+
final options = defaultTestOptions()..tracesSampleRate = 1.0;
11+
final hub = Hub(options);
12+
13+
final tx1 = hub.startTransaction('tx1', 'op') as SentryTracer;
14+
final rand1 = tx1.samplingDecision?.sampleRand;
15+
16+
// Sanity check
17+
expect(rand1, isNotNull);
18+
19+
final tx2 = hub.startTransaction('tx2', 'op') as SentryTracer;
20+
final rand2 = tx2.samplingDecision?.sampleRand;
21+
22+
expect(rand2, equals(rand1));
23+
});
24+
25+
test('is generated within a transaction in a new trace', () {
26+
final options = defaultTestOptions()..tracesSampleRate = 1.0;
27+
final hub = Hub(options);
28+
29+
final tx1 = hub.startTransaction('tx1', 'op') as SentryTracer;
30+
final rand1 = tx1.samplingDecision?.sampleRand;
31+
expect(rand1, isNotNull);
32+
33+
// Start a new trace
34+
hub.generateNewTrace();
35+
36+
final tx2 = hub.startTransaction('tx2', 'op') as SentryTracer;
37+
final rand2 = tx2.samplingDecision?.sampleRand;
38+
39+
expect(rand2, isNotNull);
40+
expect(rand2, isNot(equals(rand1)));
41+
});
42+
});
43+
}

dart/test/sentry_traces_sampler_test.dart

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'test_utils.dart';
66

77
void main() {
88
late Fixture fixture;
9+
const _sampleRand = 1.0;
910

1011
setUp(() {
1112
fixture = Fixture();
@@ -21,7 +22,7 @@ void main() {
2122
);
2223
final context = SentrySamplingContext(trContext, {});
2324

24-
expect(sut.sample(context).sampled, true);
25+
expect(sut.sample(context, _sampleRand).sampled, true);
2526
});
2627

2728
test('options has sampler', () {
@@ -40,7 +41,7 @@ void main() {
4041
);
4142
final context = SentrySamplingContext(trContext, {});
4243

43-
expect(sut.sample(context).sampled, true);
44+
expect(sut.sample(context, _sampleRand).sampled, true);
4445
});
4546

4647
test('transactionContext has parentSampled', () {
@@ -53,7 +54,7 @@ void main() {
5354
);
5455
final context = SentrySamplingContext(trContext, {});
5556

56-
expect(sut.sample(context).sampled, true);
57+
expect(sut.sample(context, _sampleRand).sampled, true);
5758
});
5859

5960
test('options has rate 1.0', () {
@@ -65,7 +66,7 @@ void main() {
6566
);
6667
final context = SentrySamplingContext(trContext, {});
6768

68-
expect(sut.sample(context).sampled, true);
69+
expect(sut.sample(context, _sampleRand).sampled, true);
6970
});
7071

7172
test('options has rate 0.0', () {
@@ -77,7 +78,7 @@ void main() {
7778
);
7879
final context = SentrySamplingContext(trContext, {});
7980

80-
expect(sut.sample(context).sampled, false);
81+
expect(sut.sample(context, _sampleRand).sampled, false);
8182
});
8283

8384
test('does not sample if tracesSampleRate and tracesSampleRate are null', () {
@@ -88,7 +89,7 @@ void main() {
8889
'op',
8990
);
9091
final context = SentrySamplingContext(trContext, {});
91-
final samplingDecision = sut.sample(context);
92+
final samplingDecision = sut.sample(context, _sampleRand);
9293

9394
expect(samplingDecision.sampleRate, isNull);
9495
expect(samplingDecision.sampleRand, isNull);
@@ -111,7 +112,7 @@ void main() {
111112
'op',
112113
);
113114
final context = SentrySamplingContext(trContext, {});
114-
sut.sample(context);
115+
sut.sample(context, _sampleRand);
115116

116117
expect(fixture.loggedException, exception);
117118
expect(fixture.loggedLevel, SentryLevel.error);

drift/test/mocks/mocks.mocks.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -429,9 +429,9 @@ class MockHub extends _i1.Mock implements _i2.Hub {
429429
) as _i2.ISentrySpan);
430430

431431
@override
432-
void generateNewTraceId() => super.noSuchMethod(
432+
void generateNewTrace() => super.noSuchMethod(
433433
Invocation.method(
434-
#generateNewTraceId,
434+
#generateNewTrace,
435435
[],
436436
),
437437
returnValueForMissingStub: null,

0 commit comments

Comments
 (0)