From 02593ece9d14947cc3e7206eeff55b93c807112c Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Mon, 10 Feb 2025 18:43:01 +1300 Subject: [PATCH 01/10] Implemented FnvHash --- src/Sentry/Internal/FnvHash.cs | 43 +++++++++++++++++++++ test/Sentry.Tests/Internals/FnvHashTests.cs | 24 ++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 src/Sentry/Internal/FnvHash.cs create mode 100644 test/Sentry.Tests/Internals/FnvHashTests.cs diff --git a/src/Sentry/Internal/FnvHash.cs b/src/Sentry/Internal/FnvHash.cs new file mode 100644 index 0000000000..10281e90bd --- /dev/null +++ b/src/Sentry/Internal/FnvHash.cs @@ -0,0 +1,43 @@ +namespace Sentry.Internal; + +/// +/// FNV is a non-cryptographic hash. +/// +/// See https://en.wikipedia.org/wiki/Fowler%E2%80%93Noll%E2%80%93Vo_hash_function#FNV_hash_parameters +/// +/// +/// We use a struct to avoid heap allocations. +/// +internal struct FnvHash +{ + public FnvHash() + { + } + + private const int Offset = unchecked((int)2166136261); + private const int Prime = 16777619; + + private int HashCode { get; set; } = Offset; + + private void Combine(byte data) + { + unchecked + { + HashCode ^= data; + HashCode *= Prime; + } + } + + private static int ComputeHash(byte[] data) + { + var result = new FnvHash(); + foreach (var b in data) + { + result.Combine(b); + } + + return result.HashCode; + } + + public static int ComputeHash(string data) => ComputeHash(Encoding.UTF8.GetBytes(data)); +} diff --git a/test/Sentry.Tests/Internals/FnvHashTests.cs b/test/Sentry.Tests/Internals/FnvHashTests.cs new file mode 100644 index 0000000000..757882c3de --- /dev/null +++ b/test/Sentry.Tests/Internals/FnvHashTests.cs @@ -0,0 +1,24 @@ +namespace Sentry.Tests.Internals; + +public class FnvHashTests +{ + [Theory] + [InlineData("", 2_166_136_261)] + [InlineData("h", 3_977_000_791)] + [InlineData("he", 1_547_363_254)] + [InlineData("hel", 179_613_742)] + [InlineData("hell", 477_198_310)] + [InlineData("hello", 1_335_831_723)] + [InlineData("hello ", 3_801_292_497)] + [InlineData("hello w", 1_402_552_146)] + [InlineData("hello wo", 3_611_200_775)] + [InlineData("hello wor", 1_282_977_583)] + [InlineData("hello worl", 2_767_971_961)] + [InlineData("hello world", 3_582_672_807)] + public void ComputeHash_WithString_ReturnsExpected(string input, uint expected) + { + var actual = FnvHash.ComputeHash(input); + + Assert.Equal(unchecked((int)expected), actual); + } +} From 3e7034d2571b440c44530b9a992e35d7ee5674d9 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 11 Feb 2025 15:03:50 +1300 Subject: [PATCH 02/10] sample_rand now added to DSC from BaggageHeaders --- src/Sentry/BaggageHeader.cs | 2 +- src/Sentry/DynamicSamplingContext.cs | 16 +++ .../DynamicSamplingContextTests.cs | 100 ++++++++++++++++++ 3 files changed, 117 insertions(+), 1 deletion(-) diff --git a/src/Sentry/BaggageHeader.cs b/src/Sentry/BaggageHeader.cs index 83fe9f0a84..66846a1fcd 100644 --- a/src/Sentry/BaggageHeader.cs +++ b/src/Sentry/BaggageHeader.cs @@ -26,7 +26,7 @@ private BaggageHeader(IEnumerable> members) => // We can safely return a dictionary of Sentry members, as we are in control over the keys added. // Just to be safe though, we'll group by key and only take the first of each one. - internal IReadOnlyDictionary GetSentryMembers() => + internal Dictionary GetSentryMembers() => Members .Where(kvp => kvp.Key.StartsWith(SentryKeyPrefix)) .GroupBy(kvp => kvp.Key, kvp => kvp.Value) diff --git a/src/Sentry/DynamicSamplingContext.cs b/src/Sentry/DynamicSamplingContext.cs index 26056d17cc..e5a0253f93 100644 --- a/src/Sentry/DynamicSamplingContext.cs +++ b/src/Sentry/DynamicSamplingContext.cs @@ -1,3 +1,4 @@ +using Sentry.Internal; using Sentry.Internal.Extensions; namespace Sentry; @@ -111,6 +112,21 @@ private DynamicSamplingContext( return null; } + // See https://develop.sentry.dev/sdk/telemetry/traces/#propagated-random-value + if (items.TryGetValue("sample_rand", out var sampleRand)) + { + if (!double.TryParse(sampleRand, NumberStyles.Float, CultureInfo.InvariantCulture, out var rand) || + rand is < 0.0 or >= 1.0) + { + return null; + } + } + else + { + var seed = FnvHash.ComputeHash(traceId); + var rand = new Random(seed).NextDouble() * rate; + items.Add("sample_rand", rand.ToString("N4", CultureInfo.InvariantCulture)); + } return new DynamicSamplingContext(items); } diff --git a/test/Sentry.Tests/DynamicSamplingContextTests.cs b/test/Sentry.Tests/DynamicSamplingContextTests.cs index b23a1c4962..a5ecfb57db 100644 --- a/test/Sentry.Tests/DynamicSamplingContextTests.cs +++ b/test/Sentry.Tests/DynamicSamplingContextTests.cs @@ -142,6 +142,104 @@ public void CreateFromBaggage_SampleRate_TooHigh() Assert.Null(dsc); } + [Fact] + public void CreateFromBaggage_SampleRand_Invalid() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "1.0"}, + {"sentry-sample_rand", "not-a-number"}, + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + Assert.Null(dsc); + } + + [Fact] + public void CreateFromBaggage_SampleRand_TooLow() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "1.0"}, + {"sentry-sample_rand", "-0.1"} + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + Assert.Null(dsc); + } + + [Fact] + public void CreateFromBaggage_SampleRand_TooHigh() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "1.0"}, + {"sentry-sample_rand", "1.0"} // Must be less than 1 + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + Assert.Null(dsc); + } + + [Fact] + public void CreateFromBaggage_NotSampledNoSampleRand_GeneratesSampleRand() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "0.5"} + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + using var scope = new AssertionScope(); + Assert.NotNull(dsc); + var sampleRandItem = Assert.Contains("sample_rand", dsc.Items); + var sampleRand = double.Parse(sampleRandItem, NumberStyles.Float, CultureInfo.InvariantCulture); + Assert.True(sampleRand >= 0.0); + Assert.True(sampleRand < 1.0); + } + + [Theory] + [InlineData("true")] + public void CreateFromBaggage_SampledNoSampleRand_GeneratesConsistentSampleRand(string sampled) + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "0.5"}, + {"sentry-sampled", sampled}, + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + using var scope = new AssertionScope(); + Assert.NotNull(dsc); + var sampleRandItem = Assert.Contains("sample_rand", dsc.Items); + var sampleRand = double.Parse(sampleRandItem, NumberStyles.Float, CultureInfo.InvariantCulture); + if (sampled == "true") + { + Assert.True(sampleRand >= 0.0); + Assert.True(sampleRand < 0.5); + } + else + { + Assert.True(sampleRand >= 0.5); + Assert.True(sampleRand < 1.0); + } + } + [Fact] public void CreateFromBaggage_Sampled_MalFormed() { @@ -186,6 +284,7 @@ public void CreateFromBaggage_Valid_Complete() {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, {"sentry-sampled", "true"}, {"sentry-sample_rate", "1.0"}, + {"sentry-sample_rand", "0.1234"}, {"sentry-release", "test@1.0.0+abc"}, {"sentry-environment", "production"}, {"sentry-user_segment", "Group B"}, @@ -200,6 +299,7 @@ public void CreateFromBaggage_Valid_Complete() Assert.Equal("d4d82fc1c2c4032a83f3a29aa3a3aff", Assert.Contains("public_key", dsc.Items)); Assert.Equal("true", Assert.Contains("sampled", dsc.Items)); Assert.Equal("1.0", Assert.Contains("sample_rate", dsc.Items)); + Assert.Equal("0.1234", Assert.Contains("sample_rand", dsc.Items)); Assert.Equal("test@1.0.0+abc", Assert.Contains("release", dsc.Items)); Assert.Equal("production", Assert.Contains("environment", dsc.Items)); Assert.Equal("Group B", Assert.Contains("user_segment", dsc.Items)); From 0b22582d043ada841debe4ae0a6b4f981398f4bb Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 11 Feb 2025 16:29:02 +1300 Subject: [PATCH 03/10] Fixed unit tests --- src/Sentry/DynamicSamplingContext.cs | 11 +++++++++-- .../SentryTracingMiddlewareTests.cs | 3 +++ test/Sentry.Tests/DynamicSamplingContextTests.cs | 5 ++++- test/Sentry.Tests/SentryPropagationContextTests.cs | 3 ++- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/Sentry/DynamicSamplingContext.cs b/src/Sentry/DynamicSamplingContext.cs index e5a0253f93..cbd32413b9 100644 --- a/src/Sentry/DynamicSamplingContext.cs +++ b/src/Sentry/DynamicSamplingContext.cs @@ -100,7 +100,7 @@ private DynamicSamplingContext( return null; } - if (items.TryGetValue("sampled", out var sampledString) && !bool.TryParse(sampledString, out _)) + if (items.TryGetValue("sampled", out var sampledString) && !bool.TryParse(sampledString, out var sampled)) { return null; } @@ -124,7 +124,14 @@ private DynamicSamplingContext( else { var seed = FnvHash.ComputeHash(traceId); - var rand = new Random(seed).NextDouble() * rate; + var rand = new Random(seed).NextDouble(); + if (!string.IsNullOrEmpty(sampledString)) + { + // Ensure sample_rand is consistent with the sampling decision that has already been made + rand = bool.Parse(sampledString) + ? rand * rate // 0 <= sampleRand < rate + : rate + (1 - rate) * rand; // rate < sampleRand < 1 + } items.Add("sample_rand", rand.ToString("N4", CultureInfo.InvariantCulture)); } return new DynamicSamplingContext(items); diff --git a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs index 3c2d440c7f..cabd08654d 100644 --- a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs +++ b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs @@ -285,6 +285,7 @@ public async Task Baggage_header_propagates_to_outbound_requests(bool shouldProp const string incomingBaggage = "sentry-trace_id=75302ac48a024bde9a3b3734a82e36c8, " + "sentry-public_key=d4d82fc1c2c4032a83f3a29aa3a3aff, " + + "sentry-sample_rand=0.1234, " + "sentry-sample_rate=0.5, " + "foo-bar=abc123"; @@ -299,6 +300,7 @@ public async Task Baggage_header_propagates_to_outbound_requests(bool shouldProp "other-value=abc123, " + "sentry-trace_id=75302ac48a024bde9a3b3734a82e36c8, " + "sentry-public_key=d4d82fc1c2c4032a83f3a29aa3a3aff, " + + "sentry-sample_rand=0.1234, " + "sentry-sample_rate=0.5"; } else @@ -382,6 +384,7 @@ public async Task Baggage_header_sets_dynamic_sampling_context() const string baggage = "sentry-trace_id=75302ac48a024bde9a3b3734a82e36c8, " + "sentry-public_key=d4d82fc1c2c4032a83f3a29aa3a3aff, " + + "sentry-sample_rand=0.1234, " + "sentry-sample_rate=0.5"; // Arrange diff --git a/test/Sentry.Tests/DynamicSamplingContextTests.cs b/test/Sentry.Tests/DynamicSamplingContextTests.cs index a5ecfb57db..5ab03f97c5 100644 --- a/test/Sentry.Tests/DynamicSamplingContextTests.cs +++ b/test/Sentry.Tests/DynamicSamplingContextTests.cs @@ -212,6 +212,7 @@ public void CreateFromBaggage_NotSampledNoSampleRand_GeneratesSampleRand() [Theory] [InlineData("true")] + [InlineData("false")] public void CreateFromBaggage_SampledNoSampleRand_GeneratesConsistentSampleRand(string sampled) { var baggage = BaggageHeader.Create(new List> @@ -269,10 +270,11 @@ public void CreateFromBaggage_Valid_Minimum() var dsc = baggage.CreateDynamicSamplingContext(); Assert.NotNull(dsc); - Assert.Equal(3, dsc.Items.Count); + Assert.Equal(4, dsc.Items.Count); Assert.Equal("43365712692146d08ee11a729dfbcaca", Assert.Contains("trace_id", dsc.Items)); Assert.Equal("d4d82fc1c2c4032a83f3a29aa3a3aff", Assert.Contains("public_key", dsc.Items)); Assert.Equal("1.0", Assert.Contains("sample_rate", dsc.Items)); + Assert.Contains("sample_rand", dsc.Items); } [Fact] @@ -314,6 +316,7 @@ public void ToBaggageHeader() {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, {"sentry-sample_rate", "1.0"}, + {"sentry-sample_rand", "0.1234"}, {"sentry-release", "test@1.0.0+abc"}, {"sentry-environment", "production"}, {"sentry-user_segment", "Group B"}, diff --git a/test/Sentry.Tests/SentryPropagationContextTests.cs b/test/Sentry.Tests/SentryPropagationContextTests.cs index 89dfa87957..dc658d2ed9 100644 --- a/test/Sentry.Tests/SentryPropagationContextTests.cs +++ b/test/Sentry.Tests/SentryPropagationContextTests.cs @@ -83,6 +83,7 @@ public void CreateFromHeaders_BaggageHeaderNotNull_CreatesPropagationContextWith var baggageHeader = BaggageHeader.Create(new List> { { "sentry-sample_rate", "1.0" }, + { "sentry-sample_rand", "0.1234" }, { "sentry-trace_id", "75302ac48a024bde9a3b3734a82e36c8" }, { "sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff" }, { "sentry-replay_id", "bfd31b89a59d41c99d96dc2baf840ecd" } @@ -90,6 +91,6 @@ public void CreateFromHeaders_BaggageHeaderNotNull_CreatesPropagationContextWith var propagationContext = SentryPropagationContext.CreateFromHeaders(null, traceHeader, baggageHeader); - Assert.Equal(4, propagationContext.GetOrCreateDynamicSamplingContext(new SentryOptions()).Items.Count); + Assert.Equal(5, propagationContext.GetOrCreateDynamicSamplingContext(new SentryOptions()).Items.Count); } } From 79ca7e6a91106531b1647137496bdd7acbfcd7d7 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 11 Feb 2025 22:44:36 +1300 Subject: [PATCH 04/10] generate sample_rand when starting new transactions --- src/Sentry/DynamicSamplingContext.cs | 16 ++- src/Sentry/Internal/Hub.cs | 8 +- src/Sentry/Internal/SampleRandHelper.cs | 15 +++ src/Sentry/TransactionTracer.cs | 2 + .../DynamicSamplingContextTests.cs | 4 +- test/Sentry.Tests/HubTests.cs | 110 ++++++++++++++++++ 6 files changed, 150 insertions(+), 5 deletions(-) create mode 100644 src/Sentry/Internal/SampleRandHelper.cs diff --git a/src/Sentry/DynamicSamplingContext.cs b/src/Sentry/DynamicSamplingContext.cs index cbd32413b9..8090e99e32 100644 --- a/src/Sentry/DynamicSamplingContext.cs +++ b/src/Sentry/DynamicSamplingContext.cs @@ -25,6 +25,7 @@ private DynamicSamplingContext( string publicKey, bool? sampled, double? sampleRate = null, + double? sampleRand = null, string? release = null, string? environment = null, string? transactionName = null) @@ -45,6 +46,11 @@ private DynamicSamplingContext( throw new ArgumentOutOfRangeException(nameof(sampleRate)); } + if (sampleRand is < 0.0 or >= 1.0) + { + throw new ArgumentOutOfRangeException(nameof(sampleRand)); + } + var items = new Dictionary(capacity: 7) { ["trace_id"] = traceId.ToString(), @@ -62,6 +68,11 @@ private DynamicSamplingContext( items.Add("sample_rate", sampleRate.Value.ToString(CultureInfo.InvariantCulture)); } + if (sampleRand is not null) + { + items.Add("sample_rand", sampleRand.Value.ToString("N4", CultureInfo.InvariantCulture)); + } + if (!string.IsNullOrWhiteSpace(release)) { items.Add("release", release); @@ -123,8 +134,7 @@ private DynamicSamplingContext( } else { - var seed = FnvHash.ComputeHash(traceId); - var rand = new Random(seed).NextDouble(); + var rand = SampleRandHelper.GenerateSampleRand(traceId); if (!string.IsNullOrEmpty(sampledString)) { // Ensure sample_rand is consistent with the sampling decision that has already been made @@ -144,6 +154,7 @@ public static DynamicSamplingContext CreateFromTransaction(TransactionTracer tra var traceId = transaction.TraceId; var sampled = transaction.IsSampled; var sampleRate = transaction.SampleRate!.Value; + var sampleRand = transaction.SampleRand; var transactionName = transaction.NameSource.IsHighQuality() ? transaction.Name : null; // 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 publicKey, sampled, sampleRate, + sampleRand, release, environment, transactionName); diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index b90f4ca603..4bb5484fa4 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -129,6 +129,10 @@ internal ITransactionTracer StartTransaction( { var transaction = new TransactionTracer(this, context); + transaction.SampleRand = (dynamicSamplingContext is not null) + ? double.Parse(dynamicSamplingContext.Items["sample_rand"], NumberStyles.Float, CultureInfo.InvariantCulture) + : transaction.SampleRand = SampleRandHelper.GenerateSampleRand(context.TraceId.ToString()); + // If the hub is disabled, we will always sample out. In other words, starting a transaction // after disposing the hub will result in that transaction not being sent to Sentry. // Additionally, we will always sample out if tracing is explicitly disabled. @@ -151,7 +155,7 @@ internal ITransactionTracer StartTransaction( if (tracesSampler(samplingContext) is { } sampleRate) { - transaction.IsSampled = _randomValuesFactory.NextBool(sampleRate); + transaction.IsSampled = SampleRandHelper.IsSampled(transaction.SampleRand.Value, sampleRate); transaction.SampleRate = sampleRate; } } @@ -160,7 +164,7 @@ internal ITransactionTracer StartTransaction( if (transaction.IsSampled == null) { var sampleRate = _options.TracesSampleRate ?? 0.0; - transaction.IsSampled = _randomValuesFactory.NextBool(sampleRate); + transaction.IsSampled = SampleRandHelper.IsSampled(transaction.SampleRand.Value, sampleRate); transaction.SampleRate = sampleRate; } diff --git a/src/Sentry/Internal/SampleRandHelper.cs b/src/Sentry/Internal/SampleRandHelper.cs new file mode 100644 index 0000000000..5e420c70f8 --- /dev/null +++ b/src/Sentry/Internal/SampleRandHelper.cs @@ -0,0 +1,15 @@ +namespace Sentry.Internal; + +internal static class SampleRandHelper +{ + internal static double GenerateSampleRand(string traceId) + => new Random(FnvHash.ComputeHash(traceId)).NextDouble(); + + internal static bool IsSampled(double sampleRand, double rate) => rate switch + { + >= 1 => true, + <= 0 => false, + _ => sampleRand < rate + }; + +} diff --git a/src/Sentry/TransactionTracer.cs b/src/Sentry/TransactionTracer.cs index 69376ad71f..ecf0ee756c 100644 --- a/src/Sentry/TransactionTracer.cs +++ b/src/Sentry/TransactionTracer.cs @@ -100,6 +100,8 @@ internal set /// public double? SampleRate { get; internal set; } + internal double? SampleRand { get; set; } + /// public SentryLevel? Level { get; set; } diff --git a/test/Sentry.Tests/DynamicSamplingContextTests.cs b/test/Sentry.Tests/DynamicSamplingContextTests.cs index 5ab03f97c5..374be0c371 100644 --- a/test/Sentry.Tests/DynamicSamplingContextTests.cs +++ b/test/Sentry.Tests/DynamicSamplingContextTests.cs @@ -356,6 +356,7 @@ public void CreateFromTransaction(bool? isSampled) NameSource = TransactionNameSource.Route, IsSampled = isSampled, SampleRate = 0.5, + SampleRand = (isSampled ?? true) ? 0.4000 : 0.6000, // Lower than the sample rate means sampled == true User = { }, @@ -364,7 +365,7 @@ public void CreateFromTransaction(bool? isSampled) var dsc = transaction.CreateDynamicSamplingContext(options); Assert.NotNull(dsc); - Assert.Equal(isSampled.HasValue ? 7 : 6, dsc.Items.Count); + Assert.Equal(isSampled.HasValue ? 8 : 7, dsc.Items.Count); Assert.Equal(traceId.ToString(), Assert.Contains("trace_id", dsc.Items)); Assert.Equal("d4d82fc1c2c4032a83f3a29aa3a3aff", Assert.Contains("public_key", dsc.Items)); if (transaction.IsSampled is { } sampled) @@ -376,6 +377,7 @@ public void CreateFromTransaction(bool? isSampled) Assert.DoesNotContain("sampled", dsc.Items); } Assert.Equal("0.5", Assert.Contains("sample_rate", dsc.Items)); + Assert.Equal((isSampled ?? true) ? "0.4000" : "0.6000", Assert.Contains("sample_rand", dsc.Items)); Assert.Equal("foo@2.4.5", Assert.Contains("release", dsc.Items)); Assert.Equal("staging", Assert.Contains("environment", dsc.Items)); Assert.Equal("GET /person/{id}", Assert.Contains("transaction", dsc.Items)); diff --git a/test/Sentry.Tests/HubTests.cs b/test/Sentry.Tests/HubTests.cs index 17206a5c4f..506d69657a 100644 --- a/test/Sentry.Tests/HubTests.cs +++ b/test/Sentry.Tests/HubTests.cs @@ -680,6 +680,116 @@ public void StartTransaction_SameInstrumenter_SampledIn() transaction.IsSampled.Should().BeTrue(); } + [Fact] + public void StartTransaction_NoDynamicSamplingContext_GeneratesSampleRand() + { + // Arrange + var transactionContext = new TransactionContext("name", "operation"); + var customContext = new Dictionary(); + + var hub = _fixture.GetSut(); + + // Act + var transaction = hub.StartTransaction(transactionContext, customContext); + + // Assert + var transactionTracer = ((TransactionTracer)transaction); + transactionTracer.SampleRand.Should().NotBeNull(); + transactionTracer.DynamicSamplingContext.Should().NotBeNull(); + transactionTracer.DynamicSamplingContext!.Items.Should().ContainKey("sample_rand"); + transactionTracer.DynamicSamplingContext.Items["sample_rand"].Should().Be(transactionTracer.SampleRand!.Value.ToString("N4", CultureInfo.InvariantCulture)); + } + + [Fact] + public void StartTransaction_DynamicSamplingContext_InheritsSampleRand() + { + // Arrange + var transactionContext = new TransactionContext("name", "operation"); + var customContext = new Dictionary(); + var dsc = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sampled", "true"}, + {"sentry-sample_rate", "0.5"}, // Required in the baggage header, but ignored by sampling logic + {"sentry-sample_rand", "0.1234"} + }).CreateDynamicSamplingContext(); + + _fixture.Options.TracesSampleRate = 0.4; + var hub = _fixture.GetSut(); + + // Act + var transaction = hub.StartTransaction(transactionContext, customContext, dsc); + + // Assert + var transactionTracer = ((TransactionTracer)transaction); + transactionTracer.IsSampled.Should().Be(true); + transactionTracer.SampleRate.Should().Be(0.4); + transactionTracer.SampleRand.Should().Be(0.1234); + transactionTracer.DynamicSamplingContext.Should().Be(dsc); + } + + [Theory] + [InlineData(0.1, false)] + [InlineData(0.2, true)] + public void StartTransaction_TraceSampler_UsesSampleRand(double sampleRate, bool expectedIsSampled) + { + // Arrange + var transactionContext = new TransactionContext("name", "operation"); + var customContext = new Dictionary(); + var dsc = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sampled", "true"}, + {"sentry-sample_rate", "0.5"}, + {"sentry-sample_rand", "0.1234"} + }).CreateDynamicSamplingContext(); + + _fixture.Options.TracesSampler = _ => sampleRate; + var hub = _fixture.GetSut(); + + // Act + var transaction = hub.StartTransaction(transactionContext, customContext, dsc); + + // Assert + var transactionTracer = ((TransactionTracer)transaction); + transactionTracer.IsSampled.Should().Be(expectedIsSampled); + transactionTracer.SampleRate.Should().Be(sampleRate); + transactionTracer.SampleRand.Should().Be(0.1234); + transactionTracer.DynamicSamplingContext.Should().Be(dsc); + } + + [Theory] + [InlineData(0.1, false)] + [InlineData(0.2, true)] + public void StartTransaction_StaticSampler_UsesSampleRand(double sampleRate, bool expectedIsSampled) + { + // Arrange + var transactionContext = new TransactionContext("name", "operation"); + var customContext = new Dictionary(); + var dsc = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "0.5"}, // Static sampling ignores this and uses options.TracesSampleRate instead + {"sentry-sample_rand", "0.1234"} + }).CreateDynamicSamplingContext(); + + _fixture.Options.TracesSampleRate = sampleRate; + var hub = _fixture.GetSut(); + + // Act + var transaction = hub.StartTransaction(transactionContext, customContext, dsc); + + // Assert + var transactionTracer = ((TransactionTracer)transaction); + transactionTracer.IsSampled.Should().Be(expectedIsSampled); + transactionTracer.SampleRate.Should().Be(sampleRate); + transactionTracer.SampleRand.Should().Be(0.1234); + transactionTracer.DynamicSamplingContext.Should().Be(dsc); + } + [Fact] public void StartTransaction_DifferentInstrumenter_SampledIn() { From 6feb7b74f5090e170808285290574ab216d3eaa0 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 12 Feb 2025 14:11:54 +1300 Subject: [PATCH 05/10] Srcub sample_rand values from verify tests --- src/Sentry/DynamicSamplingContext.cs | 2 +- src/Sentry/Internal/Hub.cs | 11 +++++----- .../SentryTracingMiddlewareTests.cs | 3 --- test/Sentry.Testing/VerifyExtensions.cs | 1 + ...rocessorTests.WithTransaction.verified.txt | 1 + ...ctionEndedAsCrashed.DotNet8_0.verified.txt | 2 ++ ...ctionEndedAsCrashed.DotNet9_0.verified.txt | 2 ++ test/Sentry.Tests/HubTests.cs | 22 ++++++++++++++++++- ...sactionProcessorTests.Discard.verified.txt | 1 + ...nsactionProcessorTests.Simple.verified.txt | 2 ++ 10 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/Sentry/DynamicSamplingContext.cs b/src/Sentry/DynamicSamplingContext.cs index 8090e99e32..3e996f1132 100644 --- a/src/Sentry/DynamicSamplingContext.cs +++ b/src/Sentry/DynamicSamplingContext.cs @@ -51,7 +51,7 @@ private DynamicSamplingContext( throw new ArgumentOutOfRangeException(nameof(sampleRand)); } - var items = new Dictionary(capacity: 7) + var items = new Dictionary(capacity: 8) { ["trace_id"] = traceId.ToString(), ["public_key"] = publicKey, diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index 4bb5484fa4..a564ab1791 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -127,11 +127,12 @@ internal ITransactionTracer StartTransaction( IReadOnlyDictionary customSamplingContext, DynamicSamplingContext? dynamicSamplingContext) { - var transaction = new TransactionTracer(this, context); - - transaction.SampleRand = (dynamicSamplingContext is not null) - ? double.Parse(dynamicSamplingContext.Items["sample_rand"], NumberStyles.Float, CultureInfo.InvariantCulture) - : transaction.SampleRand = SampleRandHelper.GenerateSampleRand(context.TraceId.ToString()); + var transaction = new TransactionTracer(this, context) + { + SampleRand = dynamicSamplingContext?.Items.TryGetValue("sample_rand", out var sampleRand) ?? false + ? double.Parse(sampleRand, NumberStyles.Float, CultureInfo.InvariantCulture) + : SampleRandHelper.GenerateSampleRand(context.TraceId.ToString()) + }; // If the hub is disabled, we will always sample out. In other words, starting a transaction // after disposing the hub will result in that transaction not being sent to Sentry. diff --git a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs index cabd08654d..13b34e7385 100644 --- a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs +++ b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs @@ -1,4 +1,3 @@ -#if NETCOREAPP3_1_OR_GREATER using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; @@ -680,5 +679,3 @@ public async Task Transaction_TransactionNameProviderSetUnset_TransactionNameSet transaction.NameSource.Should().Be(TransactionNameSource.Url); } } - -#endif diff --git a/test/Sentry.Testing/VerifyExtensions.cs b/test/Sentry.Testing/VerifyExtensions.cs index 7027580f9c..c3b3a1585f 100644 --- a/test/Sentry.Testing/VerifyExtensions.cs +++ b/test/Sentry.Testing/VerifyExtensions.cs @@ -11,6 +11,7 @@ public static SettingsTask IgnoreStandardSentryMembers(this SettingsTask setting return settings .ScrubMachineName() .ScrubUserName() + .ScrubMember("sample_rand") .AddExtraSettings(_ => { _.Converters.Add(new SpansConverter()); diff --git a/test/Sentry.Tests/EventProcessorTests.WithTransaction.verified.txt b/test/Sentry.Tests/EventProcessorTests.WithTransaction.verified.txt index af9d84138f..a39b4497ea 100644 --- a/test/Sentry.Tests/EventProcessorTests.WithTransaction.verified.txt +++ b/test/Sentry.Tests/EventProcessorTests.WithTransaction.verified.txt @@ -9,6 +9,7 @@ environment: production, public_key: d4d82fc1c2c4032a83f3a29aa3a3aff, release: release, + sample_rand: {Scrubbed}, sample_rate: 1, sampled: true, trace_id: Guid_2, diff --git a/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.DotNet8_0.verified.txt b/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.DotNet8_0.verified.txt index 34439cfbcc..c510d50d43 100644 --- a/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.DotNet8_0.verified.txt +++ b/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.DotNet8_0.verified.txt @@ -31,6 +31,7 @@ environment: production, public_key: d4d82fc1c2c4032a83f3a29aa3a3aff, release: release, + sample_rand: {Scrubbed}, sample_rate: 1, sampled: true, trace_id: Guid_3, @@ -146,6 +147,7 @@ environment: production, public_key: d4d82fc1c2c4032a83f3a29aa3a3aff, release: release, + sample_rand: {Scrubbed}, sample_rate: 1, sampled: true, trace_id: Guid_3, diff --git a/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.DotNet9_0.verified.txt b/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.DotNet9_0.verified.txt index 34439cfbcc..c510d50d43 100644 --- a/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.DotNet9_0.verified.txt +++ b/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.DotNet9_0.verified.txt @@ -31,6 +31,7 @@ environment: production, public_key: d4d82fc1c2c4032a83f3a29aa3a3aff, release: release, + sample_rand: {Scrubbed}, sample_rate: 1, sampled: true, trace_id: Guid_3, @@ -146,6 +147,7 @@ environment: production, public_key: d4d82fc1c2c4032a83f3a29aa3a3aff, release: release, + sample_rand: {Scrubbed}, sample_rate: 1, sampled: true, trace_id: Guid_3, diff --git a/test/Sentry.Tests/HubTests.cs b/test/Sentry.Tests/HubTests.cs index 506d69657a..b00cc89e7f 100644 --- a/test/Sentry.Tests/HubTests.cs +++ b/test/Sentry.Tests/HubTests.cs @@ -701,7 +701,27 @@ public void StartTransaction_NoDynamicSamplingContext_GeneratesSampleRand() } [Fact] - public void StartTransaction_DynamicSamplingContext_InheritsSampleRand() + public void StartTransaction_DynamicSamplingContextWithoutSampleRand_SampleRandNotPropagated() + { + // Arrange + var transactionContext = new TransactionContext("name", "operation"); + var customContext = new Dictionary(); + + var hub = _fixture.GetSut(); + + // Act + var transaction = hub.StartTransaction(transactionContext, customContext, DynamicSamplingContext.Empty); + + // Assert + var transactionTracer = ((TransactionTracer)transaction); + transactionTracer.SampleRand.Should().NotBeNull(); + transactionTracer.DynamicSamplingContext.Should().NotBeNull(); + // See https://develop.sentry.dev/sdk/telemetry/traces/dynamic-sampling-context/#freezing-dynamic-sampling-context + transactionTracer.DynamicSamplingContext!.Items.Should().NotContainKey("sample_rand"); + } + + [Fact] + public void StartTransaction_DynamicSamplingContextWithSampleRand_InheritsSampleRand() { // Arrange var transactionContext = new TransactionContext("name", "operation"); diff --git a/test/Sentry.Tests/TransactionProcessorTests.Discard.verified.txt b/test/Sentry.Tests/TransactionProcessorTests.Discard.verified.txt index 6ffc1f77f4..1c801c9b72 100644 --- a/test/Sentry.Tests/TransactionProcessorTests.Discard.verified.txt +++ b/test/Sentry.Tests/TransactionProcessorTests.Discard.verified.txt @@ -9,6 +9,7 @@ environment: production, public_key: d4d82fc1c2c4032a83f3a29aa3a3aff, release: release, + sample_rand: {Scrubbed}, sample_rate: 1, sampled: true, trace_id: Guid_2, diff --git a/test/Sentry.Tests/TransactionProcessorTests.Simple.verified.txt b/test/Sentry.Tests/TransactionProcessorTests.Simple.verified.txt index b8a5f7a62b..0e9cc2a73b 100644 --- a/test/Sentry.Tests/TransactionProcessorTests.Simple.verified.txt +++ b/test/Sentry.Tests/TransactionProcessorTests.Simple.verified.txt @@ -9,6 +9,7 @@ environment: production, public_key: d4d82fc1c2c4032a83f3a29aa3a3aff, release: release, + sample_rand: {Scrubbed}, sample_rate: 1, sampled: true, trace_id: Guid_2, @@ -53,6 +54,7 @@ environment: production, public_key: d4d82fc1c2c4032a83f3a29aa3a3aff, release: release, + sample_rand: {Scrubbed}, sample_rate: 1, sampled: true, trace_id: Guid_2, From 54d3293bbcacc4f32e0b4307b461022c7768a800 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 12 Feb 2025 14:14:35 +1300 Subject: [PATCH 06/10] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 844ba4106e..8f99ac3e19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +### Features +- The sample seed used for sampling decisions is now propagated, for use in downstream custom trace samplers ([#3951](https://github.com/getsentry/sentry-dotnet/pull/3951)) + ### Fixes - Native SIGSEGV errors resulting from managed NullReferenceExceptions are now suppressed on Android ([#3903](https://github.com/getsentry/sentry-dotnet/pull/3903)) From e486d84f5f55a62595f672c7ad4910173cf6ddab Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 12 Feb 2025 14:46:11 +1300 Subject: [PATCH 07/10] Update HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.Net4_8.verified.txt --- ...handledExceptionTransactionEndedAsCrashed.Net4_8.verified.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.Net4_8.verified.txt b/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.Net4_8.verified.txt index b453712534..6dbb863e8f 100644 --- a/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.Net4_8.verified.txt +++ b/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.Net4_8.verified.txt @@ -31,6 +31,7 @@ environment: production, public_key: d4d82fc1c2c4032a83f3a29aa3a3aff, release: release, + sample_rand: {Scrubbed}, sample_rate: 1, sampled: true, trace_id: Guid_3, From 03d948fdfc1b2021886f8414388fe44f1d020a39 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 12 Feb 2025 17:14:45 +1300 Subject: [PATCH 08/10] Update HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.Net4_8.verified.txt --- ...handledExceptionTransactionEndedAsCrashed.Net4_8.verified.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.Net4_8.verified.txt b/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.Net4_8.verified.txt index 6dbb863e8f..f41bcaa626 100644 --- a/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.Net4_8.verified.txt +++ b/test/Sentry.Tests/HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.Net4_8.verified.txt @@ -147,6 +147,7 @@ environment: production, public_key: d4d82fc1c2c4032a83f3a29aa3a3aff, release: release, + sample_rand: {Scrubbed}, sample_rate: 1, sampled: true, trace_id: Guid_3, From 724880d31c075b292a32a6cbe6a5848e03948e18 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Fri, 14 Feb 2025 14:26:15 +1300 Subject: [PATCH 09/10] Review feedback --- CHANGELOG.md | 1 + src/Sentry/DynamicSamplingContext.cs | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f99ac3e19..55bb8077c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased ### Features + - The sample seed used for sampling decisions is now propagated, for use in downstream custom trace samplers ([#3951](https://github.com/getsentry/sentry-dotnet/pull/3951)) ### Fixes diff --git a/src/Sentry/DynamicSamplingContext.cs b/src/Sentry/DynamicSamplingContext.cs index 3e996f1132..e85ec88b84 100644 --- a/src/Sentry/DynamicSamplingContext.cs +++ b/src/Sentry/DynamicSamplingContext.cs @@ -33,22 +33,22 @@ private DynamicSamplingContext( // Validate and set required values if (traceId == SentryId.Empty) { - throw new ArgumentOutOfRangeException(nameof(traceId)); + throw new ArgumentOutOfRangeException(nameof(traceId), "cannot be empty"); } if (string.IsNullOrWhiteSpace(publicKey)) { - throw new ArgumentException(default, nameof(publicKey)); + throw new ArgumentException("cannot be empty", nameof(publicKey)); } if (sampleRate is < 0.0 or > 1.0) { - throw new ArgumentOutOfRangeException(nameof(sampleRate)); + throw new ArgumentOutOfRangeException(nameof(sampleRate), "Arg invalid if < 0.0 or > 1.0"); } if (sampleRand is < 0.0 or >= 1.0) { - throw new ArgumentOutOfRangeException(nameof(sampleRand)); + throw new ArgumentOutOfRangeException(nameof(sampleRand), "Arg invalid if < 0.0 or >= 1.0"); } var items = new Dictionary(capacity: 8) From 8fea485f2912783f3b6fb5ffc1d57bf56b121d6e Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Fri, 14 Feb 2025 14:27:05 +1300 Subject: [PATCH 10/10] Update CHANGELOG.md --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1de994855..0897759199 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,13 @@ # Changelog -## 5.1.1 +## Unreleased ### Features - The sample seed used for sampling decisions is now propagated, for use in downstream custom trace samplers ([#3951](https://github.com/getsentry/sentry-dotnet/pull/3951)) +## 5.1.1 + ### Fixes - Emit transaction.data inside contexts.trace.data ([#3936](https://github.com/getsentry/sentry-dotnet/pull/3936))