From 5cab90d2702ef11e84ba801284a33062da7c41f7 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Wed, 17 Jan 2018 15:15:25 -0800 Subject: [PATCH] log: remove log "module" system After comtemplation, the complexity of the logging module system outweighs its usefulness. This changeset removes the system and restores lighter weight code paths. As a concession, we can always provide more context when necessary to log messages to understand them without having to fork the context for a certain set of calls. Signed-off-by: Stephen J Day --- cmd/containerd/main.go | 8 +++---- log/context.go | 41 --------------------------------- log/context_test.go | 23 ------------------- plugin/context.go | 3 +-- server/server.go | 52 +----------------------------------------- 5 files changed, 6 insertions(+), 121 deletions(-) diff --git a/cmd/containerd/main.go b/cmd/containerd/main.go index d9836be9d..4ddd7363b 100644 --- a/cmd/containerd/main.go +++ b/cmd/containerd/main.go @@ -79,7 +79,7 @@ func main() { start = time.Now() signals = make(chan os.Signal, 2048) serverC = make(chan *server.Server, 1) - ctx = log.WithModule(gocontext.Background(), "containerd") + ctx = gocontext.Background() config = defaultConfig() ) @@ -114,21 +114,21 @@ func main() { if err != nil { return errors.Wrapf(err, "failed to get listener for debug endpoint") } - serve(log.WithModule(ctx, "debug"), l, server.ServeDebug) + serve(ctx, l, server.ServeDebug) } if config.Metrics.Address != "" { l, err := net.Listen("tcp", config.Metrics.Address) if err != nil { return errors.Wrapf(err, "failed to get listener for metrics endpoint") } - serve(log.WithModule(ctx, "metrics"), l, server.ServeMetrics) + serve(ctx, l, server.ServeMetrics) } l, err := sys.GetLocalListener(address, config.GRPC.UID, config.GRPC.GID) if err != nil { return errors.Wrapf(err, "failed to get listener for main endpoint") } - serve(log.WithModule(ctx, "grpc"), l, server.ServeGRPC) + serve(ctx, l, server.ServeGRPC) log.G(ctx).Infof("containerd successfully booted in %fs", time.Since(start).Seconds()) <-done diff --git a/log/context.go b/log/context.go index 1081719c1..471c35270 100644 --- a/log/context.go +++ b/log/context.go @@ -2,7 +2,6 @@ package log import ( "context" - "path" "github.com/sirupsen/logrus" ) @@ -20,7 +19,6 @@ var ( type ( loggerKey struct{} - moduleKey struct{} ) // WithLogger returns a new context with the provided logger. Use in @@ -40,42 +38,3 @@ func GetLogger(ctx context.Context) *logrus.Entry { return logger.(*logrus.Entry) } - -// WithModule adds the module to the context, appending it with a slash if a -// module already exists. A module is just an roughly correlated defined by the -// call tree for a given context. -// -// As an example, we might have a "node" module already part of a context. If -// this function is called with "tls", the new value of module will be -// "node/tls". -// -// Modules represent the call path. If the new module and last module are the -// same, a new module entry will not be created. If the new module and old -// older module are the same but separated by other modules, the cycle will be -// represented by the module path. -func WithModule(ctx context.Context, module string) context.Context { - parent := GetModulePath(ctx) - - if parent != "" { - // don't re-append module when module is the same. - if path.Base(parent) == module { - return ctx - } - - module = path.Join(parent, module) - } - - ctx = WithLogger(ctx, GetLogger(ctx).WithField("module", module)) - return context.WithValue(ctx, moduleKey{}, module) -} - -// GetModulePath returns the module path for the provided context. If no module -// is set, an empty string is returned. -func GetModulePath(ctx context.Context) string { - module := ctx.Value(moduleKey{}) - if module == nil { - return "" - } - - return module.(string) -} diff --git a/log/context_test.go b/log/context_test.go index 6c59874c0..88c25175c 100644 --- a/log/context_test.go +++ b/log/context_test.go @@ -16,26 +16,3 @@ func TestLoggerContext(t *testing.T) { assert.Equal(t, GetLogger(ctx).Data["test"], "one") assert.Equal(t, G(ctx), GetLogger(ctx)) // these should be the same. } - -func TestModuleContext(t *testing.T) { - ctx := context.Background() - assert.Equal(t, GetModulePath(ctx), "") - - ctx = WithModule(ctx, "a") // basic behavior - assert.Equal(t, GetModulePath(ctx), "a") - logger := GetLogger(ctx) - assert.Equal(t, logger.Data["module"], "a") - - parent, ctx := ctx, WithModule(ctx, "a") - assert.Equal(t, ctx, parent) // should be a no-op - assert.Equal(t, GetModulePath(ctx), "a") - assert.Equal(t, GetLogger(ctx).Data["module"], "a") - - ctx = WithModule(ctx, "b") // new module - assert.Equal(t, GetModulePath(ctx), "a/b") - assert.Equal(t, GetLogger(ctx).Data["module"], "a/b") - - ctx = WithModule(ctx, "c") // new module - assert.Equal(t, GetModulePath(ctx), "a/b/c") - assert.Equal(t, GetLogger(ctx).Data["module"], "a/b/c") -} diff --git a/plugin/context.go b/plugin/context.go index 87e53b84f..eca4a0204 100644 --- a/plugin/context.go +++ b/plugin/context.go @@ -6,7 +6,6 @@ import ( "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/events/exchange" - "github.com/containerd/containerd/log" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" ) @@ -28,7 +27,7 @@ type InitContext struct { // NewContext returns a new plugin InitContext func NewContext(ctx context.Context, r *Registration, plugins *Set, root, state string) *InitContext { return &InitContext{ - Context: log.WithModule(ctx, r.URI()), + Context: ctx, Root: filepath.Join(root, r.URI()), State: filepath.Join(state, r.URI()), Meta: &Meta{ diff --git a/server/server.go b/server/server.go index 6053a1bdd..1b94a9be3 100644 --- a/server/server.go +++ b/server/server.go @@ -10,17 +10,6 @@ import ( "strings" "github.com/boltdb/bolt" - containers "github.com/containerd/containerd/api/services/containers/v1" - contentapi "github.com/containerd/containerd/api/services/content/v1" - diff "github.com/containerd/containerd/api/services/diff/v1" - eventsapi "github.com/containerd/containerd/api/services/events/v1" - images "github.com/containerd/containerd/api/services/images/v1" - introspection "github.com/containerd/containerd/api/services/introspection/v1" - leasesapi "github.com/containerd/containerd/api/services/leases/v1" - namespaces "github.com/containerd/containerd/api/services/namespaces/v1" - snapshotsapi "github.com/containerd/containerd/api/services/snapshots/v1" - tasks "github.com/containerd/containerd/api/services/tasks/v1" - version "github.com/containerd/containerd/api/services/version/v1" "github.com/containerd/containerd/content" "github.com/containerd/containerd/content/local" "github.com/containerd/containerd/events/exchange" @@ -34,7 +23,6 @@ import ( "golang.org/x/net/context" "google.golang.org/grpc" - "google.golang.org/grpc/health/grpc_health_v1" ) // New creates and initializes a new containerd server @@ -62,7 +50,7 @@ func New(ctx context.Context, config *Config) (*Server, error) { return nil, err } rpc := grpc.NewServer( - grpc.UnaryInterceptor(interceptor), + grpc.UnaryInterceptor(grpc_prometheus.UnaryServerInterceptor), grpc.StreamInterceptor(grpc_prometheus.StreamServerInterceptor), ) var ( @@ -235,44 +223,6 @@ func loadPlugins(config *Config) ([]*plugin.Registration, error) { return plugin.Graph(), nil } -func interceptor( - ctx context.Context, - req interface{}, - info *grpc.UnaryServerInfo, - handler grpc.UnaryHandler, -) (interface{}, error) { - ctx = log.WithModule(ctx, "containerd") - switch info.Server.(type) { - case tasks.TasksServer: - ctx = log.WithModule(ctx, "tasks") - case containers.ContainersServer: - ctx = log.WithModule(ctx, "containers") - case contentapi.ContentServer: - ctx = log.WithModule(ctx, "content") - case images.ImagesServer: - ctx = log.WithModule(ctx, "images") - case grpc_health_v1.HealthServer: - // No need to change the context - case version.VersionServer: - ctx = log.WithModule(ctx, "version") - case snapshotsapi.SnapshotsServer: - ctx = log.WithModule(ctx, "snapshot") - case diff.DiffServer: - ctx = log.WithModule(ctx, "diff") - case namespaces.NamespacesServer: - ctx = log.WithModule(ctx, "namespaces") - case eventsapi.EventsServer: - ctx = log.WithModule(ctx, "events") - case introspection.IntrospectionServer: - ctx = log.WithModule(ctx, "introspection") - case leasesapi.LeasesServer: - ctx = log.WithModule(ctx, "leases") - default: - log.G(ctx).Warnf("unknown GRPC server type: %#v\n", info.Server) - } - return grpc_prometheus.UnaryServerInterceptor(ctx, req, info, handler) -} - func trapClosedConnErr(err error) error { if err == nil { return nil