diff --git a/src/Sentry.Hangfire/GlobalConfigurationExtensions.cs b/src/Sentry.Hangfire/GlobalConfigurationExtensions.cs index 8d096d4002..fee90a9359 100644 --- a/src/Sentry.Hangfire/GlobalConfigurationExtensions.cs +++ b/src/Sentry.Hangfire/GlobalConfigurationExtensions.cs @@ -19,6 +19,25 @@ public static IGlobalConfiguration UseSentry(this IGlobalConfiguration configura return configuration; } + /// + /// Adds the Sentry filter that captures check-ins for jobs marked with . + /// + /// The Hangfire configuration. + /// Configures the Sentry Hangfire integration. + /// The Hangfire configuration. + public static IGlobalConfiguration UseSentry(this IGlobalConfiguration configuration, Action configureOptions) + { + if (configureOptions is null) + { + throw new ArgumentNullException(nameof(configureOptions)); + } + + var options = new SentryHangfireOptions(); + configureOptions(options); + configuration.UseFilter(new SentryServerFilter(null, null, options)); + return configuration; + } + /// /// For testing /// diff --git a/src/Sentry.Hangfire/Sentry.Hangfire.csproj b/src/Sentry.Hangfire/Sentry.Hangfire.csproj index 4e2f001c85..ff3b319e63 100644 --- a/src/Sentry.Hangfire/Sentry.Hangfire.csproj +++ b/src/Sentry.Hangfire/Sentry.Hangfire.csproj @@ -19,6 +19,11 @@ + + + + + diff --git a/src/Sentry.Hangfire/SentryHangfireOptions.cs b/src/Sentry.Hangfire/SentryHangfireOptions.cs new file mode 100644 index 0000000000..3bba1cb648 --- /dev/null +++ b/src/Sentry.Hangfire/SentryHangfireOptions.cs @@ -0,0 +1,25 @@ +namespace Sentry.Hangfire; + +/// +/// Options for the Sentry Hangfire integration. +/// +public class SentryHangfireOptions +{ + /// + /// When enabled, the in-progress check-in of a recurring job includes the job's cron expression and time zone + /// as the monitor config, so Sentry creates the monitor or updates its schedule. Defaults to true. + /// Set to false to manage the monitor's schedule in Sentry instead. + /// + /// + /// + /// The monitor slug still comes from . If several recurring jobs run the + /// same method, they share one monitor, and each run overwrites the monitor's schedule with its own job's. + /// + /// + /// Schedules that Sentry can't represent are not sent. This includes crons that use seconds other than a fixed + /// value, and time zones that can't be converted to an IANA ID. On .NET Framework, Windows time zone IDs can't be + /// converted, so only UTC and IANA IDs are sent. + /// + /// + public bool SendMonitorConfig { get; set; } = true; +} diff --git a/src/Sentry.Hangfire/SentryServerFilter.cs b/src/Sentry.Hangfire/SentryServerFilter.cs index fab9a64212..5c76d3f269 100644 --- a/src/Sentry.Hangfire/SentryServerFilter.cs +++ b/src/Sentry.Hangfire/SentryServerFilter.cs @@ -1,5 +1,7 @@ using Hangfire.Server; +using Hangfire.Storage; using Sentry.Extensibility; +using Sentry.Internal; namespace Sentry.Hangfire; @@ -7,16 +9,21 @@ internal class SentryServerFilter : IServerFilter { internal const string SentryMonitorSlugKey = "SentryMonitorSlug"; internal const string SentryCheckInIdKey = "SentryCheckInIdKey"; + internal const string RecurringJobIdKey = "RecurringJobId"; private readonly IHub _hub; private readonly IDiagnosticLogger? _logger; + private readonly SentryHangfireOptions _options; + + internal SentryHangfireOptions Options => _options; public SentryServerFilter() : this(null, null) { } - internal SentryServerFilter(IHub? hub, IDiagnosticLogger? logger) + internal SentryServerFilter(IHub? hub, IDiagnosticLogger? logger, SentryHangfireOptions? options = null) { _hub = hub ?? HubAdapter.Instance; + _options = options ?? new SentryHangfireOptions(); #pragma warning disable CS0618 // Type or member is obsolete _logger = logger ?? _hub.GetInternalSentryOptions()?.DiagnosticLogger; #pragma warning restore CS0618 // Type or member is obsolete @@ -35,7 +42,7 @@ public void OnPerforming(PerformingContext context) return; } - var checkInId = _hub.CaptureCheckIn(monitorSlug, CheckInStatus.InProgress); + var checkInId = CaptureInProgressCheckIn(context, monitorSlug); // 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 @@ -43,6 +50,148 @@ public void OnPerforming(PerformingContext context) context.Items[SentryCheckInIdKey] = checkInId; } + private SentryId CaptureInProgressCheckIn(PerformingContext context, string monitorSlug) + { + Action? configureMonitorOptions = null; + if (_options.SendMonitorConfig && GetRecurringJobSchedule(context) is (var crontab, var timeZone)) + { + configureMonitorOptions = options => + { + options.Interval(crontab); + options.TimeZone = timeZone; + }; + } + + // Created here, so the final check-in has the same ID even when this one isn't sent + var checkInId = SentryId.Create(); + _hub.CaptureCheckIn(monitorSlug, CheckInStatus.InProgress, checkInId, configureMonitorOptions: configureMonitorOptions); + return checkInId; + } + + private (string Crontab, string TimeZone)? GetRecurringJobSchedule(PerformingContext context) + { + string? recurringJobId = null; + try + { + recurringJobId = context.GetJobParameter(RecurringJobIdKey); + if (string.IsNullOrEmpty(recurringJobId)) + { + return null; + } + + var recurringJob = context.Connection.GetRecurringJobs([recurringJobId]).SingleOrDefault(); + if (recurringJob is null || recurringJob.Removed) + { + _logger?.LogDebug("Not sending the schedule of recurring job '{0}'. The job no longer exists.", recurringJobId); + return null; + } + + var crontab = ToCrontab(recurringJob.Cron); + var timeZone = ToIanaTimeZoneId(recurringJob.TimeZoneId); + if (crontab is null || timeZone is null) + { + _logger?.LogDebug("Not sending the schedule of recurring job '{0}'. Sentry doesn't support " + + "the cron expression '{1}' with time zone '{2}'.", recurringJobId, recurringJob.Cron, recurringJob.TimeZoneId); + return null; + } + + return (crontab, timeZone); + } + catch (Exception e) + { + _logger?.LogError(e, "Failed to read the schedule of recurring job '{0}'.", recurringJobId); + return null; + } + } + + // Hangfire also accepts a leading seconds field, which Sentry doesn't support. + internal static string? ToCrontab(string? cron) + { + if (string.IsNullOrWhiteSpace(cron)) + { + return null; + } + + var fields = cron!.Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries); + if (fields.Length == 6 + && int.TryParse(fields[0], NumberStyles.None, CultureInfo.InvariantCulture, out var seconds) + && seconds < 60) + { + fields = fields.Skip(1).ToArray(); + } + + if (fields.Length != 5) + { + return null; + } + + // Hangfire runs a job only on days that match both day fields, while Sentry runs it on days that match + // either one unless one of them starts with '*'. + if (!fields[2].StartsWith("*") && !fields[4].StartsWith("*")) + { + return null; + } + + var crontab = string.Join(" ", fields); + return CrontabValidator.IsValid(crontab) && !HasReversedRange(fields) ? crontab : null; + } + + private static readonly string[] DayNames = ["SUN", "MON", "TUE", "WED", "THU", "FRI", "SAT"]; + + // Sentry rejects ranges whose start is after their end, such as 5-1 or SAT-SUN + private static bool HasReversedRange(string[] fields) + { + foreach (var field in fields) + { + foreach (var item in field.Split(',')) + { + var range = item.Split('/')[0].Split('-'); + if (range.Length == 2 && ToNumber(range[0]) > ToNumber(range[1])) + { + return true; + } + } + } + + return false; + + static int ToNumber(string value) => + int.TryParse(value, NumberStyles.None, CultureInfo.InvariantCulture, out var number) + ? number + : Array.FindIndex(DayNames, name => string.Equals(name, value, StringComparison.OrdinalIgnoreCase)); + } + + internal static string? ToIanaTimeZoneId(string? timeZoneId) + { + // Hangfire's default is TimeZoneInfo.Utc, whose ID is "UTC" on every platform + if (string.IsNullOrWhiteSpace(timeZoneId) || timeZoneId == "UTC") + { + return "UTC"; + } + +#if NET6_0_OR_GREATER + TimeZoneInfo timeZone; + try + { + timeZone = TimeZoneInfo.FindSystemTimeZoneById(timeZoneId!); + } + catch (Exception) + { + return null; + } + + if (timeZone.HasIanaId) + { + return timeZone.Id; + } + + return TimeZoneInfo.TryConvertWindowsIdToIanaId(timeZone.Id, out var ianaId) ? ianaId : null; +#else + // .NET Framework can neither look up nor convert IANA IDs, so only pass on IDs that look like one + return timeZoneId!.Contains("/") ? timeZoneId : null; +#endif + } + public void OnPerformed(PerformedContext context) { var monitorSlug = context.GetJobParameter(SentryMonitorSlugKey); diff --git a/src/Sentry/Internal/CrontabValidator.cs b/src/Sentry/Internal/CrontabValidator.cs new file mode 100644 index 0000000000..8af7d9a90d --- /dev/null +++ b/src/Sentry/Internal/CrontabValidator.cs @@ -0,0 +1,43 @@ +using System.Text.RegularExpressions; + +namespace Sentry.Internal; + +/// +/// Validates the crontab format Sentry accepts for monitor schedules. +/// +/// +/// Also compiled into Sentry.Hangfire, which can't see Sentry's internals because it isn't strong-named. +/// +internal static partial class CrontabValidator +{ + // Breakdown of the validation regex pattern: + // For each time field (minute, hour, day, month, weekday): + // - Allows * for "any value" + // - Allows */n for step values where n must be any positive integer (except zero) + // - Allows single values within their valid ranges + // - Allows ranges (e.g., 8-10) + // - Allows step values with ranges (e.g., 8-18/4) + // - Allows lists of values and ranges (e.g., 6,8,9 or 8-10,12-14) + // - Allows weekday names (MON, TUE, WED, THU, FRI, SAT, SUN) + // + // Valid ranges for each field: + // - Minutes: 0-59 + // - Hours: 0-23 + // - Days: 1-31 + // - Months: 1-12 + // - Weekdays: 0-7 (0 and 7 both represent Sunday) or MON-SUN + private const string ValidCrontabPattern = @"^(\*(\/([1-9][0-9]*))?|([0-5]?\d)|([0-5]?\d)-([0-5]?\d)(\/([1-9][0-9]*))?)(,(\*(\/([1-9][0-9]*))?|([0-5]?\d)|([0-5]?\d)-([0-5]?\d)(\/([1-9][0-9]*))?))*(\s+)(\*(\/([1-9][0-9]*))?|([01]?\d|2[0-3])|([01]?\d|2[0-3])-([01]?\d|2[0-3])(\/([1-9][0-9]*))?)(,(\*(\/([1-9][0-9]*))?|([01]?\d|2[0-3])|([01]?\d|2[0-3])-([01]?\d|2[0-3])(\/([1-9][0-9]*))?))*(\s+)(\*(\/([1-9][0-9]*))?|([1-9]|[12]\d|3[01])|([1-9]|[12]\d|3[01])-([1-9]|[12]\d|3[01])(\/([1-9][0-9]*))?)(,(\*(\/([1-9][0-9]*))?|([1-9]|[12]\d|3[01])|([1-9]|[12]\d|3[01])-([1-9]|[12]\d|3[01])(\/([1-9][0-9]*))?))*(\s+)(\*(\/([1-9][0-9]*))?|([1-9]|1[0-2])|([1-9]|1[0-2])-([1-9]|1[0-2])(\/([1-9][0-9]*))?)(,(\*(\/([1-9][0-9]*))?|([1-9]|1[0-2])|([1-9]|1[0-2])-([1-9]|1[0-2])(\/([1-9][0-9]*))?))*(\s+)(\*(\/([1-9][0-9]*))?|[0-7]|(MON|TUE|WED|THU|FRI|SAT|SUN)|[0-7]-[0-7](\/([1-9][0-9]*))?|(MON|TUE|WED|THU|FRI|SAT|SUN)-(MON|TUE|WED|THU|FRI|SAT|SUN)(\/([1-9][0-9]*))?)(,(\*(\/([1-9][0-9]*))?|[0-7]|(MON|TUE|WED|THU|FRI|SAT|SUN)|[0-7]-[0-7](\/([1-9][0-9]*))?|(MON|TUE|WED|THU|FRI|SAT|SUN)-(MON|TUE|WED|THU|FRI|SAT|SUN)(\/([1-9][0-9]*))?))*$"; + +#if NET9_0_OR_GREATER + [GeneratedRegex(ValidCrontabPattern, RegexOptions.CultureInvariant | RegexOptions.IgnoreCase | RegexOptions.ExplicitCapture)] + private static partial Regex ValidCrontab { get; } +#elif NET8_0 + [GeneratedRegex(ValidCrontabPattern, RegexOptions.CultureInvariant | RegexOptions.IgnoreCase | RegexOptions.ExplicitCapture)] + private static partial Regex ValidCrontabRegex(); + private static readonly Regex ValidCrontab = ValidCrontabRegex(); +#else + private static readonly Regex ValidCrontab = new(ValidCrontabPattern, RegexOptions.Compiled | RegexOptions.CultureInvariant | RegexOptions.IgnoreCase | RegexOptions.ExplicitCapture); +#endif + + public static bool IsValid(string crontab) => ValidCrontab.IsMatch(crontab); +} diff --git a/src/Sentry/SentryMonitorOptions.cs b/src/Sentry/SentryMonitorOptions.cs index 8d566beedc..e814c89a0c 100644 --- a/src/Sentry/SentryMonitorOptions.cs +++ b/src/Sentry/SentryMonitorOptions.cs @@ -1,4 +1,5 @@ using Sentry.Extensibility; +using Sentry.Internal; using Sentry.Internal.Extensions; namespace Sentry; @@ -49,42 +50,13 @@ public enum SentryMonitorInterval /// /// Sentry's options for monitors /// -public partial class SentryMonitorOptions : ISentryJsonSerializable +public class SentryMonitorOptions : ISentryJsonSerializable { - // Breakdown of the validation regex pattern: - // For each time field (minute, hour, day, month, weekday): - // - Allows * for "any value" - // - Allows */n for step values where n must be any positive integer (except zero) - // - Allows single values within their valid ranges - // - Allows ranges (e.g., 8-10) - // - Allows step values with ranges (e.g., 8-18/4) - // - Allows lists of values and ranges (e.g., 6,8,9 or 8-10,12-14) - // - Allows weekday names (MON, TUE, WED, THU, FRI, SAT, SUN) - // - // Valid ranges for each field: - // - Minutes: 0-59 - // - Hours: 0-23 - // - Days: 1-31 - // - Months: 1-12 - // - Weekdays: 0-7 (0 and 7 both represent Sunday) or MON-SUN - private const string ValidCrontabPattern = @"^(\*(\/([1-9][0-9]*))?|([0-5]?\d)|([0-5]?\d)-([0-5]?\d)(\/([1-9][0-9]*))?)(,(\*(\/([1-9][0-9]*))?|([0-5]?\d)|([0-5]?\d)-([0-5]?\d)(\/([1-9][0-9]*))?))*(\s+)(\*(\/([1-9][0-9]*))?|([01]?\d|2[0-3])|([01]?\d|2[0-3])-([01]?\d|2[0-3])(\/([1-9][0-9]*))?)(,(\*(\/([1-9][0-9]*))?|([01]?\d|2[0-3])|([01]?\d|2[0-3])-([01]?\d|2[0-3])(\/([1-9][0-9]*))?))*(\s+)(\*(\/([1-9][0-9]*))?|([1-9]|[12]\d|3[01])|([1-9]|[12]\d|3[01])-([1-9]|[12]\d|3[01])(\/([1-9][0-9]*))?)(,(\*(\/([1-9][0-9]*))?|([1-9]|[12]\d|3[01])|([1-9]|[12]\d|3[01])-([1-9]|[12]\d|3[01])(\/([1-9][0-9]*))?))*(\s+)(\*(\/([1-9][0-9]*))?|([1-9]|1[0-2])|([1-9]|1[0-2])-([1-9]|1[0-2])(\/([1-9][0-9]*))?)(,(\*(\/([1-9][0-9]*))?|([1-9]|1[0-2])|([1-9]|1[0-2])-([1-9]|1[0-2])(\/([1-9][0-9]*))?))*(\s+)(\*(\/([1-9][0-9]*))?|[0-7]|(MON|TUE|WED|THU|FRI|SAT|SUN)|[0-7]-[0-7](\/([1-9][0-9]*))?|(MON|TUE|WED|THU|FRI|SAT|SUN)-(MON|TUE|WED|THU|FRI|SAT|SUN)(\/([1-9][0-9]*))?)(,(\*(\/([1-9][0-9]*))?|[0-7]|(MON|TUE|WED|THU|FRI|SAT|SUN)|[0-7]-[0-7](\/([1-9][0-9]*))?|(MON|TUE|WED|THU|FRI|SAT|SUN)-(MON|TUE|WED|THU|FRI|SAT|SUN)(\/([1-9][0-9]*))?))*$"; - private SentryMonitorScheduleType _type = SentryMonitorScheduleType.None; private string? _crontab; private int? _interval; private SentryMonitorInterval? _unit; -#if NET9_0_OR_GREATER - [GeneratedRegex(ValidCrontabPattern, RegexOptions.CultureInvariant | RegexOptions.IgnoreCase | RegexOptions.ExplicitCapture)] - private static partial Regex ValidCrontab { get; } -#elif NET8_0 - [GeneratedRegex(ValidCrontabPattern, RegexOptions.CultureInvariant | RegexOptions.IgnoreCase | RegexOptions.ExplicitCapture)] - private static partial Regex ValidCrontabRegex(); - private static readonly Regex ValidCrontab = ValidCrontabRegex(); -#else - private static readonly Regex ValidCrontab = new(ValidCrontabPattern, RegexOptions.Compiled | RegexOptions.CultureInvariant | RegexOptions.IgnoreCase | RegexOptions.ExplicitCapture); -#endif - /// /// Set Interval /// @@ -96,7 +68,7 @@ public void Interval(string crontab) throw new ArgumentException("You tried to set the interval twice. The Check-Ins interval is supposed to be set only once."); } - if (!ValidCrontab.IsMatch(crontab)) + if (!CrontabValidator.IsValid(crontab)) { throw new ArgumentException("The provided crontab does not match the expected format of '* * * * *' " + "translating to 'minute', 'hour', 'day of the month', 'month', and 'day of the week'."); diff --git a/test/Sentry.Hangfire.Tests/HangfireFixture.cs b/test/Sentry.Hangfire.Tests/HangfireFixture.cs index 949f02c693..3b9f5cd0f9 100644 --- a/test/Sentry.Hangfire.Tests/HangfireFixture.cs +++ b/test/Sentry.Hangfire.Tests/HangfireFixture.cs @@ -26,9 +26,16 @@ public HangfireFixture() _monitoringApi = JobStorage.Current.GetMonitoringApi(); } - public Task Enqueue(Expression> methodCall) + public Task Enqueue(Expression> methodCall) => WaitForJob(BackgroundJob.Enqueue(methodCall)); + + public Task TriggerRecurringJob(string recurringJobId, Expression> methodCall, string cron, TimeZoneInfo timeZone) + { + RecurringJob.AddOrUpdate(recurringJobId, methodCall, cron, new RecurringJobOptions { TimeZone = timeZone }); + return WaitForJob(RecurringJob.TriggerJob(recurringJobId)); + } + + private Task WaitForJob(string jobId) { - var jobId = BackgroundJob.Enqueue(methodCall); var checkJobState = Task.Run(() => { while (true) diff --git a/test/Sentry.Hangfire.Tests/HangfireTests.cs b/test/Sentry.Hangfire.Tests/HangfireTests.cs index 0e4601363e..2efbc1a391 100644 --- a/test/Sentry.Hangfire.Tests/HangfireTests.cs +++ b/test/Sentry.Hangfire.Tests/HangfireTests.cs @@ -1,3 +1,5 @@ +using Hangfire; + namespace Sentry.Hangfire.Tests; public class HangfireTests : IClassFixture @@ -12,28 +14,24 @@ public HangfireTests(HangfireFixture hangfireFixture) [Fact] public async Task ExecuteJobWithAttribute_CapturesCheckInInProgressAndOkWithDuration() { - var sentryId = SentryId.Create(); - _fixture.Hub.CaptureCheckIn(Arg.Any(), Arg.Any()).Returns(sentryId); - await _fixture.Enqueue(job => job.ExecuteJobWithAttribute()); _fixture.Hub.Received(1).CaptureCheckIn( Arg.Is("test-job"), Arg.Is(status => status == CheckInStatus.InProgress), Arg.Any()); + var checkInId = _fixture.Hub.ReceivedInProgressCheckInId("test-job"); + checkInId.Should().NotBeNull().And.NotBe(SentryId.Empty); _fixture.Hub.Received(1).CaptureCheckIn( Arg.Is("test-job"), Arg.Is(status => status == CheckInStatus.Ok), - Arg.Is(id => id == sentryId), + checkInId, Arg.Is(duration => duration != null), Arg.Any()); } [Fact] public async Task ExecuteJobWithException_CapturesCheckInInProgressAndErrorWithDuration() { - var sentryId = SentryId.Create(); - _fixture.Hub.CaptureCheckIn(Arg.Any(), Arg.Any()).Returns(sentryId); - await _fixture.Enqueue(job => job.ExecuteJobWithException()); await Task.Delay(1000); @@ -42,10 +40,12 @@ public async Task ExecuteJobWithException_CapturesCheckInInProgressAndErrorWithD Arg.Is("test-job-with-exception"), Arg.Is(status => status == CheckInStatus.InProgress), Arg.Any()); + var checkInId = _fixture.Hub.ReceivedInProgressCheckInId("test-job-with-exception"); + checkInId.Should().NotBeNull().And.NotBe(SentryId.Empty); _fixture.Hub.Received(1).CaptureCheckIn( Arg.Is("test-job-with-exception"), Arg.Is(status => status == CheckInStatus.Error), - Arg.Is(id => id == sentryId), + checkInId, Arg.Is(duration => duration != null), Arg.Any()); } @@ -60,6 +60,53 @@ public async Task ExecuteJobWithoutAttribute_DoesNotCapturesCheckInButLogs() _fixture.Hub.DidNotReceive().CaptureCheckIn(Arg.Any(), Arg.Any(), Arg.Is(id => id == sentryId)); _fixture.Logger.Received(1).Log(SentryLevel.Debug, Arg.Is(message => message.Contains("Skipping creating a check-in for")), null, Arg.Any(), Arg.Any()); } + + [SkippableFact] + public async Task ExecuteRecurringJob_SendMonitorConfigEnabled_CapturesCheckInWithMonitorConfig() + { + Skip.If(!TestEnvironment.HasTimeZone("Europe/Berlin"), "No time zone database"); + + var timeZone = TimeZoneInfo.FindSystemTimeZoneById("Europe/Berlin"); + + await _fixture.TriggerRecurringJob("test-recurring-job-id", job => job.ExecuteRecurringJob(), "30 0 0 1 1 *", timeZone); + + var configureMonitorOptions = _fixture.Hub.ReceivedConfigureMonitorOptions("test-recurring-job"); + configureMonitorOptions.Should().NotBeNull(); + var monitorConfig = MonitorConfigJson.Render(configureMonitorOptions!); + monitorConfig.GetProperty("schedule").GetProperty("type").GetString().Should().Be("crontab"); + monitorConfig.GetProperty("schedule").GetProperty("value").GetString().Should().Be("0 0 1 1 *"); + monitorConfig.GetProperty("timezone").GetString().Should().Be("Europe/Berlin"); + } + + // In this class so it runs serially with the fixture's jobs, which would also pick up the filter it adds + [Fact] + public void UseSentry_ConfigureOptions_AddsFilterWithOptions() + { + var configuration = Substitute.For(); + var existingFilters = SentryFilters().ToList(); + + configuration.UseSentry(options => options.SendMonitorConfig = false); + + var addedFilters = SentryFilters().Except(existingFilters).ToList(); + foreach (var filter in addedFilters) + { + GlobalJobFilters.Filters.Remove(filter); + } + addedFilters.Should().ContainSingle().Which.Options.SendMonitorConfig.Should().BeFalse(); + + static IEnumerable SentryFilters() => + GlobalJobFilters.Filters.Select(filter => filter.Instance).OfType(); + } + + [Fact] + public void UseSentry_NullConfigureOptions_Throws() + { + var configuration = Substitute.For(); + + var act = () => configuration.UseSentry((Action)null!); + + act.Should().Throw(); + } } public class TestJob @@ -76,4 +123,8 @@ public void ExecuteJobWithException() public void ExecuteJobWithoutAttribute() { } + + [SentryMonitorSlug("test-recurring-job")] + public void ExecuteRecurringJob() + { } } diff --git a/test/Sentry.Hangfire.Tests/MonitorConfigJson.cs b/test/Sentry.Hangfire.Tests/MonitorConfigJson.cs new file mode 100644 index 0000000000..27b06cb280 --- /dev/null +++ b/test/Sentry.Hangfire.Tests/MonitorConfigJson.cs @@ -0,0 +1,32 @@ +namespace Sentry.Hangfire.Tests; + +internal static class MonitorConfigJson +{ + public static JsonElement Render(Action configureMonitorOptions) + { + var options = new SentryMonitorOptions(); + configureMonitorOptions(options); + var checkIn = new SentryCheckIn("monitor-slug", CheckInStatus.InProgress) { MonitorOptions = options }; + + using var stream = new MemoryStream(); + using (var writer = new Utf8JsonWriter(stream)) + { + checkIn.WriteTo(writer, null); + } + + using var document = JsonDocument.Parse(stream.ToArray()); + return document.RootElement.GetProperty("monitor_config").Clone(); + } + + public static Action? ReceivedConfigureMonitorOptions(this IHub hub, string monitorSlug) => + hub.ReceivedInProgressCheckIn(monitorSlug)[5] as Action; + + public static SentryId? ReceivedInProgressCheckInId(this IHub hub, string monitorSlug) => + (SentryId?)hub.ReceivedInProgressCheckIn(monitorSlug)[2]; + + private static object?[] ReceivedInProgressCheckIn(this IHub hub, string monitorSlug) => + hub.ReceivedCalls() + .Where(call => call.GetMethodInfo().Name == nameof(IHub.CaptureCheckIn)) + .Select(call => call.GetArguments()) + .Single(args => (string?)args[0] == monitorSlug && (CheckInStatus?)args[1] == CheckInStatus.InProgress); +} diff --git a/test/Sentry.Hangfire.Tests/ServerFilterTests.cs b/test/Sentry.Hangfire.Tests/ServerFilterTests.cs index 0ac1a1f8a5..a2c798ec44 100644 --- a/test/Sentry.Hangfire.Tests/ServerFilterTests.cs +++ b/test/Sentry.Hangfire.Tests/ServerFilterTests.cs @@ -30,7 +30,6 @@ public void OnPerforming_IsReentrant() var performingContext = new PerformingContext(performContext); var hub = Substitute.For(); - hub.CaptureCheckIn(monitorSlug, CheckInStatus.InProgress).Returns(SentryId.Create()); var logger = Substitute.For(); var filter = new SentryServerFilter(hub, logger); @@ -47,6 +46,255 @@ public void OnPerforming_IsReentrant() // Assert performContext.Items.ContainsKey(SentryServerFilter.SentryCheckInIdKey).Should().BeTrue(); - performingContext.Items[SentryServerFilter.SentryCheckInIdKey].Should().NotBeSameAs(firstKey); + performingContext.Items[SentryServerFilter.SentryCheckInIdKey].Should().NotBe(firstKey); + } + + [Fact] + public void OnPerformed_InProgressCheckInNotCaptured_FinishesWithSameNonEmptyId() + { + // Arrange + var fixture = new RecurringJobFixture { CheckInId = SentryId.Empty }; + var filter = fixture.GetSut(); + filter.OnPerforming(fixture.PerformingContext); + var performedContext = new PerformedContext(fixture.PerformingContext, null, false, null); + + // Act + filter.OnPerformed(performedContext); + + // Assert + var checkInId = fixture.Hub.ReceivedInProgressCheckInId(RecurringJobFixture.MonitorSlug); + checkInId.Should().NotBeNull().And.NotBe(SentryId.Empty); + fixture.Hub.Received(1).CaptureCheckIn(RecurringJobFixture.MonitorSlug, CheckInStatus.Ok, checkInId, Arg.Any()); + } + + [SkippableTheory] + [InlineData("0 */2 * * *", "Europe/Berlin", "0 */2 * * *", "Europe/Berlin")] + [InlineData("30 0 */2 * * *", "UTC", "0 */2 * * *", "UTC")] + [InlineData("0 0 * * MON", null, "0 0 * * MON", "UTC")] + public void OnPerforming_RecurringJobWithSendMonitorConfigEnabled_SendsMonitorConfig( + string cron, string? timeZoneId, string expectedCrontab, string expectedTimeZone) + { + Skip.If(timeZoneId is not (null or "UTC") && !TestEnvironment.HasTimeZone(timeZoneId), "No time zone database"); + + // Arrange + var fixture = new RecurringJobFixture { Cron = cron, TimeZoneId = timeZoneId }; + var filter = fixture.GetSut(sendMonitorConfig: true); + + // Act + filter.OnPerforming(fixture.PerformingContext); + + // Assert + var monitorConfig = fixture.GetSentMonitorConfig(); + monitorConfig.Should().NotBeNull(); + monitorConfig!.Value.GetProperty("schedule").GetProperty("type").GetString().Should().Be("crontab"); + monitorConfig.Value.GetProperty("schedule").GetProperty("value").GetString().Should().Be(expectedCrontab); + monitorConfig.Value.GetProperty("timezone").GetString().Should().Be(expectedTimeZone); + fixture.PerformingContext.Items[SentryServerFilter.SentryCheckInIdKey].Should().Be(fixture.Hub.ReceivedInProgressCheckInId(RecurringJobFixture.MonitorSlug)); + } + + [Fact] + public void OnPerforming_DefaultOptions_SendsMonitorConfig() + { + // Arrange + var fixture = new RecurringJobFixture(); + var filter = fixture.GetSut(); + + // Act + filter.OnPerforming(fixture.PerformingContext); + + // Assert + var monitorConfig = fixture.GetSentMonitorConfig(); + monitorConfig.Should().NotBeNull(); + monitorConfig!.Value.GetProperty("schedule").GetProperty("value").GetString().Should().Be(fixture.Cron); + monitorConfig.Value.GetProperty("timezone").GetString().Should().Be("UTC"); + } + + [Fact] + public void OnPerforming_SendMonitorConfigDisabled_SendsCheckInWithoutMonitorConfig() + { + // Arrange + var fixture = new RecurringJobFixture(); + var filter = fixture.GetSut(sendMonitorConfig: false); + + // Act + filter.OnPerforming(fixture.PerformingContext); + + // Assert + fixture.AssertCheckInSentWithoutMonitorConfig(); + fixture.Connection.DidNotReceive().GetAllEntriesFromHash(Arg.Any()); + } + + [Fact] + public void OnPerforming_NotRecurringJob_SendsCheckInWithoutMonitorConfig() + { + // Arrange + var fixture = new RecurringJobFixture { RecurringJobId = null }; + var filter = fixture.GetSut(sendMonitorConfig: true); + + // Act + filter.OnPerforming(fixture.PerformingContext); + + // Assert + fixture.AssertCheckInSentWithoutMonitorConfig(); + } + + [Fact] + public void OnPerforming_RecurringJobDeleted_SendsCheckInWithoutMonitorConfig() + { + // Arrange + var fixture = new RecurringJobFixture { RecurringJobExists = false }; + var filter = fixture.GetSut(sendMonitorConfig: true); + + // Act + filter.OnPerforming(fixture.PerformingContext); + + // Assert + fixture.AssertCheckInSentWithoutMonitorConfig(); + } + + [Theory] + [InlineData("0 0 L * *", "UTC")] + [InlineData("60 0 * * * *", "UTC")] + [InlineData("*/10 * * * * *", "UTC")] + [InlineData("0 * * * *", "Not A Time Zone")] + public void OnPerforming_ScheduleNotSupportedBySentry_SendsCheckInWithoutMonitorConfig(string cron, string timeZoneId) + { + // Arrange + var fixture = new RecurringJobFixture { Cron = cron, TimeZoneId = timeZoneId }; + var filter = fixture.GetSut(sendMonitorConfig: true); + + // Act + filter.OnPerforming(fixture.PerformingContext); + + // Assert + fixture.AssertCheckInSentWithoutMonitorConfig(); + fixture.Logger.DidNotReceive().Log(SentryLevel.Error, Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Fact] + public void OnPerforming_StorageThrows_SendsCheckInWithoutMonitorConfig() + { + // Arrange + var fixture = new RecurringJobFixture(); + var filter = fixture.GetSut(sendMonitorConfig: true); + fixture.Connection.GetAllEntriesFromHash(Arg.Any()).Throws(new InvalidOperationException()); + + // Act + filter.OnPerforming(fixture.PerformingContext); + + // Assert + fixture.AssertCheckInSentWithoutMonitorConfig(); + } + + [Theory] + [InlineData("0 */2 * * *", "0 */2 * * *")] + [InlineData("30 0 */2 * * *", "0 */2 * * *")] + [InlineData("0 0 * * 1", "0 0 * * 1")] + [InlineData("60 0 * * * *", null)] + [InlineData("*/10 * * * * *", null)] + [InlineData("0,30 * * * * *", null)] + [InlineData("0 0 L * *", null)] + [InlineData("@daily", null)] + [InlineData("", null)] + [InlineData("0 9 * * MON", "0 9 * * MON")] + [InlineData("0 9 1-7 * *", "0 9 1-7 * *")] + [InlineData("0 9 */2 * MON", "0 9 */2 * MON")] + [InlineData("0 9 1-7 * */2", "0 9 1-7 * */2")] + [InlineData("0 9 1-7 * MON", null)] + [InlineData("0 0 9 15 * 1-5", null)] + [InlineData("0 9 * * MON-FRI", "0 9 * * MON-FRI")] + [InlineData("0 9 * * 5-1", null)] + [InlineData("0 9 * * SAT-SUN", null)] + [InlineData("0 9 * * mon-sun", null)] + [InlineData("0 22-2 * * *", null)] + [InlineData("0 9 * * 1-3,6-0", null)] + [InlineData("0 8-18/2 * * *", "0 8-18/2 * * *")] + public void ToCrontab_HangfireCron_ReturnsSentryCrontabOrNull(string cron, string? expected) + { + SentryServerFilter.ToCrontab(cron).Should().Be(expected); + } + + [SkippableTheory] + [InlineData(null, "UTC")] + [InlineData("", "UTC")] + [InlineData("UTC", "UTC")] + [InlineData("Etc/UTC", "Etc/UTC")] + [InlineData("Europe/Berlin", "Europe/Berlin")] + [InlineData("W. Europe Standard Time", "Europe/Berlin")] + [InlineData("Not A Time Zone", null)] + [InlineData("Not/A_Time_Zone", null)] + public void ToIanaTimeZoneId_HangfireTimeZoneId_ReturnsIanaIdOrNull(string? timeZoneId, string? expected) + { + Skip.If(expected is not (null or "UTC") && !TestEnvironment.HasTimeZone(timeZoneId!), "No time zone database"); + + SentryServerFilter.ToIanaTimeZoneId(timeZoneId).Should().Be(expected); + } + + private class RecurringJobFixture + { + public const string MonitorSlug = "test-monitor-slug"; + private const string JobId = "test-id"; + + public string? RecurringJobId { get; set; } = "test-recurring-job"; + public bool RecurringJobExists { get; set; } = true; + public string Cron { get; set; } = "0 * * * *"; + public string? TimeZoneId { get; set; } + + public IStorageConnection Connection { get; } = Substitute.For(); + public IHub Hub { get; } = Substitute.For(); + public IDiagnosticLogger Logger { get; } = Substitute.For(); + public SentryId CheckInId { get; set; } = SentryId.Create(); + public PerformingContext PerformingContext { get; private set; } = null!; + + public SentryServerFilter GetSut(bool? sendMonitorConfig = null) + { + Logger.IsEnabled(Arg.Any()).Returns(true); + Connection.GetJobParameter(JobId, SentryServerFilter.SentryMonitorSlugKey) + .Returns(SerializationHelper.Serialize(MonitorSlug)); + if (RecurringJobId is not null) + { + Connection.GetJobParameter(JobId, SentryServerFilter.RecurringJobIdKey) + .Returns(SerializationHelper.Serialize(RecurringJobId)); + if (RecurringJobExists) + { + var recurringJob = new Dictionary { ["Cron"] = Cron }; + if (TimeZoneId is not null) + { + recurringJob["TimeZoneId"] = TimeZoneId; + } + Connection.GetAllEntriesFromHash($"recurring-job:{RecurringJobId}").Returns(recurringJob); + } + } + + Hub.CaptureCheckIn( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any?>()) + .Returns(CheckInId); + + var backgroundJob = new BackgroundJob(JobId, null, DateTime.Now); + var performContext = new PerformContext(null, Connection, backgroundJob, Substitute.For()); + PerformingContext = new PerformingContext(performContext); + + var options = new SentryHangfireOptions(); + if (sendMonitorConfig is { } send) + { + options.SendMonitorConfig = send; + } + return new SentryServerFilter(Hub, Logger, options); + } + + public JsonElement? GetSentMonitorConfig() => + Hub.ReceivedConfigureMonitorOptions(MonitorSlug) is { } configure ? MonitorConfigJson.Render(configure) : null; + + public void AssertCheckInSentWithoutMonitorConfig() + { + Hub.Received(1).CaptureCheckIn(MonitorSlug, CheckInStatus.InProgress, Arg.Any()); + Hub.ReceivedConfigureMonitorOptions(MonitorSlug).Should().BeNull(); + PerformingContext.Items[SentryServerFilter.SentryCheckInIdKey].Should().Be(Hub.ReceivedInProgressCheckInId(MonitorSlug)); + } } } diff --git a/test/Sentry.Testing/TestEnvironment.cs b/test/Sentry.Testing/TestEnvironment.cs index e70c571fd8..42e426863d 100644 --- a/test/Sentry.Testing/TestEnvironment.cs +++ b/test/Sentry.Testing/TestEnvironment.cs @@ -13,4 +13,20 @@ public static bool IsGitHubActions return isGitHubActions?.Equals("true", StringComparison.OrdinalIgnoreCase) == true; } } + + /// + /// Some containers (e.g. Alpine) ship without a time zone database. + /// + public static bool HasTimeZone(string id) + { + try + { + _ = TimeZoneInfo.FindSystemTimeZoneById(id); + return true; + } + catch (Exception e) when (e is TimeZoneNotFoundException or InvalidTimeZoneException) + { + return false; + } + } }