Skip to content

Commit

Permalink
Merge pull request #635 from gingerlime/630-cache-key-performance-fix
Browse files Browse the repository at this point in the history
fixes #630 potential performance hit logging cache key
  • Loading branch information
delner authored Jan 3, 2019
2 parents fa5abaf + 280af3d commit ae1710d
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 1 deletion.
3 changes: 2 additions & 1 deletion lib/ddtrace/contrib/rails/active_support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ def self.finish_trace_cache(payload)
# discard parameters from the cache_store configuration
store, = *Array.wrap(::Rails.configuration.cache_store).flatten
span.set_tag(Ext::TAG_CACHE_BACKEND, store)
cache_key = Datadog::Utils.truncate(payload.fetch(:key), Ext::QUANTIZE_CACHE_MAX_KEY_SIZE)
normalized_key = ::ActiveSupport::Cache.expand_cache_key(payload.fetch(:key))
cache_key = Datadog::Utils.truncate(normalized_key, Ext::QUANTIZE_CACHE_MAX_KEY_SIZE)
span.set_tag(Ext::TAG_CACHE_KEY, cache_key)
span.set_error(payload[:exception]) if payload[:exception]
ensure
Expand Down
14 changes: 14 additions & 0 deletions test/contrib/rails/cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,18 @@ def test_cache_key_truncation_regression
assert_equal(max_key_size, span.get_tag('rails.cache.key').length)
assert(span.get_tag('rails.cache.key').end_with?('...'))
end

test 'cache key is expanded using ActiveSupport' do
class User
def cache_key
'User:3'
end
end

Rails.cache.write(['custom-key', %w[x y], User.new], 50)
spans = @tracer.writer.spans()
assert_equal(1, spans.length)
span = spans[0]
assert_equal(span.get_tag('rails.cache.key'), 'custom-key/x/y/User:3')
end
end
14 changes: 14 additions & 0 deletions test/contrib/rails/redis_cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,20 @@ class RedisCacheTracingTest < ActionController::TestCase
assert_equal(cache.span_id, del.parent_id)
end

test 'cache key is expanded using ActiveSupport' do
class User
def cache_key
'User:3'
end
end

Rails.cache.write(['custom-key', %w[x y], User.new], 50)
spans = @tracer.writer.spans()
assert_equal(spans.length, 2)
cache, _redis = spans
assert_equal(cache.get_tag('rails.cache.key'), 'custom-key/x/y/User:3')
end

private

def client_from_driver(driver)
Expand Down

0 comments on commit ae1710d

Please sign in to comment.