diff --git a/client/tailscale/localclient.go b/client/tailscale/localclient.go index 150eed0ed..fde465bac 100644 --- a/client/tailscale/localclient.go +++ b/client/tailscale/localclient.go @@ -259,13 +259,12 @@ func (lc *LocalClient) DaemonMetrics(ctx context.Context) ([]byte, error) { return lc.get200(ctx, "/localapi/v0/metrics") } -// IncrementMetric increments the value of a Tailscale daemon's metric by -// the given delta. If the metric has yet to exist, a new counter metric is -// created and initialized to delta. +// IncrementCounter increments the value of a Tailscale daemon's counter +// metric by the given delta. If the metric has yet to exist, a new counter +// metric is created and initialized to delta. // -// IncrementMetric only supports counter metrics and non-negative delta values. -// Gauge metrics are unsupported. -func (lc *LocalClient) IncrementMetric(ctx context.Context, name string, delta int) error { +// IncrementCounter does not support gauge metrics or negative delta values. +func (lc *LocalClient) IncrementCounter(ctx context.Context, name string, delta int) error { type metricUpdate struct { Name string `json:"name"` Type string `json:"type"` diff --git a/cmd/tailscale/cli/funnel.go b/cmd/tailscale/cli/funnel.go index b27390c1c..cf6ae15de 100644 --- a/cmd/tailscale/cli/funnel.go +++ b/cmd/tailscale/cli/funnel.go @@ -94,8 +94,13 @@ func (e *serveEnv) runFunnel(ctx context.Context, args []string) error { } port := uint16(port64) - if err := e.verifyFunnelEnabled(ctx, st, port); err != nil { - return err + if on { + // Don't block from turning off existing Funnel if + // network configuration/capabilities have changed. + // Only block from starting new Funnels. + if err := e.verifyFunnelEnabled(ctx, st, port); err != nil { + return err + } } dnsName := strings.TrimSuffix(st.Self.DNSName, ".") @@ -176,7 +181,7 @@ func printFunnelWarning(sc *ipn.ServeConfig) { p, _ := strconv.ParseUint(portStr, 10, 16) if _, ok := sc.TCP[uint16(p)]; !ok { warn = true - fmt.Fprintf(os.Stderr, "Warning: funnel=on for %s, but no serve config\n", hp) + fmt.Fprintf(os.Stderr, "\nWarning: funnel=on for %s, but no serve config\n", hp) } } if warn { diff --git a/cmd/tailscale/cli/serve.go b/cmd/tailscale/cli/serve.go index 4688012dd..8979a90ed 100644 --- a/cmd/tailscale/cli/serve.go +++ b/cmd/tailscale/cli/serve.go @@ -133,6 +133,7 @@ type localServeClient interface { SetServeConfig(context.Context, *ipn.ServeConfig) error QueryFeature(ctx context.Context, feature string) (*tailcfg.QueryFeatureResponse, error) WatchIPNBus(ctx context.Context, mask ipn.NotifyWatchOpt) (*tailscale.IPNBusWatcher, error) + IncrementCounter(ctx context.Context, name string, delta int) error } // serveEnv is the environment the serve command runs within. All I/O should be @@ -796,17 +797,19 @@ func (e *serveEnv) enableFeatureInteractive(ctx context.Context, feature string, return nil // already enabled } if info.Text != "" { - fmt.Fprintln(os.Stdout, info.Text) + fmt.Fprintln(os.Stdout, "\n"+info.Text) } if info.URL != "" { - fmt.Fprintln(os.Stdout, "\n "+info.URL) + fmt.Fprintln(os.Stdout, "\n "+info.URL+"\n") } if !info.ShouldWait { + e.lc.IncrementCounter(ctx, fmt.Sprintf("%s_not_awaiting_enablement", feature), 1) // The feature has not been enabled yet, // but the CLI should not block on user action. // Once info.Text is printed, exit the CLI. os.Exit(0) } + e.lc.IncrementCounter(ctx, fmt.Sprintf("%s_awaiting_enablement", feature), 1) // Block until feature is enabled. watchCtx, cancelWatch := context.WithCancel(ctx) defer cancelWatch() @@ -816,6 +819,7 @@ func (e *serveEnv) enableFeatureInteractive(ctx context.Context, feature string, // don't block. We still present the URL in the CLI, // then close the process. Swallow the error. log.Fatalf("lost connection to tailscaled: %v", err) + e.lc.IncrementCounter(ctx, fmt.Sprintf("%s_enablement_lost_connection", feature), 1) return err } defer watcher.Close() @@ -826,11 +830,13 @@ func (e *serveEnv) enableFeatureInteractive(ctx context.Context, feature string, // Let the user finish enablement then rerun their // command themselves. log.Fatalf("lost connection to tailscaled: %v", err) + e.lc.IncrementCounter(ctx, fmt.Sprintf("%s_enablement_lost_connection", feature), 1) return err } if nm := n.NetMap; nm != nil && nm.SelfNode != nil { if hasRequiredCapabilities(nm.SelfNode.Capabilities) { - fmt.Fprintln(os.Stdout, "\nSuccess.") + e.lc.IncrementCounter(ctx, fmt.Sprintf("%s_enabled", feature), 1) + fmt.Fprintln(os.Stdout, "Success.") return nil } } diff --git a/cmd/tailscale/cli/serve_test.go b/cmd/tailscale/cli/serve_test.go index 88e7aeb87..cae6b2bee 100644 --- a/cmd/tailscale/cli/serve_test.go +++ b/cmd/tailscale/cli/serve_test.go @@ -893,7 +893,11 @@ func (lc *fakeLocalServeClient) QueryFeature(ctx context.Context, feature string } func (lc *fakeLocalServeClient) WatchIPNBus(ctx context.Context, mask ipn.NotifyWatchOpt) (*tailscale.IPNBusWatcher, error) { - return nil, nil + return nil, nil // unused in tests +} + +func (lc *fakeLocalServeClient) IncrementCounter(ctx context.Context, name string, delta int) error { + return nil // unused in tests } // exactError returns an error checker that wants exactly the provided want error.