diff --git a/CHANGELOG.md b/CHANGELOG.md index d48316eef..8aa5b4a09 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/sentry-ruby/lib/sentry/profiler.rb b/sentry-ruby/lib/sentry/profiler.rb index e5bed005e..4d9532caf 100644 --- a/sentry-ruby/lib/sentry/profiler.rb +++ b/sentry-ruby/lib/sentry/profiler.rb @@ -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 @@ -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 = {} @@ -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 @@ -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 diff --git a/sentry-ruby/lib/sentry/transport.rb b/sentry-ruby/lib/sentry/transport.rb index cdfc23359..8fafefdc1 100644 --- a/sentry-ruby/lib/sentry/transport.rb +++ b/sentry-ruby/lib/sentry/transport.rb @@ -18,7 +18,8 @@ class Transport :network_error, :sample_rate, :before_send, - :event_processor + :event_processor, + :insufficient_data, ] include LoggingHelper diff --git a/sentry-ruby/spec/sentry/profiler_spec.rb b/sentry-ruby/spec/sentry/profiler_spec.rb index ec54efebb..215e739be 100644 --- a/sentry-ruby/spec/sentry/profiler_spec.rb +++ b/sentry-ruby/spec/sentry/profiler_spec.rb @@ -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.pwd) } + data + end + describe '#start' do context 'without sampling decision' do it 'does not start StackProf' do @@ -129,9 +151,19 @@ 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 @@ -139,38 +171,50 @@ 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.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)