From e4acee63223c9f54d1a098566621c7f1462daad9 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Thu, 29 Oct 2026 11:12:01 -0700 Subject: [PATCH] fix(hangfire): Skip jobs with unsupported schedules when monitoring all jobs MonitorAllRecurringJobs only covers recurring jobs whose schedule Sentry can represent, matching Quartz and sentry-python. Jobs with SentryMonitorSlug keep checking in without a config. --- src/Sentry.Hangfire/SentryHangfireOptions.cs | 6 +- src/Sentry.Hangfire/SentryServerFilter.cs | 30 +++++-- .../ServerFilterTests.cs | 83 ++++++++++++++++++- 3 files changed, 108 insertions(+), 11 deletions(-) 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(); }