We have a bunch of information passed around in context.Context (such as request id) that we want included in all logs coming from that context's scope. However, at present that context does not get passed to the aws.Logger instance, since there's no API parameter for it. This means that the debug logs that are emitted do not include this additional information.
I proposed deprecated the existing aws.Logger interface and replacing it with:
package aws
...
type ContextLogger interface {
LogWithContext(ctx context.Context, v ...interface{})
}
With regards to aws.Config, there are two choices:
Logger field remains as-is. For backwards compatibility, if the provided aws.Logger does not also implement aws.ContextLogger, then Log will be called (without the context as a parameter).Logger field is deprecated and replaced with a new ContextLogger field of type aws.ContextLogger. At runtime, use ContextLogger if set, otherwise use Logger if set.ETA: From looking at the SDK, it would also be handy if the log method were replaced with level-specific methods.
package aws
...
type ContextLogger interface {
LogDebug(ctx context.Context, v ...interface{})
LogInfo(ctx context.Context, v ...interface{})
LogWarning(ctx context.Context, v ...interface{})
LogError(ctx context.Context, v ...interface{})
}
Hi @rittneje, thanks for reaching out to us about this. You can define a custom logger to use with a service client and pass the context for a call via the logger's Log() method to print the context details in the same location as the rest of the debug logs:
myLogger := aws.LoggerFunc(func(ctx context.Context, args ...interface{}) {
fmt.Fprintln(os.Stdout, args...)
})
sess := session.New(&aws.Config{
Region: aws.String("us-west-2"),
LogLevel: aws.LogLevel(aws.LogDebugWithSigning),
Logger: myLogger,
})
svc := s3.New(sess)
ctx := context.WithValue(context.Background(), "foo", "bar")
myLogger.Log("Context:", ctx)
If this does not work for your use case please provide more details about your use case so we can better understand the need for the feature you're requesting.
@diehlaws No, that will not work. The context is a per-request concept.
For example, suppose I am using the dynamodb package.
type logger struct{}
func (logger) LogWithContext(ctx context.Context, v ...interface{}) {
// include request id in the log, for ease of traceability/searching
fmt.Println(ctx.Value("request-id"), fmt.Sprint(v...))
}
config := &aws.Config{
Logger: logger{},
...,
}
dbClient := dynamodb.New(config)
...
func handleRequest(rw http.ResponseWriter, req *http.Request) {
...
ctx := context.WithValue(req.Context(), "request-id", req.Header.Get("X-Request-ID"))
dbClient.GetItemWithContext(ctx, ...)
...
}
The context passed to GetItemWithContext needs to be passed all the way through to the logger (for those logs made by the call to GetItemWithContext). However, since the Logger.Log method does not take a context.Context parameter, there is no way to do this without making a separate *dynamodb.DynamoDB instance for each request, which does not really make sense.
@diehlaws Just following up on this.
@diehlaws I raised #3485 with the necessary changes. This patch is invaluable for troubleshooting, as the existing aws.Logger interface is unusable in any multi-threaded environment.
To put my two cents in, it goes further than a multi-threaded environment. It's about how to add contextual information to each log message. The introduction of context in the SDK has been a giant leap forward, but it has been used mostly for cancellation purposes. Though, there is more to contexts. In a distributed environment, we need contextual information (request id, correlation ids, trace ids, etc.) to make sense of running services. The natural place of this kind of transient information is in the context and the lack of the context in the logger interface prevents the enrichment of log messages emitted by the SDK.