Skip to content

Commit 5e7a336

Browse files
findleyrfindleyr
committed
internal/telemetry: encapsulate the telemetry directory
In several places throughout tests, we set all of telemetry.{LocalDir, UploadDir, Mode} according to the standard schema ("local/", "upload/", and "mode"). In the interest of reducing or eliminating global state to improve testability, introduce a telemetry.Dir type which encapsulates the telemetry directory layout. The default layout is accessed through telemetry.Default. This makes it easier for future components to close over or pass around a single piece of state (the Dir). In subsequent CLs, this will be used to make uploading more testable. For golang/go#66003 Change-Id: I31db8df20f133d834219ff17c2fe2d4f9e446b5d Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/575059 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Peter Weinberger <[email protected]>
1 parent b93185a commit 5e7a336

File tree

18 files changed

+193
-180
lines changed

18 files changed

+193
-180
lines changed

‎cmd/gotelemetry/internal/csv/csv.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ type file struct {
3232
}
3333

3434
func Csv() {
35-
files, err := readdir(telemetry.LocalDir, nil)
35+
files, err := readdir(telemetry.Default.LocalDir(), nil)
3636
if err != nil {
3737
log.Fatal(err)
3838
}

‎cmd/gotelemetry/internal/view/view.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,16 +114,17 @@ func (s *Server) handleIndex(fsys fs.FS) handlerFunc {
114114
if err != nil {
115115
return err
116116
}
117-
if _, err := os.Stat(telemetry.LocalDir); err != nil {
117+
localDir := telemetry.Default.LocalDir()
118+
if _, err := os.Stat(localDir); err != nil {
118119
return fmt.Errorf(
119120
`The telemetry dir %s does not exist.
120-
There is nothing to report.`, telemetry.LocalDir)
121+
There is nothing to report.`, telemetry.Default.LocalDir())
121122
}
122-
reports, err := reports(telemetry.LocalDir, cfg)
123+
reports, err := reports(localDir, cfg)
123124
if err != nil {
124125
return err
125126
}
126-
files, err := files(telemetry.LocalDir, cfg)
127+
files, err := files(localDir, cfg)
127128
if err != nil {
128129
return err
129130
}

‎cmd/gotelemetry/main.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -213,10 +213,10 @@ func help(name string) {
213213
}
214214

215215
func runOn(_ []string) {
216-
if old, _ := telemetry.Mode(); old == "on" {
216+
if old, _ := telemetry.Default.Mode(); old == "on" {
217217
return
218218
}
219-
if err := telemetry.SetMode("on"); err != nil {
219+
if err := telemetry.Default.SetMode("on"); err != nil {
220220
failf("Failed to enable telemetry: %v", err)
221221
}
222222
// We could perhaps only show the telemetry on message when the mode goes
@@ -236,19 +236,19 @@ To disable both collection and uploading, run “gotelemetry off“.`
236236
}
237237

238238
func runLocal(_ []string) {
239-
if old, _ := telemetry.Mode(); old == "local" {
239+
if old, _ := telemetry.Default.Mode(); old == "local" {
240240
return
241241
}
242-
if err := telemetry.SetMode("local"); err != nil {
242+
if err := telemetry.Default.SetMode("local"); err != nil {
243243
failf("Failed to set the telemetry mode to local: %v", err)
244244
}
245245
}
246246

247247
func runOff(_ []string) {
248-
if old, _ := telemetry.Mode(); old == "off" {
248+
if old, _ := telemetry.Default.Mode(); old == "off" {
249249
return
250250
}
251-
if err := telemetry.SetMode("off"); err != nil {
251+
if err := telemetry.Default.SetMode("off"); err != nil {
252252
failf("Failed to disable telemetry: %v", err)
253253
}
254254
}
@@ -258,21 +258,21 @@ func runView(_ []string) {
258258
}
259259

260260
func runEnv(_ []string) {
261-
m, t := telemetry.Mode()
261+
m, t := telemetry.Default.Mode()
262262
fmt.Printf("mode: %s %s\n", m, t)
263263
fmt.Println()
264-
fmt.Println("modefile:", telemetry.ModeFile)
265-
fmt.Println("localdir:", telemetry.LocalDir)
266-
fmt.Println("uploaddir:", telemetry.UploadDir)
264+
fmt.Println("modefile:", telemetry.Default.ModeFile())
265+
fmt.Println("localdir:", telemetry.Default.LocalDir())
266+
fmt.Println("uploaddir:", telemetry.Default.UploadDir())
267267
}
268268

269269
func runClean(_ []string) {
270270
// For now, be careful to only remove counter files and reports.
271271
// It would probably be OK to just remove everything, but it may
272272
// be useful to preserve the weekends file.
273273
for dir, suffixes := range map[string][]string{
274-
telemetry.LocalDir: {"." + counter.FileVersion + ".count", ".json"},
275-
telemetry.UploadDir: {".json"},
274+
telemetry.Default.LocalDir(): {"." + counter.FileVersion + ".count", ".json"},
275+
telemetry.Default.UploadDir(): {".json"},
276276
} {
277277
entries, err := os.ReadDir(dir)
278278
if err != nil {
@@ -307,7 +307,7 @@ func runCSV(_ []string) {
307307

308308
func runDump(args []string) {
309309
if len(args) == 0 {
310-
localdir := telemetry.LocalDir
310+
localdir := telemetry.Default.LocalDir()
311311
fi, err := os.ReadDir(localdir)
312312
if err != nil && len(args) == 0 {
313313
log.Fatal(err)

‎counter/countertest/countertest.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
package countertest
88

99
import (
10-
"path/filepath"
1110
"sync"
1211

1312
"golang.org/x/telemetry/counter"
@@ -39,9 +38,7 @@ func Open(telemetryDir string) {
3938
if opened {
4039
panic("Open was called more than once")
4140
}
42-
telemetry.ModeFile = telemetry.ModeFilePath(filepath.Join(telemetryDir, "mode"))
43-
telemetry.LocalDir = filepath.Join(telemetryDir, "local")
44-
telemetry.UploadDir = filepath.Join(telemetryDir, "upload")
41+
telemetry.Default = telemetry.NewDir(telemetryDir)
4542

4643
counter.Open()
4744
opened = true

‎internal/counter/counter_test.go

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func TestBasic(t *testing.T) {
7070

7171
func TestMissingLocalDir(t *testing.T) {
7272
testenv.SkipIfUnsupportedPlatform(t)
73-
err := os.RemoveAll(telemetry.LocalDir)
73+
err := os.RemoveAll(telemetry.Default.LocalDir())
7474
if err != nil {
7575
t.Fatal(err)
7676
}
@@ -236,7 +236,7 @@ func TestNewFile(t *testing.T) {
236236
var f file
237237
c := f.New("gophers")
238238
// shouldn't see a file yet
239-
fi, err := os.ReadDir(telemetry.LocalDir)
239+
fi, err := os.ReadDir(telemetry.Default.LocalDir())
240240
if err != nil {
241241
t.Fatal(err)
242242
}
@@ -245,7 +245,7 @@ func TestNewFile(t *testing.T) {
245245
}
246246
c.Add(9)
247247
// still shouldn't see a file
248-
fi, err = os.ReadDir(telemetry.LocalDir)
248+
fi, err = os.ReadDir(telemetry.Default.LocalDir())
249249
if err != nil {
250250
close(&f)
251251
t.Fatal(err)
@@ -256,7 +256,7 @@ func TestNewFile(t *testing.T) {
256256
}
257257
f.rotate()
258258
// now we should see a count file and a weekends file
259-
fi, _ = os.ReadDir(telemetry.LocalDir)
259+
fi, _ = os.ReadDir(telemetry.Default.LocalDir())
260260
if len(fi) != 2 {
261261
close(&f)
262262
t.Fatalf("len(fi) = %d, want 2", len(fi))
@@ -267,7 +267,7 @@ func TestNewFile(t *testing.T) {
267267
case "weekends":
268268
weekendsFile = f.Name()
269269
// while we're here, check that is ok
270-
buf, err := os.ReadFile(filepath.Join(telemetry.LocalDir, weekendsFile))
270+
buf, err := os.ReadFile(filepath.Join(telemetry.Default.LocalDir(), weekendsFile))
271271
if err != nil {
272272
t.Fatal(err)
273273
}
@@ -280,7 +280,7 @@ func TestNewFile(t *testing.T) {
280280
}
281281
}
282282

283-
buf, err := os.ReadFile(filepath.Join(telemetry.LocalDir, countFile))
283+
buf, err := os.ReadFile(filepath.Join(telemetry.Default.LocalDir(), countFile))
284284
if err != nil {
285285
close(&f)
286286
t.Fatal(err)
@@ -303,8 +303,8 @@ func TestNewFile(t *testing.T) {
303303
}
304304
close(&f)
305305
// remove the file for the next iteration of the loop
306-
os.Remove(filepath.Join(telemetry.LocalDir, countFile))
307-
os.Remove(filepath.Join(telemetry.LocalDir, weekendsFile))
306+
os.Remove(filepath.Join(telemetry.Default.LocalDir(), countFile))
307+
os.Remove(filepath.Join(telemetry.Default.LocalDir(), weekendsFile))
308308
}
309309
}
310310

@@ -316,12 +316,12 @@ func TestWeekends(t *testing.T) {
316316
for i := 0; i < 7; i++ {
317317
counterTime = future(i)
318318
for index := range "0123456" {
319-
os.WriteFile(filepath.Join(telemetry.LocalDir, "weekends"), []byte{byte(index + '0')}, 0666)
319+
os.WriteFile(filepath.Join(telemetry.Default.LocalDir(), "weekends"), []byte{byte(index + '0')}, 0666)
320320
var f file
321321
c := f.New("gophers")
322322
c.Add(7)
323323
f.rotate()
324-
fis, err := os.ReadDir(telemetry.LocalDir)
324+
fis, err := os.ReadDir(telemetry.Default.LocalDir())
325325
if err != nil {
326326
t.Fatal(err)
327327
}
@@ -330,11 +330,11 @@ func TestWeekends(t *testing.T) {
330330
for _, fi := range fis {
331331
// ignore errors for brevity: something else will fail
332332
if fi.Name() == "weekends" {
333-
buf, _ := os.ReadFile(filepath.Join(telemetry.LocalDir, fi.Name()))
333+
buf, _ := os.ReadFile(filepath.Join(telemetry.Default.LocalDir(), fi.Name()))
334334
buf = bytes.TrimSpace(buf)
335335
weekends = time.Weekday(buf[0] - '0')
336336
} else if strings.HasSuffix(fi.Name(), ".count") {
337-
buf, _ := os.ReadFile(filepath.Join(telemetry.LocalDir, fi.Name()))
337+
buf, _ := os.ReadFile(filepath.Join(telemetry.Default.LocalDir(), fi.Name()))
338338
parsed, _ := Parse(fi.Name(), buf)
339339
begins, _ = time.Parse(time.RFC3339, parsed.Meta["TimeBegin"])
340340
ends, _ = time.Parse(time.RFC3339, parsed.Meta["TimeEnd"])
@@ -361,7 +361,7 @@ func TestWeekends(t *testing.T) {
361361
close(&f)
362362
// remove files for the next iteration of the loop
363363
for _, f := range fis {
364-
os.Remove(filepath.Join(telemetry.LocalDir, f.Name()))
364+
os.Remove(filepath.Join(telemetry.Default.LocalDir(), f.Name()))
365365
}
366366
}
367367
}
@@ -505,13 +505,9 @@ func fn(t *testing.T, n int, c *StackCounter) {
505505

506506
func setup(t *testing.T) {
507507
log.SetFlags(log.Lshortfile)
508-
tmpDir := t.TempDir() // new dir for each test
509-
telemetry.LocalDir = tmpDir + "/local"
510-
telemetry.UploadDir = tmpDir + "/upload"
511-
os.MkdirAll(telemetry.LocalDir, 0777)
512-
os.MkdirAll(telemetry.UploadDir, 0777)
513-
telemetry.ModeFile = telemetry.ModeFilePath(filepath.Join(tmpDir, "mode"))
514-
// os.UserConfigDir() is "" in tests so no point in looking at it
508+
telemetry.Default = telemetry.NewDir(t.TempDir()) // new dir for each test
509+
os.MkdirAll(telemetry.Default.LocalDir(), 0777)
510+
os.MkdirAll(telemetry.Default.UploadDir(), 0777)
515511
}
516512

517513
func restore() {

‎internal/counter/file.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,11 @@ func (f *file) init(begin, end time.Time) {
124124
f.err = errNoBuildInfo
125125
return
126126
}
127-
if mode, _ := telemetry.Mode(); mode == "off" {
127+
if mode, _ := telemetry.Default.Mode(); mode == "off" {
128128
f.err = ErrDisabled
129129
return
130130
}
131-
dir := telemetry.LocalDir
131+
dir := telemetry.Default.LocalDir()
132132

133133
if err := os.MkdirAll(dir, 0777); err != nil {
134134
f.err = err
@@ -203,11 +203,11 @@ func fileValidity(now time.Time) (int, error) {
203203
// If there is no 'weekends' file create it and initialize it
204204
// to a random day of the week. There is a short interval for
205205
// a race.
206-
weekends := filepath.Join(telemetry.LocalDir, "weekends")
206+
weekends := filepath.Join(telemetry.Default.LocalDir(), "weekends")
207207
day := fmt.Sprintf("%d\n", rand.Intn(7))
208208
if _, err := os.ReadFile(weekends); err != nil {
209-
if err := os.MkdirAll(telemetry.LocalDir, 0777); err != nil {
210-
debugPrintf("%v: could not create telemetry.LocalDir %s", err, telemetry.LocalDir)
209+
if err := os.MkdirAll(telemetry.Default.LocalDir(), 0777); err != nil {
210+
debugPrintf("%v: could not create telemetry.LocalDir %s", err, telemetry.Default.LocalDir())
211211
return 7, err
212212
}
213213
if err = os.WriteFile(weekends, []byte(day), 0666); err != nil {
@@ -357,7 +357,7 @@ func Open() func() {
357357
if telemetry.DisabledOnPlatform {
358358
return func() {}
359359
}
360-
if mode, _ := telemetry.Mode(); mode == "off" {
360+
if mode, _ := telemetry.Default.Mode(); mode == "off" {
361361
// Don't open the file when telemetry is off.
362362
defaultFile.err = ErrDisabled
363363
return func() {} // No need to clean up.

‎internal/counter/rotate_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func TestRotate(t *testing.T) {
147147
setup(t)
148148
defer restore()
149149
// pretend something was uploaded
150-
os.WriteFile(filepath.Join(telemetry.UploadDir, "anything"), []byte{}, 0666)
150+
os.WriteFile(filepath.Join(telemetry.Default.UploadDir(), "anything"), []byte{}, 0666)
151151
var f file
152152
defer close(&f)
153153
c := f.New("gophers")
@@ -156,7 +156,7 @@ func TestRotate(t *testing.T) {
156156
for i := 0; i < 2; i++ {
157157
// nothing should change on the second rotate
158158
f.rotate()
159-
fi, err := os.ReadDir(telemetry.LocalDir)
159+
fi, err := os.ReadDir(telemetry.Default.LocalDir())
160160
if err != nil || len(fi) != 2 {
161161
t.Fatalf("err=%v, len(fi) = %d, want 2", err, len(fi))
162162
}
@@ -170,7 +170,7 @@ func TestRotate(t *testing.T) {
170170
if us != now {
171171
t.Errorf("us = %v, want %v, i=%d y=%s", us, now, i, y)
172172
}
173-
fd, err := os.Open(filepath.Join(telemetry.LocalDir, fi[0].Name()))
173+
fd, err := os.Open(filepath.Join(telemetry.Default.LocalDir(), fi[0].Name()))
174174
if err != nil {
175175
t.Fatal(err)
176176
}
@@ -189,7 +189,7 @@ func TestRotate(t *testing.T) {
189189
}
190190
counterTime = func() time.Time { return now.Add(7 * 24 * time.Hour) }
191191
f.rotate()
192-
fi, err := os.ReadDir(telemetry.LocalDir)
192+
fi, err := os.ReadDir(telemetry.Default.LocalDir())
193193
if err != nil || len(fi) != 3 {
194194
t.Fatalf("err=%v, len(fi) = %d, want 3", err, len(fi))
195195
}

‎internal/regtest/e2e_test.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,19 @@ func TestE2E_off(t *testing.T) {
7373

7474
for _, test := range tests {
7575
t.Run(fmt.Sprintf("mode=%s", test.mode), func(t *testing.T) {
76-
telemetryDir := t.TempDir()
76+
dir := telemetry.NewDir(t.TempDir())
7777
if test.mode != "" {
78-
if err := telemetry.ModeFilePath(filepath.Join(telemetryDir, "mode")).SetMode(test.mode); err != nil {
78+
if err := dir.SetMode(test.mode); err != nil {
7979
t.Fatalf("SetMode failed: %v", err)
8080
}
8181
}
82-
out, err := RunProg(t, telemetryDir, prog)
82+
out, err := RunProg(t, dir.Dir(), prog)
8383
if err != nil {
8484
t.Fatalf("program failed unexpectedly (%v)\n%s", err, out)
8585
}
86-
localDir := filepath.Join(telemetryDir, "local")
87-
_, err = os.Stat(localDir)
86+
_, err = os.Stat(dir.LocalDir())
8887
if err != nil && !os.IsNotExist(err) {
89-
t.Fatalf("os.Stat(%q): unexpected error: %v", localDir, err)
88+
t.Fatalf("os.Stat(%q): unexpected error: %v", dir.LocalDir(), err)
9089
}
9190
if gotLocalDir := err == nil; gotLocalDir != test.wantLocalDir {
9291
t.Errorf("got /local dir: %v, want %v; out:\n%s", gotLocalDir, test.wantLocalDir, string(out))

0 commit comments

Comments
 (0)