newrelic / newrelic-ruby-agent

New Relic RPM Ruby Agent
https://docs.newrelic.com/docs/apm/agents/ruby-agent/getting-started/introduction-new-relic-ruby/
Apache License 2.0
1.19k stars 599 forks source link

New Relic doesn't track metrics for redis-clustering gem #2444

Open praveen-ks opened 4 months ago

praveen-ks commented 4 months ago

redis-rb moved the cluster support to redis-clustering gem companion gem which internally uses redis-cluster-client gem. I am using ElastiCache cluster and upgrading redis-rb gem to latest version.

Issue: New Relic is tracking the data for connections created using redis-client gem but not for redis-cluster-client gem. What can I do to make New Relic work with redis cluster & redis-clustering gem.

Redis::Client support is there but Redis::Cluster::Client is not supported.

workato-integration[bot] commented 4 months ago

https://new-relic.atlassian.net/browse/NR-228914

kaylareopelle commented 4 months ago

Hi @praveen-ks, thanks for bringing this gap in our instrumentation to our attention.

To start collecting the same data you see for the redis-rb gem, the agent will need to add instrumentation for the redis-clustering and redis-cluster-client gems. We'll start with trying to provide the same data you see for the redis-rb gem when these new gems are in play.

Are there any features unique to the redis-clustering gem that you'd like to see instrumented?

praveen-ks commented 3 months ago

Hi @praveen-ks, thanks for bringing this gap in our instrumentation to our attention.

To start collecting the same data you see for the redis-rb gem, the agent will need to add instrumentation for the redis-clustering and redis-cluster-client gems. We'll start with trying to provide the same data you see for the redis-rb gem when these new gems are in play.

Are there any features unique to the redis-clustering gem that you'd like to see instrumented?

That should be enough @kaylareopelle, nothing specific to redis-clustering gem.

praveen-ks commented 3 months ago

@kaylareopelle I have tried adding the instrumentation for redis-clustering gem using:

Redis::Cluster::Client.prepend(NewRelic::Agent::Instrumentation::Redis::Prepend)

It worked, I was able to get redis-cluster connection calls data.

But it caused performance decline for my application. Because of this override in my application the redis connection calls become **~9-10 times** slower.

Let me know if I am missing anything.

Thanks in advance for any leads :pray:

kaylareopelle commented 3 months ago

Hi @praveen-ks, I'm excited to hear that prepending helped! It's troubling to hear about the performance decline.

redis-client, a dependency of redis-cluster-client has a structure to register middleware which is encouraged over prepending for instrumentation on Redis 6+.

Could you try swapping the prepend call with the following, and let me know how it goes? RedisClient.register(NewRelic::Agent::Instrumentation::RedisClient::Middleware)

praveen-ks commented 3 months ago

Hi @praveen-ks, I'm excited to hear that prepending helped! It's troubling to hear about the performance decline.

redis-client, a dependency of redis-cluster-client has a structure to register middleware which is encouraged over prepending for instrumentation on Redis 6+.

Could you try swapping the prepend call with the following, and let me know how it goes? RedisClient.register(NewRelic::Agent::Instrumentation::RedisClient::Middleware)

I am using latest version of both redis and redis-clustering and this code is already taking care of registering middleware for RedisClient.

That's why I don't feel this can help or am I missing something here?

kaylareopelle commented 3 months ago

Ah, I see. I didn't realize you had redis gem installed in addition to the redis-clustering gem.

If it was just the redis-clustering gem, the Redis instrumentation may not have installed on agent startup, preventing the middleware from being registered.

I'll take another look at the issue and see if I have something else to recommend.

praveen-ks commented 3 months ago

Ah, I see. I didn't realize you had redis gem installed in addition to the redis-clustering gem.

If it was just the redis-clustering gem, the Redis instrumentation may not have installed on agent startup, preventing the middleware from being registered.

I'll take another look at the issue and see if I have something else to recommend.

Thanks @kaylareopelle

It will be really helpful if I can get some workaround for time being.

kaylareopelle commented 3 days ago

Hi @praveen-ks! I've had a chance to look a little more closely at this issue. I believe part of the slowdown may have been caused by errors silently being raised while trying to capture the db name.

I have a branch, redis-clustering, that fixes this issue and also adds instrumentation for users of this gem for the call and connect methods via the RedisClient middleware reigstration. This should bring the instrumentation experience much closer to what you're familiar with on earlier versions of Redis.

If you'd like to test out the functionality, you can do so by updating the way the newrelic_rpm gem is installed to use the branch:

gem 'newrelic_rpm', github: 'newrelic/newrelic-ruby-agent', branch: 'redis-clustering'