migrate downsampling & fix tests

src/Sentry.Profiling/Downsampler.cs

@@ -1,52 +1,34 @@
 using Microsoft.Diagnostics.Tracing;
 using Microsoft.Diagnostics.Tracing.EventPipe;
 
-// TODO remove
 /// <summary>
 /// Reduce sampling rate from 1 Hz that is the default for the provider to the configured SamplingRateMs.
 /// </summary>
 internal class Downsampler
 {
-    public double SamplingRateMs { get; set; } = (double)1_000 / 101; // 101 Hz
-    private double NextSampleCounter = 0;
-    private double NextSampleLow = 0;
-    private double NextSampleHigh = -1;
+    private static double _samplingRateMs = (double)1_000 / 101; // 101 Hz
+    private double _samplingGapMs = _samplingRateMs * 0.9;
 
-    public void AttachTo(TraceEventDispatcher source)
+    // Maps from ThreadIndex to the last sample timestamp for that thread.
+    private GrowableArray<double> _prevThreadSamples = new(10);
+
+    public void NewThreadAdded(int threadIndex)
     {
-        source.AddDispatchHook(DispatchHook);
+        if (threadIndex >= _prevThreadSamples.Count)
+        {
+            _prevThreadSamples.Count = threadIndex + 1;
+            _prevThreadSamples[threadIndex] = Double.MinValue;
+        }
     }
 
-    // Downsamples to the configured SamplingRateMs by keeping a shifting window of where the timestamp must fall.
-    // Alternatively, we could keep a map of the previous sample for each thread and check that instead but that would
-    // be a bit less performant (albeit more precise).
-    private void DispatchHook(TraceEvent traceEvent, Action<TraceEvent> realDispatch)
+    public bool ShouldSample(int threadIndex, double timestampMs)
     {
-        if (traceEvent.ProviderGuid.Equals(SampleProfilerTraceEventParser.ProviderGuid))
+        Debug.Assert(threadIndex < _prevThreadSamples.Count, "ThreadIndex too large - you must call NewThreadAdded() if a new thread is added.");
+        if (_prevThreadSamples[threadIndex] + _samplingRateMs <= timestampMs)
         {
-            var timestampMs = traceEvent.TimeStampRelativeMSec;
-            // Don't sample until the NextSampleLow is reached.
-            if (NextSampleLow >= timestampMs)
-            {
-                NextSampleHigh = -1;
-                return; // skip the event
-            }
-
-            // This is the first sample after reaching the lower bound - configure the Upper bound to some reasonable value.
-            if (NextSampleHigh < 0)
-            {
-                NextSampleHigh = timestampMs + 0.9;
-            }
-            // After the upper bound is breached, advance the lower bound to the next window we care about.
-            else if (NextSampleHigh < timestampMs)
-            {
-                NextSampleCounter += 1;
-                NextSampleLow = SamplingRateMs * NextSampleCounter - 0.5;
-                return; // skip the event
-            }
+            _prevThreadSamples[threadIndex] = timestampMs;
+            return true;
         }
-
-        // Process the event.
-        realDispatch(traceEvent);
+        return false;
     }
 }

src/Sentry.Profiling/SampleProfileBuilder.cs

@@ -10,7 +10,7 @@ namespace Sentry.Profiling;
 using SentryProfileStackTrace = HashableGrowableArray<int>;
 
 /// <summary>
-/// Processes TraceLog to produce a SampleProfile.
+/// Build a SampleProfile from TraceEvent data.
 /// </summary>
 internal class SampleProfileBuilder
 {
@@ -20,6 +20,7 @@ internal class SampleProfileBuilder
     // Output profile being built.
     public readonly SampleProfile Profile = new();
 
+    // TODO reevaluate the use of SparseArray after setting up continous profiling. Dictionary might be better.
     // A sparse array that maps from StackSourceFrameIndex to an index in the output Profile.frames.
     private readonly SparseScalarArray<int> _frameIndexes = new(-1, 1000);
 
@@ -29,6 +30,9 @@ internal class SampleProfileBuilder
     // A sparse array mapping from a ThreadIndex to an index in Profile.Threads.
     private readonly SparseScalarArray<int> _threadIndexes = new(-1, 10);
 
+    // TODO make downsampling conditional once this is available: https://github.com/dotnet/runtime/issues/82939
+    private readonly Downsampler _downsampler = new();
+
     public SampleProfileBuilder(SentryOptions options, TraceLog traceLog)
     {
         _options = options;
@@ -51,14 +55,19 @@ internal void AddSample(TraceEvent data, double timestampMs)
             return;
         }
 
-        var stackIndex = AddStackTrace(callStackIndex);
-        if (stackIndex < 0)
+        var threadIndex = AddThread(thread);
+        if (threadIndex < 0)
         {
             return;
         }
 
-        var threadIndex = AddThread(thread);
-        if (threadIndex < 0)
+        if (!_downsampler.ShouldSample(threadIndex, timestampMs))
+        {
+            return;
+        }
+
+        var stackIndex = AddStackTrace(callStackIndex);
+        if (stackIndex < 0)
         {
             return;
         }
@@ -141,24 +150,12 @@ private int AddThread(TraceThread thread)
                 Name = thread.ThreadInfo ?? $"Thread {thread.ThreadID}",
             });
             _threadIndexes[key] = Profile.Threads.Count - 1;
+            _downsampler.NewThreadAdded(_threadIndexes[key]);
         }
 
         return _threadIndexes[key];
     }
 
-    private static string ActivityPath(TraceActivity activity)
-    {
-        var creator = activity.Creator;
-        if (creator is null || creator.IsThreadActivity)
-        {
-            return activity.Index.ToString();
-        }
-        else
-        {
-            return $"{ActivityPath(creator)}/{activity.Index.ToString()}";
-        }
-    }
-
     private SentryStackFrame CreateStackFrame(CodeAddressIndex codeAddressIndex)
     {
         var frame = new SentryStackFrame();

test/Sentry.Profiling.Tests/SamplingTransactionProfilerTests.cs

@@ -195,4 +195,17 @@ async Task VerifyAsync(HttpRequestMessage message)
             }
         }
     }
+
+    [Fact]
+    public void Downsampler_ShouldSample_Works()
+    {
+        var sut = new Downsampler();
+        sut.NewThreadAdded(0);
+        Assert.True(sut.ShouldSample(0, 5));
+        Assert.False(sut.ShouldSample(0, 3));
+        Assert.False(sut.ShouldSample(0, 6));
+        Assert.True(sut.ShouldSample(0, 15));
+        Assert.False(sut.ShouldSample(0, 6));
+        Assert.False(sut.ShouldSample(0, 16));
+    }
 }