From 6354fcb44978681be517dcbb9e4e3aab5527816d Mon Sep 17 00:00:00 2001 From: ilylia Date: Wed, 13 May 2020 15:11:19 +0800 Subject: [PATCH 1/3] override reporter config only when agent host or port was set in env Signed-off-by: ilylia --- config/config_env.go | 7 +++++- config/config_test.go | 51 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 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..950ef4c5 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -252,6 +252,57 @@ func TestReporter(t *testing.T) { assert.Equal(t, "user", cfg.User) assert.Equal(t, "password", cfg.Password) + // + unsetEnv(t, envEndpoint) + unsetEnv(t, envAgentHost) + unsetEnv(t, envAgentPort) + + rc = ReporterConfig{ + } + + // + cfg, err = rc.reporterConfigFromEnv() + assert.NoError(t, err) + + assert.Equal(t, "localhost:6831", cfg.LocalAgentHostPort) + + // + rc = ReporterConfig{ + LocalAgentHostPort: "localhost01:7777", + } + + // + cfg, err = rc.reporterConfigFromEnv() + assert.NoError(t, err) + + assert.Equal(t, "localhost01:7777", cfg.LocalAgentHostPort) + + // + setEnv(t, envAgentHost, "localhost02") + unsetEnv(t, envAgentPort) + rc = ReporterConfig{ + LocalAgentHostPort: "localhost01:7777", + } + + // + cfg, err = rc.reporterConfigFromEnv() + assert.NoError(t, err) + + assert.Equal(t, "localhost02:6831", cfg.LocalAgentHostPort) + + // + unsetEnv(t, envAgentHost) + setEnv(t, envAgentPort, "8888") + rc = ReporterConfig{ + LocalAgentHostPort: "localhost01:7777", + } + + // + cfg, err = rc.reporterConfigFromEnv() + assert.NoError(t, err) + + assert.Equal(t, "localhost:8888", cfg.LocalAgentHostPort) + // cleanup unsetEnv(t, envReporterMaxQueueSize) unsetEnv(t, envReporterFlushInterval) From 93bea656e3e6d29bdb972d65f16bab33ac3188cb Mon Sep 17 00:00:00 2001 From: ilylia Date: Wed, 13 May 2020 15:30:34 +0800 Subject: [PATCH 2/3] fmt fixed Signed-off-by: ilylia --- config/config_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index 950ef4c5..0d88faf1 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -256,9 +256,7 @@ func TestReporter(t *testing.T) { unsetEnv(t, envEndpoint) unsetEnv(t, envAgentHost) unsetEnv(t, envAgentPort) - - rc = ReporterConfig{ - } + rc = ReporterConfig{} // cfg, err = rc.reporterConfigFromEnv() From f10fbc271709da4769c21b44af8207bdd88e17e9 Mon Sep 17 00:00:00 2001 From: ilylia Date: Thu, 14 May 2020 09:26:36 +0800 Subject: [PATCH 3/3] split to new test Signed-off-by: ilylia --- config/config_test.go | 111 +++++++++++++++++++++++------------------- 1 file changed, 62 insertions(+), 49 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index 0d88faf1..c273d5c8 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -252,55 +252,6 @@ func TestReporter(t *testing.T) { assert.Equal(t, "user", cfg.User) assert.Equal(t, "password", cfg.Password) - // - unsetEnv(t, envEndpoint) - unsetEnv(t, envAgentHost) - unsetEnv(t, envAgentPort) - rc = ReporterConfig{} - - // - cfg, err = rc.reporterConfigFromEnv() - assert.NoError(t, err) - - assert.Equal(t, "localhost:6831", cfg.LocalAgentHostPort) - - // - rc = ReporterConfig{ - LocalAgentHostPort: "localhost01:7777", - } - - // - cfg, err = rc.reporterConfigFromEnv() - assert.NoError(t, err) - - assert.Equal(t, "localhost01:7777", cfg.LocalAgentHostPort) - - // - setEnv(t, envAgentHost, "localhost02") - unsetEnv(t, envAgentPort) - rc = ReporterConfig{ - LocalAgentHostPort: "localhost01:7777", - } - - // - cfg, err = rc.reporterConfigFromEnv() - assert.NoError(t, err) - - assert.Equal(t, "localhost02:6831", cfg.LocalAgentHostPort) - - // - unsetEnv(t, envAgentHost) - setEnv(t, envAgentPort, "8888") - rc = ReporterConfig{ - LocalAgentHostPort: "localhost01:7777", - } - - // - cfg, err = rc.reporterConfigFromEnv() - assert.NoError(t, err) - - assert.Equal(t, "localhost:8888", cfg.LocalAgentHostPort) - // cleanup unsetEnv(t, envReporterMaxQueueSize) unsetEnv(t, envReporterFlushInterval) @@ -447,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