Add before_send_check_in (#2703)

* Cleanup before_send to only apply to ErrorEvent
* remove the Hash deprecation message

Author: sl0thentr0py

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.
 
 ## 5.27.1
 

sentry-ruby/lib/sentry/client.rb

@@ -210,18 +210,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}
@@ -231,21 +223,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}
@@ -254,6 +235,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?

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)
 

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
 

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

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