Request context is overridden in instrumented http clients

[shturec]

It is quite common for golang http clients to make use of the context package. Example use cases are to halt stalled requests or shutdown appropriately for example. When an application uses the github.com/prometheus/client_golang/prometheus package to instrument an http client for metrics, it expects the transport wrappers to preserve the behavior and state of the wrapped client.

However, what happens with a request context set before the instrumentation of the client is that it gets replaced by the default root context and that makes quite of a difference:

r = r.WithContext(httptrace.WithClientTrace(context.Background(), trace))

From the golang docs:
"WithContext returns a shallow copy of r with its context changed to ctx."

The effect is that if you created a context with, for example, context.WithTimeout and set it to a http.Client (client.WithContext(ctx)), and then instrument the client, for example as suggested here, and then send requests to a server that is slow enough to trigger timeout, the timeout will never occur.

What is expected instead is that the InstrumentRoundTripperTrace function respects existing request context, if any.

All findings are wrt v0.9.2 and master.

[beorn7]

Thanks for the report.

I assume this is only an issue with the InstrumentRoundTripperTrace function. Please let me know if that's not true.