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 Service #135

Closed
ZeroInputCtrl opened this issue Jun 9, 2017 · 8 comments
Closed

Redis Service #135

ZeroInputCtrl opened this issue Jun 9, 2017 · 8 comments
Labels
integrations Involves tracing integrations
Milestone

Comments

@ZeroInputCtrl
Copy link

I have an application where it has to connect to 2 separate redis servers. I would like to set the service on each connection separately but right now it seems the redis service is a constant and can't be changed. If it could be passed into the patched initializer with the rest of the args that would be great.

@ufoot ufoot added enhancement integrations Involves tracing integrations labels Jun 12, 2017
@ufoot
Copy link
Contributor

ufoot commented Jun 12, 2017

Hi @BaneOfSerenity indeed this is clearly on our TODO list, will ping you whenever we release support for this config option.

@denmat
Copy link

denmat commented Jun 19, 2017

Would like to see this too 👍

@nerdrew
Copy link

nerdrew commented Sep 15, 2017

Want to see if this PR does what you were thinking? #190

It adds config for default_redis_service and redis_service_include_hostname.

@palazzem palazzem added this to the 0.11.0 milestone Jan 16, 2018
@palazzem
Copy link
Contributor

palazzem commented Jan 17, 2018

Hello @BaneOfSerenity! currently we're shipping the 0.11.0 that introduces a new way to configure the Tracer and our integrations. It's now possible to specify the default service name that will be used automatically by all Redis clients, though you can set it also for each instance. Here an example:

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')

I'm closing the issue since it can be considered fixed, but feel free to write your thoughts here! Also I want to say thank you again to @nerdrew because his work has been very important to us and improved a lot our configuration system.

@ZeroInputCtrl
Copy link
Author

Sorry about the lack of response to the last reply. This looks great. Thanks!

@rromanchuk
Copy link

Hey @palazzem @nerdrew i've got an implementation question. I was wondering, since i'm running two separate redis endpoints for sidekiq and rails cache_store via config.cache_store = :redis_cache_store, { url: Credentials[:redis_cache_store_url], driver: :hiredis, connect_timeout: 1, read_timeout: 1} ....what's the best way to inject a different service name for these?

Do I do something like this?

Datadog.configure(Rails.cache.redis, service_name: 'redis-rails-cache')

I'm not sure how i grab the redis object for sidekiq though? Also where would i set these up, given that i want these objects to exist when i'm calling Datadog.configure should i use

config.after_initialize do
  Datadog.configure([whereever the sidekiq redis client is?], service_name: 'redis-sidekiq')
end

@delner
Copy link
Contributor

delner commented Oct 25, 2018

@rromanchuk The Datadog.configure in your first block should work.

As for getting Redis for Sidekiq, that's a bit trickier... looking at Sidekiq's Redis connection code, it looks like they have a connection pool of Redis clients, which seems to suggest there are many of them. The trick would be to inject code to wrap each of those clients with the appropriate configuration, which will require a bit of monkey patching.

I might try something like the following (although this is entirely untested, and might require changes):

module DatadogSidekiqRedisPatch
  def self.extended(base)
    class << base
      prepend ClassMethods
    end
  end

  module ClassMethods
    def build_client(options)
      super.tap do |client|
        if client.is_a?(Redis)
          Datadog.configure(client, service_name: 'redis-sidekiq')
        end
      end
    end
  end
end

Sidekiq::RedisConnection.send(:extend, DatadogSidekiqRedisPatch)

@Jcambass
Copy link

Jcambass commented Nov 26, 2018

If you're using rails own RedisCacheStore you're fine using:

Datadog.configure(Rails.cache.redis._store, service_name: 'my-app-cache-redis')

Notice that the Datadog::Pin is set on Redis::Client rather than Redis, thus you need to passredis._client.

If you're setting any pooling options Redis::Distributed is gonna be used and some monkeypatching is needed. For Sidekiq you also need to do monkeypatching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrations Involves tracing integrations
Projects
None yet
Development

No branches or pull requests

8 participants