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

Add support for cluster client in go-redis client tracing library. #3004

Open
sfc-gh-jlai opened this issue Dec 3, 2024 · 2 comments
Open
Assignees
Labels
ack waiting-for-info waiting for answer from issue creator

Comments

@sfc-gh-jlai
Copy link

The official go-redis client exposes a ClusterClient which is exposed via NewClusterClient. However, this tracing library only exposes the UniversalClient:

func NewClient(opt *redis.Options, opts ...ClientOption) redis.UniversalClient {

This is problematic, as the implementation of the UniversalClient is entirely dependent on the length of addresses sent under the Addrs key of UniversalClient:

https://github.com/redis/go-redis/blob/e63669e1706936ac794277340c51a51c5facca70/universal.go#L243-L250

However, when using AWS ElastiCache for Redis, their implementation only exposes a single "configuration endpoint":

Once your cluster is “available,” it is ready for use. You can connect to the cluster using the Configuration Endpoint listed in the AWS Management Console.

This single address is used to connect into the cluster. Therefore, the go-redis UniversalClient will assume single-node mode, and fail to connect to the cluster. Therefore, when using AWS ElastiCache for Redis and go-redis, the ClusterClient must be manually specified.

Therefore, this request is to expose ClusterClient in this tracing pacakge.

@sfc-gh-jlai sfc-gh-jlai added the enhancement quick change/addition that does not need full team approval label Dec 3, 2024
@github-actions github-actions bot added the needs-triage New issues that have not yet been triaged label Dec 3, 2024
@darccio
Copy link
Member

darccio commented Dec 9, 2024

@DataDog/apm-idm-go For your consideration.

@darccio darccio removed the needs-triage New issues that have not yet been triaged label Dec 9, 2024
@rarguelloF
Copy link
Contributor

Hello @sfc-gh-jlai

The redis contrib package also exposes a WrapClient function you can use with your redis.ClusterClient:

clusterClient := redis.NewClusterClient(&redis.ClusterOptions{Addrs: []string{":7000", ":7001", ":7002", ":7003", ":7004", ":7005"}})
redistrace.WrapClient(clusterClient)

Please let me know if this solves your issue.

@rarguelloF rarguelloF self-assigned this Dec 11, 2024
@rarguelloF rarguelloF added waiting-for-info waiting for answer from issue creator ack labels Dec 11, 2024
@rarguelloF rarguelloF removed the enhancement quick change/addition that does not need full team approval label Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack waiting-for-info waiting for answer from issue creator
Projects
None yet
Development

No branches or pull requests

3 participants