diff --git a/src/Sentry.Hangfire/SentryHangfireOptions.cs b/src/Sentry.Hangfire/SentryHangfireOptions.cs
index d8d72018d2..3d493d6187 100644
--- a/src/Sentry.Hangfire/SentryHangfireOptions.cs
+++ b/src/Sentry.Hangfire/SentryHangfireOptions.cs
@@ -33,8 +33,10 @@ public class SentryHangfireOptions
/// dropped, and cut to 50 characters. Defaults to false.
///
///
- /// Jobs marked with keep their monitor slug.
- /// Use to leave out some recurring jobs.
+ /// Recurring jobs whose schedule Sentry can't represent are skipped, and so are recurring jobs that share a monitor
+ /// slug with another recurring job on a different schedule. lists the schedules
+ /// Sentry can represent. Jobs marked with keep their monitor slug and
+ /// still send check-ins. Use to leave out some recurring jobs.
///
public bool MonitorAllRecurringJobs { get; set; }
diff --git a/src/Sentry.Hangfire/SentryServerFilter.cs b/src/Sentry.Hangfire/SentryServerFilter.cs
index 2ca302bf73..a7adcf4452 100644
--- a/src/Sentry.Hangfire/SentryServerFilter.cs
+++ b/src/Sentry.Hangfire/SentryServerFilter.cs
@@ -41,7 +41,7 @@ internal SentryServerFilter(IHub? hub, IDiagnosticLogger? logger, SentryHangfire
public void OnPerforming(PerformingContext context)
{
- var monitorSlug = GetMonitorSlug(context);
+ var (monitorSlug, recurringJobId) = GetMonitorSlug(context);
if (monitorSlug is null)
{
var jobType = context.BackgroundJob.Job.Type;
@@ -52,7 +52,18 @@ public void OnPerforming(PerformingContext context)
return;
}
- var checkInId = CaptureInProgressCheckIn(context, monitorSlug);
+ var monitoredByDefault = recurringJobId is not null;
+ var schedule = _options.SendMonitorConfig || monitoredByDefault
+ ? GetRecurringJobSchedule(context, monitorSlug)
+ : null;
+ if (monitoredByDefault && schedule is null)
+ {
+ _logger?.LogDebug("Skipping creating a check-in for recurring job '{0}'. MonitorAllRecurringJobs only " +
+ "monitors jobs whose schedule Sentry supports.", recurringJobId);
+ return;
+ }
+
+ var checkInId = CaptureInProgressCheckIn(monitorSlug, _options.SendMonitorConfig ? schedule : null);
// Note that we may be overwriting context.Items[SentryCheckInIdKey] here, which is intentional. If that happens
// then implicitly OnPerforming was called previously with the same context, but we never made it to OnPerformed
@@ -61,30 +72,33 @@ public void OnPerforming(PerformingContext context)
context.Items[SentryMonitorSlugKey] = monitorSlug;
}
- private string? GetMonitorSlug(PerformContext context)
+ // The recurring job ID is set only when MonitorAllRecurringJobs supplies the slug.
+ private (string? MonitorSlug, string? RecurringJobId) GetMonitorSlug(PerformContext context)
{
var monitorSlug = context.GetJobParameter(SentryMonitorSlugKey);
if (monitorSlug is not null || !_options.MonitorAllRecurringJobs)
{
- return monitorSlug;
+ return (monitorSlug, null);
}
try
{
var recurringJobId = context.GetJobParameter(RecurringJobIdKey);
- return string.IsNullOrEmpty(recurringJobId) ? null : _options.GetRecurringJobSlug(recurringJobId!);
+ return string.IsNullOrEmpty(recurringJobId) || _options.GetRecurringJobSlug(recurringJobId!) is not { } slug
+ ? (null, null)
+ : (slug, recurringJobId);
}
catch (Exception e)
{
_logger?.LogError(e, "Failed to read the recurring job ID of job '{0}'.", context.BackgroundJob.Id);
- return null;
+ return (null, null);
}
}
- private SentryId CaptureInProgressCheckIn(PerformingContext context, string monitorSlug)
+ private SentryId CaptureInProgressCheckIn(string monitorSlug, (string Crontab, string TimeZone)? schedule)
{
Action? configureMonitorOptions = null;
- if (_options.SendMonitorConfig && GetRecurringJobSchedule(context, monitorSlug) is (var crontab, var timeZone))
+ if (schedule is (var crontab, var timeZone))
{
configureMonitorOptions = options =>
{
diff --git a/test/Sentry.Hangfire.Tests/ServerFilterTests.cs b/test/Sentry.Hangfire.Tests/ServerFilterTests.cs
index 7d86ea582c..47089dd6e1 100644
--- a/test/Sentry.Hangfire.Tests/ServerFilterTests.cs
+++ b/test/Sentry.Hangfire.Tests/ServerFilterTests.cs
@@ -419,7 +419,7 @@ public void OnPerforming_ExcludedRecurringJobWithAttribute_CapturesCheckIn()
}
[Fact]
- public void OnPerforming_MonitorAllRecurringJobsIdsShareSlugWithDifferentCron_SendsCheckInWithoutMonitorConfig()
+ public void OnPerforming_MonitorAllRecurringJobsIdsShareSlugWithDifferentCron_DoesNotCaptureCheckIn()
{
// Arrange
var fixture = new RecurringJobFixture
@@ -437,6 +437,87 @@ public void OnPerforming_MonitorAllRecurringJobsIdsShareSlugWithDifferentCron_Se
// Act
filter.OnPerforming(fixture.PerformingContext);
+ // Assert
+ fixture.Hub.DidNotReceiveWithAnyArgs().CaptureCheckIn(default!, default);
+ fixture.PerformingContext.Items.Should().NotContainKey(SentryServerFilter.SentryCheckInIdKey);
+ }
+
+ [Theory]
+ [InlineData("0 0 L * *", "UTC", true)]
+ [InlineData("0 * * * *", "Not A Time Zone", true)]
+ [InlineData("0 0 L * *", "UTC", false)]
+ public void OnPerforming_MonitorAllRecurringJobsScheduleNotSupportedBySentry_DoesNotCaptureCheckIn(
+ string cron, string timeZoneId, bool sendMonitorConfig)
+ {
+ // Arrange
+ var fixture = new RecurringJobFixture
+ {
+ AttributeSlug = null,
+ Slug = "test-recurring-job",
+ Cron = cron,
+ TimeZoneId = timeZoneId,
+ Options = { MonitorAllRecurringJobs = true }
+ };
+ var filter = fixture.GetSut(sendMonitorConfig);
+
+ // Act
+ filter.OnPerforming(fixture.PerformingContext);
+ filter.OnPerformed(new PerformedContext(fixture.PerformingContext, null, false, null));
+
+ // Assert
+ fixture.Hub.DidNotReceiveWithAnyArgs().CaptureCheckIn(default!, default);
+ fixture.PerformingContext.Items.Should().NotContainKey(SentryServerFilter.SentryCheckInIdKey);
+ }
+
+ [Fact]
+ public void OnPerforming_MonitorAllRecurringJobsRecurringJobDeleted_DoesNotCaptureCheckIn()
+ {
+ // Arrange
+ var fixture = new RecurringJobFixture
+ {
+ AttributeSlug = null,
+ Slug = "test-recurring-job",
+ RecurringJobExists = false,
+ Options = { MonitorAllRecurringJobs = true }
+ };
+ var filter = fixture.GetSut();
+
+ // Act
+ filter.OnPerforming(fixture.PerformingContext);
+
+ // Assert
+ fixture.Hub.DidNotReceiveWithAnyArgs().CaptureCheckIn(default!, default);
+ }
+
+ [Fact]
+ public void OnPerforming_MonitorAllRecurringJobsSendMonitorConfigDisabled_SendsCheckInWithoutMonitorConfig()
+ {
+ // Arrange
+ var fixture = new RecurringJobFixture
+ {
+ AttributeSlug = null,
+ Slug = "test-recurring-job",
+ Options = { MonitorAllRecurringJobs = true }
+ };
+ var filter = fixture.GetSut(sendMonitorConfig: false);
+
+ // Act
+ filter.OnPerforming(fixture.PerformingContext);
+
+ // Assert
+ fixture.AssertCheckInSentWithoutMonitorConfig();
+ }
+
+ [Fact]
+ public void OnPerforming_MonitorAllRecurringJobsAttributeWithScheduleNotSupportedBySentry_SendsCheckInWithoutMonitorConfig()
+ {
+ // Arrange
+ var fixture = new RecurringJobFixture { Cron = "0 0 L * *", Options = { MonitorAllRecurringJobs = true } };
+ var filter = fixture.GetSut();
+
+ // Act
+ filter.OnPerforming(fixture.PerformingContext);
+
// Assert
fixture.AssertCheckInSentWithoutMonitorConfig();
}