Merge pull request juju#13097 from achilleasa/2.8-logsink-error-if-pe…

…rsisting-logs-to-db-fails

juju#13097

This improves visibility in cases such as LP1930899 where
underprovisioned mongodb instance prevented log entries from being
persisted with no additional error output.

As the DB error was returned back to the caller (log sender worker), the
worker would keep restarting thus preventing models from logging
anything.

While this commit does not fix the underlying problem (underprovisioned
mongo) it should at least make the cause of the problem more obvious to
users that are trying to diagnose similar issues by inspecting the
logsink.log file contents.

## Bug reference

https://bugs.launchpad.net/juju/+bug/1930899

(NOTE: the PR does not fix ^ but helps diagnose the root cause of the issue)

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),

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")
+}