From 9982c5dcb28e161f3e8f985e6314432390ede176 Mon Sep 17 00:00:00 2001
From: Achilleas Anagnostopoulos <achilleas.anagnostopoulos@canonical.com>
Date: Tue, 22 Jun 2021 13:37:43 +0100
Subject: [PATCH] Ensure all DB errors when inserting logs are always logged to
 logsink

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.
---
 apiserver/logsink.go               | 13 ++++++++
 apiserver/logsink_internal_test.go | 48 ++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 apiserver/logsink_internal_test.go

diff --git a/apiserver/logsink.go b/apiserver/logsink.go
index a76f3b61f8d..b99c5a83736 100644
--- 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
new file mode 100644
index 00000000000..5b8e9f3543c
--- /dev/null
+++ 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")
+}