From 239ad574460883b49ea4981c5471c3f06db2022b Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 16 Aug 2023 20:03:57 -0700 Subject: [PATCH] tailcfg: move LogHeapPprof from Debug to c2n [capver 69] And delete Debug.GoroutineDumpURL, which was already in c2n. Updates #8923 Signed-off-by: Brad Fitzpatrick --- cmd/tailscaled/depaware.txt | 5 ++-- control/controlclient/debug.go | 40 ----------------------------- control/controlclient/direct.go | 7 ----- ipn/ipnlocal/c2n.go | 9 +++++++ ipn/ipnlocal/c2n_pprof.go | 17 +++++++++++++ log/logheap/logheap.go | 45 --------------------------------- log/logheap/logheap_js.go | 7 ----- tailcfg/tailcfg.go | 16 ++---------- tstest/iosdeps/iosdeps.go | 1 - types/netmap/netmap_test.go | 4 +-- 10 files changed, 32 insertions(+), 119 deletions(-) delete mode 100644 control/controlclient/debug.go create mode 100644 ipn/ipnlocal/c2n_pprof.go delete mode 100644 log/logheap/logheap.go delete mode 100644 log/logheap/logheap_js.go diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 0a685d61f..6d417c725 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -242,7 +242,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/ipn/store/mem from tailscale.com/ipn/store+ L tailscale.com/kube from tailscale.com/ipn/store/kubestore tailscale.com/log/filelogger from tailscale.com/logpolicy - tailscale.com/log/logheap from tailscale.com/control/controlclient tailscale.com/log/sockstatlog from tailscale.com/ipn/ipnlocal tailscale.com/logpolicy from tailscale.com/cmd/tailscaled+ tailscale.com/logtail from tailscale.com/control/controlclient+ @@ -325,7 +324,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de 💣 tailscale.com/util/deephash from tailscale.com/ipn/ipnlocal+ L 💣 tailscale.com/util/dirwalk from tailscale.com/metrics+ tailscale.com/util/dnsname from tailscale.com/hostinfo+ - tailscale.com/util/goroutines from tailscale.com/control/controlclient+ + tailscale.com/util/goroutines from tailscale.com/ipn/ipnlocal tailscale.com/util/groupmember from tailscale.com/ipn/ipnauth 💣 tailscale.com/util/hashx from tailscale.com/util/deephash tailscale.com/util/httpm from tailscale.com/client/tailscale+ @@ -495,7 +494,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de regexp from github.com/coreos/go-iptables/iptables+ regexp/syntax from regexp runtime/debug from github.com/klauspost/compress/zstd+ - runtime/pprof from tailscale.com/log/logheap+ + runtime/pprof from net/http/pprof+ runtime/trace from net/http/pprof slices from tailscale.com/wgengine/magicsock sort from compress/flate+ diff --git a/control/controlclient/debug.go b/control/controlclient/debug.go deleted file mode 100644 index 0b77d8ecd..000000000 --- a/control/controlclient/debug.go +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -package controlclient - -import ( - "bytes" - "compress/gzip" - "context" - "log" - "net/http" - "time" - - "tailscale.com/util/goroutines" -) - -func dumpGoroutinesToURL(c *http.Client, targetURL string) { - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) - defer cancel() - - zbuf := new(bytes.Buffer) - zw := gzip.NewWriter(zbuf) - zw.Write(goroutines.ScrubbedGoroutineDump(true)) - zw.Close() - - req, err := http.NewRequestWithContext(ctx, "PUT", targetURL, zbuf) - if err != nil { - log.Printf("dumpGoroutinesToURL: %v", err) - return - } - req.Header.Set("Content-Encoding", "gzip") - t0 := time.Now() - _, err = c.Do(req) - d := time.Since(t0).Round(time.Millisecond) - if err != nil { - log.Printf("dumpGoroutinesToURL error: %v to %v (after %v)", err, targetURL, d) - } else { - log.Printf("dumpGoroutinesToURL complete to %v (after %v)", targetURL, d) - } -} diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 1762c4eec..6aa395d7d 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -32,7 +32,6 @@ "tailscale.com/health" "tailscale.com/hostinfo" "tailscale.com/ipn/ipnstate" - "tailscale.com/log/logheap" "tailscale.com/logtail" "tailscale.com/net/dnscache" "tailscale.com/net/dnsfallback" @@ -1096,12 +1095,6 @@ func (c *Direct) sendMapRequest(ctx context.Context, isStreaming bool, nu Netmap logtail.Disable() envknob.SetNoLogsNoSupport() } - if resp.Debug.LogHeapPprof { - go logheap.LogHeap(resp.Debug.LogHeapURL) - } - if resp.Debug.GoroutineDumpURL != "" { - go dumpGoroutinesToURL(c.httpc, resp.Debug.GoroutineDumpURL) - } if sleep := time.Duration(resp.Debug.SleepSeconds * float64(time.Second)); sleep > 0 { if err := sleepAsRequested(ctx, c.logf, timeoutReset, sleep, c.clock); err != nil { return err diff --git a/ipn/ipnlocal/c2n.go b/ipn/ipnlocal/c2n.go index 554b9fc43..0810eb9a1 100644 --- a/ipn/ipnlocal/c2n.go +++ b/ipn/ipnlocal/c2n.go @@ -25,6 +25,8 @@ "tailscale.com/version/distro" ) +var c2nLogHeap func(http.ResponseWriter, *http.Request) // non-nil on most platforms (c2n_pprof.go) + func (b *LocalBackend) handleC2N(w http.ResponseWriter, r *http.Request) { writeJSON := func(v any) { w.Header().Set("Content-Type", "application/json") @@ -70,6 +72,13 @@ func (b *LocalBackend) handleC2N(w http.ResponseWriter, r *http.Request) { res.Error = err.Error() } writeJSON(res) + case "/debug/logheap": + if c2nLogHeap != nil { + c2nLogHeap(w, r) + } else { + http.Error(w, "not implemented", http.StatusNotImplemented) + return + } case "/ssh/usernames": var req tailcfg.C2NSSHUsernamesRequest if r.Method == "POST" { diff --git a/ipn/ipnlocal/c2n_pprof.go b/ipn/ipnlocal/c2n_pprof.go new file mode 100644 index 000000000..9341548ee --- /dev/null +++ b/ipn/ipnlocal/c2n_pprof.go @@ -0,0 +1,17 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !js && !wasm + +package ipnlocal + +import ( + "net/http" + "runtime/pprof" +) + +func init() { + c2nLogHeap = func(w http.ResponseWriter, r *http.Request) { + pprof.WriteHeapProfile(w) + } +} diff --git a/log/logheap/logheap.go b/log/logheap/logheap.go deleted file mode 100644 index f645ec4c9..000000000 --- a/log/logheap/logheap.go +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -//go:build !js - -// Package logheap logs a heap pprof profile. -package logheap - -import ( - "bytes" - "context" - "log" - "net/http" - "runtime" - "runtime/pprof" - "time" -) - -// LogHeap uploads a JSON logtail record with the base64 heap pprof by means -// of an HTTP POST request to the endpoint referred to in postURL. -func LogHeap(postURL string) { - if postURL == "" { - return - } - runtime.GC() - buf := new(bytes.Buffer) - if err := pprof.WriteHeapProfile(buf); err != nil { - log.Printf("LogHeap: %v", err) - return - } - - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - req, err := http.NewRequestWithContext(ctx, "POST", postURL, buf) - if err != nil { - log.Printf("LogHeap: %v", err) - return - } - res, err := http.DefaultClient.Do(req) - if err != nil { - log.Printf("LogHeap: %v", err) - return - } - defer res.Body.Close() -} diff --git a/log/logheap/logheap_js.go b/log/logheap/logheap_js.go deleted file mode 100644 index 35453b482..000000000 --- a/log/logheap/logheap_js.go +++ /dev/null @@ -1,7 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -package logheap - -func LogHeap(postURL string) { -} diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 20a3fb6bf..0c34cb8cf 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -106,7 +106,8 @@ // - 66: 2023-07-23: UserProfile.Groups added (available via WhoIs) // - 67: 2023-07-25: Client understands PeerCapMap // - 68: 2023-08-09: Client has dedicated updateRoutine; MapRequest.Stream true means ignore Hostinfo+Endpoints -const CurrentCapabilityVersion CapabilityVersion = 68 +// - 69: 2023-08-16: removed Debug.LogHeap* + GoroutineDumpURL; added c2n /debug/logheap +const CurrentCapabilityVersion CapabilityVersion = 69 type StableID string @@ -1749,15 +1750,6 @@ type ControlIPCandidate struct { // // TODO(bradfitz): start migrating the imperative ones to c2n requests. type Debug struct { - // LogHeapPprof controls whether the client should log - // its heap pprof data. Each true value sent from the server - // means that client should do one more log. - LogHeapPprof bool `json:",omitempty"` - - // LogHeapURL is the URL to POST its heap pprof to. - // Empty means to not log. - LogHeapURL string `json:",omitempty"` - // ForceBackgroundSTUN controls whether magicsock should // always do its background STUN queries (see magicsock's // periodicReSTUN), regardless of inactivity. @@ -1787,10 +1779,6 @@ type Debug struct { // disabled if WPAD is present on the network. DisableSubnetsIfPAC opt.Bool `json:",omitempty"` - // GoroutineDumpURL, if non-empty, requests that the client do - // a one-time dump of its active goroutines to the given URL. - GoroutineDumpURL string `json:",omitempty"` - // SleepSeconds requests that the client sleep for the // provided number of seconds. // The client can (and should) limit the value (such as 5 diff --git a/tstest/iosdeps/iosdeps.go b/tstest/iosdeps/iosdeps.go index f50fcde6e..07e45d0e5 100644 --- a/tstest/iosdeps/iosdeps.go +++ b/tstest/iosdeps/iosdeps.go @@ -42,7 +42,6 @@ _ "tailscale.com/ipn" _ "tailscale.com/ipn/ipnlocal" _ "tailscale.com/ipn/localapi" - _ "tailscale.com/log/logheap" _ "tailscale.com/logtail" _ "tailscale.com/logtail/filch" _ "tailscale.com/net/dns" diff --git a/types/netmap/netmap_test.go b/types/netmap/netmap_test.go index 84f52bb44..9d2a97e70 100644 --- a/types/netmap/netmap_test.go +++ b/types/netmap/netmap_test.go @@ -71,9 +71,9 @@ func TestNetworkMapConcise(t *testing.T) { name: "debug_values", nm: &NetworkMap{ NodeKey: testNodeKey(1), - Debug: &tailcfg.Debug{LogHeapPprof: true}, + Debug: &tailcfg.Debug{SetForceBackgroundSTUN: "true"}, }, - want: "netmap: self: [AQEBA] auth=machine-unknown u=? debug={\"LogHeapPprof\":true} []\n", + want: "netmap: self: [AQEBA] auth=machine-unknown u=? debug={\"SetForceBackgroundSTUN\":true} []\n", }, } { t.Run(tt.name, func(t *testing.T) {