Skip to content

test: add ASP.NET Core blocking detection integration tests - #5578

Merged
jamescrosswell merged 2 commits into
getsentry:mainfrom
z0rimo:test/aspnetcore-blocking-detection-5394
Sep 18, 2026
Merged

jamescrosswell merged 2 commits into
getsentry:mainfrom
z0rimo:test/aspnetcore-blocking-detection-5394

Conversation

@z0rimo

@z0rimo z0rimo commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds ASP.NET Core integration coverage for CaptureBlockingCalls.

The tests verify that:

  • multiple requests create transient SentryMiddleware instances while reusing the same IBlockingMonitor and TaskBlockingListener
  • a synchronous blocking call is captured as a Sentry event with the BlockingCallDetector mechanism
  • disabling CaptureBlockingCalls avoids creating the blocking detector services and does not capture a blocking-call event

This uses the existing TestServer integration test infrastructure and does not change production code.

Fixes #5394

Testing

NO_MOBILE=true dotnet test test/Sentry.AspNetCore.Tests/Sentry.AspNetCore.Tests.csproj -f net10.0

Passed: 289
Failed: 0
Skipped: 0

The three new tests were also run 10 consecutive times with no failures.

@github-actions github-actions Bot added the risk: medium PR risk score: medium label Sep 14, 2026
@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.82%. Comparing base (3985304) to head (430de37).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5578      +/-   ##
==========================================
+ Coverage   74.75%   74.82%   +0.07%     
==========================================
  Files         515      515              
  Lines       18963    18963              
  Branches     3694     3694              
==========================================
+ Hits        14175    14190      +15     
+ Misses       3909     3894      -15     
  Partials      879      879              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jamescrosswell jamescrosswell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @z0rimo - overall, looks good. I made a few comments/suggestions.

Comment thread test/Sentry.AspNetCore.Tests/IntegrationsTests.BlockingDetection.cs Outdated
Comment thread test/Sentry.AspNetCore.Tests/IntegrationsTests.BlockingDetection.cs
Comment thread test/Sentry.AspNetCore.Tests/IntegrationsTests.BlockingDetection.cs Outdated
@z0rimo
z0rimo force-pushed the test/aspnetcore-blocking-detection-5394 branch from 4a62a14 to 2693b92 Compare September 17, 2026 03:32
ManualResetEventSlim.Wait spins before it blocks, and only the blocking
phase goes through SynchronizationContext.Wait. On a loaded machine the
spin phase can consume the whole 25ms timeout, so no wait notification
fires and no blocking event is captured — which is why this test failed
on six of the seven CI jobs while passing locally.

Measured with a standalone probe counting SynchronizationContext.Wait
invocations on a thread-pool thread: idle, both primitives hit 50/50;
under CPU load ManualResetEventSlim dropped to 44/50 and 41/50 across
trials, while ManualResetEvent.WaitOne stayed at 50/50 (250/250 overall).

WaitHandle.WaitOne checks IsWaitNotificationRequired and routes through
the sync context in shared managed code, with no spin phase, so it
exercises the intended path on every platform.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@jamescrosswell jamescrosswell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome - thank you so much for the contribution and for integrating all my feedback 🙏🏻

I made one small fix to the last unit test you added (hope you don't mind - figured it was quicker than another round of feedback and changes).

@jamescrosswell
jamescrosswell merged commit e67eb03 into getsentry:main Sep 18, 2026
34 checks passed
This was referenced Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: medium PR risk score: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add integration tests for CaptureBlockingCalls blocking detection

2 participants