Save Logging Information in Data Directory If Provided (#2415)

Our logic should be as follows:

When Loading:

We first look inside the provided data-directory. If a config file is there, load it and return it
Otherwise, look in the global directory. If a config file is there, load it and return it.
When Saving:

If a data-directory was provided then save the config file there.
Otherwise, save the config file in the global directory

logging/telemetry.go

@@ -93,24 +93,42 @@ func makeTelemetryState(cfg TelemetryConfig, hookFactory hookFactory) (*telemetr
 func ReadTelemetryConfigOrDefault(dataDir string, genesisID string) (cfg TelemetryConfig, err error) {
 	err = nil
 	dataDirProvided := dataDir != ""
+	var configPath string
+
+	// If we have a data directory, then load the config
 	if dataDirProvided {
-		configPath := filepath.Join(dataDir, TelemetryConfigFilename)
+		configPath = filepath.Join(dataDir, TelemetryConfigFilename)
+		// Load the config, if the GUID is there then we are all set
+		// However if it isn't there then we must create it, save the file and load it.
 		cfg, err = LoadTelemetryConfig(configPath)
 	}
+
+	// We couldn't load the telemetry config for some reason
+	// If the reason is because the directory doesn't exist or we didn't provide a data directory then...
 	if (err != nil && os.IsNotExist(err)) || !dataDirProvided {
-		var configPath string
+
 		configPath, err = config.GetConfigFilePath(TelemetryConfigFilename)
 		if err != nil {
+			// In this case we don't know what to do since we couldn't
+			// create the directory.  Just create an ephemeral config.
 			cfg = createTelemetryConfig()
 			return
 		}
+
+		// Load the telemetry from the default config path
 		cfg, err = LoadTelemetryConfig(configPath)
 	}
+
+	// If there was some error loading the configuration from the config path...
 	if err != nil {
+		// Create an ephemeral config
 		cfg = createTelemetryConfig()
+
+		// If the error was that the the config wasn't there then it wasn't really an error
 		if os.IsNotExist(err) {
 			err = nil
 		} else {
+			// The error was actually due to a malformed config file...just return
 			return
 		}
 	}
@@ -134,9 +152,20 @@ func EnsureTelemetryConfig(dataDir *string, genesisID string) (TelemetryConfig,
 // EnsureTelemetryConfigCreated is the same as EnsureTelemetryConfig but it also returns a bool indicating
 // whether EnsureTelemetryConfig had to create the config.
 func EnsureTelemetryConfigCreated(dataDir *string, genesisID string) (TelemetryConfig, bool, error) {
+	/*
+		Our logic should be as follows:
+			- We first look inside the provided data-directory.  If a config file is there, load it
+			  and return it
+			- Otherwise, look in the global directory.  If a config file is there, load it and return it.
+			- Otherwise, if a data-directory was provided then save the config file there.
+			- Otherwise, save the config file in the global directory
+
+	*/
+
 	configPath := ""
 	var cfg TelemetryConfig
 	var err error
+
 	if dataDir != nil && *dataDir != "" {
 		configPath = filepath.Join(*dataDir, TelemetryConfigFilename)
 		cfg, err = LoadTelemetryConfig(configPath)
@@ -149,6 +178,8 @@ func EnsureTelemetryConfigCreated(dataDir *string, genesisID string) (TelemetryC
 		configPath, err = config.GetConfigFilePath(TelemetryConfigFilename)
 		if err != nil {
 			cfg := createTelemetryConfig()
+			// Since GetConfigFilePath failed, there is no chance that we
+			// can save the next config files
 			return cfg, true, err
 		}
 		cfg, err = LoadTelemetryConfig(configPath)
@@ -158,6 +189,23 @@ func EnsureTelemetryConfigCreated(dataDir *string, genesisID string) (TelemetryC
 		err = nil
 		created = true
 		cfg = createTelemetryConfig()
+
+		if dataDir != nil && *dataDir != "" {
+
+			/*
+				There could be a scenario where a data directory was supplied that doesn't exist.
+				In that case, we don't want to create the directory, just save in the global one
+			*/
+
+			// If the directory exists...
+			if _, err := os.Stat(*dataDir); err == nil {
+
+				// Remember, if we had a data directory supplied we want to save the config there
+				configPath = filepath.Join(*dataDir, TelemetryConfigFilename)
+			}
+
+		}
+
 		cfg.FilePath = configPath // Initialize our desired cfg.FilePath
 
 		// There was no config file, create it.

logging/telemetryConfig.go

@@ -33,6 +33,9 @@ import (
 // TelemetryConfigFilename default file name for telemetry config "logging.config"
 var TelemetryConfigFilename = "logging.config"
 
+var defaultTelemetryUsername = "telemetry-v9"
+var defaultTelemetryPassword = "oq%$FA1TOJ!yYeMEcJ7D688eEOE#MGCu"
+
 const hostnameLength = 255
 
 // TelemetryOverride Determines whether an override value is set and what it's value is.
@@ -65,8 +68,8 @@ func createTelemetryConfig() TelemetryConfig {
 		MinLogLevel:        logrus.WarnLevel,
 		ReportHistoryLevel: logrus.WarnLevel,
 		// These credentials are here intentionally. Not a bug.
-		UserName: "telemetry-v9",
-		Password: "oq%$FA1TOJ!yYeMEcJ7D688eEOE#MGCu",
+		UserName: defaultTelemetryUsername,
+		Password: defaultTelemetryPassword,
 	}
 }
 
@@ -89,6 +92,14 @@ func (cfg TelemetryConfig) Save(configPath string) error {
 	marshaledConfig.MinLogLevel = uint32(cfg.MinLogLevel)
 	marshaledConfig.ReportHistoryLevel = uint32(cfg.ReportHistoryLevel)
 
+	// If the configuration contains both default username and password for the telemetry
+	// server then we just want to substitute a blank string
+	if marshaledConfig.TelemetryConfig.UserName == defaultTelemetryUsername &&
+		marshaledConfig.TelemetryConfig.Password == defaultTelemetryPassword {
+		marshaledConfig.TelemetryConfig.UserName = ""
+		marshaledConfig.TelemetryConfig.Password = ""
+	}
+
 	enc := json.NewEncoder(f)
 	err = enc.Encode(marshaledConfig)
 	return err
@@ -127,6 +138,7 @@ func SanitizeTelemetryString(input string, maxParts int) string {
 	return input
 }
 
+// Returns err if os.Open fails or if config is mal-formed
 func loadTelemetryConfig(path string) (TelemetryConfig, error) {
 	f, err := os.Open(path)
 	if err != nil {
@@ -143,6 +155,11 @@ func loadTelemetryConfig(path string) (TelemetryConfig, error) {
 	cfg.ReportHistoryLevel = logrus.Level(marshaledConfig.ReportHistoryLevel)
 	cfg.FilePath = path
 
+	if cfg.UserName == "" && cfg.Password == "" {
+		cfg.UserName = defaultTelemetryUsername
+		cfg.Password = defaultTelemetryPassword
+	}
+
 	// Sanitize user-defined name.
 	if len(cfg.Name) > 0 {
 		cfg.Name = SanitizeTelemetryString(cfg.Name, 1)

logging/telemetryConfig_test.go

@@ -17,6 +17,7 @@
 package logging
 
 import (
+	"encoding/json"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -34,8 +35,8 @@ func Test_loadTelemetryConfig(t *testing.T) {
 		MinLogLevel:        4,
 		ReportHistoryLevel: 4,
 		// These credentials are here intentionally. Not a bug.
-		UserName: "telemetry-v9",
-		Password: "oq%$FA1TOJ!yYeMEcJ7D688eEOE#MGCu",
+		UserName: defaultTelemetryUsername,
+		Password: defaultTelemetryPassword,
 	}
 
 	a := require.New(t)
@@ -119,3 +120,54 @@ func TestLoadTelemetryConfig(t *testing.T) {
 	require.Equal(t, createTelemetryConfig().Password, tc.Password)
 
 }
+
+func TestLoadTelemetryConfigBlankUsernamePassword(t *testing.T) {
+
+	testLoggingConfigFileName := "../test/testdata/configs/logging/logging.config.test2"
+	tc, err := loadTelemetryConfig(testLoggingConfigFileName)
+	require.NoError(t, err)
+	// make sure the user name was loaded from the specified file
+	require.Equal(t, defaultTelemetryUsername, tc.UserName)
+	// ensure we know how to default correctly if some of the fields in the configuration field aren't specified.
+	require.Equal(t, defaultTelemetryPassword, tc.Password)
+}
+
+func TestSaveTelemetryConfigBlankUsernamePassword(t *testing.T) {
+
+	testDir := os.Getenv("TESTDIR")
+
+	if testDir == "" {
+		testDir, _ = ioutil.TempDir("", "tmp")
+	}
+
+	a := require.New(t)
+
+	configsPath := filepath.Join(testDir, "logging.config")
+
+	config := createTelemetryConfig()
+
+	// Ensure that config has default username and password
+	config.UserName = defaultTelemetryUsername
+	config.Password = defaultTelemetryPassword
+
+	err := config.Save(configsPath)
+	a.NoError(err)
+
+	f, err := os.Open(configsPath)
+	a.NoError(err)
+	defer f.Close()
+
+	var cfg TelemetryConfig
+
+	var marshaledConfig MarshalingTelemetryConfig
+	marshaledConfig.TelemetryConfig = createTelemetryConfig()
+
+	dec := json.NewDecoder(f)
+	err = dec.Decode(&marshaledConfig)
+	a.NoError(err)
+
+	cfg = marshaledConfig.TelemetryConfig
+	a.Equal(cfg.UserName, "")
+	a.Equal(cfg.Password, "")
+
+}

test/testdata/configs/logging/logging.config.test2

@@ -0,0 +1,4 @@
+{
+  "UserName": "",
+  "Password": ""
+}