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 <[email protected]>

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+

control/controlclient/debug.go

@@ -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)
-	}
-}

control/controlclient/direct.go

@@ -32,7 +32,6 @@ import (
 	"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

ipn/ipnlocal/c2n.go

@@ -25,6 +25,8 @@ import (
 	"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" {

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)
+	}
+}

log/logheap/logheap.go

@@ -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()
-}

log/logheap/logheap_js.go

@@ -1,7 +0,0 @@
-// Copyright (c) Tailscale Inc & AUTHORS
-// SPDX-License-Identifier: BSD-3-Clause
-
-package logheap
-
-func LogHeap(postURL string) {
-}

tailcfg/tailcfg.go

@@ -106,7 +106,8 @@ type CapabilityVersion int
 //   - 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

tstest/iosdeps/iosdeps.go

@@ -42,7 +42,6 @@ import (
 	_ "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"

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) {