telemetry/countertest: generalize Read, ReadStack

This CL removes the requirement that Open be called before
a test reads the value of a counter.

Fixes: golang/go#65600

Change-Id: Ib8b34fa6f01698e2df45158572f8fde16b9b3834
Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/562180
Run-TryBot: Peter Weinberger <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: pjweinbgo

counter/countertest/countertest.go

@@ -9,7 +9,6 @@
 package countertest
 
 import (
-	"fmt"
 	"path/filepath"
 	"sync"
 
@@ -49,16 +48,10 @@ func Open(telemetryDir string) {
 
 // ReadCounter reads the given counter.
 func ReadCounter(c *counter.Counter) (count uint64, _ error) {
-	if !isOpen() {
-		return 0, fmt.Errorf("unmet requirement - Open must be called")
-	}
 	return ic.Read(c)
 }
 
 // ReadStackCounter reads the given StackCounter.
 func ReadStackCounter(c *counter.StackCounter) (stackCounts map[string]uint64, _ error) {
-	if !isOpen() {
-		return nil, fmt.Errorf("unmet requirement - Open must be called")
-	}
 	return ic.ReadStack(c)
 }

internal/counter/counter.go

@@ -293,6 +293,9 @@ func (c *Counter) refresh() {
 // Read reads the given counter.
 // This is the implementation of x/telemetry/counter/countertest.ReadCounter.
 func Read(c *Counter) (uint64, error) {
+	if c.file.current.Load() == nil {
+		return c.state.load().extra(), nil
+	}
 	pf, err := readFile(c.file)
 	if err != nil {
 		return 0, err

internal/counter/counter_test.go

@@ -404,6 +404,17 @@ func TestStack(t *testing.T) {
 			t.Errorf("line %d differs:\n%s\n%s", i, n0[i], n1[i])
 		}
 	}
+	// check that ReadStack gives the same results
+	mp, err := ReadStack(c)
+	if len(mp) != 2 {
+		t.Errorf("ReadStack returned %d values, expected 2", len(mp))
+	}
+	for k, v := range mp {
+		if v != 1 {
+			t.Errorf("got %d for %q, expected 1", v, k)
+		}
+	}
+
 	oldnames := make(map[string]bool)
 	for _, nm := range names {
 		oldnames[nm] = true

internal/counter/rotate_test.go

@@ -44,6 +44,10 @@ func TestRotateCounters(t *testing.T) {
 		// the value of c is in its extra field
 		t.Errorf("got %d, expected 2", state.extra())
 	}
+	// Read should give the same answer
+	if v, err := Read(c); err != nil || v != 2 {
+		t.Errorf("Read got %d, %v, expected 2, nil", v, err)
+	}
 	f.rotate()
 	c.Inc() // this goes through counter.add() safely
 	if c.file.current.Load() == nil {
@@ -60,7 +64,10 @@ func TestRotateCounters(t *testing.T) {
 		// the value of c is in the mapped file
 		t.Errorf("got %d, expected 3", c.ptr.count.Load())
 	}
-
+	// and Read should give the same result
+	if v, err := Read(c); err != nil || v != 3 {
+		t.Errorf("Read gave %d, %v, expected 3, nil", v, err)
+	}
 	// move into the future and rotate the file, remapping it
 	now := getnow()
 	counterTime = func() time.Time { return now.Add(7 * 24 * time.Hour) }

internal/counter/stackcounter.go

@@ -185,17 +185,13 @@ func eq(a, b []uintptr) bool {
 // This is the implementation of
 // golang.org/x/telemetry/counter/countertest.ReadStackCounter.
 func ReadStack(c *StackCounter) (map[string]uint64, error) {
-	pf, err := readFile(c.file)
-	if err != nil {
-		return nil, err
-	}
 	ret := map[string]uint64{}
-	prefix := c.name + "\n"
-
-	for k, v := range pf.Count {
-		if strings.HasPrefix(k, prefix) {
-			ret[k] = v
+	for _, ctr := range c.Counters() {
+		v, err := Read(ctr)
+		if err != nil {
+			return nil, err
 		}
+		ret[DecodeStack(ctr.Name())] = v
 	}
 	return ret, nil
 }