Make sure isolated envelopes respect enabled_environments (#2291)

Envelopes for metrics/sessions and so on were queued directly before and
thus did not respect the config. This introduces a dedicated
`capture_envelope` on the client to centralize these checks.

Closes #2287

CHANGELOG.md

@@ -1,3 +1,10 @@
+## Unreleased
+
+### Bug Fixes
+
+- Make sure isolated envelopes respect `config.enabled_environments` [#2291](https://github.com/getsentry/sentry-ruby/pull/2291)
+  - Fixes [#2287](https://github.com/getsentry/sentry-ruby/issues/2287)
+
 ## 5.17.2
 
 ### Features

sentry-ruby/lib/sentry-ruby.rb

@@ -257,7 +257,7 @@ def close
       end
 
       if client = get_current_client
-        client.transport.flush
+        client.flush
 
         if client.configuration.include_local_variables
           exception_locals_tp.disable

sentry-ruby/lib/sentry/client.rb

@@ -77,6 +77,20 @@ def capture_event(event, scope, hint = {})
       nil
     end
 
+    # Capture an envelope directly.
+    # @param envelope [Envelope] the envelope to be captured.
+    # @return [void]
+    def capture_envelope(envelope)
+      Sentry.background_worker.perform { send_envelope(envelope) }
+    end
+
+    # Flush pending events to Sentry.
+    # @return [void]
+    def flush
+      transport.flush if configuration.sending_to_dsn_allowed?
+      spotlight_transport.flush if spotlight_transport
+    end
+
     # Initializes an Event object with the given exception. Returns `nil` if the exception's class is excluded from reporting.
     # @param exception [Exception] the exception to be reported.
     # @param hint [Hash] the hint data that'll be passed to `before_send` callback and the scope's event processors.
@@ -183,6 +197,19 @@ def send_event(event, hint = nil)
       raise
     end
 
+    # Send an envelope directly to Sentry.
+    # @param envelope [Envelope] the envelope to be sent.
+    # @return [void]
+    def send_envelope(envelope)
+      transport.send_envelope(envelope) if configuration.sending_to_dsn_allowed?
+      spotlight_transport.send_envelope(envelope) if spotlight_transport
+    rescue => e
+      # note that we don't record client reports for direct envelope types
+      # such as metrics, sessions etc
+      log_error("Envelope sending failed", e, debug: configuration.debug)
+      raise
+    end
+
     # @deprecated use Sentry.get_traceparent instead.
     #
     # Generates a Sentry trace for distribted tracing from the given Span.

sentry-ruby/lib/sentry/metrics/aggregator.rb

@@ -23,7 +23,7 @@ class Aggregator
       }
 
       # exposed only for testing
-      attr_reader :thread, :buckets, :flush_shift, :code_locations
+      attr_reader :client, :thread, :buckets, :flush_shift, :code_locations
 
       def initialize(configuration, client)
         @client = client
@@ -107,9 +107,7 @@ def flush(force: false)
           end
         end
 
-        Sentry.background_worker.perform do
-          @client.transport.send_envelope(envelope)
-        end
+        @client.capture_envelope(envelope)
       end
 
       def kill

sentry-ruby/lib/sentry/session_flusher.rb

@@ -20,12 +20,8 @@ def initialize(configuration, client)
 
     def flush
       return if @pending_aggregates.empty?
-      envelope = pending_envelope
-
-      Sentry.background_worker.perform do
-        @client.transport.send_envelope(envelope)
-      end
 
+      @client.capture_envelope(pending_envelope)
       @pending_aggregates = {}
     end
 

sentry-ruby/spec/sentry/client_spec.rb

@@ -95,6 +95,67 @@ def sentry_context
     end
   end
 
+  describe '#send_envelope' do
+    let(:envelope) do
+      envelope = Sentry::Envelope.new({ env_header: 1 })
+      envelope.add_item({ item_header: 42 }, { payload: 'test' })
+      envelope
+    end
+
+    it 'does not send envelope to either transport if disabled' do
+      configuration.dsn = nil
+
+      expect(subject.spotlight_transport).not_to receive(:send_envelope)
+      expect(subject.transport).not_to receive(:send_envelope)
+      subject.send_envelope(envelope)
+    end
+
+    it 'sends envelope to main transport if enabled' do
+      expect(subject.transport).to receive(:send_envelope).with(envelope)
+      subject.send_envelope(envelope)
+    end
+
+    it 'sends envelope with spotlight transport if enabled' do
+      configuration.spotlight = true
+
+      expect(subject.spotlight_transport).to receive(:send_envelope).with(envelope)
+      subject.send_envelope(envelope)
+    end
+
+    it 'logs error when transport failure' do
+      string_io = StringIO.new
+      configuration.debug = true
+      configuration.logger = ::Logger.new(string_io)
+      expect(subject.transport).to receive(:send_envelope).and_raise(Sentry::ExternalError.new("networking error"))
+
+      expect do
+        subject.send_envelope(envelope)
+      end.to raise_error(Sentry::ExternalError)
+
+      expect(string_io.string).to match(/Envelope sending failed: networking error/)
+      expect(string_io.string).to match(__FILE__)
+    end
+  end
+
+  describe '#capture_envelope' do
+    let(:envelope) do
+      envelope = Sentry::Envelope.new({ env_header: 1 })
+      envelope.add_item({ item_header: 42 }, { payload: 'test' })
+      envelope
+    end
+
+    before do
+      configuration.background_worker_threads = 0
+      Sentry.background_worker = Sentry::BackgroundWorker.new(configuration)
+    end
+
+    it 'queues envelope to background worker' do
+      expect(Sentry.background_worker).to receive(:perform).and_call_original
+      expect(subject).to receive(:send_envelope).with(envelope)
+      subject.capture_envelope(envelope)
+    end
+  end
+
   describe '#event_from_message' do
     let(:message) { 'This is a message' }
 

sentry-ruby/spec/sentry/metrics/aggregator_spec.rb

@@ -364,8 +364,8 @@
           expect(subject.code_locations).to eq({})
         end
 
-        it 'calls the background worker' do
-          expect(Sentry.background_worker).to receive(:perform)
+        it 'captures the envelope' do
+          expect(subject.client).to receive(:capture_envelope)
           subject.flush
         end
 
@@ -414,7 +414,7 @@
           expect(subject.buckets).to eq({})
         end
 
-        it 'calls the background worker' do
+        it 'captures the envelope' do
           expect(Sentry.background_worker).to receive(:perform)
           subject.flush(force: true)
         end

sentry-ruby/spec/sentry/session_flusher_spec.rb

@@ -5,6 +5,7 @@
 
   let(:configuration) do
     Sentry::Configuration.new.tap do |config|
+      config.dsn = Sentry::TestHelper::DUMMY_DSN
       config.release = 'test-release'
       config.environment = 'test'
       config.transport.transport_class = Sentry::DummyTransport

sentry-ruby/spec/sentry_spec.rb

@@ -1084,7 +1084,7 @@
       end
 
       it "flushes transport" do
-        expect(described_class.get_current_client.transport).to receive(:flush)
+        expect(described_class.get_current_client).to receive(:flush)
         described_class.close
       end