Add support for retrieving logger from context.Context

[wjam]

It's a common pattern to have a logger saved into the context.Context so that various pieces of information can be added to it - trace ID, user ID, etc. The problem is that the library doesn't support passing the context into the logger.

[fantapop]

This is also something I'm interested in. There is a similar request against the stripe-go logger with some implementation ideas here: stripe/stripe-go#1281

[kayvonkhosrowpour]

We are also very interested in this. I created a zaplogger interface wrapper to meet the LeveledLogger interface that the HTTP client expects (see below). But unfortunately, I can't pass in the context that contains a lot of useful information (X-Correlation-Id, for example).

It would be great for each Error(), Info(), ... function call to have a context object called as well. I imagine the context.Context could be passed in to the Do() call in the HTTP Client.

// ZapLeveledLogger wrapper for zap.logger for use with hashicorp's go-retryable LeveledLogger
type ZapLeveledLogger struct {
	Logger *zap.Logger
}

// New creates a ZapLeveledLogger with a zap.logger that satisfies standard library log.Logger interface.
func New(logger *zap.Logger) retryablehttp.LeveledLogger {
	return &ZapLeveledLogger{Logger: logger}
}

func (l *ZapLeveledLogger) Error(msg string, keysAndValues ...interface{}) {
	l.Logger.Error(msg, keysAndValuesToFields(keysAndValues)...)
}

func (l *ZapLeveledLogger) Info(msg string, keysAndValues ...interface{}) {
	l.Logger.Info(msg, keysAndValuesToFields(keysAndValues)...)
}

func (l *ZapLeveledLogger) Debug(msg string, keysAndValues ...interface{}) {
	l.Logger.Debug(msg, keysAndValuesToFields(keysAndValues)...)
}

func (l *ZapLeveledLogger) Warn(msg string, keysAndValues ...interface{}) {
	l.Logger.Warn(msg, keysAndValuesToFields(keysAndValues)...)
}

func keysAndValuesToFields(keysAndValues []interface{}) []zapcore.Field {
	fields := []zapcore.Field{}

	// we've already failed if we have an odd number of arguments
	failed := len(keysAndValues)%2 != 0

	for i := 0; (i < len(keysAndValues)-1) && !failed; {
		// get the next key safely, to check if it's a string
		key, hasKey := keysAndValues[i].(string)
		// check that there is an associated value
		hasValue := (i + 1) < len(keysAndValues)

		// if there is a key and value, include it
		if hasKey && hasValue {
			fields = append(fields, zap.Any(key, keysAndValues[i+1]))
			i += 2
		} else {
			// give up, the key-value pairs are malformed.
			// we can't know what's a key and what's a value, so just log everything.
			failed = true
			break
		}
	}

	if failed {
		fields = []zapcore.Field{
			zap.Any("leveled_logger_fields", keysAndValues),
		}
	}

	return fields
}

[kayvonkhosrowpour]

@marianoasselborn @ryanuber @claire-labry
Apologies if I'm tagging the wrong individuals (I took it from recent code commits/largest contributions), but I cannot find guidance on making requests for feature enhancements. Is there a process for gaining support on RFEs?

[jcass8695]

Hi all, adding my 0.02c, my team would really like to see this added to the library.

Some different possible implementation ideas:

• Add a ContextLeveledLogger interface, more or less mirroring LeveledLogger but with a context.Context argument. Add ContextLeveledLogger to the existing type-checks (e.g. here)

type ContextLeveledLogger interface {
	Error(ctx context.Context, msg string, keysAndValues ...interface{})
	Info(ctx context.Context, msg string, keysAndValues ...interface{})
	Debug(ctx context.Context, msg string, keysAndValues ...interface{})
	Warn(ctx context.Context, msg string, keysAndValues ...interface{})
}

• Add a field to Client, a function that accepts a context.Context argument and returns a slice of log key-values. This function could be called in the existing locations where Client.Logger is called. Somethin' like below:

type Client struct {
        HTTPClient *http.Client
	Logger     interface{}
        LoggerKeyValuesFromCtx func(context.Context) []string
        ...
}

...

func (c *Client) Do(req *Request) (*http.Response, error) {
	...

	logger := c.logger()
	if logger != nil {
		switch v := logger.(type) {
		case LeveledLogger:
			v.Debug("performing request", "method", req.Method, "url", req.URL, c.LoggerKeyValuesFromCtx(req.Ctx())...)
		case Logger:
			v.Printf("[DEBUG] %s %s", req.Method, req.URL, c.LoggerKeyValuesFromCtx(req.Ctx())...)
		}
	}
        ...
}

[jcass8695]

I have some code ready to go on a branch, but obviously don't have permission to push the branch. Should I push to a fork?

[tscales]

@jcass8695 you'll need to push to a fork, then you can open a pull request for this repository.

[whrss9527]

#231