diff --git a/CHANGELOG.md b/CHANGELOG.md index 92eca8ea3..3aa5fc105 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/sentry-ruby/lib/sentry/client.rb b/sentry-ruby/lib/sentry/client.rb index a27141d6e..d55e49a73 100644 --- a/sentry-ruby/lib/sentry/client.rb +++ b/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 event.is_a?(TransactionEvent) || configuration.sample_allowed? event = scope.apply_to_event(event, hint) diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 9de0626cf..7ca79bac1 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/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 diff --git a/sentry-ruby/lib/sentry/transport.rb b/sentry-ruby/lib/sentry/transport.rb index 05ceff34c..a4c70486f 100644 --- a/sentry-ruby/lib/sentry/transport.rb +++ b/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 diff --git a/sentry-ruby/spec/sentry/client/event_sending_spec.rb b/sentry-ruby/spec/sentry/client/event_sending_spec.rb index 3a22ef6b0..289f522ab 100644 --- a/sentry-ruby/spec/sentry/client/event_sending_spec.rb +++ b/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| diff --git a/sentry-ruby/spec/sentry/configuration_spec.rb b/sentry-ruby/spec/sentry/configuration_spec.rb index 68d29589c..fc4ecd8b0 100644 --- a/sentry-ruby/spec/sentry/configuration_spec.rb +++ b/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 diff --git a/sentry-ruby/spec/sentry/transport_spec.rb b/sentry-ruby/spec/sentry/transport_spec.rb index a318487f3..8b70c5846 100644 --- a/sentry-ruby/spec/sentry/transport_spec.rb +++ b/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)