Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable opting out of patches #2151

Merged
merged 1 commit into from Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
12 changes: 6 additions & 6 deletions sentry-ruby/lib/sentry-ruby.rb
Expand Up @@ -75,30 +75,30 @@ 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
end

# @!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 #####
Expand Down
8 changes: 8 additions & 0 deletions sentry-ruby/lib/sentry/configuration.rb
Expand Up @@ -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<Symbol>]
attr_accessor :enabled_patches

# these are not config options
# @!visibility private
attr_reader :errors, :gem_specs
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion sentry-ruby/lib/sentry/net/http.rb
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion sentry-ruby/lib/sentry/puma.rb
Expand Up @@ -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)
6 changes: 4 additions & 2 deletions sentry-ruby/lib/sentry/redis.rb
Expand Up @@ -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
11 changes: 11 additions & 0 deletions sentry-ruby/spec/sentry/configuration_spec.rb
Expand Up @@ -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
52 changes: 52 additions & 0 deletions sentry-ruby/spec/sentry_spec.rb
Expand Up @@ -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