From 63b3c825878c5270a740f64ea5ab39bc1b45a974 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 22 Apr 2024 16:45:01 -0700 Subject: [PATCH] ipn/local: log OS-specific diagnostic information as JSON (#11700) There is an undocumented 16KiB limit for text log messages. However, the limit for JSON messages is 256KiB. Even worse, logging JSON as text results in significant overhead since each double quote needs to be escaped. Instead, use logger.Logf.JSON to explicitly log the info as JSON. We also modify osdiag to return the information as structured data rather than implicitly have the package log on our behalf. This gives more control to the caller on how to log. Updates #7802 Signed-off-by: Joe Tsai --- cmd/tailscaled/tailscaled_windows.go | 2 +- ipn/localapi/localapi.go | 2 +- util/osdiag/osdiag.go | 11 ++++------- util/osdiag/osdiag_notwindows.go | 5 ++--- util/osdiag/osdiag_windows.go | 17 ++--------------- 5 files changed, 10 insertions(+), 27 deletions(-) diff --git a/cmd/tailscaled/tailscaled_windows.go b/cmd/tailscaled/tailscaled_windows.go index bf1973caa..d916cc629 100644 --- a/cmd/tailscaled/tailscaled_windows.go +++ b/cmd/tailscaled/tailscaled_windows.go @@ -131,7 +131,7 @@ func isWindowsService() bool { // Windows started. func runWindowsService(pol *logpolicy.Policy) error { go func() { - osdiag.LogSupportInfo(logger.WithPrefix(log.Printf, "Support Info: "), osdiag.LogSupportInfoReasonStartup) + logger.Logf(log.Printf).JSON(1, "SupportInfo", osdiag.SupportInfo(osdiag.LogSupportInfoReasonStartup)) }() if logSCMInteractions, _ := syspolicy.GetBoolean(syspolicy.LogSCMInteractions, false); logSCMInteractions { diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index 44a840c01..7f70dadee 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -380,7 +380,7 @@ func (h *Handler) serveBugReport(w http.ResponseWriter, r *http.Request) { envknob.LogCurrent(logger.WithPrefix(h.logf, "user bugreport: ")) // OS-specific details - osdiag.LogSupportInfo(logger.WithPrefix(h.logf, "user bugreport OS: "), osdiag.LogSupportInfoReasonBugReport) + h.logf.JSON(1, "UserBugReportOS", osdiag.SupportInfo(osdiag.LogSupportInfoReasonBugReport)) if defBool(r.URL.Query().Get("diagnose"), false) { h.b.Doctor(r.Context(), logger.WithPrefix(h.logf, "diag: ")) diff --git a/util/osdiag/osdiag.go b/util/osdiag/osdiag.go index df1bcb362..2ebecbdbf 100644 --- a/util/osdiag/osdiag.go +++ b/util/osdiag/osdiag.go @@ -4,8 +4,6 @@ // Package osdiag provides loggers for OS-specific diagnostic information. package osdiag -import "tailscale.com/types/logger" - // LogSupportInfoReason is an enumeration indicating the reason for logging // support info. type LogSupportInfoReason int @@ -15,9 +13,8 @@ LogSupportInfoReasonBugReport // a bugreport is in the process of being gathered. ) -// LogSupportInfo obtains OS-specific diagnostic information useful for -// troubleshooting and support, and writes it to logf. The reason argument is -// useful for governing the verbosity of this function's output. -func LogSupportInfo(logf logger.Logf, reason LogSupportInfoReason) { - logSupportInfo(logf, reason) +// SupportInfo obtains OS-specific diagnostic information for troubleshooting +// and support. The reason governs the verbosity of the output. +func SupportInfo(reason LogSupportInfoReason) map[string]any { + return supportInfo(reason) } diff --git a/util/osdiag/osdiag_notwindows.go b/util/osdiag/osdiag_notwindows.go index da172b967..0e46c97e5 100644 --- a/util/osdiag/osdiag_notwindows.go +++ b/util/osdiag/osdiag_notwindows.go @@ -5,7 +5,6 @@ package osdiag -import "tailscale.com/types/logger" - -func logSupportInfo(logger.Logf, LogSupportInfoReason) { +func supportInfo(LogSupportInfoReason) map[string]any { + return nil } diff --git a/util/osdiag/osdiag_windows.go b/util/osdiag/osdiag_windows.go index beb1bc26a..5dcce3bea 100644 --- a/util/osdiag/osdiag_windows.go +++ b/util/osdiag/osdiag_windows.go @@ -5,10 +5,8 @@ import ( "encoding/binary" - "encoding/json" "errors" "fmt" - "io" "path/filepath" "strings" "unicode/utf16" @@ -18,7 +16,6 @@ "github.com/dblohm7/wingoes/pe" "golang.org/x/sys/windows" "golang.org/x/sys/windows/registry" - "tailscale.com/types/logger" "tailscale.com/util/osdiag/internal/wsc" "tailscale.com/util/winutil" "tailscale.com/util/winutil/authenticode" @@ -34,15 +31,6 @@ initialValueBufLen = 80 // large enough to contain a stringified GUID encoded as UTF-16 ) -func logSupportInfo(logf logger.Logf, reason LogSupportInfoReason) { - var b strings.Builder - if err := getSupportInfo(&b, reason); err != nil { - logf("error encoding support info: %v", err) - return - } - logf("%s", b.String()) -} - const ( supportInfoKeyModules = "modules" supportInfoKeyPageFile = "pageFile" @@ -51,7 +39,7 @@ func logSupportInfo(logf logger.Logf, reason LogSupportInfoReason) { supportInfoKeyWinsockLSP = "winsockLSP" ) -func getSupportInfo(w io.Writer, reason LogSupportInfoReason) error { +func supportInfo(reason LogSupportInfoReason) map[string]any { output := make(map[string]any) regInfo, err := getRegistrySupportInfo(registry.LOCAL_MACHINE, []string{winutil.RegPolicyBase, winutil.RegBase}) @@ -86,8 +74,7 @@ func getSupportInfo(w io.Writer, reason LogSupportInfoReason) error { } } - enc := json.NewEncoder(w) - return enc.Encode(output) + return output } type getRegistrySupportInfoBufs struct {