From fcf54632603b8795667b76d7c373201e9536ed10 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Fri, 28 Jun 2024 15:49:12 +0000 Subject: [PATCH] gopls/internal/server: add counters to inform v0.17.0 Add two counters to help inform decisions for gopls@v0.17.0: - Add a gopls/gotoolchain:{auto,local,other} counter to help us understand toolchain upgradability. - Add a gopls/telemetryprompt/accepted counter to track telemetry prompt acceptance. Fixes golang/go#68240 Change-Id: I8fc06b3a266761dbf7c2781267dfb1235eef1a63 Reviewed-on: https://go-review.googlesource.com/c/tools/+/595560 LUCI-TryBot-Result: Go LUCI Reviewed-by: Hyang-Ah Hana Kim --- gopls/internal/cache/view.go | 2 ++ gopls/internal/server/general.go | 13 ++++++++ gopls/internal/server/prompt.go | 2 ++ gopls/internal/telemetry/telemetry_test.go | 9 +++-- .../test/integration/misc/misc_test.go | 11 +++++-- .../test/integration/misc/prompt_test.go | 33 +++++++++++++++++-- 6 files changed, 63 insertions(+), 7 deletions(-) diff --git a/gopls/internal/cache/view.go b/gopls/internal/cache/view.go index 1e3448d42f4..f1a13e358da 100644 --- a/gopls/internal/cache/view.go +++ b/gopls/internal/cache/view.go @@ -67,6 +67,7 @@ type GoEnv struct { GOPRIVATE string GOFLAGS string GO111MODULE string + GOTOOLCHAIN string // Go version output. GoVersion int // The X in Go 1.X @@ -992,6 +993,7 @@ func FetchGoEnv(ctx context.Context, folder protocol.DocumentURI, opts *settings "GOMODCACHE": &env.GOMODCACHE, "GOFLAGS": &env.GOFLAGS, "GO111MODULE": &env.GO111MODULE, + "GOTOOLCHAIN": &env.GOTOOLCHAIN, } if err := loadGoEnv(ctx, dir, opts.EnvSlice(), runner, envvars); err != nil { return nil, err diff --git a/gopls/internal/server/general.go b/gopls/internal/server/general.go index 340b27dc023..08b65b1bc84 100644 --- a/gopls/internal/server/general.go +++ b/gopls/internal/server/general.go @@ -468,6 +468,19 @@ func (s *server) newFolder(ctx context.Context, folder protocol.DocumentURI, nam if err != nil { return nil, err } + + // Increment folder counters. + switch { + case env.GOTOOLCHAIN == "auto" || strings.Contains(env.GOTOOLCHAIN, "+auto"): + counter.New("gopls/gotoolchain:auto").Inc() + case env.GOTOOLCHAIN == "path" || strings.Contains(env.GOTOOLCHAIN, "+path"): + counter.New("gopls/gotoolchain:path").Inc() + case env.GOTOOLCHAIN == "local": // local+auto and local+path handled above + counter.New("gopls/gotoolchain:local").Inc() + default: + counter.New("gopls/gotoolchain:other").Inc() + } + return &cache.Folder{ Dir: folder, Name: name, diff --git a/gopls/internal/server/prompt.go b/gopls/internal/server/prompt.go index 42a86240f68..cdd22048c3d 100644 --- a/gopls/internal/server/prompt.go +++ b/gopls/internal/server/prompt.go @@ -14,6 +14,7 @@ import ( "time" "golang.org/x/telemetry" + "golang.org/x/telemetry/counter" "golang.org/x/tools/gopls/internal/protocol" "golang.org/x/tools/internal/event" ) @@ -308,6 +309,7 @@ Would you like to enable Go telemetry? result = pYes if err := s.setTelemetryMode("on"); err == nil { message(protocol.Info, telemetryOnMessage(s.Options().LinkifyShowMessage)) + counter.New("gopls/telemetryprompt/accepted").Inc() } else { errorf("enabling telemetry failed: %v", err) msg := fmt.Sprintf("Failed to enable Go telemetry: %v\nTo enable telemetry manually, please run `go run golang.org/x/telemetry/cmd/gotelemetry@latest on`", err) diff --git a/gopls/internal/telemetry/telemetry_test.go b/gopls/internal/telemetry/telemetry_test.go index 6c1944b2230..7aaca41ab55 100644 --- a/gopls/internal/telemetry/telemetry_test.go +++ b/gopls/internal/telemetry/telemetry_test.go @@ -26,13 +26,13 @@ import ( ) func TestMain(m *testing.M) { - tmp, err := os.MkdirTemp("", "gopls-telemetry-test") + tmp, err := os.MkdirTemp("", "gopls-telemetry-test-counters") if err != nil { panic(err) } countertest.Open(tmp) code := Main(m) - os.RemoveAll(tmp) + os.RemoveAll(tmp) // golang/go#68243: ignore error; cleanup fails on Windows os.Exit(code) } @@ -54,6 +54,7 @@ func TestTelemetry(t *testing.T) { counter.New("gopls/client:" + editor), counter.New("gopls/goversion:1." + goversion), counter.New("fwd/vscode/linter:a"), + counter.New("gopls/gotoolchain:local"), } initialCounts := make([]uint64, len(sessionCounters)) for i, c := range sessionCounters { @@ -70,6 +71,9 @@ func TestTelemetry(t *testing.T) { Modes(Default), // must be in-process to receive the bug report below Settings{"showBugReports": true}, ClientName("Visual Studio Code"), + EnvVars{ + "GOTOOLCHAIN": "local", // so that the local counter is incremented + }, ).Run(t, "", func(_ *testing.T, env *Env) { goversion = strconv.Itoa(env.GoVersion()) addForwardedCounters(env, []string{"vscode/linter:a"}, []int64{1}) @@ -93,6 +97,7 @@ func TestTelemetry(t *testing.T) { // gopls/editor:client // gopls/goversion:1.x // fwd/vscode/linter:a + // gopls/gotoolchain:local for i, c := range sessionCounters { want := initialCounts[i] + 1 got, err := countertest.ReadCounter(c) diff --git a/gopls/internal/test/integration/misc/misc_test.go b/gopls/internal/test/integration/misc/misc_test.go index 666887f9f14..ca0125894c8 100644 --- a/gopls/internal/test/integration/misc/misc_test.go +++ b/gopls/internal/test/integration/misc/misc_test.go @@ -9,15 +9,22 @@ import ( "strings" "testing" + "golang.org/x/telemetry/counter/countertest" "golang.org/x/tools/gopls/internal/protocol" - "golang.org/x/tools/gopls/internal/test/integration" . "golang.org/x/tools/gopls/internal/test/integration" "golang.org/x/tools/gopls/internal/util/bug" ) func TestMain(m *testing.M) { bug.PanicOnBugs = true - os.Exit(integration.Main(m)) + tmp, err := os.MkdirTemp("", "gopls-misc-test-counters") + if err != nil { + panic(err) + } + countertest.Open(tmp) + code := Main(m) + os.RemoveAll(tmp) // golang/go#68243: ignore error; cleanup fails on Windows + os.Exit(code) } // TestDocumentURIFix ensures that a DocumentURI supplied by the diff --git a/gopls/internal/test/integration/misc/prompt_test.go b/gopls/internal/test/integration/misc/prompt_test.go index 9fc7bdd17dc..6eda9dabee3 100644 --- a/gopls/internal/test/integration/misc/prompt_test.go +++ b/gopls/internal/test/integration/misc/prompt_test.go @@ -13,6 +13,8 @@ import ( "testing" "time" + "golang.org/x/telemetry/counter" + "golang.org/x/telemetry/counter/countertest" "golang.org/x/tools/gopls/internal/protocol" "golang.org/x/tools/gopls/internal/protocol/command" "golang.org/x/tools/gopls/internal/server" @@ -250,6 +252,10 @@ func main() { // Test that responding to the telemetry prompt results in the expected state. func TestTelemetryPrompt_Response(t *testing.T) { + if !countertest.SupportedPlatform { + t.Skip("requires counter support") + } + const src = ` -- go.mod -- module mod.com @@ -262,18 +268,32 @@ func main() { } ` + acceptanceCounterName := "gopls/telemetryprompt/accepted" + acceptanceCounter := counter.New(acceptanceCounterName) + // We must increment the acceptance counter in order for the initial read + // below to succeed. + // + // TODO(rfindley): ReadCounter should simply return 0 for uninitialized + // counters. + acceptanceCounter.Inc() + tests := []struct { name string // subtest name response string // response to choose for the telemetry dialog wantMode string // resulting telemetry mode wantMsg string // substring contained in the follow-up popup (if empty, no popup is expected) + wantInc uint64 // expected 'prompt accepted' counter increment }{ - {"yes", server.TelemetryYes, "on", "uploading is now enabled"}, - {"no", server.TelemetryNo, "", ""}, - {"empty", "", "", ""}, + {"yes", server.TelemetryYes, "on", "uploading is now enabled", 1}, + {"no", server.TelemetryNo, "", "", 0}, + {"empty", "", "", "", 0}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { + initialCount, err := countertest.ReadCounter(acceptanceCounter) + if err != nil { + t.Fatalf("ReadCounter(%q) failed: %v", acceptanceCounterName, err) + } modeFile := filepath.Join(t.TempDir(), "mode") telemetryStartTime := time.Now().Add(-8 * 24 * time.Hour) msgRE := regexp.MustCompile(".*Would you like to enable Go telemetry?") @@ -320,6 +340,13 @@ func main() { if gotMode != test.wantMode { t.Errorf("after prompt, mode=%s, want %s", gotMode, test.wantMode) } + finalCount, err := countertest.ReadCounter(acceptanceCounter) + if err != nil { + t.Fatalf("ReadCounter(%q) failed: %v", acceptanceCounterName, err) + } + if gotInc := finalCount - initialCount; gotInc != test.wantInc { + t.Errorf("%q mismatch: got %d, want %d", acceptanceCounterName, gotInc, test.wantInc) + } }) }) }