Skip to content

Commit

Permalink
Record client reports for profiles
Browse files Browse the repository at this point in the history
  • Loading branch information
sl0thentr0py committed Sep 13, 2023
1 parent 1745db2 commit f6deea5
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 37 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
@@ -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
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 {}

Check warning on line 88 in sentry-ruby/lib/sentry/profiler.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/profiler.rb#L86-L88

Added lines #L86 - L88 were not covered by tests
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

Check warning on line 166 in sentry-ruby/lib/sentry/profiler.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/profiler.rb#L166

Added line #L166 was not covered by tests
log('Not enough samples, discarding profiler')
record_lost_event(:insufficient_data)

Check warning on line 168 in sentry-ruby/lib/sentry/profiler.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/profiler.rb#L168

Added line #L168 was not covered by tests
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
3 changes: 2 additions & 1 deletion sentry-ruby/lib/sentry/transport.rb
Expand Up @@ -18,7 +18,8 @@ class Transport
:network_error,
:sample_rate,
:before_send,
:event_processor
:event_processor,
:insufficient_data,
]

include LoggingHelper
Expand Down
104 changes: 74 additions & 30 deletions sentry-ruby/spec/sentry/profiler_spec.rb
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 f6deea5

Please sign in to comment.