Merge pull request juju#13098 from achilleasa/2.9-logsink-error-if-persisting-logs-to-db-fails

jujubot · web-flow · commit 51def44d11c5 · 2021-06-23T00:57:20.000+10:00
juju#13098 This is a forward port of juju#13097 to 2.9 ## Bug reference https://bugs.launchpad.net/juju/+bug/1930899 (NOTE: the PR does not fix ^ but helps diagnose the root cause of the issue)
diff --git a/apiserver/logsink.go b/apiserver/logsink.go
@@ -176,6 +176,19 @@ func (s *agentLoggingStrategy) WriteLog(m params.LogRecord) error {
 		Message:  m.Message,
 	}}), "logging to DB failed")
 
+	// If the log entries cannot be inserted to the DB log out an error
+	// to let users know. See LP1930899.
+	if dbErr != nil {
+		// If this fails then the next logToFile will fail as well; no
+		// need to check for errors here.
+		_ = logToFile(s.fileLogger, s.filePrefix, params.LogRecord{
+			Time:    time.Now(),
+			Module:  "juju.apiserver",
+			Level:   loggo.ERROR.String(),
+			Message: errors.Annotate(dbErr, "unable to persist log entry to the database").Error(),
+		})
+	}
+
 	m.Entity = s.entity
 	fileErr := errors.Annotate(
 		logToFile(s.fileLogger, s.filePrefix, m),
diff --git a/apiserver/logsink_internal_test.go b/apiserver/logsink_internal_test.go
@@ -0,0 +1,48 @@
+// Copyright 2021 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package apiserver
+
+import (
+	"bytes"
+	"time"
+
+	"github.com/juju/errors"
+	"github.com/juju/loggo"
+	gc "gopkg.in/check.v1"
+
+	"github.com/juju/juju/apiserver/params"
+	"github.com/juju/juju/state"
+)
+
+type loggingStrategySuite struct {
+	logs loggo.TestWriter
+}
+
+var _ = gc.Suite(&loggingStrategySuite{})
+
+func (s *loggingStrategySuite) TestLoggingOfDBInsertFailures(c *gc.C) {
+	var logBuf bytes.Buffer
+	strategy := &agentLoggingStrategy{
+		dblogger:   failingRecordLogger{},
+		fileLogger: &logBuf,
+	}
+
+	err := strategy.WriteLog(params.LogRecord{
+		Time:    time.Now(),
+		Level:   "WARN",
+		Message: "running low on resources",
+	})
+
+	// The captured DB error should be surfaced from WriteLog
+	c.Assert(err, gc.ErrorMatches, ".*spawn more overlords")
+
+	// Ensure that the DB error was also written to the sink
+	c.Assert(logBuf.String(), gc.Matches, "(?m).*spawn more overlords.*")
+}
+
+type failingRecordLogger struct{}
+
+func (failingRecordLogger) Log([]state.LogRecord) error {
+	return errors.New("spawn more overlords")
+}
