diff --git a/CHANGELOG.md b/CHANGELOG.md index eaaf46d2d..b0c039798 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ - Remove `config.async` [#1894](https://github.com/getsentry/sentry-ruby/pull/1894) - Migrate from to_hash to to_h ([#2351](https://github.com/getsentry/sentry-ruby/pull/2351)) +- Add `before_send_check_in` for applying to `CheckInEvent` ([#2703](https://github.com/getsentry/sentry-ruby/pull/2703)) +- Returning a hash from `before_send` and `before_send_transaction` is no longer supported and will drop the event. ## Unreleased diff --git a/sentry-ruby/lib/sentry/client.rb b/sentry-ruby/lib/sentry/client.rb index faa6a4bf5..e2facb2e1 100644 --- a/sentry-ruby/lib/sentry/client.rb +++ b/sentry-ruby/lib/sentry/client.rb @@ -209,18 +209,10 @@ def send_event(event, hint = nil) data_category = Envelope::Item.data_category(event.type) spans_before = event.is_a?(TransactionEvent) ? event.spans.size : 0 - if event.type != TransactionEvent::TYPE && configuration.before_send + if event.is_a?(ErrorEvent) && configuration.before_send event = configuration.before_send.call(event, hint) - case event - when ErrorEvent, CheckInEvent - # do nothing - when Hash - log_debug(<<~MSG) - Returning a Hash from before_send is deprecated and will be removed in the next major version. - Please return a Sentry::ErrorEvent object instead. - MSG - else + if !event.is_a?(ErrorEvent) # Avoid serializing the event object in this case because we aren't sure what it is and what it contains log_debug(<<~MSG) Discarded event because before_send didn't return a Sentry::ErrorEvent object but an instance of #{event.class} @@ -230,21 +222,10 @@ def send_event(event, hint = nil) end end - if event.type == TransactionEvent::TYPE && configuration.before_send_transaction + if event.is_a?(TransactionEvent) && configuration.before_send_transaction event = configuration.before_send_transaction.call(event, hint) - if event.is_a?(TransactionEvent) || event.is_a?(Hash) - spans_after = event.is_a?(TransactionEvent) ? event.spans.size : 0 - spans_delta = spans_before - spans_after - transport.record_lost_event(:before_send, "span", num: spans_delta) if spans_delta > 0 - - if event.is_a?(Hash) - log_debug(<<~MSG) - Returning a Hash from before_send_transaction is deprecated and will be removed in the next major version. - Please return a Sentry::TransactionEvent object instead. - MSG - end - else + if !event.is_a?(TransactionEvent) # Avoid serializing the event object in this case because we aren't sure what it is and what it contains log_debug(<<~MSG) Discarded event because before_send_transaction didn't return a Sentry::TransactionEvent object but an instance of #{event.class} @@ -253,6 +234,23 @@ def send_event(event, hint = nil) transport.record_lost_event(:before_send, "span", num: spans_before + 1) return end + + spans_after = event.is_a?(TransactionEvent) ? event.spans.size : 0 + spans_delta = spans_before - spans_after + transport.record_lost_event(:before_send, "span", num: spans_delta) if spans_delta > 0 + end + + if event.is_a?(CheckInEvent) && configuration.before_send_check_in + event = configuration.before_send_check_in.call(event, hint) + + if !event.is_a?(CheckInEvent) + # Avoid serializing the event object in this case because we aren't sure what it is and what it contains + log_debug(<<~MSG) + Discarded event because before_send_check_in didn't return a Sentry::CheckInEvent object but an instance of #{event.class} + MSG + transport.record_lost_event(:before_send, data_category) + return + end end transport.send_event(event) if configuration.sending_to_dsn_allowed? diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 81fce13fe..23852275a 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -68,7 +68,7 @@ class Configuration # @return [Proc] attr_reader :before_breadcrumb - # Optional Proc, called before sending an event to the server + # Optional Proc, called before sending an error event to the server # @example # config.before_send = lambda do |event, hint| # # skip ZeroDivisionError exceptions @@ -81,7 +81,7 @@ class Configuration # @return [Proc] attr_reader :before_send - # Optional Proc, called before sending an event to the server + # Optional Proc, called before sending a transaction event to the server # @example # config.before_send_transaction = lambda do |event, hint| # # skip unimportant transactions or strip sensitive data @@ -94,6 +94,18 @@ class Configuration # @return [Proc] attr_reader :before_send_transaction + # Optional Proc, called before sending a check-in event to the server + # @example + # config.before_send_check_in = lambda do |event, hint| + # if event.monitor_slug == "unimportant_job" + # nil + # else + # event + # end + # end + # @return [Proc] + attr_reader :before_send_check_in + # Optional Proc, called before sending an event to the server # @example # config.before_send_log = lambda do |log| @@ -476,6 +488,7 @@ def initialize self.before_send = nil self.before_send_transaction = nil + self.before_send_check_in = nil self.before_send_log = nil self.rack_env_whitelist = RACK_ENV_WHITELIST_DEFAULT self.traces_sampler = nil @@ -550,6 +563,12 @@ def before_send_transaction=(value) @before_send_transaction = value end + def before_send_check_in=(value) + check_callable!("before_send_check_in", value) + + @before_send_check_in = value + end + def before_breadcrumb=(value) check_callable!("before_breadcrumb", value) diff --git a/sentry-ruby/spec/sentry/client/event_sending_spec.rb b/sentry-ruby/spec/sentry/client/event_sending_spec.rb index 40e3a0d32..08d9a35ec 100644 --- a/sentry-ruby/spec/sentry/client/event_sending_spec.rb +++ b/sentry-ruby/spec/sentry/client/event_sending_spec.rb @@ -128,6 +128,8 @@ client.event_from_exception(ZeroDivisionError.new("divided by 0")) end + let(:check_in_event) { client.event_from_check_in("test_slug", :ok) } + shared_examples "Event in send_event" do context "when there's an exception" do before do @@ -140,6 +142,7 @@ end.to raise_error(Sentry::ExternalError, "networking error") end end + it "sends data through the transport" do expect(client.transport).to receive(:send_event).with(event) client.send_event(event) @@ -155,18 +158,6 @@ expect(event.tags[:called]).to eq(true) end - context "for check in events" do - let(:event_object) { client.event_from_check_in("test_slug", :ok) } - - it "does not fail due to before_send" do - configuration.before_send = lambda { |e, _h| e } - client.send_event(event) - - expect(client.transport).to receive(:send_event).with(event) - client.send_event(event) - end - end - it "doesn't apply before_send_transaction to Event" do dbl = double("before_send_transaction") allow(dbl).to receive(:call) @@ -200,19 +191,6 @@ expect(string_io.string).to include("Discarded event because before_send didn't return a Sentry::ErrorEvent object but an instance of Integer") expect(return_value).to eq(nil) end - - it "warns about Hash value's deprecation" do - string_io = StringIO.new - logger = Logger.new(string_io, level: :debug) - configuration.sdk_logger = logger - configuration.before_send = lambda do |_event, _hint| - { foo: "bar" } - end - - return_value = client.send_event(event) - expect(string_io.string).to include("Returning a Hash from before_send is deprecated and will be removed in the next major version.") - expect(return_value).to eq({ foo: "bar" }) - end end it_behaves_like "Event in send_event" do @@ -254,23 +232,60 @@ expect(string_io.string).to include("Discarded event because before_send_transaction didn't return a Sentry::TransactionEvent object but an instance of NilClass") expect(return_value).to be_nil end + end + + it_behaves_like "TransactionEvent in send_event" do + let(:event) { transaction_event } + end + + shared_examples "CheckInEvent in send_event" do + it "sends data through the transport" do + client.send_event(event) + end + + it "doesn't apply before_send to CheckInEvent" do + configuration.before_send = lambda do |event, _hint| + raise "shouldn't trigger me" + end + + client.send_event(event) + end + + it "doesn't apply before_send_transaction to CheckInEvent" do + configuration.before_send_transaction = lambda do |event, _hint| + raise "shouldn't trigger me" + end + + client.send_event(event) + end - it "warns about Hash value's deprecation" do + it "applies before_send_check_in callback before sending the event" do + called = false + configuration.before_send_check_in = lambda do |event, _hint| + called = true + event + end + + client.send_event(event) + expect(called).to eq(true) + end + + it "warns if before_send_check_in returns nil" do string_io = StringIO.new logger = Logger.new(string_io, level: :debug) configuration.sdk_logger = logger - configuration.before_send_transaction = lambda do |_event, _hint| - { foo: "bar" } + configuration.before_send_check_in = lambda do |_event, _hint| + nil end return_value = client.send_event(event) - expect(string_io.string).to include("Returning a Hash from before_send_transaction is deprecated and will be removed in the next major version.") - expect(return_value).to eq({ foo: "bar" }) + expect(string_io.string).to include("Discarded event because before_send_check_in didn't return a Sentry::CheckInEvent object but an instance of NilClass") + expect(return_value).to be_nil end end - it_behaves_like "TransactionEvent in send_event" do - let(:event) { transaction_event } + it_behaves_like "CheckInEvent in send_event" do + let(:event) { check_in_event } end end diff --git a/sentry-ruby/spec/sentry/configuration_spec.rb b/sentry-ruby/spec/sentry/configuration_spec.rb index fffeb159f..0d419f823 100644 --- a/sentry-ruby/spec/sentry/configuration_spec.rb +++ b/sentry-ruby/spec/sentry/configuration_spec.rb @@ -389,6 +389,12 @@ expect { subject.before_send_transaction = true }.to raise_error(ArgumentError, "before_send_transaction must be callable (or nil to disable)") end + it 'raises error when setting before_send_check_in to anything other than callable or nil' do + subject.before_send_check_in = -> { } + subject.before_send_check_in = nil + expect { subject.before_send_check_in = true }.to raise_error(ArgumentError, "before_send_check_in must be callable (or nil to disable)") + end + it 'raises error when setting before_breadcrumb to anything other than callable or nil' do subject.before_breadcrumb = -> { } subject.before_breadcrumb = nil diff --git a/sentry-ruby/spec/sentry/integrable_spec.rb b/sentry-ruby/spec/sentry/integrable_spec.rb index 02a2ed102..4a6677a87 100644 --- a/sentry-ruby/spec/sentry/integrable_spec.rb +++ b/sentry-ruby/spec/sentry/integrable_spec.rb @@ -82,7 +82,7 @@ module AnotherIntegration; end it "generates Sentry::FakeIntegration.capture_check_in" do hint = nil - Sentry.configuration.before_send = lambda do |event, h| + Sentry.configuration.before_send_check_in = lambda do |event, h| hint = h event end