Skip to content

Commit c8718f6

Browse files
committed
cmd/go: add file with list of all counters we collect
Maintain a list of counters we collect and test that it hasn't changed. If it has, fail a test and have the user update the list. The update process will print a reminder to update the list of collected counters. Also run go mod vendor to pull in golang.org/x/telemetry/counter/countertest. For #58894 Change-Id: I661a9c3d67cb33f42a5519f4639af7aa05c3821d Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest Reviewed-on: https://go-review.googlesource.com/c/go/+/564555 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
1 parent de487d5 commit c8718f6

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+7320
-6
lines changed

src/cmd/go/counters_test.go

+89
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Copyright 2024 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package main_test
6+
7+
import (
8+
"cmd/go/internal/base"
9+
"flag"
10+
"internal/diff"
11+
"os"
12+
"slices"
13+
"strings"
14+
"testing"
15+
)
16+
17+
var update = flag.Bool("update", false, "if true update testdata/counternames.txt")
18+
19+
func TestCounterNamesUpToDate(t *testing.T) {
20+
if !*update {
21+
t.Parallel()
22+
}
23+
24+
var counters []string
25+
// -C is a special case because it's handled by handleChdirFlag rather than
26+
// standard flag processing with FlagSets.
27+
// cmd/go/subcommand:unknown is also a special case: it's used when the subcommand
28+
// doesn't match any of the known commands.
29+
counters = append(counters, "cmd/go/flag:C", "cmd/go/subcommand:unknown")
30+
counters = append(counters, flagscounters("cmd/go/flag:", *flag.CommandLine)...)
31+
32+
for _, cmd := range base.Go.Commands {
33+
counters = append(counters, cmdcounters(nil, cmd)...)
34+
}
35+
cstr := []byte(strings.Join(counters, "\n") + "\n")
36+
37+
const counterNamesFile = "testdata/counters.txt"
38+
old, err := os.ReadFile(counterNamesFile)
39+
if err != nil {
40+
t.Fatalf("error reading %s: %v", counterNamesFile, err)
41+
}
42+
diff := diff.Diff(counterNamesFile, old, "generated counter names", cstr)
43+
if diff == nil {
44+
t.Logf("%s is up to date.", counterNamesFile)
45+
return
46+
}
47+
48+
if *update {
49+
if err := os.WriteFile(counterNamesFile, cstr, 0666); err != nil {
50+
t.Fatal(err)
51+
}
52+
t.Logf("wrote %d bytes to %s", len(cstr), counterNamesFile)
53+
t.Logf("don't forget to file a proposal to update the list of collected counters")
54+
} else {
55+
t.Logf("\n%s", diff)
56+
t.Errorf("%s is stale. To update, run 'go generate cmd/go'.", counterNamesFile)
57+
}
58+
}
59+
60+
func flagscounters(prefix string, flagSet flag.FlagSet) []string {
61+
var counters []string
62+
flagSet.VisitAll(func(f *flag.Flag) {
63+
counters = append(counters, prefix+f.Name)
64+
})
65+
return counters
66+
}
67+
68+
func cmdcounters(previous []string, cmd *base.Command) []string {
69+
const subcommandPrefix = "cmd/go/subcommand:"
70+
const flagPrefix = "cmd/go/flag:"
71+
var counters []string
72+
previousComponent := strings.Join(previous, "-")
73+
if len(previousComponent) > 0 {
74+
previousComponent += "-"
75+
}
76+
if cmd.Runnable() {
77+
counters = append(counters, subcommandPrefix+previousComponent+cmd.Name())
78+
}
79+
counters = append(counters, flagscounters(flagPrefix+previousComponent+cmd.Name()+"-", cmd.Flag)...)
80+
if len(previous) != 0 {
81+
counters = append(counters, subcommandPrefix+previousComponent+"help-"+cmd.Name())
82+
}
83+
counters = append(counters, subcommandPrefix+"help-"+previousComponent+cmd.Name())
84+
85+
for _, subcmd := range cmd.Commands {
86+
counters = append(counters, cmdcounters(append(slices.Clone(previous), cmd.Name()), subcmd)...)
87+
}
88+
return counters
89+
}

src/cmd/go/go_test.go

+11
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ import (
4444
"cmd/internal/sys"
4545

4646
cmdgo "cmd/go"
47+
48+
"golang.org/x/telemetry/counter/countertest"
4749
)
4850

4951
func init() {
@@ -153,6 +155,15 @@ func TestMain(m *testing.M) {
153155
web.EnableTestHooks(interceptors)
154156
}
155157

158+
cmdgo.TelemetryStart = func() {
159+
// TODO(matloob): we'll ideally want to call telemetry.Start here
160+
// but it calls counter.Open, which we don't want to do because
161+
// we want to call countertest.Open.
162+
if telemetryDir := os.Getenv("TESTGO_TELEMETRY_DIR"); telemetryDir != "" {
163+
countertest.Open(telemetryDir)
164+
}
165+
}
166+
156167
cmdgo.Main()
157168
os.Exit(0)
158169
}

src/cmd/go/main.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// license that can be found in the LICENSE file.
44

55
//go:generate go test cmd/go -v -run=^TestDocsUpToDate$ -fixdocs
6+
//go:generate go test cmd/go -v -run=^TestCounterNamesUpToDate$ -update
67

78
package main
89

@@ -91,7 +92,7 @@ var _ = go11tag
9192

9293
func main() {
9394
log.SetFlags(0)
94-
counter.Open() // Open the telemetry counter file so counters can be written to it.
95+
TelemetryStart() // Open the telemetry counter file so counters can be written to it.
9596
handleChdirFlag()
9697
toolchain.Select()
9798

@@ -153,7 +154,6 @@ func main() {
153154

154155
cmd, used := lookupCmd(args)
155156
cfg.CmdName = strings.Join(args[:used], " ")
156-
counter.Inc("cmd/go/subcommand:" + strings.ReplaceAll(cfg.CmdName, " ", "-"))
157157
if len(cmd.Commands) > 0 {
158158
if used >= len(args) {
159159
help.PrintUsage(os.Stderr, cmd)
@@ -162,6 +162,7 @@ func main() {
162162
}
163163
if args[used] == "help" {
164164
// Accept 'go mod help' and 'go mod help foo' for 'go help mod' and 'go help mod foo'.
165+
counter.Inc("cmd/go/subcommand:" + strings.ReplaceAll(cfg.CmdName, " ", "-") + "-" + strings.Join(args[used:], "-"))
165166
help.Help(os.Stdout, append(slices.Clip(args[:used]), args[used+1:]...))
166167
base.Exit()
167168
}
@@ -173,10 +174,12 @@ func main() {
173174
if cmdName == "" {
174175
cmdName = args[0]
175176
}
177+
counter.Inc("cmd/go/subcommand:unknown")
176178
fmt.Fprintf(os.Stderr, "go %s: unknown command\nRun 'go help%s' for usage.\n", cmdName, helpArg)
177179
base.SetExitStatus(2)
178180
base.Exit()
179181
}
182+
counter.Inc("cmd/go/subcommand:" + strings.ReplaceAll(cfg.CmdName, " ", "-"))
180183
invoke(cmd, args[used-1:])
181184
base.Exit()
182185
}
@@ -241,7 +244,7 @@ func invoke(cmd *base.Command, args []string) {
241244
} else {
242245
base.SetFromGOFLAGS(&cmd.Flag)
243246
cmd.Flag.Parse(args[1:])
244-
counter.CountFlags("cmd/go/flag:"+cmd.Name()+"-", cmd.Flag)
247+
counter.CountFlags("cmd/go/flag:"+strings.ReplaceAll(cfg.CmdName, " ", "-")+"-", cmd.Flag)
245248
args = cmd.Flag.Args()
246249
}
247250

@@ -326,7 +329,7 @@ func handleChdirFlag() {
326329
_, dir, _ = strings.Cut(a, "=")
327330
os.Args = slices.Delete(os.Args, used, used+1)
328331
}
329-
counter.Inc("cmd/go:flag-C")
332+
counter.Inc("cmd/go/flag:C")
330333

331334
if err := os.Chdir(dir); err != nil {
332335
base.Fatalf("go: %v", err)

src/cmd/go/script_test.go

+62-2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"bufio"
1414
"bytes"
1515
"context"
16+
_ "embed"
1617
"flag"
1718
"internal/testenv"
1819
"internal/txtar"
@@ -21,6 +22,7 @@ import (
2122
"path/filepath"
2223
"runtime"
2324
"strings"
25+
"sync"
2426
"testing"
2527
"time"
2628

@@ -29,6 +31,8 @@ import (
2931
"cmd/go/internal/script"
3032
"cmd/go/internal/script/scripttest"
3133
"cmd/go/internal/vcweb/vcstest"
34+
35+
"golang.org/x/telemetry/counter/countertest"
3236
)
3337

3438
var testSum = flag.String("testsum", "", `may be tidy, listm, or listall. If set, TestScript generates a go.sum file at the beginning of each test and updates test files if they pass.`)
@@ -124,7 +128,7 @@ func TestScript(t *testing.T) {
124128
if err != nil {
125129
t.Fatal(err)
126130
}
127-
initScriptDirs(t, s)
131+
telemetryDir := initScriptDirs(t, s)
128132
if err := s.ExtractFiles(a); err != nil {
129133
t.Fatal(err)
130134
}
@@ -154,6 +158,7 @@ func TestScript(t *testing.T) {
154158
// will work better seeing the full path relative to cmd/go
155159
// (where the "go test" command is usually run).
156160
scripttest.Run(t, engine, s, file, bytes.NewReader(a.Comment))
161+
checkCounters(t, telemetryDir)
157162
})
158163
}
159164
}
@@ -177,7 +182,7 @@ func tbFromContext(ctx context.Context) (testing.TB, bool) {
177182

178183
// initScriptState creates the initial directory structure in s for unpacking a
179184
// cmd/go script.
180-
func initScriptDirs(t testing.TB, s *script.State) {
185+
func initScriptDirs(t testing.TB, s *script.State) (telemetryDir string) {
181186
must := func(err error) {
182187
if err != nil {
183188
t.Helper()
@@ -188,6 +193,10 @@ func initScriptDirs(t testing.TB, s *script.State) {
188193
work := s.Getwd()
189194
must(s.Setenv("WORK", work))
190195

196+
telemetryDir = filepath.Join(work, "telemetry")
197+
must(os.MkdirAll(telemetryDir, 0777))
198+
must(s.Setenv("TESTGO_TELEMETRY_DIR", filepath.Join(work, "telemetry")))
199+
191200
must(os.MkdirAll(filepath.Join(work, "tmp"), 0777))
192201
must(s.Setenv(tempEnvName(), filepath.Join(work, "tmp")))
193202

@@ -196,6 +205,7 @@ func initScriptDirs(t testing.TB, s *script.State) {
196205
gopathSrc := filepath.Join(gopath, "src")
197206
must(os.MkdirAll(gopathSrc, 0777))
198207
must(s.Chdir(gopathSrc))
208+
return telemetryDir
199209
}
200210

201211
func scriptEnv(srv *vcstest.Server, srvCertFile string) ([]string, error) {
@@ -357,3 +367,53 @@ func updateSum(t testing.TB, e *script.Engine, s *script.State, archive *txtar.A
357367
}
358368
return rewrite
359369
}
370+
371+
func readCounters(t *testing.T, telemetryDir string) map[string]uint64 {
372+
localDir := filepath.Join(telemetryDir, "local")
373+
dirents, err := os.ReadDir(localDir)
374+
if err != nil {
375+
if os.IsNotExist(err) {
376+
return nil // The Go command didn't ever run so the local dir wasn't created
377+
}
378+
t.Fatalf("reading telemetry local dir: %v", err)
379+
}
380+
totals := map[string]uint64{}
381+
for _, dirent := range dirents {
382+
if dirent.IsDir() || !strings.HasSuffix(dirent.Name(), ".count") {
383+
// not a counter file
384+
continue
385+
}
386+
counters, _, err := countertest.ReadFile(filepath.Join(localDir, dirent.Name()))
387+
if err != nil {
388+
t.Fatalf("reading counter file: %v", err)
389+
}
390+
for k, v := range counters {
391+
totals[k] += v
392+
}
393+
}
394+
395+
return totals
396+
}
397+
398+
//go:embed testdata/counters.txt
399+
var countersTxt string
400+
401+
var (
402+
allowedCountersOnce sync.Once
403+
allowedCounters = map[string]bool{} // Set of allowed counters.
404+
)
405+
406+
func checkCounters(t *testing.T, telemetryDir string) {
407+
allowedCountersOnce.Do(func() {
408+
for _, counter := range strings.Fields(countersTxt) {
409+
allowedCounters[counter] = true
410+
}
411+
})
412+
counters := readCounters(t, telemetryDir)
413+
for name := range counters {
414+
if !allowedCounters[name] {
415+
t.Fatalf("incremented counter %q is not in testdata/counters.txt. "+
416+
"Please update counters_test.go to produce an entry for it.", name)
417+
}
418+
}
419+
}

src/cmd/go/telemetry.go

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Copyright 2024 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
//go:build !cmd_go_bootstrap
6+
7+
package main
8+
9+
import "golang.org/x/telemetry"
10+
11+
var TelemetryStart = func() {
12+
telemetry.Start(telemetry.Config{Upload: true})
13+
}

src/cmd/go/telemetry_bootstrap.go

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Copyright 2024 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
//go:build cmd_go_bootstrap
6+
7+
package main
8+
9+
var TelemetryStart = func() {}

0 commit comments

Comments
 (0)