Skip to content

Commit

Permalink
[gopls-release-branch.0.16] gopls/internal/server: add counters to in…
Browse files Browse the repository at this point in the history
…form 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 fcf5463)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/595836
Auto-Submit: Robert Findley <rfindley@google.com>
  • Loading branch information
findleyr authored and gopherbot committed Jun 28, 2024
1 parent 76e6044 commit 9c895dd
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 7 deletions.
2 changes: 2 additions & 0 deletions gopls/internal/cache/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions gopls/internal/server/general.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions gopls/internal/server/prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,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"
)
Expand Down Expand Up @@ -236,6 +237,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)
Expand Down
9 changes: 7 additions & 2 deletions gopls/internal/telemetry/telemetry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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 {
Expand All @@ -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})
Expand All @@ -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)
Expand Down
11 changes: 9 additions & 2 deletions gopls/internal/test/integration/misc/misc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 30 additions & 3 deletions gopls/internal/test/integration/misc/prompt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"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"
Expand Down Expand Up @@ -69,6 +71,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
Expand All @@ -81,18 +87,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")
msgRE := regexp.MustCompile(".*Would you like to enable Go telemetry?")
respond := func(m *protocol.ShowMessageRequestParams) (*protocol.MessageActionItem, error) {
Expand Down Expand Up @@ -136,6 +156,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)
}
})
})
}
Expand Down

0 comments on commit 9c895dd

Please sign in to comment.