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

contrib/go-redis/redis.v8: support wrapping a go-redis v8 client #803

Closed
seancaffery opened this issue Jan 4, 2021 · 2 comments · Fixed by #808
Closed

contrib/go-redis/redis.v8: support wrapping a go-redis v8 client #803

seancaffery opened this issue Jan 4, 2021 · 2 comments · Fixed by #808
Labels
proposal more in depth change that requires full team approval

Comments

@seancaffery
Copy link
Contributor

seancaffery commented Jan 4, 2021

The current redis-go v8 integration only supports a regular Redis client. I'd like to add support for adding a hook to an existing UniversalClient so I can add a tracing hook to a FailOverClient.

The API would be similar to the one provided for the previous redis-go integration. Something like:

func WrapClient(c redis.UniversalClient, opts ...ClientOption) redis.UniversalClient {
  // set up client
  // add hook
  // return client
}

We can likely share much of the code from NewClient.

An alternative could be to change NewClient to accept redis.UniversalOptions but that would be a breaking change and I imagine would be more difficult to roll out.

@seancaffery seancaffery changed the title contrib/go-redis/redis.v8 contrib/go-redis/redis.v8: support wrapping a go-redis v8 client Jan 4, 2021
@knusbaum
Copy link
Contributor

knusbaum commented Jan 4, 2021

👍 This looks like something we want to support.

We should be careful to consider any differences (things worth making into tags, etc.) between the UniversalClient compared to the regular Client.

@seancaffery
Copy link
Contributor Author

I've created #808. It'd be great to get your thoughts on how we should handle tags for each kind of client.

@knusbaum knusbaum added the proposal more in depth change that requires full team approval label Jan 8, 2021
knusbaum pushed a commit that referenced this issue Jan 13, 2021
Introduce a new function, WrapClient, in the go-redis.v8 integration. This
is similar to the older go-redis integration in allowing a user to supply
an already configured Redis client and have the tracing added to that
rather than having the library build and return a client.

Fixes #803
dianashevchenko pushed a commit that referenced this issue Feb 8, 2021
Introduce a new function, WrapClient, in the go-redis.v8 integration. This
is similar to the older go-redis integration in allowing a user to supply
an already configured Redis client and have the tracing added to that
rather than having the library build and return a client.

Fixes #803
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal more in depth change that requires full team approval
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants