diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index 9b508a58c7..cf5a8efbae 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -50,8 +50,8 @@ type config struct { // propagator propagates span context cross-process propagator Propagator - // httpRoundTripper defines the http.RoundTripper used by the agent transport. - httpRoundTripper http.RoundTripper + // httpClient specifies the HTTP client to be used by the agent's transport. + httpClient *http.Client // hostname is automatically assigned when the DD_TRACE_REPORT_HOSTNAME is set to true, // and is added as a special tag to the root span of traces. @@ -243,12 +243,18 @@ func WithSampler(s Sampler) StartOption { } } -// WithHTTPRoundTripper allows customizing the underlying HTTP transport for -// emitting spans. This is useful for advanced customization such as emitting -// spans to a unix domain socket. The default should be used in most cases. +// WithHTTPRoundTripper is deprecated. Please consider using WithHTTPClient instead. func WithHTTPRoundTripper(r http.RoundTripper) StartOption { + return WithHTTPClient(&http.Client{ + Transport: r, + Timeout: defaultHTTPTimeout, + }) +} + +// WithHTTPClient specifies the HTTP client to use when emitting spans to the agent. +func WithHTTPClient(client *http.Client) StartOption { return func(c *config) { - c.httpRoundTripper = r + c.httpClient = client } } diff --git a/ddtrace/tracer/option_test.go b/ddtrace/tracer/option_test.go index a1f1da8324..f4a0190753 100644 --- a/ddtrace/tracer/option_test.go +++ b/ddtrace/tracer/option_test.go @@ -38,7 +38,7 @@ func TestTracerOptionsDefaults(t *testing.T) { assert.Equal("tracer.test", c.serviceName) assert.Equal("localhost:8126", c.agentAddr) assert.Equal("localhost:8125", c.dogstatsdAddr) - assert.Equal(nil, c.httpRoundTripper) + assert.Nil(nil, c.httpClient) }) t.Run("analytics", func(t *testing.T) { diff --git a/ddtrace/tracer/tracer.go b/ddtrace/tracer/tracer.go index ecdda30448..1a38d9e04e 100644 --- a/ddtrace/tracer/tracer.go +++ b/ddtrace/tracer/tracer.go @@ -137,7 +137,7 @@ func newUnstartedTracer(opts ...StartOption) *tracer { fn(c) } if c.transport == nil { - c.transport = newTransport(c.agentAddr, c.httpRoundTripper) + c.transport = newTransport(c.agentAddr, c.httpClient) } if c.propagator == nil { c.propagator = NewPropagator(nil) diff --git a/ddtrace/tracer/tracer_test.go b/ddtrace/tracer/tracer_test.go index b9bfd9a310..19b84fb644 100644 --- a/ddtrace/tracer/tracer_test.go +++ b/ddtrace/tracer/tracer_test.go @@ -499,7 +499,7 @@ func TestTracerPrioritySampler(t *testing.T) { addr := srv.Listener.Addr().String() tr, _, flush, stop := startTestTracer(t, - withTransport(newHTTPTransport(addr, defaultRoundTripper)), + withTransport(newHTTPTransport(addr, defaultClient)), ) defer stop() diff --git a/ddtrace/tracer/transport.go b/ddtrace/tracer/transport.go index 32c2290099..bfc7abaa6b 100644 --- a/ddtrace/tracer/transport.go +++ b/ddtrace/tracer/transport.go @@ -21,11 +21,11 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/version" ) -var ( +var defaultClient = &http.Client{ // We copy the transport to avoid using the default one, as it might be // augmented with tracing and we don't want these calls to be recorded. // See https://golang.org/pkg/net/http/#DefaultTransport . - defaultRoundTripper = &http.Transport{ + Transport: &http.Transport{ Proxy: http.ProxyFromEnvironment, DialContext: (&net.Dialer{ Timeout: 30 * time.Second, @@ -36,14 +36,15 @@ var ( IdleConnTimeout: 90 * time.Second, TLSHandshakeTimeout: 10 * time.Second, ExpectContinueTimeout: 1 * time.Second, - } -) + }, + Timeout: defaultHTTPTimeout, +} const ( defaultHostname = "localhost" defaultPort = "8126" defaultAddress = defaultHostname + ":" + defaultPort - defaultHTTPTimeout = time.Second // defines the current timeout before giving up with the send process + defaultHTTPTimeout = 2 * time.Second // defines the current timeout before giving up with the send process traceCountHeader = "X-Datadog-Trace-Count" // header containing the number of traces in the payload ) @@ -64,16 +65,16 @@ type transport interface { // running on a non-default port, if it's located on another machine, or when // otherwise needing to customize the transport layer, for instance when using // a unix domain socket. -func newTransport(addr string, roundTripper http.RoundTripper) transport { - if roundTripper == nil { - roundTripper = defaultRoundTripper +func newTransport(addr string, client *http.Client) transport { + if client == nil { + client = defaultClient } - return newHTTPTransport(addr, roundTripper) + return newHTTPTransport(addr, client) } // newDefaultTransport return a default transport for this tracing client func newDefaultTransport() transport { - return newHTTPTransport(defaultAddress, defaultRoundTripper) + return newHTTPTransport(defaultAddress, defaultClient) } type httpTransport struct { @@ -83,7 +84,7 @@ type httpTransport struct { } // newHTTPTransport returns an httpTransport for the given endpoint -func newHTTPTransport(addr string, roundTripper http.RoundTripper) *httpTransport { +func newHTTPTransport(addr string, client *http.Client) *httpTransport { // initialize the default EncoderPool with Encoder headers defaultHeaders := map[string]string{ "Datadog-Meta-Lang": "go", @@ -101,11 +102,8 @@ func newHTTPTransport(addr string, roundTripper http.RoundTripper) *httpTranspor } return &httpTransport{ traceURL: fmt.Sprintf("http://%s/v0.4/traces", resolveAddr(addr)), - client: &http.Client{ - Transport: roundTripper, - Timeout: defaultHTTPTimeout, - }, - headers: defaultHeaders, + client: client, + headers: defaultHeaders, } } diff --git a/ddtrace/tracer/transport_test.go b/ddtrace/tracer/transport_test.go index 1e2f22be7d..f5c9e959f5 100644 --- a/ddtrace/tracer/transport_test.go +++ b/ddtrace/tracer/transport_test.go @@ -127,7 +127,7 @@ func TestTracesAgentIntegration(t *testing.T) { } for _, tc := range testCases { - transport := newHTTPTransport(defaultAddress, defaultRoundTripper) + transport := newHTTPTransport(defaultAddress, defaultClient) p, err := encode(tc.payload) assert.NoError(err) _, err = transport.send(p) @@ -196,7 +196,7 @@ func TestTransportResponse(t *testing.T) { })) defer ln.Close() addr := ln.Addr().String() - transport := newHTTPTransport(addr, defaultRoundTripper) + transport := newHTTPTransport(addr, defaultClient) rc, err := transport.send(newPayload()) if tt.err != "" { assert.Equal(tt.err, err.Error()) @@ -230,7 +230,7 @@ func TestTraceCountHeader(t *testing.T) { assert.Nil(err) assert.NotEmpty(port, "port should be given, as it's chosen randomly") for _, tc := range testCases { - transport := newHTTPTransport(host, defaultRoundTripper) + transport := newHTTPTransport(host, defaultClient) p, err := encode(tc.payload) assert.NoError(err) _, err = transport.send(p) @@ -246,7 +246,7 @@ type recordingRoundTripper struct { func (r *recordingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { r.reqs = append(r.reqs, req) - return defaultRoundTripper.RoundTrip(req) + return defaultClient.Transport.RoundTrip(req) } func TestCustomTransport(t *testing.T) { @@ -263,7 +263,7 @@ func TestCustomTransport(t *testing.T) { assert.NotEmpty(port, "port should be given, as it's chosen randomly") customRoundTripper := new(recordingRoundTripper) - transport := newHTTPTransport(host, customRoundTripper) + transport := newHTTPTransport(host, &http.Client{Transport: customRoundTripper}) p, err := encode(getTestTrace(1, 1)) assert.NoError(err) _, err = transport.send(p) @@ -273,6 +273,24 @@ func TestCustomTransport(t *testing.T) { assert.Len(customRoundTripper.reqs, 1) } +func TestWithHTTPClient(t *testing.T) { + assert := assert.New(t) + srv := mockDatadogAPINewServer(t) + defer srv.Close() + + u, err := url.Parse(srv.URL) + assert.NoError(err) + rt := new(recordingRoundTripper) + trc := newTracer(WithAgentAddr(u.Host), WithHTTPClient(&http.Client{Transport: rt})) + defer trc.Stop() + + p, err := encode(getTestTrace(1, 1)) + assert.NoError(err) + _, err = trc.config.transport.send(p) + assert.NoError(err) + assert.Len(rt.reqs, 1) +} + // TestTransportHTTPRace defines a regression tests where the request body was being // read even after http.Client.Do returns. See golang/go#33244 func TestTransportHTTPRace(t *testing.T) {