ipn/ipnserver, log/filelogger, logpolicy: when tailscaled is running as a windows service, ensure the service's log messages are written to fileaaron/loglog

This patch moves the Windows-only initialization of the filelogger into logpolicy.
Previously we only did it when babysitting the tailscaled subprocess, but this meant
that log messages from the service itself never made it to disk. This meant that
if logtail could not dial out, its log messages would be lost.

I modified filelogger.New to work a bit differently and added a `maybeWrapForPlatform`
to logpolicy to ensure that the filelogger is plugged in front of logtail ASAP.

Fixes https://github.com/tailscale/tailscale/issues/3570

Signed-off-by: Aaron Klotz <aaron@tailscale.com>

6 files changed, 54 insertions, 20 deletions

diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt
index d0951910a..dd11678a6 100644
--- a/cmd/tailscaled/depaware.txt
+++ b/cmd/tailscaled/depaware.txt
@@ -181,7 +181,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/ipn/policy                                     from tailscale.com/ipn/ipnlocal
         tailscale.com/ipn/store/aws                                  from tailscale.com/ipn/ipnserver
         tailscale.com/kube                                           from tailscale.com/ipn
-        tailscale.com/log/filelogger                                 from tailscale.com/ipn/ipnserver
+   W    tailscale.com/log/filelogger                                 from tailscale.com/logpolicy
         tailscale.com/log/logheap                                    from tailscale.com/control/controlclient
         tailscale.com/logpolicy                                      from tailscale.com/cmd/tailscaled
         tailscale.com/logtail                                        from tailscale.com/logpolicy+
diff --git a/ipn/ipnserver/server.go b/ipn/ipnserver/server.go
index ad16621c3..802a0a0fd 100644
--- a/ipn/ipnserver/server.go
+++ b/ipn/ipnserver/server.go
@@ -35,7 +35,6 @@ import (
 	"tailscale.com/ipn/ipnlocal"
 	"tailscale.com/ipn/localapi"
 	"tailscale.com/ipn/store/aws"
-	"tailscale.com/log/filelogger"
 	"tailscale.com/logtail/backoff"
 	"tailscale.com/net/netstat"
 	"tailscale.com/net/tsdial"
@@ -869,14 +868,6 @@ func BabysitProc(ctx context.Context, args []string, logf logger.Logf) {
 		panic("cannot determine executable: " + err.Error())
 	}
 
-	if runtime.GOOS == "windows" {
-		if len(args) != 2 && args[0] != "/subproc" {
-			panic(fmt.Sprintf("unexpected arguments %q", args))
-		}
-		logID := args[1]
-		logf = filelogger.New("tailscale-service", logID, logf)
-	}
-
 	var proc struct {
 		mu sync.Mutex
 		p  *os.Process
diff --git a/log/filelogger/log.go b/log/filelogger/log.go
index f8c4da65b..ef3ad1e9f 100644
--- a/log/filelogger/log.go
+++ b/log/filelogger/log.go
@@ -9,6 +9,7 @@ package filelogger
 import (
 	"bytes"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"log"
 	"os"
@@ -26,30 +27,30 @@ const (
 	maxFiles = 50
 )
 
-// New returns a logf wrapper that appends to local disk log
+// New returns a Writer that appends to local disk log
 // files on Windows, rotating old log files as needed to stay under
 // file count & byte limits.
-func New(fileBasePrefix, logID string, logf logger.Logf) logger.Logf {
+func New(fileBasePrefix, logID string, inner *log.Logger) io.Writer {
 	if runtime.GOOS != "windows" {
 		panic("not yet supported on any platform except Windows")
 	}
-	if logf == nil {
-		panic("nil logf")
+	if inner == nil {
+		panic("nil inner logger")
 	}
 	dir := filepath.Join(os.Getenv("ProgramData"), "Tailscale", "Logs")
 
 	if err := os.MkdirAll(dir, 0700); err != nil {
-		log.Printf("failed to create local log directory; not writing logs to disk: %v", err)
-		return logf
+		inner.Printf("failed to create local log directory; not writing logs to disk: %v", err)
+		return inner.Writer()
 	}
-	logf("local disk logdir: %v", dir)
+	inner.Printf("local disk logdir: %v", dir)
 	lfw := &logFileWriter{
 		fileBasePrefix: fileBasePrefix,
 		logID:          logID,
 		dir:            dir,
-		wrappedLogf:    logf,
+		wrappedLogf:    inner.Printf,
 	}
-	return lfw.Logf
+	return logger.FuncWriter(lfw.Logf)
 }
 
 // logFileWriter is the state for the log writer & rotator.
diff --git a/logpolicy/logpolicy.go b/logpolicy/logpolicy.go
index caa647acc..2532ca443 100644
--- a/logpolicy/logpolicy.go
+++ b/logpolicy/logpolicy.go
@@ -525,7 +525,7 @@ func New(collection string) *Policy {
 	}
 	lw := logtail.NewLogger(c, log.Printf)
 	log.SetFlags(0) // other logflags are set on console, not here
-	log.SetOutput(lw)
+	log.SetOutput(maybeWrapForPlatform(lw, cmdName, newc.PublicID.String()))
 
 	log.Printf("Program starting: v%v, Go %v: %#v",
 		version.Long,
diff --git a/logpolicy/logpolicy_notwindows.go b/logpolicy/logpolicy_notwindows.go
new file mode 100644
index 000000000..1b8f30469
--- /dev/null
+++ b/logpolicy/logpolicy_notwindows.go
@@ -0,0 +1,16 @@
+// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build !windows
+// +build !windows
+
+package logpolicy
+
+import (
+	"io"
+)
+
+func maybeWrapForPlatform(lw io.Writer, cmdName, logID string) io.Writer {
+	return lw
+}
diff --git a/logpolicy/logpolicy_windows.go b/logpolicy/logpolicy_windows.go
new file mode 100644
index 000000000..90572ec12
--- /dev/null
+++ b/logpolicy/logpolicy_windows.go
@@ -0,0 +1,26 @@
+// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package logpolicy
+
+import (
+	"io"
+	"log"
+
+	"golang.org/x/sys/windows/svc"
+	"tailscale.com/log/filelogger"
+)
+
+func maybeWrapForPlatform(lw io.Writer, cmdName, logID string) io.Writer {
+	if cmdName != "tailscaled" {
+		return lw
+	}
+
+	isSvc, err := svc.IsWindowsService()
+	if err != nil || !isSvc {
+		return lw
+	}
+
+	return filelogger.New("tailscale-service", logID, log.New(lw, "", 0))
+}