Skip to content

Commit

Permalink
Forward keyword arguments on Ruby 2.7 and 3.0 (appsignal#693)
Browse files Browse the repository at this point in the history
Cherry-picked appsignal#692 on main branch.

Update Object instrumentation for Ruby 2.7 and 3.0 so that it doesn't
print deprecation warnings on Ruby 2.7 and raises an error on Ruby 3.0.

It didn't look broken because we didn't actually test argument
forwarding so the tests didn't break for Ruby 3.0.

I had to implement it twice, once for Ruby 2.7 and higher, and once for
everything older. This is because the two instrumentation methods are
incompatible and this is the easiest way to instrument all versions.
This means two implementation files and two tests because Ruby 1.9 can't
parse the keyword argument syntax introduced in Ruby 2.0. This is an
ugly fix, but it's also present for a very short time, since our develop
branch already drops support for Ruby 1.9 and we can remove this
workaround when this gets merged into the develop branch.
I justed wanted to get this merged into the main branch so we can fully
support Ruby 3.0 in the Ruby gem 2.x series, rather than only fully
support it in the Ruby gem 3.x series.

This change should make our Ruby 3.0 support complete.

Fixes appsignal#656
tombruijn authored Jan 21, 2021
1 parent 308a630 commit eef9432
Showing 8 changed files with 413 additions and 49 deletions.
2 changes: 2 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -13,6 +13,8 @@ AllCops:
- "gemfiles/vendor/**/*"
- "vendor/**/*"
- "benchmark.rake"
- "lib/appsignal/integrations/object_ruby_modern.rb"
- "spec/lib/appsignal/integrations/object_spec.rb"
DisplayCopNames: true
UseCache: true
CacheRootDirectory: ./tmp
12 changes: 7 additions & 5 deletions Rakefile
Original file line number Diff line number Diff line change
@@ -374,19 +374,21 @@ end
begin
require "rspec/core/rake_task"
is_jruby = defined?(RUBY_ENGINE) && RUBY_ENGINE == "jruby"
unless is_jruby
jruby_opts = "--exclude-pattern=spec/lib/appsignal/extension/jruby_spec.rb"
end
excludes = []
excludes << "spec/lib/appsignal/extension/jruby_spec.rb" unless is_jruby
is_ruby19 = RUBY_VERSION < "2.0"
excludes << "spec/lib/appsignal/integrations/object_spec.rb" if is_ruby19
exclude_pattern = "--exclude-pattern=#{excludes.join(",")}" if excludes.any?

desc "Run the AppSignal gem test suite."
RSpec::Core::RakeTask.new :test do |t|
t.rspec_opts = jruby_opts
t.rspec_opts = exclude_pattern
end

namespace :test do
desc "Run the Appsignal gem test in an extension failure scenario"
RSpec::Core::RakeTask.new :failure do |t|
t.rspec_opts = "#{jruby_opts} --tag extension_installation_failure"
t.rspec_opts = "#{exclude_pattern} --tag extension_installation_failure"
end
end
rescue LoadError # rubocop:disable Lint/HandleExceptions
38 changes: 4 additions & 34 deletions lib/appsignal/integrations/object.rb
Original file line number Diff line number Diff line change
@@ -4,38 +4,8 @@
Appsignal::Environment.report_enabled("object_instrumentation")
end

class Object
def self.appsignal_instrument_class_method(method_name, options = {})
singleton_class.send \
:alias_method, "appsignal_uninstrumented_#{method_name}", method_name
singleton_class.send(:define_method, method_name) do |*args, &block|
name = options.fetch(:name) do
"#{method_name}.class_method.#{appsignal_reverse_class_name}.other"
end
Appsignal.instrument name do
send "appsignal_uninstrumented_#{method_name}", *args, &block
end
end
end

def self.appsignal_instrument_method(method_name, options = {})
alias_method "appsignal_uninstrumented_#{method_name}", method_name
define_method method_name do |*args, &block|
name = options.fetch(:name) do
"#{method_name}.#{appsignal_reverse_class_name}.other"
end
Appsignal.instrument name do
send "appsignal_uninstrumented_#{method_name}", *args, &block
end
end
end

def self.appsignal_reverse_class_name
return "AnonymousClass" unless name
name.split("::").reverse.join(".")
end

def appsignal_reverse_class_name
self.class.appsignal_reverse_class_name
end
if RUBY_VERSION < "2.0"
require "appsignal/integrations/object_ruby_19"
else
require "appsignal/integrations/object_ruby_modern"
end
37 changes: 37 additions & 0 deletions lib/appsignal/integrations/object_ruby_19.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# frozen_string_literal: true

class Object
def self.appsignal_instrument_class_method(method_name, options = {})
singleton_class.send \
:alias_method, "appsignal_uninstrumented_#{method_name}", method_name
singleton_class.send(:define_method, method_name) do |*args, &block|
name = options.fetch(:name) do
"#{method_name}.class_method.#{appsignal_reverse_class_name}.other"
end
Appsignal.instrument name do
send "appsignal_uninstrumented_#{method_name}", *args, &block
end
end
end

def self.appsignal_instrument_method(method_name, options = {})
alias_method "appsignal_uninstrumented_#{method_name}", method_name
define_method method_name do |*args, &block|
name = options.fetch(:name) do
"#{method_name}.#{appsignal_reverse_class_name}.other"
end
Appsignal.instrument name do
send "appsignal_uninstrumented_#{method_name}", *args, &block
end
end
end

def self.appsignal_reverse_class_name
return "AnonymousClass" unless name
name.split("::").reverse.join(".")
end

def appsignal_reverse_class_name
self.class.appsignal_reverse_class_name
end
end
64 changes: 64 additions & 0 deletions lib/appsignal/integrations/object_ruby_modern.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# frozen_string_literal: true

class Object
if Appsignal::System.ruby_2_7_or_newer?
def self.appsignal_instrument_class_method(method_name, options = {})
singleton_class.send \
:alias_method, "appsignal_uninstrumented_#{method_name}", method_name
singleton_class.send(:define_method, method_name) do |*args, **kwargs, &block|
name = options.fetch(:name) do
"#{method_name}.class_method.#{appsignal_reverse_class_name}.other"
end
Appsignal.instrument name do
send "appsignal_uninstrumented_#{method_name}", *args, **kwargs, &block
end
end
end

def self.appsignal_instrument_method(method_name, options = {})
alias_method "appsignal_uninstrumented_#{method_name}", method_name
define_method method_name do |*args, **kwargs, &block|
name = options.fetch(:name) do
"#{method_name}.#{appsignal_reverse_class_name}.other"
end
Appsignal.instrument name do
send "appsignal_uninstrumented_#{method_name}", *args, **kwargs, &block
end
end
end
else
def self.appsignal_instrument_class_method(method_name, options = {})
singleton_class.send \
:alias_method, "appsignal_uninstrumented_#{method_name}", method_name
singleton_class.send(:define_method, method_name) do |*args, &block|
name = options.fetch(:name) do
"#{method_name}.class_method.#{appsignal_reverse_class_name}.other"
end
Appsignal.instrument name do
send "appsignal_uninstrumented_#{method_name}", *args, &block
end
end
end

def self.appsignal_instrument_method(method_name, options = {})
alias_method "appsignal_uninstrumented_#{method_name}", method_name
define_method method_name do |*args, &block|
name = options.fetch(:name) do
"#{method_name}.#{appsignal_reverse_class_name}.other"
end
Appsignal.instrument name do
send "appsignal_uninstrumented_#{method_name}", *args, &block
end
end
end
end

def self.appsignal_reverse_class_name
return "AnonymousClass" unless name
name.split("::").reverse.join(".")
end

def appsignal_reverse_class_name
self.class.appsignal_reverse_class_name
end
end
4 changes: 4 additions & 0 deletions lib/appsignal/system.rb
Original file line number Diff line number Diff line change
@@ -79,5 +79,9 @@ def self.extract_ldd_version(string)
def self.jruby?
RUBY_PLATFORM == "java"
end

def self.ruby_2_7_or_newer?
RUBY_VERSION > "2.7"
end
end
end
Loading

0 comments on commit eef9432

Please sign in to comment.