From 9bd69e7e67fb67123acacc69bdd5e27fe25b6e9f Mon Sep 17 00:00:00 2001 From: David Elner Date: Tue, 13 Aug 2019 16:43:51 -0400 Subject: [PATCH] Added: Compatibility with Rails 5.2 built-in Redis store. --- Appraisals | 12 ++++++++++++ Rakefile | 2 ++ .../contrib/active_support/cache/patcher.rb | 1 - .../contrib/active_support/cache/redis.rb | 7 ++++++- lib/ddtrace/contrib/rails/framework.rb | 2 +- test/contrib/rails/apps/application.rb | 6 +++++- test/contrib/rails/redis_cache_test.rb | 19 +++++++++++++------ 7 files changed, 39 insertions(+), 10 deletions(-) diff --git a/Appraisals b/Appraisals index f63a0d0cfd9..fd39b6898c5 100644 --- a/Appraisals +++ b/Appraisals @@ -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' @@ -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' diff --git a/Rakefile b/Rakefile index b9d9f1e86cf..a925b2f5250 100644 --- a/Rakefile +++ b/Rakefile @@ -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' @@ -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' diff --git a/lib/ddtrace/contrib/active_support/cache/patcher.rb b/lib/ddtrace/contrib/active_support/cache/patcher.rb index 2dea1768e83..45752252d75 100644 --- a/lib/ddtrace/contrib/active_support/cache/patcher.rb +++ b/lib/ddtrace/contrib/active_support/cache/patcher.rb @@ -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 diff --git a/lib/ddtrace/contrib/active_support/cache/redis.rb b/lib/ddtrace/contrib/active_support/cache/redis.rb index 1faafc4f297..1e31dc77da8 100644 --- a/lib/ddtrace/contrib/active_support/cache/redis.rb +++ b/lib/ddtrace/contrib/active_support/cache/redis.rb @@ -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 diff --git a/lib/ddtrace/contrib/rails/framework.rb b/lib/ddtrace/contrib/rails/framework.rb index 586ee14d0fa..001ed5f8fa6 100644 --- a/lib/ddtrace/contrib/rails/framework.rb +++ b/lib/ddtrace/contrib/rails/framework.rb @@ -60,7 +60,7 @@ def self.activate_active_support!(config) Datadog.configuration.use( :active_support, - service_name: config[:cache_service], + cache_service: config[:cache_service], tracer: config[:tracer] ) end diff --git a/test/contrib/rails/apps/application.rb b/test/contrib/rails/apps/application.rb index 5525b15a6b9..a9902894e50 100644 --- a/test/contrib/rails/apps/application.rb +++ b/test/contrib/rails/apps/application.rb @@ -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' diff --git a/test/contrib/rails/redis_cache_test.rb b/test/contrib/rails/redis_cache_test.rb index bd8b403a649..868286b93ae 100644 --- a/test/contrib/rails/redis_cache_test.rb +++ b/test/contrib/rails/redis_cache_test.rb @@ -18,9 +18,6 @@ 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 @@ -28,6 +25,16 @@ class RedisCacheTracingTest < ActionController::TestCase 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. @@ -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') @@ -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') @@ -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')