config: added tests for various hostname acquisition strategies

cmd/trace-agent/main.go

@@ -91,17 +91,8 @@ func runAgent(ctx context.Context) {
 
 	cfg, err := config.Load(flags.ConfigPath)
 	if err != nil {
-		if !os.IsNotExist(err) {
-			osutil.Exitf("%v", err)
-		}
-	} else {
-		log.Infof("Loaded configuration: %s", cfg.ConfigPath)
-	}
-	cfg.LoadEnv()
-	if err := cfg.Validate(); err != nil {
 		osutil.Exitf("%v", err)
 	}
-
 	err = info.InitInfo(cfg) // for expvar & -info option
 	if err != nil {
 		panic(err)

config/agent.go

@@ -165,11 +165,8 @@ func (c *AgentConfig) LoadYaml(path string) error {
 	return nil
 }
 
-// LoadEnv reads environment variable values into the config.
-func (c *AgentConfig) LoadEnv() { c.loadEnv() }
-
 // Validate validates if the current configuration is good for the agent to start with.
-func (c *AgentConfig) Validate() error {
+func (c *AgentConfig) validate() error {
 	if c.APIKey == "" {
 		return ErrMissingAPIKey
 	}
@@ -181,6 +178,10 @@ func (c *AgentConfig) Validate() error {
 	return nil
 }
 
+// fallbackHostnameFunc specifies the function to use for obtaining the hostname
+// when it can not be obtained by any other means. It is replaced in tests.
+var fallbackHostnameFunc = os.Hostname
+
 // acquireHostname attempts to acquire a hostname for this configuration. It
 // tries to shell out to the infrastructure agent for this, if DD_AGENT_BIN is
 // set, otherwise falling back to os.Hostname.
@@ -205,17 +206,31 @@ func (c *AgentConfig) acquireHostname() error {
 	err := cmd.Run()
 	c.Hostname = strings.TrimSpace(out.String())
 	if err != nil || c.Hostname == "" {
-		c.Hostname, err = os.Hostname()
+		c.Hostname, err = fallbackHostnameFunc()
 	}
 	if c.Hostname == "" {
 		err = ErrMissingHostname
 	}
 	return err
 }
 
-// Load attempts to load the configuration from the given path. If it's not found
-// it returns an error and a default configuration.
+// Load returns a new configuration based on the given path. The path must not necessarily exist
+// and a valid configuration can be returned based on defaults and environment variables. If a
+// valid configuration can not be obtained, an error is returned.
 func Load(path string) (*AgentConfig, error) {
+	cfg, err := loadFile(path)
+	if err != nil {
+		if !os.IsNotExist(err) {
+			return nil, err
+		}
+	} else {
+		log.Infof("Loaded configuration: %s", cfg.ConfigPath)
+	}
+	cfg.loadEnv()
+	return cfg, cfg.validate()
+}
+
+func loadFile(path string) (*AgentConfig, error) {
 	cfgPath := path
 	if cfgPath == flags.DefaultConfigPath && !osutil.Exists(cfgPath) && osutil.Exists(agent5Config) {
 		// attempting to load inexistent default path, but found existing Agent 5

config/agent_test.go

@@ -8,6 +8,61 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func TestConfigHostname(t *testing.T) {
+	t.Run("nothing", func(t *testing.T) {
+		assert := assert.New(t)
+		fallbackHostnameFunc = func() (string, error) {
+			return "", nil
+		}
+		defer func() {
+			fallbackHostnameFunc = os.Hostname
+		}()
+		_, err := Load("./testdata/multi_api_keys.ini")
+		assert.Equal(ErrMissingHostname, err)
+	})
+
+	t.Run("fallback", func(t *testing.T) {
+		host, err := os.Hostname()
+		if err != nil || host == "" {
+			// can't say
+			t.Skip()
+		}
+		assert := assert.New(t)
+		cfg, err := Load("./testdata/multi_api_keys.ini")
+		assert.NoError(err)
+		assert.Equal(host, cfg.Hostname)
+	})
+
+	t.Run("file", func(t *testing.T) {
+		assert := assert.New(t)
+		cfg, err := Load("./testdata/full.yaml")
+		assert.NoError(err)
+		assert.Equal("mymachine", cfg.Hostname)
+	})
+
+	t.Run("env", func(t *testing.T) {
+		// hostname from env
+		assert := assert.New(t)
+		err := os.Setenv(envHostname, "onlyenv")
+		defer os.Unsetenv(envHostname)
+		assert.NoError(err)
+		cfg, err := Load("./testdata/multi_api_keys.ini")
+		assert.NoError(err)
+		assert.Equal("onlyenv", cfg.Hostname)
+	})
+
+	t.Run("file+env", func(t *testing.T) {
+		// hostname from file, overwritten from env
+		assert := assert.New(t)
+		err := os.Setenv(envHostname, "envoverride")
+		defer os.Unsetenv(envHostname)
+		assert.NoError(err)
+		cfg, err := Load("./testdata/full.yaml")
+		assert.NoError(err)
+		assert.Equal("envoverride", cfg.Hostname)
+	})
+}
+
 func TestDefaultConfig(t *testing.T) {
 	assert := assert.New(t)
 	c := New()
@@ -29,7 +84,7 @@ func TestOnlyEnvConfig(t *testing.T) {
 	os.Setenv("DD_API_KEY", "apikey_from_env")
 
 	c := New()
-	c.LoadEnv()
+	c.loadEnv()
 	assert.Equal(t, "apikey_from_env", c.APIKey)
 
 	os.Setenv("DD_API_KEY", "")
@@ -38,7 +93,7 @@ func TestOnlyEnvConfig(t *testing.T) {
 func TestOnlyDDAgentConfig(t *testing.T) {
 	assert := assert.New(t)
 
-	c, err := Load("./test_cases/no_apm_config.ini")
+	c, err := loadFile("./testdata/no_apm_config.ini")
 	assert.NoError(err)
 
 	assert.Equal("thing", c.Hostname)
@@ -53,7 +108,7 @@ func TestDDAgentMultiAPIKeys(t *testing.T) {
 	// TODO: at some point, expire this case
 	assert := assert.New(t)
 
-	c, err := Load("./test_cases/multi_api_keys.ini")
+	c, err := loadFile("./testdata/multi_api_keys.ini")
 	assert.NoError(err)
 
 	assert.Equal("foo", c.APIKey)
@@ -62,7 +117,7 @@ func TestDDAgentMultiAPIKeys(t *testing.T) {
 func TestFullIniConfig(t *testing.T) {
 	assert := assert.New(t)
 
-	c, err := Load("./test_cases/full.ini")
+	c, err := loadFile("./testdata/full.ini")
 	assert.NoError(err)
 
 	assert.Equal("api_key_test", c.APIKey)
@@ -82,7 +137,7 @@ func TestFullIniConfig(t *testing.T) {
 func TestFullYamlConfig(t *testing.T) {
 	assert := assert.New(t)
 
-	c, err := Load("./test_cases/full.yaml")
+	c, err := loadFile("./testdata/full.yaml")
 	assert.NoError(err)
 
 	assert.Equal("api_key_test", c.APIKey)
@@ -114,7 +169,7 @@ func TestFullYamlConfig(t *testing.T) {
 func TestUndocumentedYamlConfig(t *testing.T) {
 	assert := assert.New(t)
 
-	c, err := Load("./test_cases/undocumented.yaml")
+	c, err := loadFile("./testdata/undocumented.yaml")
 	assert.NoError(err)
 
 	assert.Equal("thing", c.Hostname)
@@ -188,7 +243,7 @@ func TestAcquireHostname(t *testing.T) {
 func TestUndocumentedIni(t *testing.T) {
 	assert := assert.New(t)
 
-	c, err := Load("./test_cases/undocumented.ini")
+	c, err := loadFile("./testdata/undocumented.ini")
 	assert.NoError(err)
 
 	// analysis legacy
@@ -209,7 +264,7 @@ func TestAnalyzedSpansEnvConfig(t *testing.T) {
 	defer os.Unsetenv("DD_APM_ANALYZED_SPANS")
 
 	c := New()
-	c.LoadEnv()
+	c.loadEnv()
 
 	assert.Len(c.AnalyzedSpansByService, 2)
 	assert.Len(c.AnalyzedSpansByService["service1"], 2)

info/info_test.go

@@ -25,7 +25,7 @@ type testServerHandler struct {
 func (h *testServerHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
 
-	json, err := ioutil.ReadFile("./test_cases/okay.json")
+	json, err := ioutil.ReadFile("./testdata/okay.json")
 	if err != nil {
 		h.t.Errorf("error loading json file: %v", err)
 	}
@@ -56,7 +56,7 @@ type testServerWarningHandler struct {
 func (h *testServerWarningHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
 
-	json, err := ioutil.ReadFile("./test_cases/warning.json")
+	json, err := ioutil.ReadFile("./testdata/warning.json")
 	if err != nil {
 		h.t.Errorf("error loading json file: %v", err)
 	}
@@ -142,7 +142,7 @@ func TestInfo(t *testing.T) {
 	assert.NoError(err)
 	info := buf.String()
 	t.Logf("Info:\n%s\n", info)
-	expectedInfo, err := ioutil.ReadFile("./test_cases/okay.info")
+	expectedInfo, err := ioutil.ReadFile("./testdata/okay.info")
 	assert.NoError(err)
 	assert.Equal(string(expectedInfo), info)
 }
@@ -171,7 +171,7 @@ func TestWarning(t *testing.T) {
 	assert.Nil(err)
 	info := buf.String()
 
-	expectedWarning, err := ioutil.ReadFile("./test_cases/warning.info")
+	expectedWarning, err := ioutil.ReadFile("./testdata/warning.info")
 	assert.NoError(err)
 	assert.Equal(string(expectedWarning), info)