ddtrace/tracer: fixed precedence ordering of configuration options (#…

…1232)

* ddtrace/tracer: fixed precedence ordering of configuration options

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) {

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,
 	}
@@ -186,23 +186,19 @@ 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
+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 = 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)
 }

ddtrace/tracer/transport_test.go

@@ -78,25 +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"},
-		{"", "", "", 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"},
+		{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 != "" {
@@ -107,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)
 		})
 	}
 }