From c6853b832c9e9580c7836a741fa93d2ac0dd092d Mon Sep 17 00:00:00 2001 From: ilylia Date: Thu, 14 May 2020 11:20:18 +0800 Subject: [PATCH] Override reporter config only when agent host/port is set in env (#513) * override reporter config only when agent host or port was set in env Signed-off-by: ilylia * fmt fixed Signed-off-by: ilylia * split to new test Signed-off-by: ilylia --- config/config_env.go | 7 ++++- config/config_test.go | 62 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/config/config_env.go b/config/config_env.go index 66d63408..f38eb9d9 100644 --- a/config/config_env.go +++ b/config/config_env.go @@ -187,20 +187,25 @@ func (rc *ReporterConfig) reporterConfigFromEnv() (*ReporterConfig, error) { rc.User = user rc.Password = pswd } else { + useEnv := false host := jaeger.DefaultUDPSpanServerHost if e := os.Getenv(envAgentHost); e != "" { host = e + useEnv = true } port := jaeger.DefaultUDPSpanServerPort if e := os.Getenv(envAgentPort); e != "" { if value, err := strconv.ParseInt(e, 10, 0); err == nil { port = int(value) + useEnv = true } else { return nil, errors.Wrapf(err, "cannot parse env var %s=%s", envAgentPort, e) } } - rc.LocalAgentHostPort = fmt.Sprintf("%s:%d", host, port) + if useEnv || rc.LocalAgentHostPort == "" { + rc.LocalAgentHostPort = fmt.Sprintf("%s:%d", host, port) + } } return rc, nil diff --git a/config/config_test.go b/config/config_test.go index f5d9e4c4..c273d5c8 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -398,6 +398,68 @@ func TestReporterConfigFromEnv(t *testing.T) { unsetEnv(t, envPassword) } +func TestReporterAgentConfigFromEnv(t *testing.T) { + // prepare + unsetEnv(t, envEndpoint) + unsetEnv(t, envAgentHost) + unsetEnv(t, envAgentPort) + + // No config and no env check + rc := ReporterConfig{} + + // test + cfg, err := rc.reporterConfigFromEnv() + assert.NoError(t, err) + + // verify + assert.Equal(t, "localhost:6831", cfg.LocalAgentHostPort) + + // No env check + rc = ReporterConfig{ + LocalAgentHostPort: "localhost01:7777", + } + + // test + cfg, err = rc.reporterConfigFromEnv() + assert.NoError(t, err) + + // verify + assert.Equal(t, "localhost01:7777", cfg.LocalAgentHostPort) + + // Only host env check + setEnv(t, envAgentHost, "localhost02") + unsetEnv(t, envAgentPort) + rc = ReporterConfig{ + LocalAgentHostPort: "localhost01:7777", + } + + // test + cfg, err = rc.reporterConfigFromEnv() + assert.NoError(t, err) + + // verify + assert.Equal(t, "localhost02:6831", cfg.LocalAgentHostPort) + + // Only port env check + unsetEnv(t, envAgentHost) + setEnv(t, envAgentPort, "8888") + rc = ReporterConfig{ + LocalAgentHostPort: "localhost01:7777", + } + + // test + cfg, err = rc.reporterConfigFromEnv() + assert.NoError(t, err) + + // verify + assert.Equal(t, "localhost:8888", cfg.LocalAgentHostPort) + + // cleanup + unsetEnv(t, envEndpoint) + unsetEnv(t, envAgentHost) + unsetEnv(t, envAgentPort) +} + func TestParsingErrorsFromEnv(t *testing.T) { setEnv(t, envAgentHost, "localhost") // we require this in order to test the parsing of the port