Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure isolated envelopes respect enabled_environments #2291

Merged
merged 1 commit into from Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions 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
Expand Down
2 changes: 1 addition & 1 deletion sentry-ruby/lib/sentry-ruby.rb
Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions sentry-ruby/lib/sentry/client.rb
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Comment on lines +207 to +208
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have data categories for both sessions and metrics, though.

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.
Expand Down
6 changes: 2 additions & 4 deletions sentry-ruby/lib/sentry/metrics/aggregator.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 1 addition & 5 deletions sentry-ruby/lib/sentry/session_flusher.rb
Expand Up @@ -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

Expand Down
61 changes: 61 additions & 0 deletions sentry-ruby/spec/sentry/client_spec.rb
Expand Up @@ -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' }

Expand Down
6 changes: 3 additions & 3 deletions sentry-ruby/spec/sentry/metrics/aggregator_spec.rb
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions sentry-ruby/spec/sentry/session_flusher_spec.rb
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion sentry-ruby/spec/sentry_spec.rb
Expand Up @@ -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

Expand Down