From 40e2b312b62a4663de991447b539c2a42c24ffea Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 15 Dec 2021 19:07:52 -0800 Subject: [PATCH] ipn/ipnserver, logpolicy: move Windows disk logging up earlier This 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. Examples that weren't logged to disk: * logtail unable to dial out, * DNS flush messages from the service * svc.ChangeRequest messages (#3581) This is basically the same fix as #3571 but staying in the Logf type, and avoiding build-tagged file (which wasn't quite a goal, but happened and seemed nice) Fixes #3570 Co-authored-by: Aaron Klotz Change-Id: Iacd80c4720b7218365ec80ae143339d030842702 --- cmd/tailscaled/depaware.txt | 2 +- ipn/ipnserver/server.go | 9 --------- logpolicy/logpolicy.go | 16 +++++++++++++++- logtail/logtail.go | 5 +++++ 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index d0951910a..d30960342 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 + 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/logpolicy/logpolicy.go b/logpolicy/logpolicy.go index caa647acc..c67557697 100644 --- a/logpolicy/logpolicy.go +++ b/logpolicy/logpolicy.go @@ -13,6 +13,7 @@ import ( "crypto/tls" "encoding/json" "fmt" + "io" "io/ioutil" "log" "net" @@ -29,6 +30,7 @@ import ( "golang.org/x/term" "tailscale.com/atomicfile" + "tailscale.com/log/filelogger" "tailscale.com/logtail" "tailscale.com/logtail/filch" "tailscale.com/net/dnscache" @@ -524,8 +526,20 @@ func New(collection string) *Policy { } } lw := logtail.NewLogger(c, log.Printf) + + var logOutput io.Writer = lw + + if runtime.GOOS == "windows" && c.Collection == logtail.CollectionNode { + logID := newc.PublicID.String() + exe, _ := os.Executable() + if strings.EqualFold(filepath.Base(exe), "tailscaled.exe") { + diskLogf := filelogger.New("tailscale-service", logID, lw.Logf) + logOutput = logger.FuncWriter(diskLogf) + } + } + log.SetFlags(0) // other logflags are set on console, not here - log.SetOutput(lw) + log.SetOutput(logOutput) log.Printf("Program starting: v%v, Go %v: %#v", version.Long, diff --git a/logtail/logtail.go b/logtail/logtail.go index 1506cb948..4e217bac1 100644 --- a/logtail/logtail.go +++ b/logtail/logtail.go @@ -523,6 +523,11 @@ func (l *Logger) encode(buf []byte) []byte { return b } +// Logf logs to l using the provided fmt-style format and optional arguments. +func (l *Logger) Logf(format string, args ...interface{}) { + fmt.Fprintf(l, format, args...) +} + // Write logs an encoded JSON blob. // // If the []byte passed to Write is not an encoded JSON blob,