internal/telemetry: centralize telemetry settings (directories and mode)

The LocalDir/UploadDir/Enabled in types.go are computed based on
internal/telemetry.{LocalDir,UploadDir,ModeFile}.

Previously, the init function in types.go created the telemetry
directories, which means, simply depending on the package creates
directories in user's system default configuration location. For
example, by running tests in this repo, users may end up having
telemetry directories. Avoid this eager directory creation.

The previous behavior allowed a process to completely skip telemetry
collection if there is no writable disk space, even when the user
didn't turn off the telemetry. However, we guess os.UserConfigDir
misconfiguration is rare, so we drop this behavior.

We introduce the undocumented EXP_GOTELEMETRYDIR env var here.
A process' default telemetry setting uses directories and files under
a directory determined by the EXP_GOTELEMETRYDIR env var, or
os.UserConfigDir.

EXP_GOTELEMETRYDIR env var allows us to test instrumented
binary's behavior without polluting the user's default
telemetry mode file or data collection directory.
This environment variable is intentionally undocumented
because this may change or get dropped.

And, rename LookupMode to Mode.

Updates golang/go#60967

Change-Id: Ie8ad9bf52a9b5d7fe70b92f6c8ccdeda6dbb1a90
Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/506916
Run-TryBot: Hyang-Ah Hana Kim <[email protected]>
Reviewed-by: Jamal Carvalho <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: hyangah

cmd/gotelemetry/main.go

@@ -20,7 +20,7 @@ func main() {
 
 	args := flag.Args()
 	if len(args) == 0 {
-		fmt.Println(telemetry.LookupMode())
+		fmt.Println(telemetry.Mode())
 		return
 	}
 	switch cmd := args[0]; cmd {

internal/telemetry/mode.go

@@ -13,32 +13,60 @@ import (
 	"strings"
 )
 
-var userConfigDir = os.UserConfigDir
+// The followings are the process' default Settings.
+// The values are subdirectories and a file under
+// os.UserConfigDir()/go/telemetry.
+// For convenience, each field is made to global
+// and they are not supposed to be changed.
+var (
+	// Default directory containing count files and local reports (not yet uploaded)
+	LocalDir string
+	// Default directory containing uploaded reports.
+	UploadDir string
+	// Default file path that holds the telemetry mode info.
+	ModeFile ModeFilePath
+)
 
-// filename returns the default telemetry mode file name.
-func filename() (string, error) {
-	cfgDir, err := userConfigDir()
-	if err != nil {
-		return "", err
-	}
-	// TODO(hyangah): should we consider GOTELEMETRYDIR?
+// ModeFilePath is the telemetry mode file path with methods to manipulate the file contents.
+type ModeFilePath string
 
-	return filepath.Join(cfgDir, "go", "telemetry", "mode"), nil
+func init() {
+	// First check if the undocumented EXP_GOTELEMETRYDIR
+	// env var is set. This variable name may change or
+	// get abandoned in the future.
+	// This knob helps running instrumented binaries in test
+	// without affecting or being affected by the system
+	// default telemetry settings.
+	gotelemetrydir := os.Getenv("EXP_GOTELEMETRYDIR")
+	if gotelemetrydir == "" {
+		cfgDir, err := os.UserConfigDir()
+		if err != nil {
+			return
+		}
+		gotelemetrydir = filepath.Join(cfgDir, "go", "telemetry")
+	}
+	LocalDir = filepath.Join(gotelemetrydir, "local")
+	UploadDir = filepath.Join(gotelemetrydir, "upload")
+	ModeFile = ModeFilePath(filepath.Join(gotelemetrydir, "mode"))
 }
 
 // SetMode updates the telemetry mode with the given mode.
 // Acceptable values for mode are "on", "off", "local", or https:// urls.
 func SetMode(mode string) error {
+	return ModeFile.SetMode(mode)
+}
+
+func (m ModeFilePath) SetMode(mode string) error {
 	switch mode {
 	case "on", "off", "local":
 	default:
 		if !strings.HasPrefix(mode, "https://") {
 			return errors.New("invalid telemetry mode value")
 		}
 	}
-	fname, err := filename()
-	if err != nil {
-		return fmt.Errorf("cannot create a telemetry mode file: %w", err)
+	fname := string(m)
+	if fname == "" {
+		return fmt.Errorf("cannot determine telemetry mode file name")
 	}
 	if err := os.MkdirAll(filepath.Dir(fname), 0755); err != nil {
 		return fmt.Errorf("cannot create a telemetry mode file: %w", err)
@@ -47,11 +75,15 @@ func SetMode(mode string) error {
 	return os.WriteFile(fname, data, 0666)
 }
 
-// LookupMode returns the current telemetry mode.
-func LookupMode() string {
-	fname, err := filename()
-	if err != nil {
-		return "local" // default
+// Mode returns the current telemetry mode.
+func Mode() string {
+	return ModeFile.Mode()
+}
+
+func (m ModeFilePath) Mode() string {
+	fname := string(m)
+	if fname == "" {
+		return "off" // it's likely LocalDir/UploadDir are empty too. Turn off telemetry.
 	}
 	data, err := os.ReadFile(fname)
 	if err != nil {

internal/telemetry/mode_test.go

@@ -6,28 +6,42 @@
 package telemetry
 
 import (
+	"fmt"
 	"os"
+	"path/filepath"
 	"testing"
 )
 
+func TestTelemetryDefault(t *testing.T) {
+	gotelemetrydir := os.Getenv("GOTELEMETRYDIR")
+	if _, err := os.UserConfigDir(); gotelemetrydir == "" && err != nil {
+		if LocalDir != "" || UploadDir != "" || ModeFile != "" {
+			t.Errorf("DefaultSetting: (%q, %q, %q), want non-empty LocalDir/UploadDir/ModeFile", LocalDir, UploadDir, ModeFile)
+		}
+	} else {
+		if LocalDir == "" || UploadDir == "" || ModeFile == "" {
+			t.Errorf("DefaultSetting: (%q, %q, %q), want non-empty LocalDir/UploadDir/ModeFile", LocalDir, UploadDir, ModeFile)
+		}
+	}
+}
 func TestTelemetryModeWithNoModeConfig(t *testing.T) {
-	defer func() { userConfigDir = os.UserConfigDir }()
-
 	tmp := t.TempDir()
-	userConfigDir = func() (string, error) { return tmp, nil }
-
-	got := LookupMode()
-	if got != "local" {
-		t.Fatalf("LookupMode() = %q, want local", got)
+	tests := []struct {
+		modefile ModeFilePath
+		want     string
+	}{
+		{ModeFilePath(filepath.Join(tmp, "mode")), "local"},
+		{"", "off"},
+	}
+	for _, tt := range tests {
+		if got := tt.modefile.Mode(); got != tt.want {
+			t.Logf("Mode file: %q", tt.modefile)
+			t.Errorf("Mode() = %v, want %v", got, tt.want)
+		}
 	}
 }
 
 func TestTelemetryMode(t *testing.T) {
-	defer func() { userConfigDir = os.UserConfigDir }()
-
-	tmp := t.TempDir()
-	userConfigDir = func() (string, error) { return tmp, nil }
-
 	tests := []struct {
 		in      string
 		wantErr bool // want error when setting.
@@ -40,16 +54,18 @@ func TestTelemetryMode(t *testing.T) {
 		{"bogus", true},
 		{"", true},
 	}
-	for _, tt := range tests {
+	tmp := t.TempDir()
+	for i, tt := range tests {
 		t.Run("mode="+tt.in, func(t *testing.T) {
-			setErr := SetMode(tt.in)
+			modefile := ModeFilePath(filepath.Join(tmp, fmt.Sprintf("modefile%d", i)))
+			setErr := modefile.SetMode(tt.in)
 			if (setErr != nil) != tt.wantErr {
 				t.Fatalf("Set() error = %v, wantErr %v", setErr, tt.wantErr)
 			}
 			if setErr != nil {
 				return
 			}
-			if got := LookupMode(); got != tt.in {
+			if got := modefile.Mode(); got != tt.in {
 				t.Errorf("LookupMode() = %q, want %q", got, tt.in)
 			}
 		})

types.go

@@ -5,9 +5,6 @@
 package telemetry
 
 import (
-	"os"
-	"path/filepath"
-
 	"golang.org/x/telemetry/internal/telemetry"
 )
 
@@ -59,36 +56,9 @@ type ProgramReport struct {
 
 var (
 	// directory containing count files and local (not to be uploaded) reports
-	LocalDir string
+	LocalDir = telemetry.LocalDir
 	// directory containing uploaded reports
-	UploadDir string
+	UploadDir = telemetry.UploadDir
 	// whether telemetry is enabled
-	Enabled bool
+	Enabled bool = telemetry.Mode() != "off"
 )
-
-// init() sets LocalDir and UploadDir. Users must not change these.
-// If the directories cannot be found or set, telemetry is disabled.
-func init() {
-	mode := telemetry.LookupMode()
-	if mode == "off" {
-		return
-	}
-
-	env, err := os.UserConfigDir()
-	if err != nil {
-		return
-	}
-	env = filepath.Join(env, "go", "telemetry")
-
-	l := filepath.Join(env, "local")
-	u := filepath.Join(env, "upload")
-	if err := os.MkdirAll(l, 0755); err != nil {
-		return
-	}
-	if err := os.MkdirAll(u, 0755); err != nil {
-		return
-	}
-	LocalDir = l
-	UploadDir = u
-	Enabled = true
-}