ref(crons): Extract Fugit schedule conversion

sentry-ruby/lib/sentry-ruby.rb

@@ -27,6 +27,7 @@
 require "sentry/session_flusher"
 require "sentry/backpressure_monitor"
 require "sentry/cron/monitor_check_ins"
+require "sentry/cron/fugit_schedule"
 require "sentry/vernier/profiler"
 require "sentry/metrics"
 

sentry-ruby/lib/sentry/cron/fugit_schedule.rb

@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+module Sentry
+  module Cron
+    # Converts fugit schedules into monitor configs. Returns nil for schedules
+    # Sentry would read differently.
+    module FugitSchedule
+      class << self
+        def from_cron(cron)
+          crontab = crontab_for(cron)
+          timezone = timezone_for(cron) if crontab
+          MonitorConfig.from_crontab(crontab, timezone: timezone, **options) if timezone
+        end
+
+        def from_seconds(seconds)
+          seconds = seconds.to_f
+          return unless seconds > 0 && (seconds % 60).zero?
+
+          seconds = seconds.to_i
+          if (seconds % 86_400).zero?
+            MonitorConfig.from_interval(seconds / 86_400, :day, **options)
+          elsif (seconds % 3_600).zero?
+            MonitorConfig.from_interval(seconds / 3_600, :hour, **options)
+          else
+            MonitorConfig.from_interval(seconds / 60, :minute, **options)
+          end
+        end
+
+        private
+
+        def options
+          cron_config = Sentry.configuration.cron
+          { checkin_margin: cron_config.default_checkin_margin, max_runtime: cron_config.default_max_runtime }
+        end
+
+        # Rebuilds a 5-field crontab from fugit's expanded fields so Sentry
+        # (Debian cron rules) reads it the same way. Returns nil for fugit-only
+        # syntax: sub-minute seconds, `L`, `#`, `%` and `&`.
+        def crontab_for(cron)
+          return unless defined?(::Fugit::Cron) && cron.is_a?(::Fugit::Cron)
+          return unless cron.seconds&.size == 1
+          return if cron.monthdays&.any?(&:negative?)
+          return if cron.weekdays&.any? { |day| day.size > 1 }
+          return if cron.monthdays && cron.weekdays && cron.instance_variable_get(:@day_and)
+
+          [
+            crontab_field(cron.minutes, 0, 59, step: true),
+            crontab_field(cron.hours, 0, 23, step: true),
+            # Day fields never use `*/n`: Debian cron treats a field starting
+            # with `*` as unrestricted when combining day-of-month and weekday.
+            crontab_field(cron.monthdays, 1, 31),
+            crontab_field(cron.months, 1, 12, step: true),
+            crontab_field(cron.weekdays&.map(&:first), 0, 6)
+          ].join(" ")
+        end
+
+        def crontab_field(values, min, max, step: false)
+          return "*" unless values
+
+          if step && values.size > 1 && values.first == min
+            increment = values[1] - values[0]
+            return "*/#{increment}" if values == min.step(max, increment).to_a
+          end
+
+          values.chunk_while { |a, b| b == a + 1 }.map { |run| run.size > 1 ? "#{run.first}-#{run.last}" : run.first.to_s }.join(",")
+        end
+
+        # Sentry only accepts IANA names, so offsets such as "+05:30" get no config.
+        def timezone_for(cron)
+          zone = cron.timezone || ::EtOrbi.determine_local_tzone
+          return Sentry.configuration.cron.default_timezone unless zone
+
+          ::TZInfo::Timezone.get(zone.name).identifier
+        rescue StandardError
+          nil
+        end
+      end
+    end
+  end
+end

sentry-ruby/lib/sentry/rufus_scheduler.rb

@@ -98,76 +98,15 @@ def slug_for(job)
       end
 
       def monitor_config_for(job)
-        cron_config = Sentry.configuration.cron
-        options = { checkin_margin: cron_config.default_checkin_margin, max_runtime: cron_config.default_max_runtime }
-
         case job
         when ::Rufus::Scheduler::CronJob
-          crontab = crontab_for(job.cron_line)
-          timezone = timezone_for(job.cron_line)
-          Sentry::Cron::MonitorConfig.from_crontab(crontab, timezone: timezone, **options) if crontab && timezone
+          Sentry::Cron::FugitSchedule.from_cron(job.cron_line)
         when ::Rufus::Scheduler::EveryJob
-          interval_for(job.frequency, options)
+          Sentry::Cron::FugitSchedule.from_seconds(job.frequency)
         end
         # IntervalJob waits `interval` after each run ends, so it has no fixed
         # schedule to monitor against.
       end
-
-      def interval_for(seconds, options)
-        seconds = seconds.to_f
-        return unless seconds > 0 && (seconds % 60).zero?
-
-        seconds = seconds.to_i
-        if (seconds % 86_400).zero?
-          Sentry::Cron::MonitorConfig.from_interval(seconds / 86_400, :day, **options)
-        elsif (seconds % 3_600).zero?
-          Sentry::Cron::MonitorConfig.from_interval(seconds / 3_600, :hour, **options)
-        else
-          Sentry::Cron::MonitorConfig.from_interval(seconds / 60, :minute, **options)
-        end
-      end
-
-      # Rebuilds a 5-field crontab from fugit's expanded fields so Sentry
-      # (Debian cron rules) reads it the same way. Returns nil for fugit-only
-      # syntax: sub-minute seconds, `L`, `#`, `%` and `&`.
-      def crontab_for(cron)
-        return unless defined?(::Fugit::Cron) && cron.is_a?(::Fugit::Cron)
-        return unless cron.seconds&.size == 1
-        return if cron.monthdays&.any?(&:negative?)
-        return if cron.weekdays&.any? { |day| day.size > 1 }
-        return if cron.monthdays && cron.weekdays && cron.instance_variable_get(:@day_and)
-
-        [
-          crontab_field(cron.minutes, 0, 59, step: true),
-          crontab_field(cron.hours, 0, 23, step: true),
-          # Day fields never use `*/n`: Debian cron treats a field starting
-          # with `*` as unrestricted when combining day-of-month and weekday.
-          crontab_field(cron.monthdays, 1, 31),
-          crontab_field(cron.months, 1, 12, step: true),
-          crontab_field(cron.weekdays&.map(&:first), 0, 6)
-        ].join(" ")
-      end
-
-      def crontab_field(values, min, max, step: false)
-        return "*" unless values
-
-        if step && values.size > 1 && values.first == min
-          increment = values[1] - values[0]
-          return "*/#{increment}" if values == min.step(max, increment).to_a
-        end
-
-        values.chunk_while { |a, b| b == a + 1 }.map { |run| run.size > 1 ? "#{run.first}-#{run.last}" : run.first.to_s }.join(",")
-      end
-
-      # Sentry only accepts IANA names, so offsets such as "+05:30" get no config.
-      def timezone_for(cron)
-        zone = cron.timezone || ::EtOrbi.determine_local_tzone
-        return Sentry.configuration.cron.default_timezone unless zone
-
-        ::TZInfo::Timezone.get(zone.name).identifier
-      rescue StandardError
-        nil
-      end
     end
   end
 end

sentry-ruby/spec/sentry/cron/fugit_schedule_spec.rb

@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+require "fugit"
+
+RSpec.describe Sentry::Cron::FugitSchedule do
+  before { perform_basic_setup }
+
+  def from_cron(cron)
+    described_class.from_cron(Fugit.parse_cron(cron))&.to_h
+  end
+
+  describe ".from_cron" do
+    it "sends the crontab and time zone" do
+      expect(from_cron("0 9 * * 1-5 Europe/Vienna")).to eq(
+        schedule: { type: :crontab, value: "0 9 * * 1-5" },
+        timezone: "Europe/Vienna"
+      )
+    end
+
+    it "applies the cron config defaults" do
+      Sentry.configuration.cron.default_checkin_margin = 5
+      Sentry.configuration.cron.default_max_runtime = 30
+
+      expect(from_cron("0 9 * * * UTC")).to include(checkin_margin: 5, max_runtime: 30)
+    end
+
+    {
+      "*/15 * * * *" => "*/15 * * * *",
+      "0 9 * * mon-fri" => "0 9 * * 1-5",
+      "0 0 */2 * 1" => "0 0 1,3,5,7,9,11,13,15,17,19,21,23,25,27,29,31 * 1",
+      "30 0 9 * * *" => "0 9 * * *",
+      "@daily" => "0 0 * * *"
+    }.each do |cron, crontab|
+      it "sends #{crontab.inspect} for #{cron.inspect}" do
+        expect(from_cron("#{cron} UTC")[:schedule]).to eq(type: :crontab, value: crontab)
+      end
+    end
+
+    ["0 12 L * *", "0 12 * * mon#2", "0 12 * * mon%2", "0 0 1-7 * mon&", "*/10 * * * * *"].each do |cron|
+      it "returns nil for #{cron.inspect}" do
+        expect(from_cron(cron)).to be_nil
+      end
+    end
+
+    it "returns nil for an offset time zone" do
+      expect(from_cron("0 9 * * * +05:30")).to be_nil
+    end
+
+    it "returns nil when the local time zone is an offset" do
+      allow(EtOrbi).to receive(:determine_local_tzone).and_return(EtOrbi.get_tzone("+05:30"))
+
+      expect(from_cron("0 9 * * *")).to be_nil
+    end
+
+    it "uses the local time zone when the cron line has none" do
+      allow(EtOrbi).to receive(:determine_local_tzone).and_return(TZInfo::Timezone.get("Asia/Tokyo"))
+
+      expect(from_cron("0 9 * * *")[:timezone]).to eq("Asia/Tokyo")
+    end
+
+    it "returns nil for non-cron schedules" do
+      expect(described_class.from_cron(Fugit.parse("10m"))).to be_nil
+    end
+  end
+
+  describe ".from_seconds" do
+    { 600 => { value: 10, unit: :minute }, 7_200 => { value: 2, unit: :hour }, 86_400 => { value: 1, unit: :day } }.each do |seconds, interval|
+      it "sends an interval config for #{seconds}s" do
+        expect(described_class.from_seconds(seconds).to_h[:schedule]).to eq(type: :interval, **interval)
+      end
+    end
+
+    it "returns nil for sub-minute frequencies" do
+      expect(described_class.from_seconds(90)).to be_nil
+    end
+  end
+end

sentry-ruby/spec/sentry/rufus_scheduler_spec.rb

@@ -59,71 +59,18 @@ def monitor_config(type, schedule, **opts)
     expect(sentry_events.map(&:status)).to eq([:in_progress, :error])
   end
 
-  it "applies the cron config defaults" do
-    Sentry.configuration.cron.default_checkin_margin = 5
-    Sentry.configuration.cron.default_max_runtime = 30
-
-    expect(monitor_config(:cron, "0 9 * * * UTC", name: "job")).to include(checkin_margin: 5, max_runtime: 30)
+  it "sends no config for cron lines Sentry would read differently" do
+    expect(monitor_config(:cron, "0 12 L * *", name: "job")).to be_nil
+    expect(sentry_events.count).to eq(2)
   end
 
-  describe "crontab conversion" do
-    {
-      "*/15 * * * *" => "*/15 * * * *",
-      "0 9 * * mon-fri" => "0 9 * * 1-5",
-      "0 0 */2 * 1" => "0 0 1,3,5,7,9,11,13,15,17,19,21,23,25,27,29,31 * 1",
-      "30 0 9 * * *" => "0 9 * * *",
-      "@daily" => "0 0 * * *"
-    }.each do |cron, crontab|
-      it "sends #{crontab.inspect} for #{cron.inspect}" do
-        expect(monitor_config(:cron, "#{cron} UTC", name: "job")[:schedule]).to eq(type: :crontab, value: crontab)
-      end
-    end
-
-    ["0 12 L * *", "0 12 * * mon#2", "0 12 * * mon%2", "0 0 1-7 * mon&", "*/10 * * * * *"].each do |cron|
-      it "sends no config for #{cron.inspect}" do
-        expect(monitor_config(:cron, cron, name: "job")).to be_nil
-        expect(sentry_events.count).to eq(2)
-      end
-    end
-
-    it "sends no config for an offset time zone" do
-      expect(monitor_config(:cron, "0 9 * * * +05:30", name: "job")).to be_nil
-      expect(sentry_events.count).to eq(2)
-    end
-
-    it "sends no config when the local time zone is an offset" do
-      allow(EtOrbi).to receive(:determine_local_tzone).and_return(EtOrbi.get_tzone("+05:30"))
-
-      expect(monitor_config(:cron, "0 9 * * *", name: "job")).to be_nil
-    end
-
-    it "uses the local time zone when the cron line has none" do
-      allow(EtOrbi).to receive(:determine_local_tzone).and_return(TZInfo::Timezone.get("Asia/Tokyo"))
-
-      expect(monitor_config(:cron, "0 9 * * *", name: "job")[:timezone]).to eq("Asia/Tokyo")
-    end
+  it "sends an interval config for every jobs" do
+    expect(monitor_config(:every, "2h", name: "job")[:schedule]).to eq(type: :interval, value: 2, unit: :hour)
   end
 
-  describe "every and interval jobs" do
-    {
-      "10m" => { value: 10, unit: :minute },
-      "2h" => { value: 2, unit: :hour },
-      "1d" => { value: 1, unit: :day }
-    }.each do |every, interval|
-      it "sends an interval config for every #{every}" do
-        expect(monitor_config(:every, every, name: "job")[:schedule]).to eq(type: :interval, **interval)
-      end
-    end
-
-    it "sends check-ins without config for sub-minute frequencies" do
-      expect(monitor_config(:every, "90s", name: "job")).to be_nil
-      expect(sentry_events.count).to eq(2)
-    end
-
-    it "sends check-ins without config for interval jobs" do
-      expect(monitor_config(:interval, "5m", name: "job")).to be_nil
-      expect(sentry_events.count).to eq(2)
-    end
+  it "sends check-ins without config for interval jobs" do
+    expect(monitor_config(:interval, "5m", name: "job")).to be_nil
+    expect(sentry_events.count).to eq(2)
   end
 
   it "does not monitor one-off jobs" do