Skip to content

Commit 79ca7e6

Browse files
jamescrosswelljamescrosswell
committed
generate sample_rand when starting new transactions
1 parent 0b22582 commit 79ca7e6

File tree

6 files changed

+150
-5
lines changed

6 files changed

+150
-5
lines changed

‎src/Sentry/DynamicSamplingContext.cs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ private DynamicSamplingContext(
2525
string publicKey,
2626
bool? sampled,
2727
double? sampleRate = null,
28+
double? sampleRand = null,
2829
string? release = null,
2930
string? environment = null,
3031
string? transactionName = null)
@@ -45,6 +46,11 @@ private DynamicSamplingContext(
4546
throw new ArgumentOutOfRangeException(nameof(sampleRate));
4647
}
4748

49+
if (sampleRand is < 0.0 or >= 1.0)
50+
{
51+
throw new ArgumentOutOfRangeException(nameof(sampleRand));
52+
}
53+
4854
var items = new Dictionary<string, string>(capacity: 7)
4955
{
5056
["trace_id"] = traceId.ToString(),
@@ -62,6 +68,11 @@ private DynamicSamplingContext(
6268
items.Add("sample_rate", sampleRate.Value.ToString(CultureInfo.InvariantCulture));
6369
}
6470

71+
if (sampleRand is not null)
72+
{
73+
items.Add("sample_rand", sampleRand.Value.ToString("N4", CultureInfo.InvariantCulture));
74+
}
75+
6576
if (!string.IsNullOrWhiteSpace(release))
6677
{
6778
items.Add("release", release);
@@ -123,8 +134,7 @@ private DynamicSamplingContext(
123134
}
124135
else
125136
{
126-
var seed = FnvHash.ComputeHash(traceId);
127-
var rand = new Random(seed).NextDouble();
137+
var rand = SampleRandHelper.GenerateSampleRand(traceId);
128138
if (!string.IsNullOrEmpty(sampledString))
129139
{
130140
// Ensure sample_rand is consistent with the sampling decision that has already been made
@@ -144,6 +154,7 @@ public static DynamicSamplingContext CreateFromTransaction(TransactionTracer tra
144154
var traceId = transaction.TraceId;
145155
var sampled = transaction.IsSampled;
146156
var sampleRate = transaction.SampleRate!.Value;
157+
var sampleRand = transaction.SampleRand;
147158
var transactionName = transaction.NameSource.IsHighQuality() ? transaction.Name : null;
148159

149160
// These two may not have been set yet on the transaction, but we can get them directly.
@@ -155,6 +166,7 @@ public static DynamicSamplingContext CreateFromTransaction(TransactionTracer tra
155166
publicKey,
156167
sampled,
157168
sampleRate,
169+
sampleRand,
158170
release,
159171
environment,
160172
transactionName);

‎src/Sentry/Internal/Hub.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@ internal ITransactionTracer StartTransaction(
129129
{
130130
var transaction = new TransactionTracer(this, context);
131131

132+
transaction.SampleRand = (dynamicSamplingContext is not null)
133+
? double.Parse(dynamicSamplingContext.Items["sample_rand"], NumberStyles.Float, CultureInfo.InvariantCulture)
134+
: transaction.SampleRand = SampleRandHelper.GenerateSampleRand(context.TraceId.ToString());
135+
132136
// If the hub is disabled, we will always sample out. In other words, starting a transaction
133137
// after disposing the hub will result in that transaction not being sent to Sentry.
134138
// Additionally, we will always sample out if tracing is explicitly disabled.
@@ -151,7 +155,7 @@ internal ITransactionTracer StartTransaction(
151155

152156
if (tracesSampler(samplingContext) is { } sampleRate)
153157
{
154-
transaction.IsSampled = _randomValuesFactory.NextBool(sampleRate);
158+
transaction.IsSampled = SampleRandHelper.IsSampled(transaction.SampleRand.Value, sampleRate);
155159
transaction.SampleRate = sampleRate;
156160
}
157161
}
@@ -160,7 +164,7 @@ internal ITransactionTracer StartTransaction(
160164
if (transaction.IsSampled == null)
161165
{
162166
var sampleRate = _options.TracesSampleRate ?? 0.0;
163-
transaction.IsSampled = _randomValuesFactory.NextBool(sampleRate);
167+
transaction.IsSampled = SampleRandHelper.IsSampled(transaction.SampleRand.Value, sampleRate);
164168
transaction.SampleRate = sampleRate;
165169
}
166170

‎src/Sentry/Internal/SampleRandHelper.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
namespace Sentry.Internal;
2+
3+
internal static class SampleRandHelper
4+
{
5+
internal static double GenerateSampleRand(string traceId)
6+
=> new Random(FnvHash.ComputeHash(traceId)).NextDouble();
7+
8+
internal static bool IsSampled(double sampleRand, double rate) => rate switch
9+
{
10+
>= 1 => true,
11+
<= 0 => false,
12+
_ => sampleRand < rate
13+
};
14+
15+
}

‎src/Sentry/TransactionTracer.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ internal set
100100
/// </summary>
101101
public double? SampleRate { get; internal set; }
102102

103+
internal double? SampleRand { get; set; }
104+
103105
/// <inheritdoc />
104106
public SentryLevel? Level { get; set; }
105107

‎test/Sentry.Tests/DynamicSamplingContextTests.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,7 @@ public void CreateFromTransaction(bool? isSampled)
356356
NameSource = TransactionNameSource.Route,
357357
IsSampled = isSampled,
358358
SampleRate = 0.5,
359+
SampleRand = (isSampled ?? true) ? 0.4000 : 0.6000, // Lower than the sample rate means sampled == true
359360
User =
360361
{
361362
},
@@ -364,7 +365,7 @@ public void CreateFromTransaction(bool? isSampled)
364365
var dsc = transaction.CreateDynamicSamplingContext(options);
365366

366367
Assert.NotNull(dsc);
367-
Assert.Equal(isSampled.HasValue ? 7 : 6, dsc.Items.Count);
368+
Assert.Equal(isSampled.HasValue ? 8 : 7, dsc.Items.Count);
368369
Assert.Equal(traceId.ToString(), Assert.Contains("trace_id", dsc.Items));
369370
Assert.Equal("d4d82fc1c2c4032a83f3a29aa3a3aff", Assert.Contains("public_key", dsc.Items));
370371
if (transaction.IsSampled is { } sampled)
@@ -376,6 +377,7 @@ public void CreateFromTransaction(bool? isSampled)
376377
Assert.DoesNotContain("sampled", dsc.Items);
377378
}
378379
Assert.Equal("0.5", Assert.Contains("sample_rate", dsc.Items));
380+
Assert.Equal((isSampled ?? true) ? "0.4000" : "0.6000", Assert.Contains("sample_rand", dsc.Items));
379381
Assert.Equal("[email protected]", Assert.Contains("release", dsc.Items));
380382
Assert.Equal("staging", Assert.Contains("environment", dsc.Items));
381383
Assert.Equal("GET /person/{id}", Assert.Contains("transaction", dsc.Items));

‎test/Sentry.Tests/HubTests.cs‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -680,6 +680,116 @@ public void StartTransaction_SameInstrumenter_SampledIn()
680680
transaction.IsSampled.Should().BeTrue();
681681
}
682682

683+
[Fact]
684+
public void StartTransaction_NoDynamicSamplingContext_GeneratesSampleRand()
685+
{
686+
// Arrange
687+
var transactionContext = new TransactionContext("name", "operation");
688+
var customContext = new Dictionary<string, object>();
689+
690+
var hub = _fixture.GetSut();
691+
692+
// Act
693+
var transaction = hub.StartTransaction(transactionContext, customContext);
694+
695+
// Assert
696+
var transactionTracer = ((TransactionTracer)transaction);
697+
transactionTracer.SampleRand.Should().NotBeNull();
698+
transactionTracer.DynamicSamplingContext.Should().NotBeNull();
699+
transactionTracer.DynamicSamplingContext!.Items.Should().ContainKey("sample_rand");
700+
transactionTracer.DynamicSamplingContext.Items["sample_rand"].Should().Be(transactionTracer.SampleRand!.Value.ToString("N4", CultureInfo.InvariantCulture));
701+
}
702+
703+
[Fact]
704+
public void StartTransaction_DynamicSamplingContext_InheritsSampleRand()
705+
{
706+
// Arrange
707+
var transactionContext = new TransactionContext("name", "operation");
708+
var customContext = new Dictionary<string, object>();
709+
var dsc = BaggageHeader.Create(new List<KeyValuePair<string, string>>
710+
{
711+
{"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"},
712+
{"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"},
713+
{"sentry-sampled", "true"},
714+
{"sentry-sample_rate", "0.5"}, // Required in the baggage header, but ignored by sampling logic
715+
{"sentry-sample_rand", "0.1234"}
716+
}).CreateDynamicSamplingContext();
717+
718+
_fixture.Options.TracesSampleRate = 0.4;
719+
var hub = _fixture.GetSut();
720+
721+
// Act
722+
var transaction = hub.StartTransaction(transactionContext, customContext, dsc);
723+
724+
// Assert
725+
var transactionTracer = ((TransactionTracer)transaction);
726+
transactionTracer.IsSampled.Should().Be(true);
727+
transactionTracer.SampleRate.Should().Be(0.4);
728+
transactionTracer.SampleRand.Should().Be(0.1234);
729+
transactionTracer.DynamicSamplingContext.Should().Be(dsc);
730+
}
731+
732+
[Theory]
733+
[InlineData(0.1, false)]
734+
[InlineData(0.2, true)]
735+
public void StartTransaction_TraceSampler_UsesSampleRand(double sampleRate, bool expectedIsSampled)
736+
{
737+
// Arrange
738+
var transactionContext = new TransactionContext("name", "operation");
739+
var customContext = new Dictionary<string, object>();
740+
var dsc = BaggageHeader.Create(new List<KeyValuePair<string, string>>
741+
{
742+
{"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"},
743+
{"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"},
744+
{"sentry-sampled", "true"},
745+
{"sentry-sample_rate", "0.5"},
746+
{"sentry-sample_rand", "0.1234"}
747+
}).CreateDynamicSamplingContext();
748+
749+
_fixture.Options.TracesSampler = _ => sampleRate;
750+
var hub = _fixture.GetSut();
751+
752+
// Act
753+
var transaction = hub.StartTransaction(transactionContext, customContext, dsc);
754+
755+
// Assert
756+
var transactionTracer = ((TransactionTracer)transaction);
757+
transactionTracer.IsSampled.Should().Be(expectedIsSampled);
758+
transactionTracer.SampleRate.Should().Be(sampleRate);
759+
transactionTracer.SampleRand.Should().Be(0.1234);
760+
transactionTracer.DynamicSamplingContext.Should().Be(dsc);
761+
}
762+
763+
[Theory]
764+
[InlineData(0.1, false)]
765+
[InlineData(0.2, true)]
766+
public void StartTransaction_StaticSampler_UsesSampleRand(double sampleRate, bool expectedIsSampled)
767+
{
768+
// Arrange
769+
var transactionContext = new TransactionContext("name", "operation");
770+
var customContext = new Dictionary<string, object>();
771+
var dsc = BaggageHeader.Create(new List<KeyValuePair<string, string>>
772+
{
773+
{"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"},
774+
{"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"},
775+
{"sentry-sample_rate", "0.5"}, // Static sampling ignores this and uses options.TracesSampleRate instead
776+
{"sentry-sample_rand", "0.1234"}
777+
}).CreateDynamicSamplingContext();
778+
779+
_fixture.Options.TracesSampleRate = sampleRate;
780+
var hub = _fixture.GetSut();
781+
782+
// Act
783+
var transaction = hub.StartTransaction(transactionContext, customContext, dsc);
784+
785+
// Assert
786+
var transactionTracer = ((TransactionTracer)transaction);
787+
transactionTracer.IsSampled.Should().Be(expectedIsSampled);
788+
transactionTracer.SampleRate.Should().Be(sampleRate);
789+
transactionTracer.SampleRand.Should().Be(0.1234);
790+
transactionTracer.DynamicSamplingContext.Should().Be(dsc);
791+
}
792+
683793
[Fact]
684794
public void StartTransaction_DifferentInstrumenter_SampledIn()
685795
{

0 commit comments

Comments
 (0)