feat: blocking detection (#2709)

* feat: blocking detection

* half assed stuff from the weekend

* verify

* ref

* binding

* context

* wip

* wip

* Fixed compiler errors

* Update CHANGELOG.md

* Format code

* Reintroducing Bruno's changes with tests

* Reintroduced changes to MainSentryEventProcessor

* Reintroduced the core blocking capability

* Update SentryStackTraceFactory.cs

* Update HubTests.CaptureEvent_ActiveTransaction_UnhandledExceptionTransactionEndedAsCrashed.Net4_8.verified.txt

* Added blocking detection suppression

* Format code

* Reverted unintentional changes to ApiDefinitions

* Fixed stack traces for blocking calls

* Update BlockingMonitor.cs

* Some basic tests for BlockingMonitor

* Added code to ensure blocking events only get sent once per day

* Format code

* Added System.Diagnostics.Metrics for blocking calls so users can know the number of blocking calls for a particular code location

* removed throttling mechanism

* Update CHANGELOG.md

* merge issues

* Refactored to improve testability

* Format code

* Moved IBlockingMonitor to a separate file

* Applied review feedback

---------

Co-authored-by: James Crosswell <[email protected]>
Co-authored-by: Sentry Github Bot <[email protected]>
Co-authored-by: James Crosswell <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -4,6 +4,11 @@
 
 ### Features
 
+- ASP.NET Core: Blocking call detection. An event with the stack trace of the blocking call will be captured as event. ([#2709](https://github.com/getsentry/sentry-dotnet/pull/2709))
+    - IMPORTANT: Verify this in test/staging before prod! Blocking calls in hot paths could create a lot of events for your Sentry project.
+    - Opt-in via `options.CaptureBlockingCalls = true`
+    - Disabled for specific code blocks with `using (new SuppressBlockingDetection())`
+    - Doesn't detect everything. See original [Caveats described by Ben Adams](https://github.com/benaadams/Ben.BlockingDetector?tab=readme-ov-file#caveats).
 - Added Crons support via `SentrySdk.CaptureCheckIn` and an integration with Hangfire ([#3128](https://github.com/getsentry/sentry-dotnet/pull/3128))
 - Common tags set automatically for metrics and metrics summaries are attached to Spans ([#3191](https://github.com/getsentry/sentry-dotnet/pull/3191))
 

samples/Sentry.Samples.AspNetCore.Mvc/Controllers/HomeController.cs

@@ -1,6 +1,7 @@
 using System.Data;
 using System.Diagnostics;
 using Microsoft.AspNetCore.Mvc;
+using Sentry.Ben.BlockingDetector;
 using Sentry.Samples.AspNetCore.Mvc.Models;
 
 namespace Samples.AspNetCore.Mvc.Controllers;
@@ -12,6 +13,53 @@ public IActionResult Index()
         return View();
     }
 
+    // GET /home/block/true or /home/block/false to observe events
+    [HttpGet("[controller]/block/{block?}")]
+    public async Task<string> Block([FromRoute] bool block)
+    {
+        if (block)
+        {
+            logger.LogInformation("\ud83d\ude31 Calling a blocking API on an async method \ud83d\ude31");
+
+            // This will result in an event in Sentry
+            Task.Delay(10).Wait(); // This is a blocking call. Same with '.Result'
+        }
+        else
+        {
+            logger.LogInformation("\ud83d\ude31 No blocking call made \ud83d\ude31");
+
+            // Non-blocking, no event captured
+            await Task.Delay(10);
+        }
+
+        return "Was blocking? " + block;
+    }
+
+    // GET /home/suppress/true or /home/suppress/false to observe events
+    [HttpGet("[controller]/suppress/{suppress?}")]
+    public async Task<string> Suppress([FromRoute] bool suppress)
+    {
+        if (suppress)
+        {
+            logger.LogInformation("Blocking suppression enabled");
+            using (new SuppressBlockingDetection())
+            {
+                Task.Delay(10).Wait(); // This is blocking but won't trigger an event, due to suppression
+            }
+            logger.LogInformation("Blocking suppression disabled");
+        }
+        else
+        {
+            logger.LogInformation("\ud83d\ude31 Unsuppressed blocking call on an async method \ud83d\ude31");
+            Task.Delay(10).Wait(); // This is blocking but won't trigger an event, due to suppression
+        }
+
+        // Non-blocking, no event captured
+        await Task.Delay(10);
+
+        return "Was suppressed? " + suppress;
+    }
+
     // Example: An exception that goes unhandled by the app will be captured by Sentry:
     [HttpPost]
     public async Task PostIndex(string? @params)

samples/Sentry.Samples.AspNetCore.Mvc/Program.cs

@@ -25,6 +25,9 @@
         // Example: Disabling support to compressed responses:
         options.DecompressionMethods = DecompressionMethods.None;
 
+        // Call GET /home/block/true to see this in action
+        options.CaptureBlockingCalls = true;
+
         options.MaxQueueItems = 100;
         options.ShutdownTimeout = TimeSpan.FromSeconds(5);
 

samples/Sentry.Samples.AspNetCore.Mvc/README.md

@@ -0,0 +1,12 @@
+Sample usages of the Sentry SDK for ASP.NET Core on an MVC app
+
+Start by changing the DSN in `appsettings.json`, `Sentry:Dsn` property with your own.
+No DSN yet? Get one for free at https://sentry.io/ to give this sample a run.
+
+Blocking detection:
+* It's turned on via option `CaptureBlockingCalls`
+* In the `HomeController` there's an action that causes a blocking call on an async method.
+* You can trigger it with:
+  * `GET http://localhost:5001/home/block/true`
+
+Results `Was blocking? True` and an event captured in Sentry.

samples/Sentry.Samples.AspNetCore.Mvc/Sentry.Samples.AspNetCore.Mvc.csproj

@@ -11,55 +11,4 @@
     <ProjectReference Include="..\..\src\Sentry.AspNetCore\Sentry.AspNetCore.csproj" />
   </ItemGroup>
 
-  <ItemGroup>
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-grid.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-grid.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-grid.min.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-grid.min.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-grid.rtl.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-grid.rtl.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-grid.rtl.min.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-grid.rtl.min.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-reboot.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-reboot.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-reboot.min.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-reboot.min.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-reboot.rtl.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-reboot.rtl.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-reboot.rtl.min.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-reboot.rtl.min.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-utilities.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-utilities.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-utilities.min.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-utilities.min.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-utilities.rtl.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-utilities.rtl.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-utilities.rtl.min.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap-utilities.rtl.min.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap.min.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap.min.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap.rtl.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap.rtl.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap.rtl.min.css" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\css\bootstrap.rtl.min.css.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\js\bootstrap.bundle.js" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\js\bootstrap.bundle.js.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\js\bootstrap.bundle.min.js" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\js\bootstrap.bundle.min.js.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\js\bootstrap.esm.js" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\js\bootstrap.esm.js.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\js\bootstrap.esm.min.js" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\js\bootstrap.esm.min.js.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\js\bootstrap.js" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\js\bootstrap.js.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\js\bootstrap.min.js" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\dist\js\bootstrap.min.js.map" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\bootstrap\LICENSE" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\jquery-validation-unobtrusive\jquery.validate.unobtrusive.js" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\jquery-validation-unobtrusive\jquery.validate.unobtrusive.min.js" />
-    <_ContentIncludedByDefault Remove="wwwroot\lib\jquery-validation-unobtrusive\LICENSE.txt" />
-  </ItemGroup>
-
 </Project>

src/Sentry.AspNetCore/BindableSentryAspNetCoreOptions.cs

@@ -16,6 +16,7 @@ internal class BindableSentryAspNetCoreOptions : BindableSentryLoggingOptions
     public bool? FlushBeforeRequestCompleted { get; set; }
     public bool? AdjustStandardEnvironmentNameCasing { get; set; }
     public bool? AutoRegisterTracing { get; set; }
+    public bool? CaptureBlockingCalls { get; set; }
 
     public void ApplyTo(SentryAspNetCoreOptions options)
     {
@@ -26,5 +27,6 @@ public void ApplyTo(SentryAspNetCoreOptions options)
         options.FlushBeforeRequestCompleted = FlushBeforeRequestCompleted ?? options.FlushBeforeRequestCompleted;
         options.AdjustStandardEnvironmentNameCasing = AdjustStandardEnvironmentNameCasing ?? options.AdjustStandardEnvironmentNameCasing;
         options.AutoRegisterTracing = AutoRegisterTracing ?? options.AutoRegisterTracing;
+        options.CaptureBlockingCalls = CaptureBlockingCalls ?? options.CaptureBlockingCalls;
     }
 }

src/Sentry.AspNetCore/SentryAspNetCoreOptions.cs

@@ -42,6 +42,18 @@ public class SentryAspNetCoreOptions : SentryLoggingOptions
     /// </summary>
     internal bool FlushBeforeRequestCompleted { get; set; }
 
+    /// <summary>
+    /// Will capture an event if a blocking call is detected.
+    /// </summary>
+    /// <remarks>
+    /// Get a Sentry event for a sync over async call such as (await SomeTask).Result
+    /// Note that when ConfigureAwait(false) is used, blocking won't be detected.
+    /// Additionally, detects blocking calls over CLR waits such as lock, ReaderWriterLock, etc.
+    /// Blocking calls through system calls are also not captured.
+    /// </remarks>
+    /// <seealso href="https://github.com/getsentry/Ben.BlockingDetector/"/>
+    public bool CaptureBlockingCalls { get; set; }
+
     /// <summary>
     /// The strategy to define the name of a transaction based on the <see cref="HttpContext"/>.
     /// </summary>

src/Sentry.AspNetCore/SentryMiddleware.cs

@@ -3,6 +3,7 @@
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using Sentry.AspNetCore.Extensions;
+using Sentry.Ben.BlockingDetector;
 using Sentry.Extensibility;
 using Sentry.Internal;
 using Sentry.Reflection;
@@ -32,6 +33,11 @@ internal static readonly SdkVersion NameAndVersion
 
     private static readonly string ProtocolPackageName = "nuget:" + NameAndVersion.Name;
 
+    // Ben.BlockingDetector
+    private readonly BlockingMonitor? _monitor;
+    private readonly DetectBlockingSynchronizationContext? _detectBlockingSyncCtx;
+    private readonly TaskBlockingListener? _listener;
+
     /// <summary>
     /// Initializes a new instance of the <see cref="SentryMiddleware"/> class.
     /// </summary>
@@ -63,6 +69,13 @@ public SentryMiddleware(
         _eventExceptionProcessors = eventExceptionProcessors;
         _eventProcessors = eventProcessors;
         _transactionProcessors = transactionProcessors;
+
+        if (_options.CaptureBlockingCalls)
+        {
+            _monitor = new BlockingMonitor(_getHub, _options);
+            _detectBlockingSyncCtx = new DetectBlockingSynchronizationContext(_monitor);
+            _listener = new TaskBlockingListener(_monitor);
+        }
     }
 
     /// <summary>
@@ -132,7 +145,24 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next)
             try
             {
                 var originalMethod = context.Request.Method;
-                await next(context).ConfigureAwait(false);
+                if (_options.CaptureBlockingCalls && _monitor is not null)
+                {
+                    var syncCtx = SynchronizationContext.Current;
+                    SynchronizationContext.SetSynchronizationContext(syncCtx == null ? _detectBlockingSyncCtx : new DetectBlockingSynchronizationContext(_monitor, syncCtx));
+                    try
+                    {
+                        // For detection to work we need ConfigureAwait=true
+                        await next(context).ConfigureAwait(true);
+                    }
+                    finally
+                    {
+                        SynchronizationContext.SetSynchronizationContext(syncCtx);
+                    }
+                }
+                else
+                {
+                    await next(context).ConfigureAwait(false);
+                }
                 if (_options.Instrumenter == Instrumenter.OpenTelemetry && Activity.Current is { } activity)
                 {
                     // The middleware pipeline finishes up before the Otel Activity.OnEnd callback is invoked so we need

src/Sentry/Ben.BlockingDetector/BlockingMonitor.cs

@@ -0,0 +1,107 @@
+using Sentry.Internal;
+using Sentry.Protocol;
+
+// Namespace starting with Sentry makes sure the SDK cuts frames off before reporting
+namespace Sentry.Ben.BlockingDetector
+{
+    internal class BlockingMonitor : IBlockingMonitor
+    {
+        private readonly Func<IHub> _getHub;
+        private readonly SentryOptions _options;
+        internal readonly IRecursionTracker _recursionTracker;
+
+        public BlockingMonitor(Func<IHub> getHub, SentryOptions options)
+            : this(getHub, options, new StaticRecursionTracker())
+        {
+        }
+
+        internal BlockingMonitor(Func<IHub> getHub, SentryOptions options, IRecursionTracker recursionTracker)
+        {
+            _getHub = getHub;
+            _options = options;
+            _recursionTracker = recursionTracker;
+        }
+
+        private static bool ShouldSkipFrame(string? frameInfo) =>
+            frameInfo?.StartsWith("Sentry.Ben") == true
+            // Skip frames relating to the TaskBlockingListener
+            || frameInfo?.StartsWith("System.Diagnostics") == true
+            // Skip frames relating to the async state machine
+            || frameInfo?.StartsWith("System.Threading") == true;
+
+        public void BlockingStart(DetectionSource detectionSource)
+        {
+            // From Stephen Cleary:
+            // "The default SynchronizationContext queues its asynchronous delegates to the ThreadPool but executes its
+            // synchronous delegates directly on the calling thread."
+            //
+            // Implicitly then, if we're not on a ThreadPool thread, we're not in an async context.
+            if (!Thread.CurrentThread.IsThreadPoolThread)
+            {
+                return;
+            }
+
+            _recursionTracker.Recurse();
+
+            try
+            {
+                if (!_recursionTracker.IsFirstRecursion())
+                {
+                    return;
+                }
+
+                var stackTrace = DebugStackTrace.Create(
+                    _options,
+                    new StackTrace(true),
+                    true,
+                    ShouldSkipFrame
+                );
+                var evt = new SentryEvent
+                {
+                    Level = SentryLevel.Warning,
+                    Message =
+                        "Blocking method has been invoked and blocked, this can lead to ThreadPool starvation. Learn more about it: " +
+                        "https://learn.microsoft.com/en-us/aspnet/core/fundamentals/best-practices#avoid-blocking-calls ",
+                    SentryExceptions = new[]
+                    {
+                        new SentryException
+                        {
+                            ThreadId = Environment.CurrentManagedThreadId,
+                            Mechanism = new Mechanism
+                            {
+                                Type = "BlockingCallDetector",
+                                Handled = false,
+                                Description = "Blocking calls can cause ThreadPool starvation.",
+                                Source = detectionSource.ToString()
+                            },
+                            Type = "Blocking call detected",
+                            Stacktrace = stackTrace,
+                        }
+                    },
+                };
+
+                _getHub().CaptureEvent(evt);
+            }
+            catch
+            {
+                // ignored
+            }
+        }
+
+        public void BlockingEnd()
+        {
+            if (!Thread.CurrentThread.IsThreadPoolThread)
+            {
+                return;
+            }
+
+            _recursionTracker.Backtrack();
+        }
+    }
+
+    internal enum DetectionSource
+    {
+        SynchronizationContext,
+        EventListener
+    }
+}