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

Redis 4.0 support #305

Merged
merged 9 commits into from
Jan 17, 2018
Merged

Redis 4.0 support #305

merged 9 commits into from
Jan 17, 2018

Conversation

delner
Copy link
Contributor

@delner delner commented Jan 10, 2018

Our Redis patcher isn't compatible with Redis 4.0.

According to some of the latest changes, they removed backwards compatibility for using Redis#client as we were in our patcher. That function no longer returns a client object, instead we must use the new Redis#_client function.

Relevant commits here:

redis/redis-rb@c239abb
redis/redis-rb@3138507

This pull request adds an alternative patching method for Redis version 4+, which uses the new Redis#_client instead. It also retains our older method of patching for Redis versions 3.x, to preserve backwards compatibility.

@delner delner added enhancement integrations Involves tracing integrations labels Jan 10, 2018
@delner delner self-assigned this Jan 10, 2018
@delner delner added the breaking-change Involves a breaking change label Jan 10, 2018
@delner
Copy link
Contributor Author

delner commented Jan 10, 2018

Just to keep in mind, I've found Redis 4.0 is not compatible with Ruby < 2.2.2 or Resque < 2.0. Resque 2.0+ is still currently unreleased, and when it is, they will drop support for all Ruby versions < 2.1.

Related commits on Resque:

resque/resque@59a7d61
resque/resque@fa9cd4c

@delner delner force-pushed the delner/redis_4_support branch 3 times, most recently from b6b9940 to d14e399 Compare January 12, 2018 17:06
@@ -63,7 +53,8 @@ def patch_redis_client

def initialize(*args)
service = Datadog.configuration[:redis][:service_name]
pin = Datadog::Pin.new(service, app: 'redis', app_type: Datadog::Ext::AppTypes::DB)
tracer = Datadog.configuration[:redis][:tracer]
pin = Datadog::Pin.new(service, app: 'redis', app_type: Datadog::Ext::AppTypes::DB, tracer: tracer)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

p-lambert
p-lambert previously approved these changes Jan 12, 2018
Copy link
Contributor

@palazzem palazzem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're introducing a breaking change in the Configuration system, we can have this PR merged that removes the Pin accessors from the Redis client. The new way to change tracing configs is:

require 'redis'
require 'ddtrace'

Datadog.configure do |c|
  c.use :redis, service_name: 'redis'  # default service name
end

# traced clients
customer_cache = Redis::Client.new
invoice_cache = Redis::Client.new

# change service names for each client
Datadog.configure(customer_cache, service_name: 'customer-cache')
Datadog.configure(invoice_cache, service_name: invoice-cache')

@palazzem palazzem merged commit 89ca8fc into master Jan 17, 2018
@palazzem palazzem deleted the delner/redis_4_support branch January 17, 2018 14:37
@delner delner mentioned this pull request Jan 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Involves a breaking change integrations Involves tracing integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants