diff --git a/cmd/trace-agent/main.go b/cmd/trace-agent/main.go index 00273a2f8..d9ed1da28 100644 --- a/cmd/trace-agent/main.go +++ b/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) diff --git a/config/agent.go b/config/agent.go index 06dc1e551..7a9472551 100644 --- a/config/agent.go +++ b/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,7 +206,7 @@ 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 @@ -213,9 +214,23 @@ func (c *AgentConfig) acquireHostname() error { 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 diff --git a/config/agent_test.go b/config/agent_test.go index 717d42f81..94ec12d6d 100644 --- a/config/agent_test.go +++ b/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) diff --git a/config/test_cases/full.ini b/config/testdata/full.ini similarity index 100% rename from config/test_cases/full.ini rename to config/testdata/full.ini diff --git a/config/test_cases/full.yaml b/config/testdata/full.yaml similarity index 100% rename from config/test_cases/full.yaml rename to config/testdata/full.yaml diff --git a/config/test_cases/multi_api_keys.ini b/config/testdata/multi_api_keys.ini similarity index 100% rename from config/test_cases/multi_api_keys.ini rename to config/testdata/multi_api_keys.ini diff --git a/config/test_cases/no_apm_config.ini b/config/testdata/no_apm_config.ini similarity index 100% rename from config/test_cases/no_apm_config.ini rename to config/testdata/no_apm_config.ini diff --git a/config/test_cases/undocumented.ini b/config/testdata/undocumented.ini similarity index 100% rename from config/test_cases/undocumented.ini rename to config/testdata/undocumented.ini diff --git a/config/test_cases/undocumented.yaml b/config/testdata/undocumented.yaml similarity index 100% rename from config/test_cases/undocumented.yaml rename to config/testdata/undocumented.yaml diff --git a/info/info_test.go b/info/info_test.go index 4742ef83b..675303321 100644 --- a/info/info_test.go +++ b/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) diff --git a/info/test_cases/okay.info b/info/testdata/okay.info similarity index 100% rename from info/test_cases/okay.info rename to info/testdata/okay.info diff --git a/info/test_cases/okay.json b/info/testdata/okay.json similarity index 100% rename from info/test_cases/okay.json rename to info/testdata/okay.json diff --git a/info/test_cases/warning.info b/info/testdata/warning.info similarity index 100% rename from info/test_cases/warning.info rename to info/testdata/warning.info diff --git a/info/test_cases/warning.json b/info/testdata/warning.json similarity index 100% rename from info/test_cases/warning.json rename to info/testdata/warning.json