From 286135ccfae8487d49f2c2982f032483a2d4ffa2 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 5 Oct 2023 15:54:34 +0200 Subject: [PATCH] Add new Sentry.capture_check_in API for Cron monitoring (#2117) * New `CheckInEvent` class for the envelope payload * New `Cron::MonitorConfig` class that holds the monitor configuration * New `Cron::MonitorSchedule` module that holds two types of schedules `Crontab` and `Interval` --- CHANGELOG.md | 9 +++ sentry-ruby/lib/sentry-ruby.rb | 19 +++++ sentry-ruby/lib/sentry/check_in_event.rb | 60 ++++++++++++++ sentry-ruby/lib/sentry/client.rb | 31 +++++++ sentry-ruby/lib/sentry/cron/monitor_config.rb | 53 ++++++++++++ .../lib/sentry/cron/monitor_schedule.rb | 42 ++++++++++ sentry-ruby/lib/sentry/hub.rb | 26 +++++- sentry-ruby/lib/sentry/integrable.rb | 6 ++ .../sentry/utils/argument_checking_helper.rb | 6 ++ sentry-ruby/spec/sentry/client_spec.rb | 53 ++++++++++++ .../spec/sentry/cron/monitor_config_spec.rb | 80 +++++++++++++++++++ .../spec/sentry/cron/monitor_schedule_spec.rb | 39 +++++++++ sentry-ruby/spec/sentry/hub_spec.rb | 77 ++++++++++++++++-- sentry-ruby/spec/sentry/integrable_spec.rb | 15 +++- 14 files changed, 507 insertions(+), 9 deletions(-) create mode 100644 sentry-ruby/lib/sentry/check_in_event.rb create mode 100644 sentry-ruby/lib/sentry/cron/monitor_config.rb create mode 100644 sentry-ruby/lib/sentry/cron/monitor_schedule.rb create mode 100644 sentry-ruby/spec/sentry/cron/monitor_config_spec.rb create mode 100644 sentry-ruby/spec/sentry/cron/monitor_schedule_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 930c29c98..9fa1f05d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,15 @@ - Record client reports for profiles [#2107](https://github.com/getsentry/sentry-ruby/pull/2107) - Adopt Rails 7.1's new BroadcastLogger [#2120](https://github.com/getsentry/sentry-ruby/pull/2120) +- Add `Sentry.capture_check_in` API for Cron Monitoring [#2117](https://github.com/getsentry/sentry-ruby/pull/2117) + + You can now track progress of long running scheduled jobs. + + ```rb + check_in_id = Sentry.capture_check_in('job_name', :in_progress) + # do job stuff + Sentry.capture_check_in('job_name', :ok, check_in_id: check_in_id) + ``` ### Bug Fixes diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index daed11803..3572ae8ac 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -15,6 +15,7 @@ require "sentry/event" require "sentry/error_event" require "sentry/transaction_event" +require "sentry/check_in_event" require "sentry/span" require "sentry/transaction" require "sentry/hub" @@ -430,6 +431,24 @@ def capture_event(event) get_current_hub.capture_event(event) end + # Captures a check-in and sends it to Sentry via the currently active hub. + # + # @param slug [String] identifier of this monitor + # @param status [Symbol] status of this check-in, one of {CheckInEvent::VALID_STATUSES} + # + # @param [Hash] options extra check-in options + # @option options [String] check_in_id for updating the status of an existing monitor + # @option options [Integer] duration seconds elapsed since this monitor started + # @option options [Cron::MonitorConfig] monitor_config configuration for this monitor + # + # @yieldparam scope [Scope] + # + # @return [String, nil] The {CheckInEvent#check_in_id} to use for later updates on the same slug + def capture_check_in(slug, status, **options, &block) + return unless initialized? + get_current_hub.capture_check_in(slug, status, **options, &block) + end + # Takes or initializes a new Sentry::Transaction and makes a sampling decision for it. # # @return [Transaction, nil] diff --git a/sentry-ruby/lib/sentry/check_in_event.rb b/sentry-ruby/lib/sentry/check_in_event.rb new file mode 100644 index 000000000..d2b266aa3 --- /dev/null +++ b/sentry-ruby/lib/sentry/check_in_event.rb @@ -0,0 +1,60 @@ +# frozen_string_literal + +require 'securerandom' +require 'sentry/cron/monitor_config' + +module Sentry + class CheckInEvent < Event + TYPE = 'check_in' + + # uuid to identify this check-in. + # @return [String] + attr_accessor :check_in_id + + # Identifier of the monitor for this check-in. + # @return [String] + attr_accessor :monitor_slug + + # Duration of this check since it has started in seconds. + # @return [Integer, nil] + attr_accessor :duration + + # Monitor configuration to support upserts. + # @return [Cron::MonitorConfig, nil] + attr_accessor :monitor_config + + # Status of this check-in. + # @return [Symbol] + attr_accessor :status + + VALID_STATUSES = %i(ok in_progress error) + + def initialize( + slug:, + status:, + duration: nil, + monitor_config: nil, + check_in_id: nil, + **options + ) + super(**options) + + self.monitor_slug = slug + self.status = status + self.duration = duration + self.monitor_config = monitor_config + self.check_in_id = check_in_id || SecureRandom.uuid.delete('-') + end + + # @return [Hash] + def to_hash + data = super + data[:check_in_id] = check_in_id + data[:monitor_slug] = monitor_slug + data[:status] = status + data[:duration] = duration if duration + data[:monitor_config] = monitor_config.to_hash if monitor_config + data + end + end +end diff --git a/sentry-ruby/lib/sentry/client.rb b/sentry-ruby/lib/sentry/client.rb index 80f1165c8..d9e0f5b07 100644 --- a/sentry-ruby/lib/sentry/client.rb +++ b/sentry-ruby/lib/sentry/client.rb @@ -104,6 +104,37 @@ def event_from_message(message, hint = {}, backtrace: nil) event end + # Initializes a CheckInEvent object with the given options. + # + # @param slug [String] identifier of this monitor + # @param status [Symbol] status of this check-in, one of {CheckInEvent::VALID_STATUSES} + # @param hint [Hash] the hint data that'll be passed to `before_send` callback and the scope's event processors. + # @param duration [Integer, nil] seconds elapsed since this monitor started + # @param monitor_config [Cron::MonitorConfig, nil] configuration for this monitor + # @param check_in_id [String, nil] for updating the status of an existing monitor + # + # @return [Event] + def event_from_check_in( + slug, + status, + hint = {}, + duration: nil, + monitor_config: nil, + check_in_id: nil + ) + return unless configuration.sending_allowed? + + CheckInEvent.new( + configuration: configuration, + integration_meta: Sentry.integrations[hint[:integration]], + slug: slug, + status: status, + duration: duration, + monitor_config: monitor_config, + check_in_id: check_in_id + ) + end + # Initializes an Event object with the given Transaction object. # @param transaction [Transaction] the transaction to be recorded. # @return [TransactionEvent] diff --git a/sentry-ruby/lib/sentry/cron/monitor_config.rb b/sentry-ruby/lib/sentry/cron/monitor_config.rb new file mode 100644 index 000000000..eb47288f2 --- /dev/null +++ b/sentry-ruby/lib/sentry/cron/monitor_config.rb @@ -0,0 +1,53 @@ +# frozen_string_literal + +require 'sentry/cron/monitor_schedule' + +module Sentry + module Cron + class MonitorConfig + # The monitor schedule configuration + # @return [MonitorSchedule::Crontab, MonitorSchedule::Interval] + attr_accessor :schedule + + # 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 :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 :max_runtime + + # tz database style timezone string + # @return [String, nil] + attr_accessor :timezone + + def initialize(schedule, checkin_margin: nil, max_runtime: nil, timezone: nil) + @schedule = schedule + @checkin_margin = checkin_margin + @max_runtime = max_runtime + @timezone = timezone + end + + def self.from_crontab(crontab, **options) + new(MonitorSchedule::Crontab.new(crontab), **options) + end + + def self.from_interval(num, unit, **options) + return nil unless MonitorSchedule::Interval::VALID_UNITS.include?(unit) + + new(MonitorSchedule::Interval.new(num, unit), **options) + end + + def to_hash + { + schedule: schedule.to_hash, + checkin_margin: checkin_margin, + max_runtime: max_runtime, + timezone: timezone + }.compact + end + end + end +end diff --git a/sentry-ruby/lib/sentry/cron/monitor_schedule.rb b/sentry-ruby/lib/sentry/cron/monitor_schedule.rb new file mode 100644 index 000000000..e23615b59 --- /dev/null +++ b/sentry-ruby/lib/sentry/cron/monitor_schedule.rb @@ -0,0 +1,42 @@ +# frozen_string_literal + +module Sentry + module Cron + module MonitorSchedule + class Crontab + # A crontab formatted string such as "0 * * * *". + # @return [String] + attr_accessor :value + + def initialize(value) + @value = value + end + + def to_hash + { type: :crontab, value: value } + end + end + + class Interval + # The number representing duration of the interval. + # @return [Integer] + attr_accessor :value + + # The unit representing duration of the interval. + # @return [Symbol] + attr_accessor :unit + + VALID_UNITS = %i(year month week day hour minute) + + def initialize(value, unit) + @value = value + @unit = unit + end + + def to_hash + { type: :interval, value: value, unit: unit } + end + end + end + end +end diff --git a/sentry-ruby/lib/sentry/hub.rb b/sentry-ruby/lib/sentry/hub.rb index 7d9dca4c7..36258eb33 100644 --- a/sentry-ruby/lib/sentry/hub.rb +++ b/sentry-ruby/lib/sentry/hub.rb @@ -156,6 +156,30 @@ def capture_message(message, **options, &block) capture_event(event, **options, &block) end + def capture_check_in(slug, status, **options, &block) + check_argument_type!(slug, ::String) + check_argument_includes!(status, Sentry::CheckInEvent::VALID_STATUSES) + + return unless current_client + + options[:hint] ||= {} + options[:hint][:slug] = slug + + event = current_client.event_from_check_in( + slug, + status, + options[:hint], + duration: options.delete(:duration), + monitor_config: options.delete(:monitor_config), + check_in_id: options.delete(:check_in_id) + ) + + return unless event + + capture_event(event, **options, &block) + event.check_in_id + end + def capture_event(event, **options, &block) check_argument_type!(event, Sentry::Event) @@ -178,7 +202,7 @@ def capture_event(event, **options, &block) configuration.log_debug(event.to_json_compatible) end - @last_event_id = event&.event_id unless event.is_a?(Sentry::TransactionEvent) + @last_event_id = event&.event_id if event.is_a?(Sentry::ErrorEvent) event end diff --git a/sentry-ruby/lib/sentry/integrable.rb b/sentry-ruby/lib/sentry/integrable.rb index daa3669f7..a1edeadb6 100644 --- a/sentry-ruby/lib/sentry/integrable.rb +++ b/sentry-ruby/lib/sentry/integrable.rb @@ -22,5 +22,11 @@ def capture_message(message, **options, &block) options[:hint][:integration] = integration_name Sentry.capture_message(message, **options, &block) end + + def capture_check_in(slug, status, **options, &block) + options[:hint] ||= {} + options[:hint][:integration] = integration_name + Sentry.capture_check_in(slug, status, **options, &block) + end end end diff --git a/sentry-ruby/lib/sentry/utils/argument_checking_helper.rb b/sentry-ruby/lib/sentry/utils/argument_checking_helper.rb index 5b161b872..e00eb5fc2 100644 --- a/sentry-ruby/lib/sentry/utils/argument_checking_helper.rb +++ b/sentry-ruby/lib/sentry/utils/argument_checking_helper.rb @@ -9,5 +9,11 @@ def check_argument_type!(argument, *expected_types) raise ArgumentError, "expect the argument to be a #{expected_types.join(' or ')}, got #{argument.class} (#{argument.inspect})" end end + + def check_argument_includes!(argument, values) + unless values.include?(argument) + raise ArgumentError, "expect the argument to be one of #{values.map(&:inspect).join(' or ')}, got #{argument.inspect}" + end + end end end diff --git a/sentry-ruby/spec/sentry/client_spec.rb b/sentry-ruby/spec/sentry/client_spec.rb index ec70d407a..8ffc43a44 100644 --- a/sentry-ruby/spec/sentry/client_spec.rb +++ b/sentry-ruby/spec/sentry/client_spec.rb @@ -510,6 +510,59 @@ module ExcTag; end end end + describe "#event_from_check_in" do + let(:slug) { "test_slug" } + let(:status) { :ok } + + it 'returns an event' do + event = subject.event_from_check_in(slug, status) + expect(event).to be_a(Sentry::CheckInEvent) + + hash = event.to_hash + expect(hash[:monitor_slug]).to eq(slug) + expect(hash[:status]).to eq(status) + expect(hash[:check_in_id].length).to eq(32) + end + + it 'returns an event with correct optional attributes from crontab config' do + event = subject.event_from_check_in( + slug, + status, + duration: 30, + check_in_id: "xxx-yyy", + monitor_config: Sentry::Cron::MonitorConfig.from_crontab("* * * * *") + ) + + expect(event).to be_a(Sentry::CheckInEvent) + + hash = event.to_hash + expect(hash[:monitor_slug]).to eq(slug) + expect(hash[:status]).to eq(status) + expect(hash[:check_in_id]).to eq("xxx-yyy") + expect(hash[:duration]).to eq(30) + expect(hash[:monitor_config]).to eq({ schedule: { type: :crontab, value: "* * * * *" } }) + end + + it 'returns an event with correct optional attributes from interval config' do + event = subject.event_from_check_in( + slug, + status, + duration: 30, + check_in_id: "xxx-yyy", + monitor_config: Sentry::Cron::MonitorConfig.from_interval(30, :minute) + ) + + expect(event).to be_a(Sentry::CheckInEvent) + + hash = event.to_hash + expect(hash[:monitor_slug]).to eq(slug) + expect(hash[:status]).to eq(status) + expect(hash[:check_in_id]).to eq("xxx-yyy") + expect(hash[:duration]).to eq(30) + expect(hash[:monitor_config]).to eq({ schedule: { type: :interval, value: 30, unit: :minute } }) + end + end + describe "#generate_sentry_trace" do let(:string_io) { StringIO.new } let(:logger) do diff --git a/sentry-ruby/spec/sentry/cron/monitor_config_spec.rb b/sentry-ruby/spec/sentry/cron/monitor_config_spec.rb new file mode 100644 index 000000000..15d00bfc5 --- /dev/null +++ b/sentry-ruby/spec/sentry/cron/monitor_config_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +RSpec.describe Sentry::Cron::MonitorConfig do + describe '.from_crontab' do + it 'has correct attributes' do + subject = described_class.from_crontab( + '5 * * * *', + checkin_margin: 10, + max_runtime: 30, + 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.timezone).to eq('Europe/Vienna') + end + end + + describe '.from_interval' do + it 'returns nil without valid unit' do + expect(described_class.from_interval(5, :bla)).to eq(nil) + end + + it 'has correct attributes' do + subject = described_class.from_interval( + 5, + :hour, + checkin_margin: 10, + max_runtime: 30, + timezone: 'Europe/Vienna' + ) + + expect(subject.schedule).to be_a(Sentry::Cron::MonitorSchedule::Interval) + 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.timezone).to eq('Europe/Vienna') + end + end + + describe '#to_hash' do + it 'returns hash with correct attributes for crontab' do + subject = described_class.from_crontab( + '5 * * * *', + checkin_margin: 10, + max_runtime: 30, + timezone: 'Europe/Vienna' + ) + + hash = subject.to_hash + expect(hash).to eq({ + schedule: { type: :crontab, value: '5 * * * *' }, + checkin_margin: 10, + max_runtime: 30, + timezone: 'Europe/Vienna' + }) + end + + it 'returns hash with correct attributes for interval' do + subject = described_class.from_interval( + 5, + :hour, + checkin_margin: 10, + max_runtime: 30, + timezone: 'Europe/Vienna' + ) + + hash = subject.to_hash + expect(hash).to eq({ + schedule: { type: :interval, value: 5, unit: :hour }, + checkin_margin: 10, + max_runtime: 30, + timezone: 'Europe/Vienna' + }) + end + end +end diff --git a/sentry-ruby/spec/sentry/cron/monitor_schedule_spec.rb b/sentry-ruby/spec/sentry/cron/monitor_schedule_spec.rb new file mode 100644 index 000000000..8e1b422d1 --- /dev/null +++ b/sentry-ruby/spec/sentry/cron/monitor_schedule_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +RSpec.describe Sentry::Cron::MonitorSchedule::Crontab do + let(:subject) { described_class.new('5 * * * *') } + + describe '#value' do + it 'has correct value' do + expect(subject.value).to eq('5 * * * *') + end + end + + describe '#to_hash' do + it 'has correct attributes' do + expect(subject.to_hash).to eq({ type: :crontab, value: subject.value }) + end + end +end + +RSpec.describe Sentry::Cron::MonitorSchedule::Interval do + let(:subject) { described_class.new(5, :minute) } + + describe '#value' do + it 'has correct value' do + expect(subject.value).to eq(5) + end + end + + describe '#unit' do + it 'has correct unit' do + expect(subject.unit).to eq(:minute) + end + end + + describe '#to_hash' do + it 'has correct attributes' do + expect(subject.to_hash).to eq({ type: :interval, value: subject.value, unit: subject.unit }) + end + end +end diff --git a/sentry-ruby/spec/sentry/hub_spec.rb b/sentry-ruby/spec/sentry/hub_spec.rb index ed001b69a..80abb738b 100644 --- a/sentry-ruby/spec/sentry/hub_spec.rb +++ b/sentry-ruby/spec/sentry/hub_spec.rb @@ -30,7 +30,7 @@ end it "doesn't send the event nor assign last_event_id" do - subject.send(capture_helper, capture_subject) + subject.send(capture_helper, *capture_subject) expect(transport.events).to be_empty expect(subject.last_event_id).to eq(nil) @@ -39,14 +39,14 @@ context "with custom attributes" do it "updates the event with custom attributes" do - subject.send(capture_helper, capture_subject, tags: { foo: "bar" }) + subject.send(capture_helper, *capture_subject, tags: { foo: "bar" }) event = transport.events.last expect(event.tags).to eq({ foo: "bar" }) end it "accepts custom level" do - subject.send(capture_helper, capture_subject, level: :info) + subject.send(capture_helper, *capture_subject, level: :info) event = transport.events.last expect(event.level).to eq(:info) @@ -59,7 +59,7 @@ subject.send( capture_helper, - capture_subject, + *capture_subject, tags: { new_tag: true }, contexts: { another_character: { name: "Jane", age: 20 }}, extra: { new_extra: true } @@ -89,7 +89,7 @@ end it "accepts a custom scope" do - subject.send(capture_helper, capture_subject, scope: new_scope) + subject.send(capture_helper, *capture_subject, scope: new_scope) event = transport.events.last expect(event.tags).to eq({ custom_scope: true }) @@ -102,7 +102,7 @@ end it 'yields the scope to a passed block' do - subject.send(capture_helper, capture_subject) do |scope| + subject.send(capture_helper, *capture_subject) do |scope| scope.set_tags({ temporary_scope: true }) end @@ -116,13 +116,15 @@ hint = nil configuration.before_send = ->(event, h) { hint = h } - subject.send(capture_helper, capture_subject, hint: {foo: "bar"}) + subject.send(capture_helper, *capture_subject, hint: {foo: "bar"}) case capture_subject when String expect(hint).to eq({message: capture_subject, foo: "bar"}) when Exception expect(hint).to eq({exception: capture_subject, foo: "bar"}) + when Array + expect(hint).to eq({slug: capture_subject.first, foo: "bar"}) else expect(hint).to eq({foo: "bar"}) end @@ -167,6 +169,64 @@ end end + describe '#capture_check_in' do + let(:slug) { "test_slug" } + + it "raises error when passing a non-string slug" do + expect do + subject.capture_check_in(1, :ok) + end.to raise_error(ArgumentError, 'expect the argument to be a String, got Integer (1)') + end + + it "raises error when passing an invalid status" do + expect do + subject.capture_check_in(slug, "bla") + end.to raise_error(ArgumentError, 'expect the argument to be one of :ok or :in_progress or :error, got "bla"') + end + + it "raises error when passing an invalid status symbol" do + expect do + subject.capture_check_in(slug, :bla) + end.to raise_error(ArgumentError, 'expect the argument to be one of :ok or :in_progress or :error, got :bla') + end + + it "returns a check_in id" do + check_in_id = subject.capture_check_in(slug, :ok) + expect(check_in_id).to be_a(String) + expect(check_in_id.length).to eq(32) + end + + it "initializes an Event, and sends it via the Client" do + expect do + subject.capture_check_in(slug, :ok) + end.to change { transport.events.count }.by(1) + end + + it "populates the event hash correctly" do + subject.capture_check_in( + slug, + :ok, + duration: 30, + check_in_id: "xxx-yyy", + monitor_config: Sentry::Cron::MonitorConfig.from_crontab("* * * * *") + ) + + event = transport.events.last.to_hash + expect(event).to include( + monitor_slug: slug, + status: :ok, + check_in_id: "xxx-yyy", + duration: 30, + monitor_config: { schedule: { type: :crontab, value: "* * * * *" } } + ) + end + + it_behaves_like "capture_helper" do + let(:capture_helper) { :capture_check_in } + let(:capture_subject) { [slug, :ok] } + end + end + describe '#capture_exception' do let(:exception) { ZeroDivisionError.new("divided by 0") } @@ -503,12 +563,15 @@ error_event = client.event_from_exception(exception) transaction_event = client.event_from_transaction(transaction) + check_in_event = client.event_from_check_in("test_slug", :ok) subject.capture_event(error_event) subject.capture_event(transaction_event) + subject.capture_event(check_in_event) expect(subject.last_event_id).to eq(error_event.event_id) expect(subject.last_event_id).not_to eq(transaction_event.event_id) + expect(subject.last_event_id).not_to eq(check_in_event.event_id) end end diff --git a/sentry-ruby/spec/sentry/integrable_spec.rb b/sentry-ruby/spec/sentry/integrable_spec.rb index 70fd13d9a..5dc18d045 100644 --- a/sentry-ruby/spec/sentry/integrable_spec.rb +++ b/sentry-ruby/spec/sentry/integrable_spec.rb @@ -59,7 +59,7 @@ module AnotherIntegration; end expect(hint).to eq({ additional_hint: "foo", integration: "fake_integration", exception: exception }) end - it "generates Sentry::FakeIntegration.capture_exception" do + it "generates Sentry::FakeIntegration.capture_message" do hint = nil Sentry.configuration.before_send = lambda do |event, h| @@ -72,6 +72,19 @@ module AnotherIntegration; end expect(hint).to eq({ additional_hint: "foo", integration: "fake_integration", message: message }) end + it "generates Sentry::FakeIntegration.capture_check_in" do + hint = nil + + Sentry.configuration.before_send = lambda do |event, h| + hint = h + event + end + + Sentry::FakeIntegration.capture_check_in("test_slug", :ok, hint: { additional_hint: "foo" }) + + expect(hint).to eq({ additional_hint: "foo", integration: "fake_integration", slug: "test_slug" }) + end + it "sets correct meta when the event is captured by integration helpers" do event = Sentry::FakeIntegration.capture_message(message) expect(event.sdk).to eq({ name: "sentry.ruby.fake_integration", version: "0.1.0" })