Avoid duplicated sampling on Transaction events

CHANGELOG.md

@@ -33,6 +33,7 @@
 - Connect `Sidekiq`'s transaction with its parent when possible [#1590](https://github.com/getsentry/sentry-ruby/pull/1590)
   - Fixes [#1586](https://github.com/getsentry/sentry-ruby/issues/1586)
 - Use nil instead of false to disable callable settings [#1594](https://github.com/getsentry/sentry-ruby/pull/1594)
+- Avoid duplicated sampling on Transaction events [#1601](https://github.com/getsentry/sentry-ruby/pull/1601)
 
 ## 4.7.3
 

sentry-ruby/lib/sentry/client.rb

@@ -25,6 +25,7 @@ def initialize(configuration)
 
     def capture_event(event, scope, hint = {})
       return unless configuration.sending_allowed?
+      return unless configuration.sample_allowed? || event.is_a?(TransactionEvent)
 
       event = scope.apply_to_event(event, hint)
 

sentry-ruby/lib/sentry/configuration.rb

@@ -268,9 +268,13 @@ def environment=(environment)
     def sending_allowed?
       @errors = []
 
-      valid? &&
-        capture_in_environment? &&
-        sample_allowed?
+      valid? && capture_in_environment?
+    end
+
+    def sample_allowed?
+      return true if sample_rate == 1.0
+
+      Random.rand < sample_rate
     end
 
     def error_messages
@@ -389,17 +393,6 @@ def valid?
       end
     end
 
-    def sample_allowed?
-      return true if sample_rate == 1.0
-
-      if Random.rand >= sample_rate
-        @errors << "Excluded by random sample"
-        false
-      else
-        true
-      end
-    end
-
     def environment_from_env
       ENV['SENTRY_CURRENT_ENV'] || ENV['SENTRY_ENVIRONMENT'] || ENV['RAILS_ENV'] || ENV['RACK_ENV'] || 'development'
     end

sentry-ruby/lib/sentry/transport.rb

@@ -8,11 +8,9 @@ class Transport
 
     include LoggingHelper
 
-    attr_accessor :configuration
     attr_reader :logger, :rate_limits
 
     def initialize(configuration)
-      @configuration = configuration
       @logger = configuration.logger
       @transport_configuration = configuration.transport
       @dsn = configuration.dsn
@@ -27,12 +25,6 @@ def send_event(event)
       event_hash = event.to_hash
       item_type = get_item_type(event_hash)
 
-      unless configuration.sending_allowed?
-        log_debug("Envelope [#{item_type}] not sent: #{configuration.error_messages}")
-
-        return
-      end
-
       if is_rate_limited?(item_type)
         log_info("Envelope [#{item_type}] not sent: rate limiting")
 
@@ -97,7 +89,7 @@ def encode(event)
       item_type = get_item_type(event_hash)
 
       envelope = <<~ENVELOPE
-        {"event_id":"#{event_id}","dsn":"#{configuration.dsn.to_s}","sdk":#{Sentry.sdk_meta.to_json},"sent_at":"#{Sentry.utc_now.iso8601}"}
+        {"event_id":"#{event_id}","dsn":"#{@dsn.to_s}","sdk":#{Sentry.sdk_meta.to_json},"sent_at":"#{Sentry.utc_now.iso8601}"}
         {"type":"#{item_type}","content_type":"application/json"}
         #{JSON.generate(event_hash)}
       ENVELOPE

sentry-ruby/spec/sentry/client/event_sending_spec.rb

@@ -19,6 +19,39 @@
     let(:scope) { Sentry::Scope.new }
     let(:event) { subject.event_from_message(message) }
 
+    context "with sample_rate set" do
+      before do
+        configuration.sample_rate = 0.5
+        configuration.background_worker_threads = 0
+      end
+
+      context "with Event" do
+        it "sends the event when it's sampled" do
+          allow(Random).to receive(:rand).and_return(0.49)
+          subject.capture_event(event, scope)
+          expect(subject.transport.events.count).to eq(1)
+        end
+
+        it "doesn't send the event when it's not sampled" do
+          allow(Random).to receive(:rand).and_return(0.51)
+          subject.capture_event(event, scope)
+          expect(subject.transport.events.count).to eq(0)
+        end
+      end
+
+      context "with TransactionEvent" do
+        require "debug"
+        it "ignores the sampling" do
+          transaction_event = subject.event_from_transaction(Sentry::Transaction.new(hub: hub))
+          allow(Random).to receive(:rand).and_return(0.51)
+
+          subject.capture_event(transaction_event, scope)
+
+          expect(subject.transport.events.count).to eq(1)
+        end
+      end
+    end
+
     context 'with config.async set' do
       let(:async_block) do
         lambda do |event|

sentry-ruby/spec/sentry/configuration_spec.rb

@@ -233,21 +233,19 @@
     end
   end
 
-  context "with a sample rate" do
-    before(:each) do
-      subject.dsn = 'http://12345:67890@sentry.localdomain:3000/sentry/42'
+  describe "#sample_allowed?" do
+    before do
       subject.sample_rate = 0.75
     end
 
     it 'captured_allowed false when sampled' do
       allow(Random).to receive(:rand).and_return(0.76)
-      expect(subject.sending_allowed?).to eq(false)
-      expect(subject.errors).to eq(["Excluded by random sample"])
+      expect(subject.sample_allowed?).to eq(false)
     end
 
     it 'captured_allowed true when not sampled' do
       allow(Random).to receive(:rand).and_return(0.74)
-      expect(subject.sending_allowed?).to eq(true)
+      expect(subject.sample_allowed?).to eq(true)
     end
   end
 

sentry-ruby/spec/sentry/transport_spec.rb

@@ -79,25 +79,6 @@
     let(:client) { Sentry::Client.new(configuration) }
     let(:event) { client.event_from_exception(ZeroDivisionError.new("divided by 0")) }
 
-    context "when event is not allowed (by sampling)" do
-      let(:string_io) do
-        StringIO.new
-      end
-
-      before do
-        configuration.logger = Logger.new(string_io)
-        configuration.sample_rate = 0.5
-        allow(Random).to receive(:rand).and_return(0.6)
-      end
-
-      it "logs correct message" do
-        subject.send_event(event)
-
-        logs = string_io.string
-        expect(logs).to match(/Envelope \[event\] not sent: Excluded by random sample/)
-      end
-    end
-
     context "when success" do
       before do
         allow(subject).to receive(:send_data)