Skip to content

Commit

Permalink
Updates for new logging to appease govet
Browse files Browse the repository at this point in the history 
Govet doesn't like functions that look like format handlers not ending in `f`,
such as Debug() vs Debugf(). This is changing some of the new log handling to
use 'f' function names to appease govet.

Updated the implicit handling of including the client as an arg without being
used in a format string. Now the client object simply has a Errorf function
for logging errors and it adds itself onto the format string.
  • Loading branch information
@krobertson
krobertson committed Oct 29, 2014
1 parent bc84847 commit 5623b58
Show file tree
Hide file tree
Showing 11 changed files with 103 additions and 108 deletions.
12 changes: 6 additions & 6 deletions logger/log.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func NewFileLogger(filename string, time, debug, trace bool) *Logger {
fileflags := os.O_WRONLY | os.O_APPEND | os.O_CREATE
f, err := os.OpenFile(filename, fileflags, 0660)
if err != nil {
log.Fatal("error opening file: %v", err)
log.Fatalf("error opening file: %v", err)
}

flags := 0
Expand Down Expand Up @@ -78,25 +78,25 @@ func setColoredLabelFormats(l *Logger) {
l.traceLabel = fmt.Sprintf(colorFormat, 33, "TRACE")
}

func (l *Logger) Notice(format string, v ...interface{}) {
func (l *Logger) Noticef(format string, v ...interface{}) {
l.logger.Printf(l.infoLabel+format, v...)
}

func (l *Logger) Error(format string, v ...interface{}) {
func (l *Logger) Errorf(format string, v ...interface{}) {
l.logger.Printf(l.errorLabel+format, v...)
}

func (l *Logger) Fatal(format string, v ...interface{}) {
func (l *Logger) Fatalf(format string, v ...interface{}) {
l.logger.Fatalf(l.fatalLabel+format, v)
}

func (l *Logger) Debug(format string, v ...interface{}) {
func (l *Logger) Debugf(format string, v ...interface{}) {
if l.debug == true {
l.logger.Printf(l.debugLabel+format, v...)
}
}

func (l *Logger) Trace(format string, v ...interface{}) {
func (l *Logger) Tracef(format string, v ...interface{}) {
if l.trace == true {
l.logger.Printf(l.traceLabel+format, v...)
}
Expand Down
14 changes: 7 additions & 7 deletions logger/log_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,42 +46,42 @@ func TestStdLoggerWithDebugTraceAndTime(t *testing.T) {
func TestStdLoggerNotice(t *testing.T) {
expectOutput(t, func() {
logger := NewStdLogger(false, false, false, false)
logger.Notice("foo")
logger.Noticef("foo")
}, "[INFO] foo\n")
}

func TestStdLoggerNoticeWithColor(t *testing.T) {
expectOutput(t, func() {
logger := NewStdLogger(false, false, false, true)
logger.Notice("foo")
logger.Noticef("foo")
}, "[\x1b[32mINFO\x1b[0m] foo\n")
}

func TestStdLoggerDebug(t *testing.T) {
expectOutput(t, func() {
logger := NewStdLogger(false, true, false, false)
logger.Debug("foo %s", "bar")
logger.Debugf("foo %s", "bar")
}, "[DEBUG] foo bar\n")
}

func TestStdLoggerDebugWithOutDebug(t *testing.T) {
expectOutput(t, func() {
logger := NewStdLogger(false, false, false, false)
logger.Debug("foo")
logger.Debugf("foo")
}, "")
}

func TestStdLoggerTrace(t *testing.T) {
expectOutput(t, func() {
logger := NewStdLogger(false, false, true, false)
logger.Trace("foo")
logger.Tracef("foo")
}, "[TRACE] foo\n")
}

func TestStdLoggerTraceWithOutDebug(t *testing.T) {
expectOutput(t, func() {
logger := NewStdLogger(false, false, false, false)
logger.Trace("foo")
logger.Tracef("foo")
}, "")
}

Expand All @@ -96,7 +96,7 @@ func TestFileLogger(t *testing.T) {
file.Close()

logger := NewFileLogger(file.Name(), false, false, false)
logger.Notice("foo")
logger.Noticef("foo")

buf, err := ioutil.ReadFile(file.Name())
if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions logger/syslog.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,25 +59,25 @@ func getNetworkAndAddr(fqn string) (network, addr string) {
return
}

func (l *SysLogger) Notice(format string, v ...interface{}) {
func (l *SysLogger) Noticef(format string, v ...interface{}) {
l.writer.Notice(fmt.Sprintf(format, v...))
}

func (l *SysLogger) Fatal(format string, v ...interface{}) {
func (l *SysLogger) Fatalf(format string, v ...interface{}) {
l.writer.Crit(fmt.Sprintf(format, v...))
}

func (l *SysLogger) Error(format string, v ...interface{}) {
func (l *SysLogger) Errorf(format string, v ...interface{}) {
l.writer.Err(fmt.Sprintf(format, v...))
}

func (l *SysLogger) Debug(format string, v ...interface{}) {
func (l *SysLogger) Debugf(format string, v ...interface{}) {
if l.debug {
l.writer.Debug(fmt.Sprintf(format, v...))
}
}

func (l *SysLogger) Trace(format string, v ...interface{}) {
func (l *SysLogger) Tracef(format string, v ...interface{}) {
if l.trace {
l.writer.Notice(fmt.Sprintf(format, v...))
}
Expand Down
10 changes: 5 additions & 5 deletions logger/syslog_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestRemoteSysLoggerNotice(t *testing.T) {
startServer(done)
logger := NewRemoteSysLogger(serverFQN, true, true)

logger.Notice("foo %s", "bar")
logger.Noticef("foo %s", "bar")
expectSyslogOutput(t, <-done, "foo bar\n")
}

Expand All @@ -63,7 +63,7 @@ func TestRemoteSysLoggerDebug(t *testing.T) {
startServer(done)
logger := NewRemoteSysLogger(serverFQN, true, true)

logger.Debug("foo %s", "qux")
logger.Debugf("foo %s", "qux")
expectSyslogOutput(t, <-done, "foo qux\n")
}

Expand All @@ -72,7 +72,7 @@ func TestRemoteSysLoggerDebugDisabled(t *testing.T) {
startServer(done)
logger := NewRemoteSysLogger(serverFQN, false, false)

logger.Debug("foo %s", "qux")
logger.Debugf("foo %s", "qux")
rcvd := <-done
if rcvd != "" {
t.Fatalf("Unexpected syslog response %s\n", rcvd)
Expand All @@ -84,7 +84,7 @@ func TestRemoteSysLoggerTrace(t *testing.T) {
startServer(done)
logger := NewRemoteSysLogger(serverFQN, true, true)

logger.Trace("foo %s", "qux")
logger.Tracef("foo %s", "qux")
expectSyslogOutput(t, <-done, "foo qux\n")
}

Expand All @@ -93,7 +93,7 @@ func TestRemoteSysLoggerTraceDisabled(t *testing.T) {
startServer(done)
logger := NewRemoteSysLogger(serverFQN, true, false)

logger.Trace("foo %s", "qux")
logger.Tracef("foo %s", "qux")
rcvd := <-done
if rcvd != "" {
t.Fatalf("Unexpected syslog response %s\n", rcvd)
Expand Down
43 changes: 24 additions & 19 deletions server/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func (c *client) readLoop() {
return
}
if err := c.parse(b[:n]); err != nil {
Error("Error reading from client: %s", err.Error(), c)
c.Errorf("Error reading from client: %s", err.Error())
// Auth was handled inline
if err != ErrAuthorization {
c.sendErr("Parser Error")
Expand All @@ -161,7 +161,7 @@ func (c *client) readLoop() {
err := cp.bw.Flush()
cp.nc.SetWriteDeadline(time.Time{})
if err != nil {
Debug("Error flushing: %v", err)
Debugf("Error flushing: %v", err)
cp.mu.Unlock()
cp.closeConnection()
cp.mu.Lock()
Expand All @@ -184,7 +184,7 @@ func (c *client) traceMsg(msg []byte) {

pm := fmt.Sprintf("Processing %s msg: %d", c.typeString(), c.inMsgs)
opa := []interface{}{pm, string(c.pa.subject), string(c.pa.reply), string(msg[:len(msg)-LEN_CR_LF])}
Trace("MSG: %s", opa, c)
Tracef("MSG: %s", opa, c)
}

func (c *client) traceOp(op string, arg []byte) {
Expand All @@ -196,7 +196,7 @@ func (c *client) traceOp(op string, arg []byte) {
if arg != nil {
opa = append(opa, fmt.Sprintf("%s %s", op, string(arg)))
}
Trace("OP: %s", opa, c)
Tracef("OP: %s", opa, c)
}

// Process the info message if we are a route.
Expand All @@ -214,11 +214,11 @@ func (c *client) processRouteInfo(info *Info) {
c.mu.Unlock()

if s.addRoute(c) {
Debug("Registering remote route %q", info.ID, c)
Debugf("Registering remote route %q", info.ID, c)
// Send our local subscriptions to this route.
s.sendLocalSubsToRoute(c)
} else {
Debug("Detected duplicate remote route %q", info.ID, c)
Debugf("Detected duplicate remote route %q", info.ID, c)
c.closeConnection()
}
}
Expand All @@ -236,7 +236,7 @@ func (c *client) processInfo(arg []byte) error {
}

func (c *client) processErr(errStr string) {
Error("Client error %s", errStr, c)
c.Errorf("Client error %s", errStr)
c.closeConnection()
}

Expand Down Expand Up @@ -306,7 +306,7 @@ func (c *client) processPing() {
err := c.bw.Flush()
if err != nil {
c.clearConnection()
Debug("Error on Flush, error %s", err.Error(), c)
Debugf("Error on Flush, error %s", err.Error(), c)
}
c.mu.Unlock()
}
Expand Down Expand Up @@ -483,7 +483,7 @@ func (c *client) unsubscribe(sub *subscription) {
c.mu.Lock()
defer c.mu.Unlock()
if sub.max > 0 && sub.nm < sub.max {
Debug(
Debugf(
"Deferring actual UNSUB(%s): %d max, %d received\n",
string(sub.subject), sub.max, sub.nm, c,
)
Expand Down Expand Up @@ -628,10 +628,10 @@ writeErr:
client.mu.Unlock()

if ne, ok := err.(net.Error); ok && ne.Timeout() {
Notice("Slow Consumer Detected", c)
Noticef("Slow Consumer Detected", c)
client.closeConnection()
} else {
Debug("Error writing msg: %v", err, c)
Debugf("Error writing msg: %v", err, c)
}
}

Expand Down Expand Up @@ -739,11 +739,11 @@ func (c *client) processMsg(msg []byte) {
}
if sub.client == nil || sub.client.nc == nil || sub.client.route == nil ||
sub.client.route.remoteID == "" {
Debug("Bad or Missing ROUTER Identity, not processing msg", c)
Debugf("Bad or Missing ROUTER Identity, not processing msg", c)
continue
}
if _, ok := rmap[sub.client.route.remoteID]; ok {
Debug("Ignoring route, already processed", c)
Debugf("Ignoring route, already processed", c)
continue
}
rmap[sub.client.route.remoteID] = routeSeen
Expand Down Expand Up @@ -772,12 +772,12 @@ func (c *client) processPingTimer() {
return
}

Debug("Client Ping Timer", c)
Debugf("Client Ping Timer", c)

// Check for violation
c.pout += 1
if c.pout > c.srv.opts.MaxPingsOut {
Debug("Stale Client Connection - Closing", c)
Debugf("Stale Client Connection - Closing", c)
if c.bw != nil {
c.bw.WriteString(fmt.Sprintf("-ERR '%s'\r\n", "Stale Connection"))
c.bw.Flush()
Expand All @@ -790,7 +790,7 @@ func (c *client) processPingTimer() {
c.bw.WriteString("PING\r\n")
err := c.bw.Flush()
if err != nil {
Debug("Error on Client Ping Flush, error %s", err)
Debugf("Error on Client Ping Flush, error %s", err)
c.clearConnection()
} else {
// Reset to fire again if all OK.
Expand Down Expand Up @@ -862,7 +862,7 @@ func (c *client) closeConnection() {
return
}

Debug("%s connection closed", c.typeString(), c)
Debugf("%s connection closed", c.typeString(), c)

c.clearAuthTimer()
c.clearPingTimer()
Expand Down Expand Up @@ -899,11 +899,16 @@ func (c *client) closeConnection() {
defer srv.mu.Unlock()
rid := c.route.remoteID
if rid != "" && srv.remotes[rid] != nil {
Debug("Not attempting reconnect for solicited route, already connected. Try %d", rid, c)
Debugf("Not attempting reconnect for solicited route, already connected. Try %d", rid, c)
return
} else {
Debug("Attempting reconnect for solicited route", c)
Debugf("Attempting reconnect for solicited route", c)
go srv.reConnectToRoute(c.route.url)
}
}
}

func (c *client) Errorf(format string, v ...interface{}) {
format = fmt.Sprintf("%s - %s", c, format)
Errorf(format, v...)
}
Loading

0 comments on commit 5623b58

Please sign in to comment.