gopls/internal/server: add counters to inform v0.17.0

Add two counters to help inform decisions for [email protected]: - 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 <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
golang · Jun 28, 2024 · fcf5463 · fcf5463
1 parent 70a59b2
commit fcf5463
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 7 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
+				}
 			})
 		})
 	}