From 360c24d97561814561e5fcc9ca351b642b40cb9f Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 10 Sep 2015 20:40:01 -0700 Subject: [PATCH 1/2] Allow interface{} keys when using logger Signed-off-by: Stephen J Day --- context/logger.go | 11 ++++++++--- context/util.go | 2 +- registry/storage/blobwriter.go | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/context/logger.go b/context/logger.go index 78e4212a0..8c4c6f6c1 100644 --- a/context/logger.go +++ b/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 @@ -89,7 +95,6 @@ func getLogrusLogger(ctx Context, keys ...interface{}) *logrus.Entry { } fields := logrus.Fields{} - for _, key := range keys { v := ctx.Value(key) if v != nil { diff --git a/context/util.go b/context/util.go index c0aff00d2..299edc004 100644 --- a/context/util.go +++ b/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 { diff --git a/registry/storage/blobwriter.go b/registry/storage/blobwriter.go index e0e7239c0..b384fa8a0 100644 --- a/registry/storage/blobwriter.go +++ b/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"). From 530afa5234f1924af4e10d879ae40c51916e0b2d Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 10 Sep 2015 20:41:58 -0700 Subject: [PATCH 2/2] Add WithVersion to context and other cleanup By adding WithVersion to the context package, we can simplify context setup in the application. This avoids some odd bugs where instantiation order can lead to missing instance.id or version from log messages. Signed-off-by: Stephen J Day --- context/doc.go | 13 +++++++++++++ context/logger.go | 10 +++++++++- context/version.go | 16 ++++++++++++++++ context/version_test.go | 19 +++++++++++++++++++ registry/handlers/app.go | 2 -- registry/registry.go | 13 +++++-------- 6 files changed, 62 insertions(+), 11 deletions(-) create mode 100644 context/version.go create mode 100644 context/version_test.go diff --git a/context/doc.go b/context/doc.go index a63989e54..6fe1f817d 100644 --- a/context/doc.go +++ b/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 diff --git a/context/logger.go b/context/logger.go index 8c4c6f6c1..7c5e51872 100644 --- a/context/logger.go +++ b/context/logger.go @@ -90,8 +90,16 @@ 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{} diff --git a/context/version.go b/context/version.go new file mode 100644 index 000000000..746cda02e --- /dev/null +++ b/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") +} diff --git a/context/version_test.go b/context/version_test.go new file mode 100644 index 000000000..b81652691 --- /dev/null +++ b/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) + } +} diff --git a/registry/handlers/app.go b/registry/handlers/app.go index 8c67c20b8..5103c5fbe 100644 --- a/registry/handlers/app.go +++ b/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) diff --git a/registry/registry.go b/registry/registry.go index cb0c87654..86cb6a173 100644 --- a/registry/registry.go +++ b/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{}