diff --git a/CHANGELOG.md b/CHANGELOG.md index 5af0b29b0..c37a76bbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ config.rails.active_support_logger_subscription_items.delete("sql.active_record") config.rails.active_support_logger_subscription_items["foo"] = :bar ``` +- Enable opting out of patches [#2151](https://github.com/getsentry/sentry-ruby/pull/2151) ### Bug Fixes diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index 59edf50d7..c3577d0f0 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -75,15 +75,15 @@ def exception_locals_tp ##### Patch Registration ##### # @!visibility private - def register_patch(patch = nil, target = nil, &block) + def register_patch(key, patch = nil, target = nil, &block) if patch && block raise ArgumentError.new("Please provide either a patch and its target OR a block, but not both") end if block - registered_patches << block + registered_patches[key] = block else - registered_patches << proc do + registered_patches[key] = proc do target.send(:prepend, patch) unless target.ancestors.include?(patch) end end @@ -91,14 +91,14 @@ def register_patch(patch = nil, target = nil, &block) # @!visibility private def apply_patches(config) - registered_patches.each do |patch| - patch.call(config) + registered_patches.each do |key, patch| + patch.call(config) if config.enabled_patches.include?(key) end end # @!visibility private def registered_patches - @registered_patches ||= [] + @registered_patches ||= {} end ##### Integrations ##### diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 11c2ce3bd..1f89f31a3 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -258,6 +258,11 @@ def capture_exception_frame_locals=(value) # @return [Float, nil] attr_reader :profiles_sample_rate + # Array of patches to apply. + # Default is {DEFAULT_PATCHES} + # @return [Array] + attr_accessor :enabled_patches + # these are not config options # @!visibility private attr_reader :errors, :gem_specs @@ -297,6 +302,8 @@ def capture_exception_frame_locals=(value) PROPAGATION_TARGETS_MATCH_ALL = /.*/.freeze + DEFAULT_PATCHES = %i(redis puma http).freeze + class << self # Post initialization callbacks are called at the end of initialization process # allowing extending the configuration of sentry-ruby by multiple extensions @@ -340,6 +347,7 @@ def initialize self.server_name = server_name_from_env self.instrumenter = :sentry self.trace_propagation_targets = [PROPAGATION_TARGETS_MATCH_ALL] + self.enabled_patches = DEFAULT_PATCHES.dup self.before_send = nil self.before_send_transaction = nil diff --git a/sentry-ruby/lib/sentry/net/http.rb b/sentry-ruby/lib/sentry/net/http.rb index 634753937..71c61ca3e 100644 --- a/sentry-ruby/lib/sentry/net/http.rb +++ b/sentry-ruby/lib/sentry/net/http.rb @@ -99,4 +99,4 @@ def propagate_trace?(url, configuration) end end -Sentry.register_patch(Sentry::Net::HTTP, Net::HTTP) +Sentry.register_patch(:http, Sentry::Net::HTTP, Net::HTTP) diff --git a/sentry-ruby/lib/sentry/puma.rb b/sentry-ruby/lib/sentry/puma.rb index 69a937fb3..56e874771 100644 --- a/sentry-ruby/lib/sentry/puma.rb +++ b/sentry-ruby/lib/sentry/puma.rb @@ -29,4 +29,4 @@ def lowlevel_error(e, env, status=500) end end -Sentry.register_patch(Sentry::Puma::Server, Puma::Server) +Sentry.register_patch(:puma, Sentry::Puma::Server, Puma::Server) diff --git a/sentry-ruby/lib/sentry/redis.rb b/sentry-ruby/lib/sentry/redis.rb index befe868aa..3c3832321 100644 --- a/sentry-ruby/lib/sentry/redis.rb +++ b/sentry-ruby/lib/sentry/redis.rb @@ -99,8 +99,10 @@ def call_pipelined(commands, redis_config) if defined?(::Redis::Client) if Gem::Version.new(::Redis::VERSION) < Gem::Version.new("5.0") - Sentry.register_patch(Sentry::Redis::OldClientPatch, ::Redis::Client) + Sentry.register_patch(:redis, Sentry::Redis::OldClientPatch, ::Redis::Client) elsif defined?(RedisClient) - RedisClient.register(Sentry::Redis::GlobalRedisInstrumentation) + Sentry.register_patch(:redis) do + RedisClient.register(Sentry::Redis::GlobalRedisInstrumentation) + end end end diff --git a/sentry-ruby/spec/sentry/configuration_spec.rb b/sentry-ruby/spec/sentry/configuration_spec.rb index 6cd571600..5e4beb7ee 100644 --- a/sentry-ruby/spec/sentry/configuration_spec.rb +++ b/sentry-ruby/spec/sentry/configuration_spec.rb @@ -542,4 +542,15 @@ class SentryConfigurationSample < Sentry::Configuration expect(subject.instrumenter).to eq(:sentry) end end + + describe "#enabled_patches" do + it "sets default patches" do + expect(subject.enabled_patches).to eq(%i(redis puma http)) + end + + it "can override" do + subject.enabled_patches.delete(:puma) + expect(subject.enabled_patches).to eq(%i(redis http)) + end + end end diff --git a/sentry-ruby/spec/sentry_spec.rb b/sentry-ruby/spec/sentry_spec.rb index 80c74180f..da784a6bf 100644 --- a/sentry-ruby/spec/sentry_spec.rb +++ b/sentry-ruby/spec/sentry_spec.rb @@ -1032,4 +1032,56 @@ end.to change { new_transport.events.count }.by(1) end end + + describe "patching" do + let(:target_class) { Class.new } + + let(:module_patch) do + Module.new do + def foo; end + end + end + + context "with target and patch to prepend" do + before do + described_class.register_patch(:module_patch, module_patch, target_class) + end + + it "does not prepend patch if not in enabled_patches" do + perform_basic_setup + expect(target_class.ancestors).not_to include(module_patch) + expect(target_class.instance_methods).not_to include(:foo) + end + + it "prepends patch if in enabled_patches" do + expect(target_class).to receive(:prepend).with(module_patch).and_call_original + perform_basic_setup { |c| c.enabled_patches = %i(module_patch) } + + expect(target_class.ancestors.first).to eq(module_patch) + expect(target_class.instance_methods).to include(:foo) + end + end + + context "with block" do + before do + described_class.register_patch(:block_patch) do + target_class.send(:prepend, module_patch) + end + end + + it "does not call block if not in enabled_patches" do + perform_basic_setup + expect(target_class.ancestors).not_to include(module_patch) + expect(target_class.instance_methods).not_to include(:foo) + end + + it "calls block if in enabled_patches" do + expect(target_class).to receive(:prepend).with(module_patch).and_call_original + perform_basic_setup { |c| c.enabled_patches = %i(block_patch) } + + expect(target_class.ancestors.first).to eq(module_patch) + expect(target_class.instance_methods).to include(:foo) + end + end + end end