Repository: vitessio/vitess
Pull request #6511 (2020-07-31 00:22:09)

Convert syslog events to normal log events
if the syslog daemon cannot be contacted.

Signed-off-by: Jacques Grove <aquarapid@gmail.com>


Patch:
diff --git a/go/event/syslogger/syslogger.go b/go/event/syslogger/syslogger.go
index 2c26fee083..1c8ff22136 100644
--- a/go/event/syslogger/syslogger.go
+++ b/go/event/syslogger/syslogger.go
@@ -80,11 +80,6 @@ type syslogWriter interface {
 var writer syslogWriter
 
 func listener(ev Syslogger) {
-	if writer == nil {
-		log.Errorf("no connection, dropping syslog event: %#v", ev)
-		return
-	}
-
 	// Ask the event to convert itself to a syslog message.
 	sev, msg := ev.Syslog()
 
@@ -92,21 +87,53 @@ func listener(ev Syslogger) {
 	var err error
 	switch sev {
 	case syslog.LOG_EMERG:
-		err = writer.Emerg(msg)
+		if writer != nil {
+			err = writer.Emerg(msg)
+		} else {
+			log.Errorf(msg)
+		}
 	case syslog.LOG_ALERT:
-		err = writer.Alert(msg)
+		if writer != nil {
+			err = writer.Alert(msg)
+		} else {
+			log.Errorf(msg)
+		}
 	case syslog.LOG_CRIT:
-		err = writer.Crit(msg)
+		if writer != nil {
+			err = writer.Crit(msg)
+		} else {
+			log.Errorf(msg)
+		}
 	case syslog.LOG_ERR:
-		err = writer.Err(msg)
+		if writer != nil {
+			err = writer.Err(msg)
+		} else {
+			log.Errorf(msg)
+		}
 	case syslog.LOG_WARNING:
-		err = writer.Warning(msg)
+		if writer != nil {
+			err = writer.Warning(msg)
+		} else {
+			log.Warningf(msg)
+		}
 	case syslog.LOG_NOTICE:
-		err = writer.Notice(msg)
+		if writer != nil {
+			err = writer.Notice(msg)
+		} else {
+			log.Infof(msg)
+		}
 	case syslog.LOG_INFO:
-		err = writer.Info(msg)
+		if writer != nil {
+			err = writer.Info(msg)
+		} else {
+			log.Infof(msg)
+		}
 	case syslog.LOG_DEBUG:
-		err = writer.Debug(msg)
+		if writer != nil {
+			err = writer.Debug(msg)
+		} else {
+			log.Infof(msg)
+		}
 	default:
 		err = fmt.Errorf("invalid syslog severity: %v", sev)
 	}


Test patch:
diff --git a/go/event/syslogger/syslogger_test.go b/go/event/syslogger/syslogger_test.go
index 111335d41f..44176decbb 100644
--- a/go/event/syslogger/syslogger_test.go
+++ b/go/event/syslogger/syslogger_test.go
@@ -19,9 +19,11 @@ package syslogger
 import (
 	"fmt"
 	"log/syslog"
+	"strings"
 	"testing"
 
 	"vitess.io/vitess/go/event"
+	"vitess.io/vitess/go/vt/log"
 )
 
 type TestEvent struct {
@@ -61,6 +63,60 @@ func (fw *fakeWriter) Info(msg string) error    { return fw.write(syslog.LOG_INF
 func (fw *fakeWriter) Notice(msg string) error  { return fw.write(syslog.LOG_NOTICE, msg) }
 func (fw *fakeWriter) Warning(msg string) error { return fw.write(syslog.LOG_WARNING, msg) }
 
+type loggerMsg struct {
+	msg   string
+	level string
+}
+type testLogger struct {
+	logs          []loggerMsg
+	savedInfof    func(format string, args ...interface{})
+	savedWarningf func(format string, args ...interface{})
+	savedErrorf   func(format string, args ...interface{})
+}
+
+func newTestLogger() *testLogger {
+	tl := &testLogger{
+		savedInfof:    log.Infof,
+		savedWarningf: log.Warningf,
+		savedErrorf:   log.Errorf,
+	}
+	log.Infof = tl.recordInfof
+	log.Warningf = tl.recordWarningf
+	log.Errorf = tl.recordErrorf
+	return tl
+}
+
+func (tl *testLogger) Close() {
+	log.Infof = tl.savedInfof
+	log.Warningf = tl.savedWarningf
+	log.Errorf = tl.savedErrorf
+}
+
+func (tl *testLogger) recordInfof(format string, args ...interface{}) {
+	msg := fmt.Sprintf(format, args...)
+	tl.logs = append(tl.logs, loggerMsg{msg, "INFO"})
+	tl.savedInfof(msg)
+}
+
+func (tl *testLogger) recordWarningf(format string, args ...interface{}) {
+	msg := fmt.Sprintf(format, args...)
+	tl.logs = append(tl.logs, loggerMsg{msg, "WARNING"})
+	tl.savedWarningf(msg)
+}
+
+func (tl *testLogger) recordErrorf(format string, args ...interface{}) {
+	msg := fmt.Sprintf(format, args...)
+	tl.logs = append(tl.logs, loggerMsg{msg, "ERROR"})
+	tl.savedErrorf(msg)
+}
+
+func (tl *testLogger) getLog() loggerMsg {
+	if len(tl.logs) > 0 {
+		return tl.logs[len(tl.logs)-1]
+	}
+	return loggerMsg{"no logs!", "ERROR"}
+}
+
 // TestSyslog checks that our callback works.
 func TestSyslog(t *testing.T) {
 	writer = &fakeWriter{}
@@ -73,14 +129,59 @@ func TestSyslog(t *testing.T) {
 	}
 }
 
-// TestBadWriter checks that we don't panic when the connection fails.
+// TestBadWriter verifies we are still triggering (to normal logs) if
+// the syslog connection failed
 func TestBadWriter(t *testing.T) {
+	tl := newTestLogger()
+	defer tl.Close()
+
 	writer = nil
+	wantMsg := "testing message"
+	wantLevel := "ERROR"
+	ev := &TestEvent{priority: syslog.LOG_ALERT, message: wantMsg}
+	event.Dispatch(ev)
+	if !strings.Contains(tl.getLog().msg, wantMsg) {
+		t.Errorf("error log msg [%s], want msg [%s]", tl.getLog().msg, wantMsg)
+	}
+	if !strings.Contains(tl.getLog().level, wantLevel) {
+		t.Errorf("error log level [%s], want level [%s]", tl.getLog().level, wantLevel)
+	}
+	ev = &TestEvent{priority: syslog.LOG_CRIT, message: wantMsg}
+	event.Dispatch(ev)
+	if !strings.Contains(tl.getLog().level, wantLevel) {
+		t.Errorf("error log level [%s], want level [%s]", tl.getLog().level, wantLevel)
+	}
+	ev = &TestEvent{priority: syslog.LOG_ERR, message: wantMsg}
+	event.Dispatch(ev)
+	if !strings.Contains(tl.getLog().level, wantLevel) {
+		t.Errorf("error log level [%s], want level [%s]", tl.getLog().level, wantLevel)
+	}
 
-	ev := new(TestEvent)
+	wantLevel = "WARNING"
+	ev = &TestEvent{priority: syslog.LOG_WARNING, message: wantMsg}
 	event.Dispatch(ev)
+	if !strings.Contains(tl.getLog().level, wantLevel) {
+		t.Errorf("error log level [%s], want level [%s]", tl.getLog().level, wantLevel)
+	}
 
-	if ev.triggered {
+	wantLevel = "INFO"
+	ev = &TestEvent{priority: syslog.LOG_INFO, message: wantMsg}
+	event.Dispatch(ev)
+	if !strings.Contains(tl.getLog().level, wantLevel) {
+		t.Errorf("error log level [%s], want level [%s]", tl.getLog().level, wantLevel)
+	}
+	ev = &TestEvent{priority: syslog.LOG_NOTICE, message: wantMsg}
+	event.Dispatch(ev)
+	if !strings.Contains(tl.getLog().level, wantLevel) {
+		t.Errorf("error log level [%s], want level [%s]", tl.getLog().level, wantLevel)
+	}
+	ev = &TestEvent{priority: syslog.LOG_DEBUG, message: wantMsg}
+	event.Dispatch(ev)
+	if !strings.Contains(tl.getLog().level, wantLevel) {
+		t.Errorf("error log level [%s], want level [%s]", tl.getLog().level, wantLevel)
+	}
+
+	if !ev.triggered {
 		t.Errorf("passed nil writer to client")
 	}
 }