From 507a5a79001f33ca8f50899c7e8cc5d10a5ecb16 Mon Sep 17 00:00:00 2001 From: Peter Weinberger Date: Mon, 29 Jan 2024 07:42:28 -0500 Subject: [PATCH] upload: add debug logging If the directory telemetry.LocalDir/debug exists write a per-process log of upload actions there. Fixes: https://github.com/golang/go/issues/63709 Change-Id: Ia42400fa7ec40848897e76d39e424de1c9fe8d40 Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/558640 Run-TryBot: Peter Weinberger Reviewed-by: Hyang-Ah Hana Kim Reviewed-by: Robert Findley TryBot-Result: Gopher Robot --- internal/upload/dates_test.go | 26 ++++++++++++-- internal/upload/run.go | 65 +++++++++++++++++++++++++++++++++-- upload/upload.go | 2 ++ 3 files changed, 89 insertions(+), 4 deletions(-) diff --git a/internal/upload/dates_test.go b/internal/upload/dates_test.go index 3810c6a..5d48b18 100644 --- a/internal/upload/dates_test.go +++ b/internal/upload/dates_test.go @@ -363,7 +363,11 @@ func readCountFileInfo(t *testing.T, localDir string) *countFileInfo { } func doTest(t *testing.T, u *Uploader, doing *Test, known *countFileInfo) int { - os.MkdirAll(u.LocalDir, 0777) + dbg := filepath.Join(u.LocalDir, "debug") + os.MkdirAll(dbg, 0777) + if err := LogIfDebug(dbg); err != nil { + t.Errorf("debug logging: %v", err) + } if len(doing.uploads) > 0 { os.MkdirAll(u.UploadDir, 0777) } @@ -402,7 +406,7 @@ func doTest(t *testing.T, u *Uploader, doing *Test, known *countFileInfo) int { u.Run() // check results - var cfiles, rfiles, lfiles, ufiles int + var cfiles, rfiles, lfiles, ufiles, logcnt int fis, err := os.ReadDir(u.LocalDir) if err != nil { t.Errorf("%v reading localdir %s", err, u.LocalDir) @@ -417,10 +421,20 @@ func doTest(t *testing.T, u *Uploader, doing *Test, known *countFileInfo) int { lfiles++ case strings.HasSuffix(f.Name(), ".json"): rfiles++ + case f.Name() == "debug": + dbgname := filepath.Join(u.LocalDir, "debug") + logs, err := os.ReadDir(dbgname) + if err != nil { + break + } + logcnt += len(logs) default: t.Errorf("for %s, unexpected local file %s", doing.name, f.Name()) } } + if logcnt != 1 { + t.Errorf("expected 1 log file, got %d", logcnt) + } fis, err = os.ReadDir(u.UploadDir) if err != nil { t.Errorf("%v reading uploaddir %s", err, u.UploadDir) @@ -439,6 +453,14 @@ func doTest(t *testing.T, u *Uploader, doing *Test, known *countFileInfo) int { if doing.wantUploadeds != ufiles { t.Errorf("%s: got %d uploaded files, wanted %d", doing.name, ufiles, doing.wantUploadeds) } + // close all loggers, so Windows can clean up the test + for _, lw := range seenlogwriters { + if fd, ok := lw.(*os.File); ok { + fd.Close() + } + } + // and let the next test start in a clean state + seenlogwriters = []io.Writer{} return ufiles - len(doing.uploads) } diff --git a/internal/upload/run.go b/internal/upload/run.go index e8a1286..0636fc8 100644 --- a/internal/upload/run.go +++ b/internal/upload/run.go @@ -5,8 +5,14 @@ package upload import ( + "fmt" "io" "log" + "os" + "path" + "path/filepath" + "runtime/debug" + "strings" "time" "golang.org/x/telemetry" @@ -19,11 +25,66 @@ func init() { logger = log.New(io.Discard, "", 0) } +// keep track of what SetLogOutput has seen +var seenlogwriters []io.Writer + // SetLogOutput sets the default logger's output destination. func SetLogOutput(logging io.Writer) { - if logging != nil { - logger.SetOutput(logging) + if logging == nil { + return + } + logger.SetOutput(logging) // the common case + seenlogwriters = append(seenlogwriters, logging) + if len(seenlogwriters) > 1 { + // The client asked for logging, and there is also a debug dir + logger.SetOutput(io.MultiWriter(seenlogwriters...)) + } +} + +// LogIfDebug arranges to write a log file in the directory +// dirname, if it exists. If dirname is the empty string, +// the function tries the directory it.Localdir/debug. +func LogIfDebug(dirname string) error { + dname := filepath.Join(it.LocalDir, "debug") + if dirname != "" { + dname = dirname + } + fd, err := os.Stat(dname) + if err == os.ErrExist || !fd.IsDir() { + // debug doesn't exist or isn't a directory + return nil + } + if err != nil { + return err + } + info, ok := debug.ReadBuildInfo() + if !ok { + return fmt.Errorf("no build info") + } + year, month, day := time.Now().UTC().Date() + goVers := info.GoVersion + // E.g., goVers:"go1.22-20240109-RC01 cl/597041403 +dcbe772469 X:loopvar" + words := strings.Fields(goVers) + goVers = words[0] + progPkgPath := info.Path + if progPkgPath == "" { + progPkgPath = strings.TrimSuffix(filepath.Base(os.Args[0]), ".exe") + } + prog := path.Base(progPkgPath) + progVers := info.Main.Version + fname := filepath.Join(dname, fmt.Sprintf("%s-%s-%s-%4d%02d%02d-%d.log", + prog, progVers, goVers, year, month, day, os.Getpid())) + fname = strings.ReplaceAll(fname, " ", "") + if _, err := os.Stat(fname); err == nil { + // This process previously called upload.Run + return nil + } + logfd, err := os.Create(fname) + if err != nil { + return err } + SetLogOutput(logfd) + return nil } // Uploader carries parameters needed for upload. diff --git a/upload/upload.go b/upload/upload.go index ae53f6e..0a2b13a 100644 --- a/upload/upload.go +++ b/upload/upload.go @@ -17,6 +17,8 @@ func Run(c *Control) { if c != nil && c.Logging != nil { upload.SetLogOutput(c.Logging) } + // ignore error: failed logging should not block uploads + upload.LogIfDebug("") defer func() { if err := recover(); err != nil {