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 context version Do func? #310

Closed
yancl opened this issue Jan 4, 2018 · 4 comments
Closed

Add context version Do func? #310

yancl opened this issue Jan 4, 2018 · 4 comments

Comments

@yancl
Copy link

yancl commented Jan 4, 2018

Sometimes we need to add tracing(opentracing, for example) to redis call. but the current version only supply Do function which does not have a context.Context parameter, so is it reasonable to add a DoContext function to redigo? just like the sql version here, thanks.

@garyburd
Copy link
Member

garyburd commented Jan 4, 2018

Please give a description of the problem you are trying to solve in your application and how Redigo connections will solve that problem when given a context.

@yancl
Copy link
Author

yancl commented Jan 4, 2018

OK. Suppose we have an component which has a Query method. The Query method calls redis. But we want to know what the cmd and args passed to redis of some spec trace-id. so we may do it like this:

package main

import (
	"context"
	"fmt"
	"github.com/garyburd/redigo/redis"
)

type RedisConn struct {
	redis.Conn
}

func (c *RedisConn) DoContext(ctx context.Context, cmd string, args ...interface{}) (r interface{}, err error) {
	traceId := ctx.Value("trace-id")
	fmt.Printf("trace-id:%s, cmd:%s, args:%v", traceId, cmd, args)
	//return c.Conn.DoContext(ctx, cmd, args...)
	return c.Conn.Do(cmd, args...)
}

func DialRedis(target string, opts ...redis.DialOption) (*RedisConn, error) {
	c, err := redis.Dial("tcp", target, opts...)
	if err != nil {
		return nil, err
	}
	return &RedisConn{Conn: c}, nil
}

func Query(ctx context.Context, keyword string) (string, error) {
	traceId := ctx.Value("trace-id")
        // just to show we can log the things related to one spec tace-id
	fmt.Printf("trace-id:%s", traceId)
        // we don't use connpool here for demo
	c, err := DialRedis("localhost:6389")
	if err != nil {
		return "", err
	}
	return redis.String(c.DoContext(ctx, "GET", keyword))
}

func main() {
	key := "key"
	ctx := context.WithValue(context.Background(), "trace-id", "0123456789")
	if val, err := Query(ctx, key); err != nil {
		fmt.Println(err)
	} else {
		fmt.Printf("key:%s, val:%s\n", key, val)
	}
}

As you can see from above, with context we can log the whole call chain with some spec trace-id. Also RedisConn is redis.Conn, so callers can use RedisConn as redis.Conn without code break:)

@garyburd
Copy link
Member

garyburd commented Jan 4, 2018

The application works with no change to Redigo.

Adding a method to redis.Conn is a breaking change.

You can hide RedisConn from the application by following the pattern in NewLoggingConn.

func newTracingConn(ctx context.Context, c redis.Conn) redis.Conn { }

Wrap a plain connection with the tracing connection wherever you need tracing. If you use a pool, then add the wrapper in a Get method wrapper:

type myPool struct {
    redis.Pool
}

func (p *myPool) GetContext(ctx context.Context) redis.Conn {
    return newTracingConn(ctx, p.Pool.Get())
}

@yancl
Copy link
Author

yancl commented Jan 5, 2018

Thanks, Got it:)

@yancl yancl closed this as completed Jan 5, 2018
@gomodule gomodule locked and limited conversation to collaborators Aug 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants