From ccbe539e86dfbb8749c09763ddfd73bf10ac57cc Mon Sep 17 00:00:00 2001 From: Morgan Bauer <mbauer@us.ibm.com> Date: Tue, 21 Jul 2015 15:26:52 -0700 Subject: [PATCH] golint fixes for daemon/logger/* - downcase and privatize exported variables that were unused - make accurate an error message - added package comments - remove unused var ReadLogsNotSupported - enable linter - some spelling corrections Signed-off-by: Morgan Bauer <mbauer@us.ibm.com> --- daemon/logger/factory.go | 19 +++++++++---- daemon/logger/fluentd/fluentd.go | 20 +++++++++----- daemon/logger/gelf/gelf.go | 35 ++++++++++++++---------- daemon/logger/journald/journald.go | 15 ++++++---- daemon/logger/jsonfilelog/jsonfilelog.go | 28 ++++++++++++------- daemon/logger/logger.go | 32 ++++++++++++++-------- daemon/logger/syslog/syslog.go | 18 ++++++++---- hack/make/validate-lint | 6 ++++ 8 files changed, 113 insertions(+), 60 deletions(-) diff --git a/daemon/logger/factory.go b/daemon/logger/factory.go index f40655ae8b..ffd03ea469 100644 --- a/daemon/logger/factory.go +++ b/daemon/logger/factory.go @@ -8,13 +8,14 @@ import ( "time" ) -// Creator is a method that builds a logging driver instance with given context +// Creator builds a logging driver instance with given context. type Creator func(Context) (Logger, error) -//LogOptValidator is a method that validates the log opts provided +// LogOptValidator checks the options specific to the underlying +// logging implementation. type LogOptValidator func(cfg map[string]string) error -// Context provides enough information for a logging driver to do its function +// Context provides enough information for a logging driver to do its function. type Context struct { Config map[string]string ContainerID string @@ -27,7 +28,7 @@ type Context struct { LogPath string } -// Hostname returns the hostname from the underlying OS +// Hostname returns the hostname from the underlying OS. func (ctx *Context) Hostname() (string, error) { hostname, err := os.Hostname() if err != nil { @@ -36,7 +37,9 @@ func (ctx *Context) Hostname() (string, error) { return hostname, nil } -// Command returns the command that the container being logged was started with +// Command returns the command that the container being logged was +// started with. The Entrypoint is prepended to the container +// arguments. func (ctx *Context) Command() string { terms := []string{ctx.ContainerEntrypoint} for _, arg := range ctx.ContainerArgs { @@ -68,7 +71,7 @@ func (lf *logdriverFactory) registerLogOptValidator(name string, l LogOptValidat defer lf.m.Unlock() if _, ok := lf.optValidator[name]; ok { - return fmt.Errorf("logger: log driver named '%s' is already registered", name) + return fmt.Errorf("logger: log validator named '%s' is already registered", name) } lf.optValidator[name] = l return nil @@ -101,6 +104,8 @@ func RegisterLogDriver(name string, c Creator) error { return factory.register(name, c) } +// RegisterLogOptValidator registers the logging option validator with +// the given logging driver name. func RegisterLogOptValidator(name string, l LogOptValidator) error { return factory.registerLogOptValidator(name, l) } @@ -110,6 +115,8 @@ func GetLogDriver(name string) (Creator, error) { return factory.get(name) } +// ValidateLogOpts checks the options for the given log driver. The +// options supported are specific to the LogDriver implementation. func ValidateLogOpts(name string, cfg map[string]string) error { l := factory.getLogOptValidator(name) if l != nil { diff --git a/daemon/logger/fluentd/fluentd.go b/daemon/logger/fluentd/fluentd.go index 97205ddee2..d9b94a4ddd 100644 --- a/daemon/logger/fluentd/fluentd.go +++ b/daemon/logger/fluentd/fluentd.go @@ -1,3 +1,5 @@ +// Package fluentd provides the log driver for forwarding server logs +// to fluentd endpoints. package fluentd import ( @@ -14,14 +16,14 @@ import ( "github.com/fluent/fluent-logger-golang/fluent" ) -type Fluentd struct { +type fluentd struct { tag string containerID string containerName string writer *fluent.Fluent } -type Receiver struct { +type receiver struct { ID string FullID string Name string @@ -67,7 +69,7 @@ func parseConfig(ctx logger.Context) (string, int, string, error) { } if config["fluentd-tag"] != "" { - receiver := &Receiver{ + receiver := &receiver{ ID: ctx.ContainerID[:12], FullID: ctx.ContainerID, Name: ctx.ContainerName, @@ -86,6 +88,9 @@ func parseConfig(ctx logger.Context) (string, int, string, error) { return host, port, tag, nil } +// New creates a fluentd logger using the configuration passed in on +// the context. Supported context configuration variables are +// fluentd-address & fluentd-tag. func New(ctx logger.Context) (logger.Logger, error) { host, port, tag, err := parseConfig(ctx) if err != nil { @@ -99,7 +104,7 @@ func New(ctx logger.Context) (logger.Logger, error) { if err != nil { return nil, err } - return &Fluentd{ + return &fluentd{ tag: tag, containerID: ctx.ContainerID, containerName: ctx.ContainerName, @@ -107,7 +112,7 @@ func New(ctx logger.Context) (logger.Logger, error) { }, nil } -func (f *Fluentd) Log(msg *logger.Message) error { +func (f *fluentd) Log(msg *logger.Message) error { data := map[string]string{ "container_id": f.containerID, "container_name": f.containerName, @@ -119,6 +124,7 @@ func (f *Fluentd) Log(msg *logger.Message) error { return f.writer.PostWithTime(f.tag, msg.Timestamp, data) } +// ValidateLogOpt looks for fluentd specific log options fluentd-address & fluentd-tag. func ValidateLogOpt(cfg map[string]string) error { for key := range cfg { switch key { @@ -131,10 +137,10 @@ func ValidateLogOpt(cfg map[string]string) error { return nil } -func (f *Fluentd) Close() error { +func (f *fluentd) Close() error { return f.writer.Close() } -func (f *Fluentd) Name() string { +func (f *fluentd) Name() string { return name } diff --git a/daemon/logger/gelf/gelf.go b/daemon/logger/gelf/gelf.go index 22c734bc45..49fd3ff5a9 100644 --- a/daemon/logger/gelf/gelf.go +++ b/daemon/logger/gelf/gelf.go @@ -1,5 +1,7 @@ // +build linux +// Package gelf provides the log driver for forwarding server logs to +// endpoints that support the Graylog Extended Log Format. package gelf import ( @@ -17,17 +19,17 @@ import ( const name = "gelf" -type GelfLogger struct { +type gelfLogger struct { writer *gelf.Writer ctx logger.Context - fields GelfFields + fields gelfFields } -type GelfFields struct { +type gelfFields struct { hostname string - containerId string + containerID string containerName string - imageId string + imageID string imageName string command string tag string @@ -43,6 +45,9 @@ func init() { } } +// New creates a gelf logger using the configuration passed in on the +// context. Supported context configuration variables are +// gelf-address, & gelf-tag. func New(ctx logger.Context) (logger.Logger, error) { // parse gelf address address, err := parseAddress(ctx.Config["gelf-address"]) @@ -59,11 +64,11 @@ func New(ctx logger.Context) (logger.Logger, error) { // remove trailing slash from container name containerName := bytes.TrimLeft([]byte(ctx.ContainerName), "/") - fields := GelfFields{ + fields := gelfFields{ hostname: hostname, - containerId: ctx.ContainerID, + containerID: ctx.ContainerID, containerName: string(containerName), - imageId: ctx.ContainerImageID, + imageID: ctx.ContainerImageID, imageName: ctx.ContainerImageName, command: ctx.Command(), tag: ctx.Config["gelf-tag"], @@ -76,14 +81,14 @@ func New(ctx logger.Context) (logger.Logger, error) { return nil, fmt.Errorf("gelf: cannot connect to GELF endpoint: %s %v", address, err) } - return &GelfLogger{ + return &gelfLogger{ writer: gelfWriter, ctx: ctx, fields: fields, }, nil } -func (s *GelfLogger) Log(msg *logger.Message) error { +func (s *gelfLogger) Log(msg *logger.Message) error { // remove trailing and leading whitespace short := bytes.TrimSpace([]byte(msg.Line)) @@ -99,9 +104,9 @@ func (s *GelfLogger) Log(msg *logger.Message) error { TimeUnix: float64(msg.Timestamp.UnixNano()/int64(time.Millisecond)) / 1000.0, Level: level, Extra: map[string]interface{}{ - "_container_id": s.fields.containerId, + "_container_id": s.fields.containerID, "_container_name": s.fields.containerName, - "_image_id": s.fields.imageId, + "_image_id": s.fields.imageID, "_image_name": s.fields.imageName, "_command": s.fields.command, "_tag": s.fields.tag, @@ -115,14 +120,16 @@ func (s *GelfLogger) Log(msg *logger.Message) error { return nil } -func (s *GelfLogger) Close() error { +func (s *gelfLogger) Close() error { return s.writer.Close() } -func (s *GelfLogger) Name() string { +func (s *gelfLogger) Name() string { return name } +// ValidateLogOpt looks for gelf specific log options gelf-address, & +// gelf-tag. func ValidateLogOpt(cfg map[string]string) error { for key := range cfg { switch key { diff --git a/daemon/logger/journald/journald.go b/daemon/logger/journald/journald.go index 4dd88c14f9..85ccc9ca49 100644 --- a/daemon/logger/journald/journald.go +++ b/daemon/logger/journald/journald.go @@ -1,5 +1,7 @@ // +build linux +// Package journald provides the log driver for forwarding server logs +// to endpoints that receive the systemd format. package journald import ( @@ -12,7 +14,7 @@ import ( const name = "journald" -type Journald struct { +type journald struct { Jmap map[string]string } @@ -22,6 +24,9 @@ func init() { } } +// New creates a journald logger using the configuration passed in on +// the context. Supported context configuration variables are +// syslog-address, syslog-facility, & syslog-tag. func New(ctx logger.Context) (logger.Logger, error) { if !journal.Enabled() { return nil, fmt.Errorf("journald is not enabled on this host") @@ -36,20 +41,20 @@ func New(ctx logger.Context) (logger.Logger, error) { "CONTAINER_ID": ctx.ContainerID[:12], "CONTAINER_ID_FULL": ctx.ContainerID, "CONTAINER_NAME": name} - return &Journald{Jmap: jmap}, nil + return &journald{Jmap: jmap}, nil } -func (s *Journald) Log(msg *logger.Message) error { +func (s *journald) Log(msg *logger.Message) error { if msg.Source == "stderr" { return journal.Send(string(msg.Line), journal.PriErr, s.Jmap) } return journal.Send(string(msg.Line), journal.PriInfo, s.Jmap) } -func (s *Journald) Close() error { +func (s *journald) Close() error { return nil } -func (s *Journald) Name() string { +func (s *journald) Name() string { return name } diff --git a/daemon/logger/jsonfilelog/jsonfilelog.go b/daemon/logger/jsonfilelog/jsonfilelog.go index 383aada822..e3fb98dc6b 100644 --- a/daemon/logger/jsonfilelog/jsonfilelog.go +++ b/daemon/logger/jsonfilelog/jsonfilelog.go @@ -1,3 +1,6 @@ +// Package jsonfilelog provides the default Logger implementation for +// Docker logging. This logger logs to files on the host server in the +// JSON format. package jsonfilelog import ( @@ -23,12 +26,12 @@ import ( ) const ( + // Name is the name of the file that the jsonlogger logs to. Name = "json-file" maxJSONDecodeRetry = 10 ) -// JSONFileLogger is Logger implementation for default docker logging: -// JSON objects to file +// JSONFileLogger is Logger implementation for default Docker logging. type JSONFileLogger struct { buf *bytes.Buffer f *os.File // store for closing @@ -49,7 +52,8 @@ func init() { } } -// New creates new JSONFileLogger which writes to filename +// New creates new JSONFileLogger which writes to filename passed in +// on given context. func New(ctx logger.Context) (logger.Logger, error) { log, err := os.OpenFile(ctx.LogPath, os.O_RDWR|os.O_APPEND|os.O_CREATE, 0600) if err != nil { @@ -63,14 +67,14 @@ func New(ctx logger.Context) (logger.Logger, error) { return nil, err } } - var maxFiles int = 1 + var maxFiles = 1 if maxFileString, ok := ctx.Config["max-file"]; ok { maxFiles, err = strconv.Atoi(maxFileString) if err != nil { return nil, err } if maxFiles < 1 { - return nil, fmt.Errorf("max-files cannot be less than 1.") + return nil, fmt.Errorf("max-files cannot be less than 1") } } return &JSONFileLogger{ @@ -84,7 +88,7 @@ func New(ctx logger.Context) (logger.Logger, error) { }, nil } -// Log converts logger.Message to jsonlog.JSONLog and serializes it to file +// Log converts logger.Message to jsonlog.JSONLog and serializes it to file. func (l *JSONFileLogger) Log(msg *logger.Message) error { l.mu.Lock() defer l.mu.Unlock() @@ -153,6 +157,7 @@ func rotate(name string, n int) error { return nil } +// backup renames a file from curr to old, creating an empty file curr if it does not exist. func backup(old, curr string) error { if _, err := os.Stat(old); !os.IsNotExist(err) { err := os.Remove(old) @@ -170,6 +175,7 @@ func backup(old, curr string) error { return os.Rename(curr, old) } +// ValidateLogOpt looks for json specific log options max-file & max-size. func ValidateLogOpt(cfg map[string]string) error { for key := range cfg { switch key { @@ -182,11 +188,12 @@ func ValidateLogOpt(cfg map[string]string) error { return nil } +// LogPath returns the location the given json logger logs to. func (l *JSONFileLogger) LogPath() string { return l.ctx.LogPath } -// Close closes underlying file and signals all readers to stop +// Close closes underlying file and signals all readers to stop. func (l *JSONFileLogger) Close() error { l.mu.Lock() err := l.f.Close() @@ -198,7 +205,7 @@ func (l *JSONFileLogger) Close() error { return err } -// Name returns name of this logger +// Name returns name of this logger. func (l *JSONFileLogger) Name() string { return Name } @@ -216,7 +223,8 @@ func decodeLogLine(dec *json.Decoder, l *jsonlog.JSONLog) (*logger.Message, erro return msg, nil } -// Reads from the log file +// ReadLogs implements the logger's LogReader interface for the logs +// created by this driver. func (l *JSONFileLogger) ReadLogs(config logger.ReadConfig) *logger.LogWatcher { logWatcher := logger.NewLogWatcher() @@ -326,7 +334,7 @@ func followLogs(f *os.File, logWatcher *logger.LogWatcher, notifyRotate chan int // try again because this shouldn't happen if _, ok := err.(*json.SyntaxError); ok && retries <= maxJSONDecodeRetry { dec = json.NewDecoder(f) - retries += 1 + retries++ continue } logWatcher.Err <- err diff --git a/daemon/logger/logger.go b/daemon/logger/logger.go index 96421f4b96..ed245fcc1d 100644 --- a/daemon/logger/logger.go +++ b/daemon/logger/logger.go @@ -1,3 +1,10 @@ +// Package logger defines interfaces that logger drivers implement to +// log messages. +// +// The other half of a logger driver is the implementation of the +// factory, which holds the contextual instance information that +// allows multiple loggers of the same type to perform different +// actions, such as logging to different locations. package logger import ( @@ -7,16 +14,16 @@ import ( "github.com/docker/docker/pkg/timeutils" ) -// ErrReadLogsNotSupported is returned when the logger does not support reading logs +// ErrReadLogsNotSupported is returned when the logger does not support reading logs. var ErrReadLogsNotSupported = errors.New("configured logging reader does not support reading") const ( - // TimeFormat is the time format used for timestamps sent to log readers + // TimeFormat is the time format used for timestamps sent to log readers. TimeFormat = timeutils.RFC3339NanoFixed logWatcherBufferSize = 4096 ) -// Message is datastructure that represents record from some container +// Message is datastructure that represents record from some container. type Message struct { ContainerID string Line []byte @@ -24,31 +31,31 @@ type Message struct { Timestamp time.Time } -// Logger is the interface for docker logging drivers +// Logger is the interface for docker logging drivers. type Logger interface { Log(*Message) error Name() string Close() error } -// ReadConfig is the configuration passed into ReadLogs +// ReadConfig is the configuration passed into ReadLogs. type ReadConfig struct { Since time.Time Tail int Follow bool } -// LogReader is the interface for reading log messages for loggers that support reading +// LogReader is the interface for reading log messages for loggers that support reading. type LogReader interface { // Read logs from underlying logging backend ReadLogs(ReadConfig) *LogWatcher } -// LogWatcher is used when consuming logs read from the LogReader interface +// LogWatcher is used when consuming logs read from the LogReader interface. type LogWatcher struct { - // For sending log messages to a reader + // For sending log messages to a reader. Msg chan *Message - // For sending error messages that occur while while reading logs + // For sending error messages that occur while while reading logs. Err chan error closeNotifier chan struct{} } @@ -62,13 +69,14 @@ func NewLogWatcher() *LogWatcher { } } -// Close notifies the underlying log reader to stop +// Close notifies the underlying log reader to stop. func (w *LogWatcher) Close() { close(w.closeNotifier) } -// WatchClose returns a channel receiver that receives notification when the watcher has been closed -// This should only be called from one goroutine +// WatchClose returns a channel receiver that receives notification +// when the watcher has been closed. This should only be called from +// one goroutine. func (w *LogWatcher) WatchClose() <-chan struct{} { return w.closeNotifier } diff --git a/daemon/logger/syslog/syslog.go b/daemon/logger/syslog/syslog.go index 9c8c321bb4..a402e26193 100644 --- a/daemon/logger/syslog/syslog.go +++ b/daemon/logger/syslog/syslog.go @@ -1,5 +1,6 @@ // +build linux +// Package syslog provides the logdriver for forwarding server logs to syslog endpoints. package syslog import ( @@ -43,7 +44,7 @@ var facilities = map[string]syslog.Priority{ "local7": syslog.LOG_LOCAL7, } -type Syslog struct { +type syslogger struct { writer *syslog.Writer } @@ -56,6 +57,9 @@ func init() { } } +// New creates a syslog logger using the configuration passed in on +// the context. Supported context configuration variables are +// syslog-address, syslog-facility, & syslog-tag. func New(ctx logger.Context) (logger.Logger, error) { tag := ctx.Config["syslog-tag"] if tag == "" { @@ -82,23 +86,23 @@ func New(ctx logger.Context) (logger.Logger, error) { return nil, err } - return &Syslog{ + return &syslogger{ writer: log, }, nil } -func (s *Syslog) Log(msg *logger.Message) error { +func (s *syslogger) Log(msg *logger.Message) error { if msg.Source == "stderr" { return s.writer.Err(string(msg.Line)) } return s.writer.Info(string(msg.Line)) } -func (s *Syslog) Close() error { +func (s *syslogger) Close() error { return s.writer.Close() } -func (s *Syslog) Name() string { +func (s *syslogger) Name() string { return name } @@ -132,12 +136,14 @@ func parseAddress(address string) (string, string, error) { return "", "", nil } +// ValidateLogOpt looks for syslog specific log options +// syslog-address, syslog-facility, & syslog-tag. func ValidateLogOpt(cfg map[string]string) error { for key := range cfg { switch key { case "syslog-address": - case "syslog-tag": case "syslog-facility": + case "syslog-tag": default: return fmt.Errorf("unknown log opt '%s' for syslog log driver", key) } diff --git a/hack/make/validate-lint b/hack/make/validate-lint index 5995af21d0..5caa42387b 100644 --- a/hack/make/validate-lint +++ b/hack/make/validate-lint @@ -22,6 +22,12 @@ packages=( daemon/execdriver/native/template daemon/graphdriver/aufs daemon/graphdriver/devmapper + daemon/logger + daemon/logger/fluentd + daemon/logger/gelf + daemon/logger/journald + daemon/logger/jsonfilelog + daemon/logger/syslog daemon/network docker dockerinit