DataDog / dd-trace-go

Datadog Go Library including APM tracing, profiling, and security monitoring.
https://docs.datadoghq.com/tracing/
Other
671 stars 438 forks source link

Add support for `httptrace.ClientTrace` on wrapped `http.Client` requests #1598

Open jamestelfer opened 2 years ago

jamestelfer commented 2 years ago

The standard library offers support for tracing details of the HTTP(s) connection state on requests via the httptrace.ClientTrace structure.

I have implemented this in our service code such that the http.request span includes the additional tags, for example:

image

Each of the numbers shown is the number of milliseconds consumed by a segment of the request cycle. I found it useful to discover the location of slow network connection establishment issues (spoiler: it was TLS not DNS that time!), verify the TLS cipher suites in use, and ascertain that connection pooling was working as expected.

My implementation is an http.RoundTripper wrapper that sets up the trace context and annotates the current span. I was thinking it would be a useful addition to contrib/net/http.

It could either be default behaviour (the overhead isn't high IMO), or it could be a RoundTripperOption to WrapClient.

I'm happy to contribute a PR for this feature, I'm looking for some confirmation it would be a welcome addition.

phumberdroz commented 1 year ago

We really like to see this. We have been having issues with a couple of requests going to an external API and have not been sure what was causing it, dns, tls handshake etc..

Having this easily visible (maybe even as spans?) would have been nice.

AndreySlashman commented 5 months ago

@jamestelfer Awesome idea! Do you have the implementation available somewhere? As a gist maybe?

rarguelloF commented 5 months ago

Hi @jamestelfer, please apologize me for the late response, we've somehow lost track of this issue 😅

Please consider this proposal accepted, if you or anyone else is interested to contribute I would be glad to review and merge your PR!