From a70287d3248c22cb70042e825806790113a634d5 Mon Sep 17 00:00:00 2001
From: Anton Tolchanov <commits@knyar.net>
Date: Sat, 28 Sep 2024 13:31:00 +0200
Subject: [PATCH] logpolicy: don't create a filch buffer if logging is disabled

Updates #9549

Signed-off-by: Anton Tolchanov <commits@knyar.net>
---
 logpolicy/logpolicy.go | 76 ++++++++++++++++++++++++------------------
 1 file changed, 44 insertions(+), 32 deletions(-)

diff --git a/logpolicy/logpolicy.go b/logpolicy/logpolicy.go
index 9e00a3ad4..ff0976cb2 100644
--- a/logpolicy/logpolicy.go
+++ b/logpolicy/logpolicy.go
@@ -576,37 +576,18 @@ func NewWithConfigPath(collection, dir, cmdName string, netMon *netmon.Monitor,
 	if envknob.NoLogsNoSupport() || testenv.InTest() {
 		logf("You have disabled logging. Tailscale will not be able to provide support.")
 		conf.HTTPC = &http.Client{Transport: noopPretendSuccessTransport{}}
-	} else if val := getLogTarget(); val != "" {
-		logf("You have enabled a non-default log target. Doing without being told to by Tailscale staff or your network administrator will make getting support difficult.")
-		conf.BaseURL = val
-		u, _ := url.Parse(val)
-		conf.HTTPC = &http.Client{Transport: NewLogtailTransport(u.Host, netMon, health, logf)}
-	}
-
-	filchOptions := filch.Options{
-		ReplaceStderr: redirectStderrToLogPanics(),
-	}
-	filchPrefix := filepath.Join(dir, cmdName)
-
-	// NAS disks cannot hibernate if we're writing logs to them all the time.
-	// https://github.com/tailscale/tailscale/issues/3551
-	if runtime.GOOS == "linux" && (distro.Get() == distro.Synology || distro.Get() == distro.QNAP) {
-		tmpfsLogs := "/tmp/tailscale-logs"
-		if err := os.MkdirAll(tmpfsLogs, 0755); err == nil {
-			filchPrefix = filepath.Join(tmpfsLogs, cmdName)
-			filchOptions.MaxFileSize = 1 << 20
-		} else {
-			// not a fatal error, we can leave the log files on the spinning disk
-			logf("Unable to create /tmp directory for log storage: %v\n", err)
+	} else {
+		// Only attach an on-disk filch buffer if we are going to be sending logs.
+		// No reason to persist them locally just to drop them later.
+		attachFilchBuffer(&conf, dir, cmdName, logf)
+
+		if val := getLogTarget(); val != "" {
+			logf("You have enabled a non-default log target. Doing without being told to by Tailscale staff or your network administrator will make getting support difficult.")
+			conf.BaseURL = val
+			u, _ := url.Parse(val)
+			conf.HTTPC = &http.Client{Transport: NewLogtailTransport(u.Host, netMon, health, logf)}
 		}
-	}
 
-	filchBuf, filchErr := filch.New(filchPrefix, filchOptions)
-	if filchBuf != nil {
-		conf.Buffer = filchBuf
-		if filchBuf.OrigStderr != nil {
-			conf.Stderr = filchBuf.OrigStderr
-		}
 	}
 	lw := logtail.NewLogger(conf, logf)
 
@@ -631,9 +612,6 @@ func NewWithConfigPath(collection, dir, cmdName string, netMon *netmon.Monitor,
 		goVersion(),
 		os.Args)
 	logf("LogID: %v", newc.PublicID)
-	if filchErr != nil {
-		logf("filch failed: %v", filchErr)
-	}
 	if earlyErrBuf.Len() != 0 {
 		logf("%s", earlyErrBuf.Bytes())
 	}
@@ -645,6 +623,40 @@ func NewWithConfigPath(collection, dir, cmdName string, netMon *netmon.Monitor,
 	}
 }
 
+// attachFilchBuffer creates an on-disk ring buffer using filch and attaches
+// it to the logtail config. Note that this is optional; if no buffer is set,
+// logtail will use an in-memory buffer.
+func attachFilchBuffer(conf *logtail.Config, dir, cmdName string, logf logger.Logf) {
+	filchOptions := filch.Options{
+		ReplaceStderr: redirectStderrToLogPanics(),
+	}
+	filchPrefix := filepath.Join(dir, cmdName)
+
+	// NAS disks cannot hibernate if we're writing logs to them all the time.
+	// https://github.com/tailscale/tailscale/issues/3551
+	if runtime.GOOS == "linux" && (distro.Get() == distro.Synology || distro.Get() == distro.QNAP) {
+		tmpfsLogs := "/tmp/tailscale-logs"
+		if err := os.MkdirAll(tmpfsLogs, 0755); err == nil {
+			filchPrefix = filepath.Join(tmpfsLogs, cmdName)
+			filchOptions.MaxFileSize = 1 << 20
+		} else {
+			// not a fatal error, we can leave the log files on the spinning disk
+			logf("Unable to create /tmp directory for log storage: %v\n", err)
+		}
+	}
+
+	filchBuf, filchErr := filch.New(filchPrefix, filchOptions)
+	if filchBuf != nil {
+		conf.Buffer = filchBuf
+		if filchBuf.OrigStderr != nil {
+			conf.Stderr = filchBuf.OrigStderr
+		}
+	}
+	if filchErr != nil {
+		logf("filch failed: %v", filchErr)
+	}
+}
+
 // dialLog is used by NewLogtailTransport to log the happy path of its
 // own dialing.
 //