[gopls-release-branch.0.16] 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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
(cherry picked from commit fcf54632603b8795667b76d7c373201e9536ed10)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/595836
Auto-Submit: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/cache/view.go b/gopls/internal/cache/view.go
index 1e3448d..f1a13e3 100644
--- a/gopls/internal/cache/view.go
+++ b/gopls/internal/cache/view.go
@@ -67,6 +67,7 @@
GOPRIVATE string
GOFLAGS string
GO111MODULE string
+ GOTOOLCHAIN string
// Go version output.
GoVersion int // The X in Go 1.X
@@ -992,6 +993,7 @@
"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 340b27d..08b65b1 100644
--- a/gopls/internal/server/general.go
+++ b/gopls/internal/server/general.go
@@ -468,6 +468,19 @@
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 a1dba8d..dfe778b 100644
--- a/gopls/internal/server/prompt.go
+++ b/gopls/internal/server/prompt.go
@@ -12,6 +12,7 @@
"time"
"golang.org/x/telemetry"
+ "golang.org/x/telemetry/counter"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/internal/event"
)
@@ -236,6 +237,7 @@
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 6c1944b..7aaca41 100644
--- a/gopls/internal/telemetry/telemetry_test.go
+++ b/gopls/internal/telemetry/telemetry_test.go
@@ -26,13 +26,13 @@
)
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 @@
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 @@
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 @@
// 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 666887f..ca01258 100644
--- a/gopls/internal/test/integration/misc/misc_test.go
+++ b/gopls/internal/test/integration/misc/misc_test.go
@@ -9,15 +9,22 @@
"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 26c0e93..5c8adf8 100644
--- a/gopls/internal/test/integration/misc/prompt_test.go
+++ b/gopls/internal/test/integration/misc/prompt_test.go
@@ -11,6 +11,8 @@
"regexp"
"testing"
+ "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"
@@ -69,6 +71,10 @@
// 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
@@ -81,18 +87,32 @@
}
`
+ 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")
msgRE := regexp.MustCompile(".*Would you like to enable Go telemetry?")
respond := func(m *protocol.ShowMessageRequestParams) (*protocol.MessageActionItem, error) {
@@ -136,6 +156,13 @@
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)
+ }
})
})
}
