Repository navigation
Feature/sentry.quartz - #5505
michaelmairegger wants to merge 37 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
michaelmairegger
changed the title
|
@jamescrosswell I will continue watching Quartz.Net alpha releases and adopt it until final bits of 4.0 will be released. |
Fantastic, thank you @michaelmairegger ! Maybe we mark the PR as draft until then? That let's me know it doesn't yet need my attention (you can always tag me directly if you want my input on something before v4 is released). |
michaelmairegger
changed the title
18ff2fe to
8e9c712
Compare
c05a9f7 to
59fdcd8
Compare
|
@jamescrosswell quartz.net released final bits of v4. I updated the package to the final version and would say this first version of the package is feature complete. |
michaelmairegger
changed the title
Nice - thanks @michaelmairegger. I wasn't expecting them to get v4 out so quickly! Apologies in advance... I might be a bis slow on reviewing - there's quite a but going on right now and the invention of AI has made it much easier for people to make PRs - so I'm spread a bit thin. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5505 +/- ##
==========================================
+ Coverage 74.72% 74.93% +0.21%
==========================================
Files 515 523 +8
Lines 18949 19061 +112
Branches 3696 3706 +10
==========================================
+ Hits 14159 14284 +125
+ Misses 3908 3890 -18
- Partials 882 887 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 688c054. Configure here.
… collection parameter and streamlining option configuration
…dSentryScope method, update SentryMetricsMiddleware to support configurable options
The format-code workflow can't push its auto-fix to a fork branch, so it fails the check instead. Applying the fix here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every package needs a name/packageUrl/mainDocsUrl entry so craft can create its registry entry on first publish; a missing `name` fails the whole registry target rather than just that package. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Keeps the middleware design from getsentry#5505 and adds what the cron monitors need on top: - One AddSentry() entry point on IQuartzBuilder that registers the scope and check-in middleware with HubAdapter, so no IHub is needed in DI. - The scope middleware starts a new trace per run and captures what the job throws inside the run's scope. - Check-ins only for jobs with [SentryCronMonitorSlug]. The slug comes from the trigger or job data map, the attribute, or the slugified job key, so job keys sharing a class don't share a monitor. - The monitor config is converted from cron and simple triggers, with the Quartz day-of-week shift, and left out when Sentry can't represent the schedule. SendMonitorConfig turns it off globally or per job. - ConfigureMonitorOptions runs before the trigger's schedule is applied, so it can override it, and never breaks the check-in. - The package is signed and restricted to Quartz [4.0.0,5.0.0). The schedule conversion, slug and check-in logic live in Core/ so the Quartz 3 package can compile the same files. Co-Authored-By: Claude <noreply@anthropic.com>
Sentry.Quartz3 keeps its name for Quartz.NET 3.x. Co-Authored-By: Claude <noreply@anthropic.com>
a5da5cf to
fd4beb4
Compare
- Jobs built without an identity get a new GUID name on each start, so their monitor slug falls back to the job class name, with a warning. Two jobs that resolve to the same slug also log a warning. - The scope middleware no longer captures exceptions. Quartz logs them, and the logging integration reports that. The cron middleware still sends an error check-in. In global mode the scope is left alone. - Quartz 4 accepts '*' in both day fields and runs on either day when both restrict. Convert those expressions, spelling out a '*' step as a range so Sentry unions the fields too. - ConfigureMonitorOptions takes the IJobExecutionContext and always runs while SendMonitorConfig is on. The trigger's schedule fills in when it sets none, and the config is only sent when it has a schedule. - The check-in ID is created up front, so the final check-in matches the in-progress one even when that wasn't sent. Co-Authored-By: Claude <noreply@anthropic.com>
…allback Log the unnamed-job fallback at debug level on each run instead of tracking which jobs were warned about, and remove the shared-slug detection. If ConfigureMonitorOptions throws, ignore what it set and use the trigger's schedule. Co-Authored-By: Claude <noreply@anthropic.com>
| { | ||
| return null; | ||
| } | ||
|
|
||
| return Format(start + offset) + "-" + Format(end + offset) + step; | ||
| } |
There was a problem hiding this comment.
Bug: The ToCrontab method incorrectly returns null for valid Quartz 4 cron expressions where both day-of-month and day-of-week are ?, preventing schedule monitoring.
Severity: MEDIUM
Suggested Fix
Remove the conditional check if (dayOfMonth == "?" && dayOfWeek == "?") that returns null. This check is invalid for Quartz 4, where ? in both the day-of-month and day-of-week fields is a valid expression. The logic should proceed to build the crontab string without this premature null return.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/Sentry.Quartz/Core/TriggerSchedule.cs#L190-L195
Potential issue: In Quartz 4, a cron expression is valid even if both the day-of-month
and day-of-week fields are `?`. However, the `ToCrontab` method contains a specific
check `if (dayOfMonth == "?" && dayOfWeek == "?")` that incorrectly returns `null` in
this case. This prevents valid schedules, such as `0 0 12 ? * ?`, from being converted
to a crontab and sent to Sentry's monitoring API, effectively disabling monitoring for
these jobs. Equivalent expressions like `0 0 12 * * ?` are handled correctly because
they do not trigger this specific condition.
This branch has not been deployed
3 participants
This PR implements and showcases Quartz.Net integration for Sentry
Note: Quartz.Net v4 is still alpha, therefore we should wait until v4 is finally released.
fixes #4601