Merge pull request #974 from stevvooe/context-cleanup

context: WithVersion and context package cleanup

context/doc.go

@@ -3,6 +3,19 @@
 // logging relevent request information but this package is not limited to
 // that purpose.
 //
+// The easiest way to get started is to get the background context:
+//
+// 	ctx := context.Background()
+//
+// The returned context should be passed around your application and be the
+// root of all other context instances. If the application has a version, this
+// line should be called before anything else:
+//
+// 	ctx := context.WithVersion(context.Background(), version)
+//
+// The above will store the version in the context and will be available to
+// the logger.
+//
 // Logging
 //
 // The most useful aspect of this package is GetLogger. This function takes

context/logger.go

@@ -54,8 +54,14 @@ func GetLoggerWithField(ctx Context, key, value interface{}, keys ...interface{}
 // GetLoggerWithFields returns a logger instance with the specified fields
 // without affecting the context. Extra specified keys will be resolved from
 // the context.
-func GetLoggerWithFields(ctx Context, fields map[string]interface{}, keys ...interface{}) Logger {
-	return getLogrusLogger(ctx, keys...).WithFields(logrus.Fields(fields))
+func GetLoggerWithFields(ctx Context, fields map[interface{}]interface{}, keys ...interface{}) Logger {
+	// must convert from interface{} -> interface{} to string -> interface{} for logrus.
+	lfields := make(logrus.Fields, len(fields))
+	for key, value := range fields {
+		lfields[fmt.Sprint(key)] = value
+	}
+
+	return getLogrusLogger(ctx, keys...).WithFields(lfields)
 }
 
 // GetLogger returns the logger from the current context, if present. If one
@@ -84,12 +90,19 @@ func getLogrusLogger(ctx Context, keys ...interface{}) *logrus.Entry {
 	}
 
 	if logger == nil {
+		fields := logrus.Fields{}
+
+		// Fill in the instance id, if we have it.
+		instanceID := ctx.Value("instance.id")
+		if instanceID != nil {
+			fields["instance.id"] = instanceID
+		}
+
 		// If no logger is found, just return the standard logger.
-		logger = logrus.NewEntry(logrus.StandardLogger())
+		logger = logrus.StandardLogger().WithFields(fields)
 	}
 
 	fields := logrus.Fields{}
-
 	for _, key := range keys {
 		v := ctx.Value(key)
 		if v != nil {

context/util.go

@@ -20,7 +20,7 @@ func Since(ctx Context, key interface{}) time.Duration {
 
 // GetStringValue returns a string value from the context. The empty string
 // will be returned if not found.
-func GetStringValue(ctx Context, key string) (value string) {
+func GetStringValue(ctx Context, key interface{}) (value string) {
 	stringi := ctx.Value(key)
 	if stringi != nil {
 		if valuev, ok := stringi.(string); ok {

context/version.go

@@ -0,0 +1,16 @@
+package context
+
+// WithVersion stores the application version in the context. The new context
+// gets a logger to ensure log messages are marked with the application
+// version.
+func WithVersion(ctx Context, version string) Context {
+	ctx = WithValue(ctx, "version", version)
+	// push a new logger onto the stack
+	return WithLogger(ctx, GetLogger(ctx, "version"))
+}
+
+// GetVersion returns the application version from the context. An empty
+// string may returned if the version was not set on the context.
+func GetVersion(ctx Context) string {
+	return GetStringValue(ctx, "version")
+}

context/version_test.go

@@ -0,0 +1,19 @@
+package context
+
+import "testing"
+
+func TestVersionContext(t *testing.T) {
+	ctx := Background()
+
+	if GetVersion(ctx) != "" {
+		t.Fatalf("context should not yet have a version")
+	}
+
+	expected := "2.1-whatever"
+	ctx = WithVersion(ctx, expected)
+	version := GetVersion(ctx)
+
+	if version != expected {
+		t.Fatalf("version was not set: %q != %q", version, expected)
+	}
+}

registry/handlers/app.go

@@ -77,8 +77,6 @@ func NewApp(ctx context.Context, configuration *configuration.Configuration) *Ap
 		isCache: configuration.Proxy.RemoteURL != "",
 	}
 
-	app.Context = ctxu.WithLogger(app.Context, ctxu.GetLogger(app, "instance.id"))
-
 	// Register the handler dispatchers.
 	app.register(v2.RouteNameBase, func(ctx *Context, r *http.Request) http.Handler {
 		return http.HandlerFunc(apiBase)

registry/registry.go

@@ -35,6 +35,9 @@ var Cmd = &cobra.Command{
 			return
 		}
 
+		// setup context
+		ctx := context.WithVersion(context.Background(), version.Version)
+
 		config, err := resolveConfiguration(args)
 		if err != nil {
 			fmt.Fprintf(os.Stderr, "configuration error: %v\n", err)
@@ -51,7 +54,7 @@ var Cmd = &cobra.Command{
 			}(config.HTTP.Debug.Addr)
 		}
 
-		registry, err := NewRegistry(context.Background(), config)
+		registry, err := NewRegistry(ctx, config)
 		if err != nil {
 			log.Fatalln(err)
 		}
@@ -78,9 +81,6 @@ type Registry struct {
 
 // NewRegistry creates a new registry from a context and configuration struct.
 func NewRegistry(ctx context.Context, config *configuration.Configuration) (*Registry, error) {
-	// Note this
-	ctx = context.WithValue(ctx, "version", version.Version)
-
 	var err error
 	ctx, err = configureLogging(ctx, config)
 	if err != nil {
@@ -218,7 +218,7 @@ func configureLogging(ctx context.Context, config *configuration.Configuration)
 	if config.Log.Level == "" && config.Log.Formatter == "" {
 		// If no config for logging is set, fallback to deprecated "Loglevel".
 		log.SetLevel(logLevel(config.Loglevel))
-		ctx = context.WithLogger(ctx, context.GetLogger(ctx, "version"))
+		ctx = context.WithLogger(ctx, context.GetLogger(ctx))
 		return ctx, nil
 	}
 
@@ -253,9 +253,6 @@ func configureLogging(ctx context.Context, config *configuration.Configuration)
 		log.Debugf("using %q logging formatter", config.Log.Formatter)
 	}
 
-	// log the application version with messages
-	ctx = context.WithLogger(ctx, context.GetLogger(ctx, "version"))
-
 	if len(config.Log.Fields) > 0 {
 		// build up the static fields, if present.
 		var fields []interface{}

registry/storage/blobwriter.go

@@ -241,7 +241,7 @@ func (bw *blobWriter) validateBlob(ctx context.Context, desc distribution.Descri
 
 	if !verified {
 		context.GetLoggerWithFields(ctx,
-			map[string]interface{}{
+			map[interface{}]interface{}{
 				"canonical": canonical,
 				"provided":  desc.Digest,
 			}, "canonical", "provided").