From f88aa2c1e6f4061cb4bf26550cb855fcf10d87a8 Mon Sep 17 00:00:00 2001 From: "diana.shevchenko" Date: Mon, 4 Apr 2022 13:55:58 +0200 Subject: [PATCH 1/3] ddtrace/tracer: fixed precedence ordering of configuration options --- ddtrace/tracer/option.go | 2 +- ddtrace/tracer/transport.go | 12 ++++++------ ddtrace/tracer/transport_test.go | 15 ++++++++------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index f2799288c6..a6dbc6564c 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -177,7 +177,7 @@ func forEachStringTag(str string, fn func(key string, val string)) { func newConfig(opts ...StartOption) *config { c := new(config) c.sampler = NewAllSampler() - c.agentAddr = resolveAgentAddr(defaultAddress) + c.agentAddr = resolveAgentAddr("") c.httpClient = defaultHTTPClient() if internal.BoolEnv("DD_TRACE_ANALYTICS_ENABLED", false) { diff --git a/ddtrace/tracer/transport.go b/ddtrace/tracer/transport.go index aad1ce69c8..8bb77b8342 100644 --- a/ddtrace/tracer/transport.go +++ b/ddtrace/tracer/transport.go @@ -192,17 +192,17 @@ func resolveAgentAddr(addr string) string { // no port in addr host = addr } + if v := os.Getenv("DD_AGENT_HOST"); v != "" && host == "" { + host = v + } + if v := os.Getenv("DD_TRACE_AGENT_PORT"); v != "" && port == "" { + port = v + } if host == "" { host = defaultHostname } if port == "" { port = defaultPort } - if v := os.Getenv("DD_AGENT_HOST"); v != "" { - host = v - } - if v := os.Getenv("DD_TRACE_AGENT_PORT"); v != "" { - port = v - } return fmt.Sprintf("%s:%s", host, port) } diff --git a/ddtrace/tracer/transport_test.go b/ddtrace/tracer/transport_test.go index 1e8ce3d019..6b8eb179f7 100644 --- a/ddtrace/tracer/transport_test.go +++ b/ddtrace/tracer/transport_test.go @@ -87,16 +87,17 @@ func TestResolveAgentAddr(t *testing.T) { {":1111", "", "", fmt.Sprintf("%s:1111", defaultHostname)}, {"", "", "", defaultAddress}, {"custom:1234", "", "", "custom:1234"}, - {"", "", "", defaultAddress}, {"", "ip.local", "", fmt.Sprintf("ip.local:%s", defaultPort)}, {"", "", "1234", fmt.Sprintf("%s:1234", defaultHostname)}, {"", "ip.local", "1234", "ip.local:1234"}, - {"ip.other", "ip.local", "", fmt.Sprintf("ip.local:%s", defaultPort)}, - {"ip.other:1234", "ip.local", "", "ip.local:1234"}, - {":8888", "", "1234", fmt.Sprintf("%s:1234", defaultHostname)}, - {"ip.other:8888", "", "1234", "ip.other:1234"}, - {"ip.other", "ip.local", "1234", "ip.local:1234"}, - {"ip.other:8888", "ip.local", "1234", "ip.local:1234"}, + {"ip.other", "ip.local", "", fmt.Sprintf("ip.other:%s", defaultPort)}, + {"ip.other:1234", "ip.local", "", "ip.other:1234"}, + {":8888", "", "1234", fmt.Sprintf("%s:8888", defaultHostname)}, + {"ip.other:8888", "", "1234", "ip.other:8888"}, + {"ip.other", "ip.local", "1234", "ip.other:1234"}, + {"ip.other:8888", "ip.local", "1234", "ip.other:8888"}, + {"ip.other", "ip.local", "1234", fmt.Sprintf("ip.other:%v", 1234)}, + {"ip.other:1234", "ip.local", "", "ip.other:1234"}, } { t.Run("", func(t *testing.T) { if tt.envHost != "" { From 77eff546756f1dec1c0741a805beae4f414e793c Mon Sep 17 00:00:00 2001 From: "diana.shevchenko" Date: Tue, 5 Apr 2022 14:18:45 +0200 Subject: [PATCH 2/3] removed redundant calls to resolveAgentAddr --- ddtrace/tracer/transport.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ddtrace/tracer/transport.go b/ddtrace/tracer/transport.go index 8bb77b8342..8e67da2cc9 100644 --- a/ddtrace/tracer/transport.go +++ b/ddtrace/tracer/transport.go @@ -101,8 +101,8 @@ func newHTTPTransport(addr string, client *http.Client) *httpTransport { defaultHeaders["Datadog-Container-ID"] = cid } return &httpTransport{ - traceURL: fmt.Sprintf("http://%s/v0.4/traces", resolveAgentAddr(addr)), - statsURL: fmt.Sprintf("http://%s/v0.6/stats", resolveAgentAddr(addr)), + traceURL: fmt.Sprintf("http://%s/v0.4/traces", addr), + statsURL: fmt.Sprintf("http://%s/v0.6/stats", addr), client: client, headers: defaultHeaders, } From 097480999851d7689e035596b5c7402bc1c1d23c Mon Sep 17 00:00:00 2001 From: "diana.shevchenko" Date: Tue, 19 Apr 2022 14:54:18 +0200 Subject: [PATCH 3/3] Refactored: removed inforcing of the code path; updated tests --- ddtrace/tracer/option.go | 2 +- ddtrace/tracer/transport.go | 12 ++++------- ddtrace/tracer/transport_test.go | 35 +++++++++++++++----------------- 3 files changed, 21 insertions(+), 28 deletions(-) diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index a6dbc6564c..28e1f421ed 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -177,7 +177,7 @@ func forEachStringTag(str string, fn func(key string, val string)) { func newConfig(opts ...StartOption) *config { c := new(config) c.sampler = NewAllSampler() - c.agentAddr = resolveAgentAddr("") + c.agentAddr = resolveAgentAddr() c.httpClient = defaultHTTPClient() if internal.BoolEnv("DD_TRACE_ANALYTICS_ENABLED", false) { diff --git a/ddtrace/tracer/transport.go b/ddtrace/tracer/transport.go index 8e67da2cc9..5c42347c54 100644 --- a/ddtrace/tracer/transport.go +++ b/ddtrace/tracer/transport.go @@ -186,16 +186,12 @@ func (t *httpTransport) endpoint() string { // resolveAgentAddr resolves the given agent address and fills in any missing host // and port using the defaults. Some environment variable settings will // take precedence over configuration. -func resolveAgentAddr(addr string) string { - host, port, err := net.SplitHostPort(addr) - if err != nil { - // no port in addr - host = addr - } - if v := os.Getenv("DD_AGENT_HOST"); v != "" && host == "" { +func resolveAgentAddr() string { + var host, port string + if v := os.Getenv("DD_AGENT_HOST"); v != "" { host = v } - if v := os.Getenv("DD_TRACE_AGENT_PORT"); v != "" && port == "" { + if v := os.Getenv("DD_TRACE_AGENT_PORT"); v != "" { port = v } if host == "" { diff --git a/ddtrace/tracer/transport_test.go b/ddtrace/tracer/transport_test.go index 6b8eb179f7..d0908b0c51 100644 --- a/ddtrace/tracer/transport_test.go +++ b/ddtrace/tracer/transport_test.go @@ -78,26 +78,19 @@ func TestTracesAgentIntegration(t *testing.T) { } func TestResolveAgentAddr(t *testing.T) { + c := new(config) for _, tt := range []struct { - in, envHost, envPort, out string + inOpt StartOption + envHost, envPort, out string }{ - {"host", "", "", fmt.Sprintf("host:%s", defaultPort)}, - {"www.my-address.com", "", "", fmt.Sprintf("www.my-address.com:%s", defaultPort)}, - {"localhost", "", "", fmt.Sprintf("localhost:%s", defaultPort)}, - {":1111", "", "", fmt.Sprintf("%s:1111", defaultHostname)}, - {"", "", "", defaultAddress}, - {"custom:1234", "", "", "custom:1234"}, - {"", "ip.local", "", fmt.Sprintf("ip.local:%s", defaultPort)}, - {"", "", "1234", fmt.Sprintf("%s:1234", defaultHostname)}, - {"", "ip.local", "1234", "ip.local:1234"}, - {"ip.other", "ip.local", "", fmt.Sprintf("ip.other:%s", defaultPort)}, - {"ip.other:1234", "ip.local", "", "ip.other:1234"}, - {":8888", "", "1234", fmt.Sprintf("%s:8888", defaultHostname)}, - {"ip.other:8888", "", "1234", "ip.other:8888"}, - {"ip.other", "ip.local", "1234", "ip.other:1234"}, - {"ip.other:8888", "ip.local", "1234", "ip.other:8888"}, - {"ip.other", "ip.local", "1234", fmt.Sprintf("ip.other:%v", 1234)}, - {"ip.other:1234", "ip.local", "", "ip.other:1234"}, + {nil, "", "", defaultAddress}, + {nil, "ip.local", "", fmt.Sprintf("ip.local:%s", defaultPort)}, + {nil, "", "1234", fmt.Sprintf("%s:1234", defaultHostname)}, + {nil, "ip.local", "1234", "ip.local:1234"}, + {WithAgentAddr("host:1243"), "", "", "host:1243"}, + {WithAgentAddr("ip.other:9876"), "ip.local", "", "ip.other:9876"}, + {WithAgentAddr("ip.other:1234"), "", "9876", "ip.other:1234"}, + {WithAgentAddr("ip.other:8888"), "ip.local", "1234", "ip.other:8888"}, } { t.Run("", func(t *testing.T) { if tt.envHost != "" { @@ -108,7 +101,11 @@ func TestResolveAgentAddr(t *testing.T) { os.Setenv("DD_TRACE_AGENT_PORT", tt.envPort) defer os.Unsetenv("DD_TRACE_AGENT_PORT") } - assert.Equal(t, resolveAgentAddr(tt.in), tt.out) + c.agentAddr = resolveAgentAddr() + if tt.inOpt != nil { + tt.inOpt(c) + } + assert.Equal(t, c.agentAddr, tt.out) }) } }