Skip to content

Commit

Permalink
Added: Compatibility with Rails 5.2 built-in Redis store.
Browse files Browse the repository at this point in the history
  • Loading branch information
delner committed Aug 16, 2019
1 parent c290b8c commit 9414b9f
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 9 deletions.
12 changes: 12 additions & 0 deletions Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,12 @@ elsif Gem::Version.new('2.2.0') <= Gem::Version.new(RUBY_VERSION) \
end

appraise 'rails5-postgres-redis' do
gem 'rails', '~> 5.2.1'
gem 'pg', '< 1.0', platform: :ruby
gem 'redis'
end

appraise 'rails5-postgres-redis-activesupport' do
gem 'rails', '~> 5.2.1'
gem 'pg', '< 1.0', platform: :ruby
gem 'redis-rails'
Expand Down Expand Up @@ -425,6 +431,12 @@ elsif Gem::Version.new('2.3.0') <= Gem::Version.new(RUBY_VERSION) \
end

appraise 'rails5-postgres-redis' do
gem 'rails', '~> 5.2.1'
gem 'pg', '< 1.0', platform: :ruby
gem 'redis'
end

appraise 'rails5-postgres-redis-activesupport' do
gem 'rails', '~> 5.2.1'
gem 'pg', '< 1.0', platform: :ruby
gem 'redis-rails'
Expand Down
2 changes: 2 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ task :ci do
sh 'bundle exec appraisal rails5-mysql2 rake test:rails'
sh 'bundle exec appraisal rails5-postgres rake test:rails'
sh 'bundle exec appraisal rails5-postgres-redis rake test:railsredis'
sh 'bundle exec appraisal rails5-postgres-redis-activesupport rake test:railsredis'
sh 'bundle exec appraisal rails5-postgres-sidekiq rake test:railssidekiq'
sh 'bundle exec appraisal rails5-postgres-sidekiq rake test:railsactivejob'
sh 'bundle exec appraisal rails5-postgres rake test:railsdisableenv'
Expand Down Expand Up @@ -409,6 +410,7 @@ task :ci do
sh 'bundle exec appraisal rails5-mysql2 rake test:rails'
sh 'bundle exec appraisal rails5-postgres rake test:rails'
sh 'bundle exec appraisal rails5-postgres-redis rake test:railsredis'
sh 'bundle exec appraisal rails5-postgres-redis-activesupport rake test:railsredis'
sh 'bundle exec appraisal rails5-postgres-sidekiq rake test:railssidekiq'
sh 'bundle exec appraisal rails5-postgres-sidekiq rake test:railsactivejob'
sh 'bundle exec appraisal rails5-postgres rake test:railsdisableenv'
Expand Down
1 change: 0 additions & 1 deletion lib/ddtrace/contrib/active_support/cache/patcher.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
require 'ddtrace/contrib/patcher'
require 'ddtrace/contrib/active_support/cache/instrumentation'
# require 'ddtrace/contrib/active_support/cache/redis'

module Datadog
module Contrib
Expand Down
7 changes: 6 additions & 1 deletion lib/ddtrace/contrib/active_support/cache/redis.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@ module Patcher
# be redefined, and some of them not. The latest version of redis-activesupport
# redefines write but leaves untouched read and delete:
# https://github.com/redis-store/redis-activesupport/blob/master/lib/active_support/cache/redis_store.rb
#
# For Rails >= 5.2 w/o redis-activesupport...
# ActiveSupport includes a Redis cache store internally, and does not require these overrides.
# https://github.com/rails/rails/blob/master/activesupport/lib/active_support/cache/redis_cache_store.rb
def patch_redis?(meth)
defined?(::ActiveSupport::Cache::RedisStore) \
!Gem.loaded_specs['redis-activesupport'].nil? \
&& defined?(::ActiveSupport::Cache::RedisStore) \
&& ::ActiveSupport::Cache::RedisStore.instance_methods(false).include?(meth)
end

Expand Down
6 changes: 5 additions & 1 deletion test/contrib/rails/apps/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ class TestApplication < Rails::Application
# common settings between all Rails versions
def initialize(*args)
super(*args)
redis_cache = [:redis_store, { url: ENV['REDIS_URL'] }]
redis_cache = if Gem.loaded_specs['redis-activesupport']
[:redis_store, { url: ENV['REDIS_URL'] }]
else
[:redis_cache_store, { url: ENV['REDIS_URL'] }]
end
file_cache = [:file_store, '/tmp/ddtrace-rb/cache/']

config.secret_key_base = 'f624861242e4ccf20eacb6bb48a886da'
Expand Down
19 changes: 13 additions & 6 deletions test/contrib/rails/redis_cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,23 @@ class RedisCacheTracingTest < ActionController::TestCase
@tracer = get_test_tracer
Datadog.configuration[:rails][:tracer] = @tracer
Datadog.configuration.use(:redis)

# get the Redis pin accessing private methods (only Rails 3.x)
driver = Rails.cache.instance_variable_get(:@data)
Datadog.configure(client_from_driver(driver), tracer: @tracer)
end

teardown do
Datadog.configuration[:rails][:tracer] = @original_tracer
end

def driver
# For internal Redis store (Rails 5.2+), use the #redis method.
# For redis-activesupport, get the Redis pin accessing private methods (only Rails 3.x)
Rails.cache.respond_to?(:redis) ? Rails.cache.redis : Rails.cache.instance_variable_get(:@data)
end

def cache_store_name
Gem.loaded_specs['redis-activesupport'] ? 'redis_store' : 'redis_cache_store'
end

test 'cache.read() and cache.fetch() are properly traced' do
# read and fetch should behave exactly the same, and we shall
# never see a read() having a fetch() as parent.
Expand All @@ -44,7 +51,7 @@ class RedisCacheTracingTest < ActionController::TestCase
assert_equal(cache.span_type, 'cache')
assert_equal(cache.resource, 'GET')
assert_equal(cache.service, "#{app_name}-cache")
assert_equal(cache.get_tag('rails.cache.backend').to_s, 'redis_store')
assert_equal(cache.get_tag('rails.cache.backend').to_s, cache_store_name)
assert_equal(cache.get_tag('rails.cache.key'), 'custom-key')

assert_equal(redis.name, 'redis.command')
Expand Down Expand Up @@ -110,7 +117,7 @@ class RedisCacheTracingTest < ActionController::TestCase
assert_equal(cache.span_type, 'cache')
assert_equal(cache.resource, 'SET')
assert_equal(cache.service, "#{app_name}-cache")
assert_equal(cache.get_tag('rails.cache.backend').to_s, 'redis_store')
assert_equal(cache.get_tag('rails.cache.backend').to_s, cache_store_name)
assert_equal(cache.get_tag('rails.cache.key'), 'custom-key')

assert_equal(redis.name, 'redis.command')
Expand All @@ -134,7 +141,7 @@ class RedisCacheTracingTest < ActionController::TestCase
assert_equal(cache.span_type, 'cache')
assert_equal(cache.resource, 'DELETE')
assert_equal(cache.service, "#{app_name}-cache")
assert_equal(cache.get_tag('rails.cache.backend').to_s, 'redis_store')
assert_equal(cache.get_tag('rails.cache.backend').to_s, cache_store_name)
assert_equal(cache.get_tag('rails.cache.key'), 'custom-key')

assert_equal(del.name, 'redis.command')
Expand Down

0 comments on commit 9414b9f

Please sign in to comment.