Repository navigation
feat: add SentrySdk.WithMonitor to run a job with cron check-ins and create the monitor from code - #5664
feat: add SentrySdk.WithMonitor to run a job with cron check-ins and create the monitor from code#5664wedamija wants to merge 5 commits into
Conversation
SentrySdk.WithMonitor and IHub.WithMonitor send an in-progress check-in (with optional monitor config), run the job, then send an ok or error check-in with the duration. Exceptions are rethrown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5664 +/- ##
==========================================
+ Coverage 74.93% 75.20% +0.27%
==========================================
Files 515 515
Lines 18975 19060 +85
Branches 3695 3703 +8
==========================================
+ Hits 14219 14335 +116
+ Misses 3876 3870 -6
+ Partials 880 855 -25 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…tured Hub.CaptureCheckIn returns SentryId.Empty when capturing fails, and SentryCheckIn only generates an id for null, so the final check-in was sent with an all-zero id. Also puts one parameter per line in the new signatures and covers the throwing Func<T> and Func<Task<T>> overloads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Hey @wedamija, thanks for the work here! I had a quick look and it's looking pretty good overall. Before anything else: is there any urgency to bringing this feature in? That said, I'd be fine with merging if @bruno-garcia approves it, since he definitely knows best here :) Either way, I could not find any issue to link this to, except for #3262. |
| } | ||
| catch | ||
| { | ||
| hub.CaptureCheckIn(monitorSlug, CheckInStatus.Error, checkInId, stopwatch.Elapsed); |
There was a problem hiding this comment.
Is there a way to tie the exception we're bubbling here with the check-in?
We do something similar with spans, where we keep a WeakRef of the exception and in the product we can link the two entities.
Otherwise, is this check-in error linked to the actual exception that had it fail?
There was a problem hiding this comment.
The check-in carries the current trace ID, which should be shared with the given exception, afaik
There was a problem hiding this comment.
Actually I was wrong here, I've pushed an update to fix this:
Each WithMonitor run now gets its own scope, and a new trace when no span or transaction is active. Check-ins and any exception captured during the run share that trace ID. When there's an active transaction, its trace is used.
| } | ||
| catch | ||
| { | ||
| hub.CaptureCheckIn(monitorSlug, CheckInStatus.Error, checkInId, stopwatch.Elapsed); |
There was a problem hiding this comment.
same here regarding the exception reference and linking to failed check-in
| Action<SentryMonitorOptions>? configureMonitorOptions = null) | ||
| => hub.RunWithMonitorAsync<object?>(monitorSlug, async () => | ||
| { | ||
| await job().ConfigureAwait(false); |
There was a problem hiding this comment.
It's up to the caller of WithMonitor to handle errors and capture the exception with Sentry?
There was a problem hiding this comment.
Yeah, this is how we handle this for other sdks:
https://github.com/getsentry/sentry-python/blob/c5f80df07625977c1a84836f03b77943f90589f3/sentry_sdk/crons/decorator.py#L74-L93
https://github.com/getsentry/sentry-javascript/blob/21ba35d6efdd940e162ec17c5f2625d880f97eda/packages/core/src/monitor.ts#L33-L50
The exception is typically caught by the apps unhanded exception setup
There was a problem hiding this comment.
The exception is typically caught by the apps unhanded exception setup
Makes sense for the app to handle since it's app error.. but we need to make sure the SDK is linking things correctly
@ric-oliv I haven't been too involved with .NET for a while, so it's really your call |
Hey @ric-oliv, my main motivation for this is that we've seen a pretty big bump in crons usage recently in sdks that make it easy to set up crons, and I'm hoping to get this kind of support into most SDKs. Not sure if you saw, but I also have Hangfire support here: #5665, but I wanted to get this initial work merged first before I make it ready for review I'm happy to work towards getting this into a good state. I wouldn't call it extremely urgent, but I think it's worthwhile to do and get out sooner rather than later, and I'm happy to put in the work to finish it up if you don't mind reviewing. |
A job returning ValueTask binds to WithMonitor<T>(Func<T>), so ok was sent as soon as the job returned. That path now awaits a plain ValueTask once and sends ok or error when it completes. A ValueTask<T>, or a Task that reaches Func<T> through an explicit type argument, still sends ok on return and logs a warning pointing at 'async () => await job()'. Tests pin which overload each job shape binds to. Co-Authored-By: Claude <noreply@anthropic.com>
Like sentry-java's CheckInUtils, each run pushes a scope and, unless a span is active, starts a new trace, so a run's check-ins and the errors captured during it share a trace ID. The scope stays current across awaits, including for ValueTask jobs. Also reject a null job or empty slug before sending any check-in. Co-Authored-By: Claude <noreply@anthropic.com>
The final check-in reused the ID returned for the in-progress one, which is empty when that one isn't captured. Create the ID up front and send it with both check-ins. Co-Authored-By: Claude <noreply@anthropic.com>
Description
Adds
SentrySdk.WithMonitor, which runs a job and sends its check-ins in one call, likeSentry.withMonitorin JavaScript and@monitorin Python:It sends an in-progress check-in with the optional monitor config, runs the job, then sends ok or error with the duration. Exceptions are rethrown and return values pass through. There are overloads for
Action,Func<T>,Func<Task>andFunc<Task<T>>, added as extension methods onIHubso existingIHubimplementations don't break. Each run gets its own scope and, unless a span is active, its own trace.A
ValueTask<T>job binds toFunc<T>and isn't awaited, so the SDK logs a warning; wrap it asasync () => await job().Motivation
Check-ins for a monitor that doesn't exist yet are dropped, so today users create each monitor by hand and pair the in-progress and final check-ins themselves. With the config passed here, Sentry creates the monitor, or updates its schedule, from code.
Testing
HubExtensionsMonitorTestsandSentrySdkTests; API approval files updated.Docs: getsentry/sentry-docs#19779