perf: Avoid shallow copies of DSC (#4453)

Resolves #4381
- #4381

#skip-changelog

Author: jamescrosswell

src/Sentry.AspNet/HttpContextExtensions.cs

@@ -113,7 +113,7 @@ public static ITransactionTracer StartSentryTransaction(this HttpContext httpCon
             // See:
             // https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#freezing-dynamic-sampling-context
             // https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#unified-propagation-mechanism
-            dynamicSamplingContext = DynamicSamplingContext.Empty;
+            dynamicSamplingContext = DynamicSamplingContext.Empty();
         }
 
         var transaction = SentrySdk.StartTransaction(transactionContext, customSamplingContext, dynamicSamplingContext);

src/Sentry.AspNetCore/SentryTracingMiddleware.cs

@@ -75,7 +75,7 @@ public SentryTracingMiddleware(
                 // See:
                 // https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#freezing-dynamic-sampling-context
                 // https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#unified-propagation-mechanism
-                dynamicSamplingContext = DynamicSamplingContext.Empty;
+                dynamicSamplingContext = DynamicSamplingContext.Empty();
             }
 
             var transaction = _getHub().StartTransaction(transactionContext, customSamplingContext, dynamicSamplingContext);

src/Sentry/DynamicSamplingContext.cs

@@ -9,16 +9,18 @@ namespace Sentry;
 /// <seealso href="https://develop.sentry.dev/sdk/performance/dynamic-sampling-context"/>
 internal class DynamicSamplingContext
 {
-    public IReadOnlyDictionary<string, string> Items { get; }
+    private readonly Dictionary<string, string> _items;
+
+    public IReadOnlyDictionary<string, string> Items => _items;
 
     public bool IsEmpty => Items.Count == 0;
 
-    private DynamicSamplingContext(IReadOnlyDictionary<string, string> items) => Items = items;
+    private DynamicSamplingContext(Dictionary<string, string> items) => _items = items;
 
     /// <summary>
     /// Gets an empty <see cref="DynamicSamplingContext"/> that can be used to "freeze" the DSC on a transaction.
     /// </summary>
-    public static readonly DynamicSamplingContext Empty = new(new Dictionary<string, string>().AsReadOnly());
+    public static DynamicSamplingContext Empty() => new(new Dictionary<string, string>());
 
     private DynamicSamplingContext(SentryId traceId,
         string publicKey,
@@ -93,39 +95,24 @@ private DynamicSamplingContext(SentryId traceId,
             items.Add("replay_id", replayId.ToString());
         }
 
-        Items = items;
+        _items = items;
     }
 
     public BaggageHeader ToBaggageHeader() => BaggageHeader.Create(Items, useSentryPrefix: true);
 
-    public DynamicSamplingContext WithSampleRate(double sampleRate)
+    public void SetSampleRate(double sampleRate)
     {
-        if (Items.TryGetValue("sample_rate", out var dscSampleRate))
-        {
-            if (double.TryParse(dscSampleRate, NumberStyles.Float, CultureInfo.InvariantCulture, out var rate))
-            {
-                if (Math.Abs(rate - sampleRate) > double.Epsilon)
-                {
-                    var items = Items.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
-                    items["sample_rate"] = sampleRate.ToString(CultureInfo.InvariantCulture);
-                    return new DynamicSamplingContext(items);
-                }
-            }
-        }
-
-        return this;
+        _items["sample_rate"] = sampleRate.ToString(CultureInfo.InvariantCulture);
     }
 
-    public DynamicSamplingContext WithReplayId(IReplaySession? replaySession)
+    internal DynamicSamplingContext Clone() => new(new Dictionary<string, string>(_items));
+
+    public void SetReplayId(IReplaySession? replaySession)
     {
-        if (replaySession?.ActiveReplayId is not { } replayId || replayId == SentryId.Empty)
+        if (replaySession?.ActiveReplayId is { } replayId && replayId != SentryId.Empty)
         {
-            return this;
+            _items["replay_id"] = replayId.ToString();
         }
-
-        var items = Items.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
-        items["replay_id"] = replayId.ToString();
-        return new DynamicSamplingContext(items);
     }
 
     public static DynamicSamplingContext? CreateFromBaggageHeader(BaggageHeader baggage, IReplaySession? replaySession)

src/Sentry/Internal/Hub.cs

@@ -193,7 +193,7 @@ internal ITransactionTracer StartTransaction(
                 isSampled = SampleRandHelper.IsSampled(sampleRand, samplerSampleRate);
 
                 // Ensure the actual sampleRate is set on the provided DSC (if any) when the TracesSampler reached a sampling decision
-                dynamicSamplingContext = dynamicSamplingContext?.WithSampleRate(samplerSampleRate);
+                dynamicSamplingContext?.SetSampleRate(samplerSampleRate);
             }
         }
 
@@ -207,12 +207,12 @@ internal ITransactionTracer StartTransaction(
             if (context.IsSampled is null && _options.TracesSampleRate is not null)
             {
                 // Ensure the actual sampleRate is set on the provided DSC (if any) when not IsSampled upstream but the TracesSampleRate reached a sampling decision
-                dynamicSamplingContext = dynamicSamplingContext?.WithSampleRate(_options.TracesSampleRate.Value);
+                dynamicSamplingContext?.SetSampleRate(_options.TracesSampleRate.Value);
             }
         }
 
         // Make sure there is a replayId (if available) on the provided DSC (if any).
-        dynamicSamplingContext = dynamicSamplingContext?.WithReplayId(_replaySession);
+        dynamicSamplingContext?.SetReplayId(_replaySession);
 
         if (isSampled is false)
         {

test/Sentry.Tests/DynamicSamplingContextTests.cs

@@ -25,7 +25,7 @@ public Fixture()
     [Fact]
     public void EmptyContext()
     {
-        var dsc = DynamicSamplingContext.Empty;
+        var dsc = DynamicSamplingContext.Empty();
 
         Assert.True(dsc.IsEmpty);
     }

test/Sentry.Tests/HubTests.cs

@@ -743,6 +743,7 @@ public void StartTransaction_DynamicSamplingContextWithSampleRate_OverwritesSamp
             {"sentry-sample_rate", "0.5"},
             {"sentry-sample_rand", "0.1234"},
         }).CreateDynamicSamplingContext();
+        var originalDsc = dsc?.Clone();
 
         _fixture.Options.TracesSampler = _ => tracesSampler;
         _fixture.Options.TracesSampleRate = tracesSampleRate;
@@ -759,12 +760,11 @@ public void StartTransaction_DynamicSamplingContextWithSampleRate_OverwritesSamp
             transactionTracer.SampleRate.Should().Be(expectedSampleRate);
             if (expectedDscOverwritten)
             {
-                transactionTracer.DynamicSamplingContext.Should().NotBeSameAs(dsc);
-                transactionTracer.DynamicSamplingContext.Should().BeEquivalentTo(dsc.ReplaceSampleRate(expectedSampleRate));
+                transactionTracer.DynamicSamplingContext.Should().BeEquivalentTo(originalDsc.CloneWithSampleRate(expectedSampleRate));
             }
             else
             {
-                transactionTracer.DynamicSamplingContext.Should().BeSameAs(dsc);
+                transactionTracer.DynamicSamplingContext.Should().BeEquivalentTo(originalDsc);
             }
         }
         else
@@ -773,12 +773,11 @@ public void StartTransaction_DynamicSamplingContextWithSampleRate_OverwritesSamp
             unsampledTransaction.SampleRate.Should().Be(expectedSampleRate);
             if (expectedDscOverwritten)
             {
-                unsampledTransaction.DynamicSamplingContext.Should().NotBeSameAs(dsc);
-                unsampledTransaction.DynamicSamplingContext.Should().BeEquivalentTo(dsc.ReplaceSampleRate(expectedSampleRate));
+                unsampledTransaction.DynamicSamplingContext.Should().BeEquivalentTo(originalDsc.CloneWithSampleRate(expectedSampleRate));
             }
             else
             {
-                unsampledTransaction.DynamicSamplingContext.Should().BeSameAs(dsc);
+                unsampledTransaction.DynamicSamplingContext.Should().BeEquivalentTo(originalDsc);
             }
         }
     }
@@ -814,12 +813,12 @@ public void StartTransaction_DynamicSamplingContextWithReplayId_UsesActiveReplay
         transactionTracer.IsSampled.Should().BeTrue();
         transactionTracer.DynamicSamplingContext.Should().NotBeNull();
 
-        var expectedDsc = dsc.ReplaceSampleRate(_fixture.Options.TracesSampleRate.Value);
+        var expectedDsc = dsc.CloneWithSampleRate(_fixture.Options.TracesSampleRate.Value);
         if (replaySessionIsActive)
         {
             // We overwrite the replay_id when we have an active replay session
             // Otherwise we propagate whatever was in the baggage header
-            expectedDsc = expectedDsc.ReplaceReplayId(_fixture.ReplaySession);
+            expectedDsc = expectedDsc.CloneWithReplayId(_fixture.ReplaySession);
         }
         transactionTracer.DynamicSamplingContext.Should().BeEquivalentTo(expectedDsc);
     }
@@ -881,7 +880,7 @@ public void StartTransaction_DynamicSamplingContextWithoutSampleRand_SampleRandN
         var hub = _fixture.GetSut();
 
         // Act
-        var transaction = hub.StartTransaction(transactionContext, new Dictionary<string, object>(), DynamicSamplingContext.Empty);
+        var transaction = hub.StartTransaction(transactionContext, new Dictionary<string, object>(), DynamicSamplingContext.Empty());
 
         // Assert
         var transactionTracer = transaction.Should().BeOfType<TransactionTracer>().Subject;
@@ -905,6 +904,7 @@ public void StartTransaction_DynamicSamplingContextWithSampleRand_InheritsSample
             {"sentry-sample_rate", "0.5"}, // Required in the baggage header, but ignored by sampling logic
             {"sentry-sample_rand", "0.1234"}
         }).CreateDynamicSamplingContext(dummyReplaySession);
+        var originalDsc = dsc.Clone();
 
         _fixture.Options.TracesSampleRate = 0.4;
         var hub = _fixture.GetSut();
@@ -917,8 +917,7 @@ public void StartTransaction_DynamicSamplingContextWithSampleRand_InheritsSample
         transactionTracer.IsSampled.Should().BeTrue();
         transactionTracer.SampleRate.Should().Be(0.4);
         transactionTracer.SampleRand.Should().Be(0.1234);
-        transactionTracer.DynamicSamplingContext.Should().NotBeSameAs(dsc);
-        transactionTracer.DynamicSamplingContext.Should().BeEquivalentTo(dsc.ReplaceSampleRate(0.4));
+        transactionTracer.DynamicSamplingContext.Should().BeEquivalentTo(originalDsc.CloneWithSampleRate(0.4));
     }
 
     [Theory]
@@ -937,6 +936,7 @@ public void StartTransaction_TraceSampler_UsesSampleRand(double sampleRate, bool
             {"sentry-sample_rate", "0.5"},
             {"sentry-sample_rand", "0.1234"}
         }).CreateDynamicSamplingContext(_fixture.ReplaySession);
+        var originalDsc = dsc.Clone();
 
         _fixture.Options.TracesSampler = _ => sampleRate;
         var hub = _fixture.GetSut();
@@ -951,17 +951,15 @@ public void StartTransaction_TraceSampler_UsesSampleRand(double sampleRate, bool
             transactionTracer.IsSampled.Should().BeTrue();
             transactionTracer.SampleRate.Should().Be(sampleRate);
             transactionTracer.SampleRand.Should().Be(0.1234);
-            transactionTracer.DynamicSamplingContext.Should().NotBeSameAs(dsc);
-            transactionTracer.DynamicSamplingContext.Should().BeEquivalentTo(dsc.ReplaceSampleRate(sampleRate));
+            transactionTracer.DynamicSamplingContext.Should().BeEquivalentTo(originalDsc.CloneWithSampleRate(sampleRate));
         }
         else
         {
             var unsampledTransaction = transaction.Should().BeOfType<UnsampledTransaction>().Subject;
             unsampledTransaction.IsSampled.Should().BeFalse();
             unsampledTransaction.SampleRate.Should().Be(sampleRate);
             unsampledTransaction.SampleRand.Should().Be(0.1234);
-            unsampledTransaction.DynamicSamplingContext.Should().NotBeSameAs(dsc);
-            unsampledTransaction.DynamicSamplingContext.Should().BeEquivalentTo(dsc.ReplaceSampleRate(sampleRate));
+            unsampledTransaction.DynamicSamplingContext.Should().BeEquivalentTo(originalDsc.CloneWithSampleRate(sampleRate));
         }
     }
 
@@ -981,6 +979,7 @@ public void StartTransaction_StaticSampler_UsesSampleRand(double sampleRate, boo
             {"sentry-sample_rate", "0.5"}, // Static sampling ignores this and uses options.TracesSampleRate instead
             {"sentry-sample_rand", "0.1234"}
         }).CreateDynamicSamplingContext(dummyReplaySession);
+        var originalDsc = dsc.Clone();
 
         _fixture.Options.TracesSampleRate = sampleRate;
         var hub = _fixture.GetSut();
@@ -995,17 +994,15 @@ public void StartTransaction_StaticSampler_UsesSampleRand(double sampleRate, boo
             transactionTracer.IsSampled.Should().BeTrue();
             transactionTracer.SampleRate.Should().Be(sampleRate);
             transactionTracer.SampleRand.Should().Be(0.1234);
-            transactionTracer.DynamicSamplingContext.Should().NotBeSameAs(dsc);
-            transactionTracer.DynamicSamplingContext.Should().BeEquivalentTo(dsc.ReplaceSampleRate(sampleRate));
+            transactionTracer.DynamicSamplingContext.Should().BeEquivalentTo(originalDsc.CloneWithSampleRate(sampleRate));
         }
         else
         {
             var unsampledTransaction = transaction.Should().BeOfType<UnsampledTransaction>().Subject;
             unsampledTransaction.IsSampled.Should().BeFalse();
             unsampledTransaction.SampleRate.Should().Be(sampleRate);
             unsampledTransaction.SampleRand.Should().Be(0.1234);
-            unsampledTransaction.DynamicSamplingContext.Should().NotBeSameAs(dsc);
-            unsampledTransaction.DynamicSamplingContext.Should().BeEquivalentTo(dsc.ReplaceSampleRate(sampleRate));
+            unsampledTransaction.DynamicSamplingContext.Should().BeEquivalentTo(originalDsc.CloneWithSampleRate(sampleRate));
         }
     }
 
@@ -1255,7 +1252,7 @@ public void GetBaggage_SpanActive_ReturnsBaggageFromSpan(bool isSampled)
         baggage.Members.Should().Equal([
             new KeyValuePair<string, string>("sentry-trace_id", "43365712692146d08ee11a729dfbcaca"),
             new KeyValuePair<string, string>("sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"),
-            new KeyValuePair<string, string>("sentry-sample_rate", isSampled ? "1" : "0.0"),
+            new KeyValuePair<string, string>("sentry-sample_rate", isSampled ? "1" : "0"),
             new KeyValuePair<string, string>("sentry-sample_rand", sampleRand!.Value.ToString(CultureInfo.InvariantCulture)),
         ]);
     }
@@ -2409,63 +2406,19 @@ internal partial class HubTestsJsonContext : JsonSerializerContext
 #nullable enable
 file static class DynamicSamplingContextExtensions
 {
-    public static DynamicSamplingContext ReplaceSampleRate(this DynamicSamplingContext? dsc, double sampleRate)
+    public static DynamicSamplingContext CloneWithSampleRate(this DynamicSamplingContext? dsc, double sampleRate)
     {
         Assert.NotNull(dsc);
-
-        var value = sampleRate.ToString(CultureInfo.InvariantCulture);
-        return dsc.Replace("sentry-sample_rate", value);
+        var newDsc = dsc.Clone();
+        newDsc.SetSampleRate(sampleRate);
+        return newDsc;
     }
 
-    public static DynamicSamplingContext ReplaceReplayId(this DynamicSamplingContext? dsc, IReplaySession replaySession)
+    public static DynamicSamplingContext CloneWithReplayId(this DynamicSamplingContext? dsc, IReplaySession replaySession)
     {
         Assert.NotNull(dsc);
-
-        if (!replaySession.ActiveReplayId.HasValue)
-        {
-            throw new InvalidOperationException($"No {nameof(IReplaySession.ActiveReplayId)}.");
-        }
-
-        var value = replaySession.ActiveReplayId.Value.ToString();
-        return dsc.Replace("sentry-replay_id", value);
-    }
-
-    private static DynamicSamplingContext Replace(this DynamicSamplingContext dsc, string key, string newValue)
-    {
-        var items = dsc.ToBaggageHeader().Members.ToList();
-        var index = items.FindSingleIndex(key);
-
-        var oldValue = items[index].Value;
-        if (oldValue == newValue)
-        {
-            throw new InvalidOperationException($"{key} already is {oldValue}.");
-        }
-
-        items[index] = new KeyValuePair<string, string>(key, newValue);
-
-        var baggage = BaggageHeader.Create(items);
-        var dynamicSamplingContext = DynamicSamplingContext.CreateFromBaggageHeader(baggage, null);
-        if (dynamicSamplingContext is null)
-        {
-            throw new InvalidOperationException($"Invalid {nameof(BaggageHeader)}: {baggage}");
-        }
-
-        return dynamicSamplingContext;
-    }
-
-    private static int FindSingleIndex(this List<KeyValuePair<string, string>> items, string key)
-    {
-        var index = items.FindIndex(item => item.Key == key);
-        if (index == -1)
-        {
-            throw new InvalidOperationException($"{key} not found.");
-        }
-
-        if (items.FindLastIndex(item => item.Key == key) != index)
-        {
-            throw new InvalidOperationException($"Duplicate {key} found.");
-        }
-
-        return index;
+        var newDsc = dsc.Clone();
+        newDsc.SetReplayId(replaySession);
+        return newDsc;
     }
 }