From 530afa5234f1924af4e10d879ae40c51916e0b2d Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 10 Sep 2015 20:41:58 -0700 Subject: [PATCH] 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{}