From d7f38fd4012aa50e6a508e1c2ce9143d520f3b23 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 28 Dec 2023 17:16:03 +0100 Subject: [PATCH 1/3] Allow setting defaults for MonitorConfig objects with new Cron::Configuration --- CHANGELOG.md | 11 +++++ sentry-ruby/lib/sentry/configuration.rb | 10 ++++- sentry-ruby/lib/sentry/cron/configuration.rb | 23 ++++++++++ sentry-ruby/lib/sentry/cron/monitor_config.rb | 6 +-- sentry-ruby/spec/sentry/configuration_spec.rb | 9 ++++ .../spec/sentry/cron/monitor_config_spec.rb | 45 +++++++++++++++---- 6 files changed, 91 insertions(+), 13 deletions(-) create mode 100644 sentry-ruby/lib/sentry/cron/configuration.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index a1d5d62a9..fc6adb8ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,17 @@ - Implement proper flushing logic on ``close`` for Client Reports and Sessions [#2206](https://github.com/getsentry/sentry-ruby/pull/2206) - Support cron with timezone for `sidekiq-scheduler` patch [#2209](https://github.com/getsentry/sentry-ruby/pull/2209) - Fixes [#2187](https://github.com/getsentry/sentry-ruby/issues/2187) +- Add `Cron::Configuration` object that holds defaults for all ``MonitorConfig`` objects [#2210](https://github.com/getsentry/sentry-ruby/pull/2210) + + ```ruby + Sentry.init do |config| + # ... + config.cron.default_checkin_margin = 1 + config.cron.default_max_runtime = 30 + config.cron.default_timezone = 'America/New_York' + end + ``` + ## 5.15.2 diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 5868abcf6..5bfda3f64 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -7,6 +7,7 @@ require "sentry/dsn" require "sentry/release_detector" require "sentry/transport/configuration" +require "sentry/cron/configuration" require "sentry/linecache" require "sentry/interfaces/stacktrace_builder" @@ -226,10 +227,14 @@ def capture_exception_frame_locals=(value) # @return [String] attr_accessor :server_name - # Return a Transport::Configuration object for transport-related configurations. - # @return [Transport] + # Transport related configuration. + # @return [Transport::Configuration] attr_reader :transport + # Cron related configuration. + # @return [Cron::Configuration] + attr_reader :cron + # Take a float between 0.0 and 1.0 as the sample rate for tracing events (transactions). # @return [Float, nil] attr_reader :traces_sample_rate @@ -380,6 +385,7 @@ def initialize self.enable_tracing = nil @transport = Transport::Configuration.new + @cron = Cron::Configuration.new @gem_specs = Hash[Gem::Specification.map { |spec| [spec.name, spec.version.to_s] }] if Gem::Specification.respond_to?(:map) run_post_initialization_callbacks diff --git a/sentry-ruby/lib/sentry/cron/configuration.rb b/sentry-ruby/lib/sentry/cron/configuration.rb new file mode 100644 index 000000000..897f5a511 --- /dev/null +++ b/sentry-ruby/lib/sentry/cron/configuration.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Sentry + module Cron + class Configuration + # Defaults set here will apply to all {Cron::MonitorConfig} objects unless overwritten. + + # How long (in minutes) after the expected checkin time will we wait + # until we consider the checkin to have been missed. + # @return [Integer, nil] + attr_accessor :default_checkin_margin + + # How long (in minutes) is the checkin allowed to run for in in_progress + # before it is considered failed. + # @return [Integer, nil] + attr_accessor :default_max_runtime + + # tz database style timezone string + # @return [String, nil] + attr_accessor :default_timezone + end + end +end diff --git a/sentry-ruby/lib/sentry/cron/monitor_config.rb b/sentry-ruby/lib/sentry/cron/monitor_config.rb index 68b8c709c..98cbf2aff 100644 --- a/sentry-ruby/lib/sentry/cron/monitor_config.rb +++ b/sentry-ruby/lib/sentry/cron/monitor_config.rb @@ -25,9 +25,9 @@ class MonitorConfig def initialize(schedule, checkin_margin: nil, max_runtime: nil, timezone: nil) @schedule = schedule - @checkin_margin = checkin_margin - @max_runtime = max_runtime - @timezone = timezone + @checkin_margin = checkin_margin || Sentry.configuration&.cron&.default_checkin_margin + @max_runtime = max_runtime || Sentry.configuration&.cron&.default_max_runtime + @timezone = timezone || Sentry.configuration&.cron&.default_timezone end def self.from_crontab(crontab, **options) diff --git a/sentry-ruby/spec/sentry/configuration_spec.rb b/sentry-ruby/spec/sentry/configuration_spec.rb index 367c1450e..788eda965 100644 --- a/sentry-ruby/spec/sentry/configuration_spec.rb +++ b/sentry-ruby/spec/sentry/configuration_spec.rb @@ -256,6 +256,15 @@ end end + describe "#cron" do + it "returns an initialized Cron::Configuration object" do + expect(subject.cron).to be_a(Sentry::Cron::Configuration) + expect(subject.cron.default_checkin_margin).to eq(nil) + expect(subject.cron.default_max_runtime).to eq(nil) + expect(subject.cron.default_timezone).to eq(nil) + end + end + describe "#spotlight" do it "false by default" do expect(subject.spotlight).to eq(false) diff --git a/sentry-ruby/spec/sentry/cron/monitor_config_spec.rb b/sentry-ruby/spec/sentry/cron/monitor_config_spec.rb index 15d00bfc5..7a6d91cf6 100644 --- a/sentry-ruby/spec/sentry/cron/monitor_config_spec.rb +++ b/sentry-ruby/spec/sentry/cron/monitor_config_spec.rb @@ -1,21 +1,39 @@ require 'spec_helper' RSpec.describe Sentry::Cron::MonitorConfig do + before do + perform_basic_setup do |config| + config.cron.default_checkin_margin = 1 + config.cron.default_max_runtime = 30 + config.cron.default_timezone = 'America/New_York' + end + end + describe '.from_crontab' do it 'has correct attributes' do subject = described_class.from_crontab( '5 * * * *', checkin_margin: 10, - max_runtime: 30, + max_runtime: 20, timezone: 'Europe/Vienna' ) expect(subject.schedule).to be_a(Sentry::Cron::MonitorSchedule::Crontab) expect(subject.schedule.value).to eq('5 * * * *') expect(subject.checkin_margin).to eq(10) - expect(subject.max_runtime).to eq(30) + expect(subject.max_runtime).to eq(20) expect(subject.timezone).to eq('Europe/Vienna') end + + it 'fills in correct defaults from cron configuration' do + subject = described_class.from_crontab('5 * * * *') + + expect(subject.schedule).to be_a(Sentry::Cron::MonitorSchedule::Crontab) + expect(subject.schedule.value).to eq('5 * * * *') + expect(subject.checkin_margin).to eq(1) + expect(subject.max_runtime).to eq(30) + expect(subject.timezone).to eq('America/New_York') + end end describe '.from_interval' do @@ -28,7 +46,7 @@ 5, :hour, checkin_margin: 10, - max_runtime: 30, + max_runtime: 20, timezone: 'Europe/Vienna' ) @@ -36,9 +54,20 @@ expect(subject.schedule.value).to eq(5) expect(subject.schedule.unit).to eq(:hour) expect(subject.checkin_margin).to eq(10) - expect(subject.max_runtime).to eq(30) + expect(subject.max_runtime).to eq(20) expect(subject.timezone).to eq('Europe/Vienna') end + + it 'fills in correct defaults from cron configuration' do + subject = described_class.from_interval(5, :minute) + + expect(subject.schedule).to be_a(Sentry::Cron::MonitorSchedule::Interval) + expect(subject.schedule.value).to eq(5) + expect(subject.schedule.unit).to eq(:minute) + expect(subject.checkin_margin).to eq(1) + expect(subject.max_runtime).to eq(30) + expect(subject.timezone).to eq('America/New_York') + end end describe '#to_hash' do @@ -46,7 +75,7 @@ subject = described_class.from_crontab( '5 * * * *', checkin_margin: 10, - max_runtime: 30, + max_runtime: 20, timezone: 'Europe/Vienna' ) @@ -54,7 +83,7 @@ expect(hash).to eq({ schedule: { type: :crontab, value: '5 * * * *' }, checkin_margin: 10, - max_runtime: 30, + max_runtime: 20, timezone: 'Europe/Vienna' }) end @@ -64,7 +93,7 @@ 5, :hour, checkin_margin: 10, - max_runtime: 30, + max_runtime: 20, timezone: 'Europe/Vienna' ) @@ -72,7 +101,7 @@ expect(hash).to eq({ schedule: { type: :interval, value: 5, unit: :hour }, checkin_margin: 10, - max_runtime: 30, + max_runtime: 20, timezone: 'Europe/Vienna' }) end From 976926d8db7aa2bae091886f073d6710305e717a Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 28 Dec 2023 17:24:09 +0100 Subject: [PATCH 2/3] Make danger happy? --- CHANGELOG.md | 11 +++++------ sentry-ruby/spec/sentry/hub_spec.rb | 3 ++- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc6adb8ef..ba4f8d893 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ### Features -- Enable backpressure handling by default [#2185](https://github.com/getsentry/sentry-ruby/pull/2185) +- Enable backpressure handling by default ([#2185](https://github.com/getsentry/sentry-ruby/pull/2185)) The SDK can now dynamically downsamples transactions to reduce backpressure in high throughput systems. It starts a new `BackpressureMonitor` thread to perform some health checks @@ -21,10 +21,10 @@ If your system serves heavy load, please let us know how this feature works for you! -- Implement proper flushing logic on ``close`` for Client Reports and Sessions [#2206](https://github.com/getsentry/sentry-ruby/pull/2206) -- Support cron with timezone for `sidekiq-scheduler` patch [#2209](https://github.com/getsentry/sentry-ruby/pull/2209) - - Fixes [#2187](https://github.com/getsentry/sentry-ruby/issues/2187) -- Add `Cron::Configuration` object that holds defaults for all ``MonitorConfig`` objects [#2210](https://github.com/getsentry/sentry-ruby/pull/2210) +- Implement proper flushing logic on ``close`` for Client Reports and Sessions ([#2206](https://github.com/getsentry/sentry-ruby/pull/2206)) +- Support cron with timezone for `sidekiq-scheduler` patch ([#2209](https://github.com/getsentry/sentry-ruby/pull/2209)) + - Fixes ([#2187](https://github.com/getsentry/sentry-ruby/issues/2187)) +- Add `Cron::Configuration` object that holds defaults for all ``MonitorConfig`` objects ([#2210](https://github.com/getsentry/sentry-ruby/pull/2210)) ```ruby Sentry.init do |config| @@ -35,7 +35,6 @@ end ``` - ## 5.15.2 ### Bug Fixes diff --git a/sentry-ruby/spec/sentry/hub_spec.rb b/sentry-ruby/spec/sentry/hub_spec.rb index 80abb738b..f4be2f7dc 100644 --- a/sentry-ruby/spec/sentry/hub_spec.rb +++ b/sentry-ruby/spec/sentry/hub_spec.rb @@ -217,8 +217,9 @@ status: :ok, check_in_id: "xxx-yyy", duration: 30, - monitor_config: { schedule: { type: :crontab, value: "* * * * *" } } ) + + expect(event[:monitor_config]).to include({ schedule: { type: :crontab, value: "* * * * *" } }) end it_behaves_like "capture_helper" do From 882740e3eb6effac0995d884519a1c8680e765d0 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 28 Dec 2023 17:39:18 +0100 Subject: [PATCH 3/3] dumdum --- CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba4f8d893..a396feadf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ### Features -- Enable backpressure handling by default ([#2185](https://github.com/getsentry/sentry-ruby/pull/2185)) +- Enable backpressure handling by default [#2185](https://github.com/getsentry/sentry-ruby/pull/2185) The SDK can now dynamically downsamples transactions to reduce backpressure in high throughput systems. It starts a new `BackpressureMonitor` thread to perform some health checks @@ -21,10 +21,10 @@ If your system serves heavy load, please let us know how this feature works for you! -- Implement proper flushing logic on ``close`` for Client Reports and Sessions ([#2206](https://github.com/getsentry/sentry-ruby/pull/2206)) -- Support cron with timezone for `sidekiq-scheduler` patch ([#2209](https://github.com/getsentry/sentry-ruby/pull/2209)) - - Fixes ([#2187](https://github.com/getsentry/sentry-ruby/issues/2187)) -- Add `Cron::Configuration` object that holds defaults for all ``MonitorConfig`` objects ([#2210](https://github.com/getsentry/sentry-ruby/pull/2210)) +- Implement proper flushing logic on ``close`` for Client Reports and Sessions [#2206](https://github.com/getsentry/sentry-ruby/pull/2206) +- Support cron with timezone for `sidekiq-scheduler` patch [#2209](https://github.com/getsentry/sentry-ruby/pull/2209) + - Fixes [#2187](https://github.com/getsentry/sentry-ruby/issues/2187) +- Add `Cron::Configuration` object that holds defaults for all ``MonitorConfig`` objects [#2211](https://github.com/getsentry/sentry-ruby/pull/2211) ```ruby Sentry.init do |config|