From 96188ffd2fb3f14fbe3088603a89c6b283c9e2d8 Mon Sep 17 00:00:00 2001 From: Aaron Klotz Date: Wed, 15 Dec 2021 11:43:40 -0700 Subject: [PATCH] ipn/ipnserver, log/filelogger, logpolicy: when tailscaled is running as a windows service, ensure the service's log messages are written to file 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 --- cmd/tailscaled/depaware.txt | 2 +- ipn/ipnserver/server.go | 9 --------- log/filelogger/log.go | 19 ++++++++++--------- logpolicy/logpolicy.go | 2 +- logpolicy/logpolicy_notwindows.go | 16 ++++++++++++++++ logpolicy/logpolicy_windows.go | 26 ++++++++++++++++++++++++++ 6 files changed, 54 insertions(+), 20 deletions(-) create mode 100644 logpolicy/logpolicy_notwindows.go create mode 100644 logpolicy/logpolicy_windows.go 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 @@ "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 @@ import ( "bytes" "fmt" + "io" "io/ioutil" "log" "os" @@ -26,30 +27,30 @@ 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)) +}