Skip to content

Commit

Permalink
Better profiler_class setter in config
Browse files Browse the repository at this point in the history
  • Loading branch information
solnic committed Sep 20, 2024
1 parent 5abb059 commit 282c3c2
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 12 deletions.
1 change: 1 addition & 0 deletions sentry-ruby/lib/sentry-ruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
require "sentry/backpressure_monitor"
require "sentry/cron/monitor_check_ins"
require "sentry/metrics"
require "sentry/vernier/profiler"

[
"sentry/rake",
Expand Down
14 changes: 13 additions & 1 deletion sentry-ruby/lib/sentry/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ def capture_exception_frame_locals=(value)

# The profiler class
# @return [Class]
attr_accessor :profiler_class
attr_reader :profiler_class

# Take a float between 0.0 and 1.0 as the sample rate for capturing profiles.
# Note that this rate is relative to traces_sample_rate / traces_sampler,
Expand Down Expand Up @@ -504,6 +504,18 @@ def profiles_sample_rate=(profiles_sample_rate)
@profiles_sample_rate = profiles_sample_rate
end

def profiler_class=(profiler_class)
if profiler_class == Sentry::Vernier::Profiler
begin
require "vernier"
rescue LoadError
raise ArgumentError, "Please add the 'vernier' gem to your Gemfile to use the Vernier profiler with Sentry."

Check warning on line 512 in sentry-ruby/lib/sentry/configuration.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/configuration.rb#L512

Added line #L512 was not covered by tests
end
end

@profiler_class = profiler_class
end

def sending_allowed?
spotlight || sending_to_dsn_allowed?
end
Expand Down
4 changes: 1 addition & 3 deletions sentry-ruby/lib/sentry/vernier/profiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
require_relative "../profiler/helpers"
require_relative "output"

begin; require "vernier"; rescue LoadError; return; end

module Sentry
module Vernier
class Profiler
Expand All @@ -19,7 +17,7 @@ def initialize(configuration)
@started = false
@sampled = nil

@profiling_enabled = configuration.profiling_enabled?
@profiling_enabled = defined?(Vernier) && configuration.profiling_enabled?
@profiles_sample_rate = configuration.profiles_sample_rate
@project_root = configuration.project_root
@app_dirs_pattern = configuration.app_dirs_pattern
Expand Down
17 changes: 17 additions & 0 deletions sentry-ruby/spec/sentry/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -693,4 +693,21 @@ class SentryConfigurationSample < Sentry::Configuration
expect(subject.enabled_patches).to eq(%i[redis http])
end
end

describe "#profiler_class=" do
it "sets the profiler class to Vernier when it's available", when: :vernier_installed? do
subject.profiler_class = Sentry::Vernier::Profiler
expect(subject.profiler_class).to eq(Sentry::Vernier::Profiler)
end

it "sets the profiler class to StackProf when Vernier is not available", when: { ruby_version?: [:<, "3.2"] } do
expect { subject.profiler_class = Sentry::Vernier::Profiler }

Check warning on line 704 in sentry-ruby/spec/sentry/configuration_spec.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/spec/sentry/configuration_spec.rb#L704

Added line #L704 was not covered by tests
.to raise_error(
ArgumentError,
/Please add the 'vernier' gem to your Gemfile/
)

expect(subject.profiler_class).to eq(Sentry::Profiler)

Check warning on line 710 in sentry-ruby/spec/sentry/configuration_spec.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/spec/sentry/configuration_spec.rb#L710

Added line #L710 was not covered by tests
end
end
end
9 changes: 3 additions & 6 deletions sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -706,13 +706,10 @@ def will_be_sampled_by_sdk
expect(profile[:transaction][:trace_id]).to eq(event.contexts[:trace][:trace_id])

thread_id_mapping = {
Sentry::Profiler => "0"
Sentry::Profiler => "0",
Sentry::Vernier::Profiler => Thread.current.object_id.to_s
}

if defined?(Sentry::Vernier::Profiler)
thread_id_mapping[Sentry::Vernier::Profiler] = Thread.current.object_id.to_s
end

expect(profile[:transaction][:active_thread_id]).to eq(thread_id_mapping[Sentry.configuration.profiler_class])

# detailed checking of content is done in profiler_spec,
Expand All @@ -737,7 +734,7 @@ def will_be_sampled_by_sdk
let(:app) do
->(_) do
[200, {}, "ok"]
end
end
end

let(:stackprof_results) do
Expand Down
4 changes: 2 additions & 2 deletions sentry-ruby/spec/sentry/vernier/profiler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

require "sentry/vernier/profiler"

RSpec.describe "Sentry::Vernier::Profiler", when: { ruby_version?: [:>=, "3.2.1"] } do
subject(:profiler) { Sentry::Vernier::Profiler.new(Sentry.configuration) }
RSpec.describe Sentry::Vernier::Profiler, when: { ruby_version?: [:>=, "3.2.1"] } do
subject(:profiler) { described_class.new(Sentry.configuration) }

before do
# TODO: replace with some public API once available
Expand Down
1 change: 1 addition & 0 deletions sentry-ruby/spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
require "rspec/retry"
require "redis"
require "stackprof" unless RUBY_PLATFORM == "java"
require "vernier" unless RUBY_PLATFORM == "java" || RUBY_VERSION < "3.2"

SimpleCov.start do
project_name "sentry-ruby"
Expand Down

0 comments on commit 282c3c2

Please sign in to comment.