Skip to content

Commit

Permalink
Record client reports for profiles (#2107)
Browse files Browse the repository at this point in the history
  • Loading branch information
sl0thentr0py authored Sep 13, 2023
1 parent 1745db2 commit cc91b56
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 38 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
## Unreleased

### Features

- Record client reports for profiles [#2107](https://github.com/getsentry/sentry-ruby/pull/2107)

### Bug Fixes

- Rename `http.method` to `http.request.method` in `Span::DataConventions` [#2106](https://github.com/getsentry/sentry-ruby/pull/2106)
Expand Down
23 changes: 17 additions & 6 deletions sentry-ruby/lib/sentry/profiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class Profiler
# 101 Hz in microseconds
DEFAULT_INTERVAL = 1e6 / 101
MICRO_TO_NANO_SECONDS = 1e3
MIN_SAMPLES_REQUIRED = 2

attr_reader :sampled, :started, :event_id

Expand Down Expand Up @@ -73,14 +74,19 @@ def set_initial_sample_decision(transaction_sampled)
end

def to_hash
return {} unless @sampled
unless @sampled
record_lost_event(:sample_rate)
return {}
end

return {} unless @started

results = StackProf.results
return {} unless results
return {} if results.empty?
return {} if results[:samples] == 0
return {} unless results[:raw]

if !results || results.empty? || results[:samples] == 0 || !results[:raw]
record_lost_event(:insufficient_data)
return {}
end

frame_map = {}

Expand Down Expand Up @@ -157,8 +163,9 @@ def to_hash

log('Some samples thrown away') if samples.size != results[:samples]

if samples.size <= 2
if samples.size <= MIN_SAMPLES_REQUIRED
log('Not enough samples, discarding profiler')
record_lost_event(:insufficient_data)
return {}
end

Expand Down Expand Up @@ -218,5 +225,9 @@ def split_module(name)

[function, mod]
end

def record_lost_event(reason)
Sentry.get_current_client&.transport&.record_lost_event(reason, 'profile')
end
end
end
5 changes: 3 additions & 2 deletions sentry-ruby/lib/sentry/transport.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ class Transport
:network_error,
:sample_rate,
:before_send,
:event_processor
:event_processor,
:insufficient_data
]

include LoggingHelper
Expand Down Expand Up @@ -185,7 +186,7 @@ def fetch_pending_client_report
reason, type = key

# 'event' has to be mapped to 'error'
category = type == 'transaction' ? 'transaction' : 'error'
category = type == 'event' ? 'error' : type

{ reason: reason, category: category, quantity: val }
end
Expand Down
104 changes: 74 additions & 30 deletions sentry-ruby/spec/sentry/profiler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,28 @@

let(:subject) { described_class.new(Sentry.configuration) }

# profiled with following code
# module Bar
# module Foo
# def self.foo
# 1e6.to_i.times { 2**2 }
# end
# end

# def self.bar
# Foo.foo
# sleep 0.1
# end
# end
#
# Bar.bar
let(:stackprof_results) do
data = StackProf::Report.from_file('spec/support/stackprof_results.json').data
# relative dir differs on each machine
data[:frames].each { |_id, fra| fra[:file].gsub!(/<dir>/, Dir.pwd) }
data
end

describe '#start' do
context 'without sampling decision' do
it 'does not start StackProf' do
Expand Down Expand Up @@ -129,48 +151,70 @@
end

describe '#to_hash' do
it 'returns nil unless sampled' do
subject.set_initial_sample_decision(false)
expect(subject.to_hash).to eq({})
let (:transport) { Sentry.get_current_client.transport }

context 'when not sampled' do
before { subject.set_initial_sample_decision(false) }

it 'returns nil' do
expect(subject.to_hash).to eq({})
end

it 'records lost event' do
expect(transport).to receive(:record_lost_event).with(:sample_rate, 'profile')
subject.to_hash
end
end

it 'returns nil unless started' do
subject.set_initial_sample_decision(true)
expect(subject.to_hash).to eq({})
end

it 'returns nil if empty results' do
subject.set_initial_sample_decision(true)
subject.start
subject.stop
context 'with empty results' do
before do
subject.set_initial_sample_decision(true)
subject.start
subject.stop
end

expect(StackProf).to receive(:results).and_call_original
expect(subject.to_hash).to eq({})
it 'returns empty' do
expect(StackProf).to receive(:results).and_call_original
expect(subject.to_hash).to eq({})
end

it 'records lost event' do
expect(transport).to receive(:record_lost_event).with(:insufficient_data, 'profile')
subject.to_hash
end
end

context 'with profiled code' do
# profiled with following code
# module Bar
# module Foo
# def self.foo
# 1e6.to_i.times { 2**2 }
# end
# end

# def self.bar
# Foo.foo
# sleep 0.1
# end
# end
#
# Bar.bar
let(:stackprof_results) do
data = StackProf::Report.from_file('spec/support/stackprof_results.json').data
# relative dir differs on each machine
data[:frames].each { |_id, fra| fra[:file].gsub!(/<dir>/, Dir.pwd) }
data
context 'with insufficient samples' do
let(:truncated_results) do
results = stackprof_results
frame = stackprof_results[:frames].keys.first
results[:raw] = [1, frame, 2] # 2 samples with single frame
results
end

before do
allow(StackProf).to receive(:results).and_return(truncated_results)
subject.set_initial_sample_decision(true)
subject.start
subject.stop
end

it 'returns empty' do
expect(subject.to_hash).to eq({})
end

it 'records lost event' do
expect(transport).to receive(:record_lost_event).with(:insufficient_data, 'profile')
subject.to_hash
end
end

context 'with profiled code' do
before do
allow(StackProf).to receive(:results).and_return(stackprof_results)
subject.set_initial_sample_decision(true)
Expand Down

0 comments on commit cc91b56

Please sign in to comment.