otelhttp.WithClientTrace lost span when proxy can not connect

[ddatsh]

Description

otelhttp.WithClientTrace lost span when proxy can not connect

Environment

• OS: [e.g. win11]
• Architecture: [e.g. x86]
• Go Version: [e.g. 1.23.4]
• go.opentelemetry.io/contrib version: [e.g. v0.58.0]

Steps To Reproduce

1. Using this code ...

t := &http.Transport{}
t.Proxy = func(request *http.Request) (*url.URL, error) {
  // 127.0.0.1:10809 can not connect 
  return &url.URL{Scheme: "http", Host: "127.0.0.1:10809"}, nil
}

client = &http.Client{
Transport: otelhttp.NewTransport(
	t,
	otelhttp.WithClientTrace(func(ctx context.Context) *httptrace.ClientTrace {
		return otelhttptrace.NewClientTrace(ctx)
	}),
)}

client.Get("xxxxx")

2. Run ...
3. See error ...

go.opentelemetry.io\contrib\instrumentation\net\http\httptrace\otelhttptrace clienttrace.go

func (ct *clientTracer) gotConn(info httptrace.GotConnInfo) {

 ...
 ct.end("http.getconn", nil, attrs...) //  gotConn()   do not call when  127.0.0.1:10809 can not connect

now http.getconn span lost

func (ct *clientTracer) getConn(host string) {
	ct.start("http.getconn", "http.getconn", semconv.NetHostName(host))
}

Expected behavior

when 127.0.0.1:10809 works,

net/http/transport.go:1481

case r := <-w.result:
		// Trace success but only for HTTP/1.
		// HTTP/2 calls trace.GotConn itself.
		if r.pc != nil && r.pc.alt == nil && trace != nil && trace.GotConn != nil {
			info := httptrace.GotConnInfo{
				Conn:   r.pc.conn,
				Reused: r.pc.isReused(),
			}
			if !r.idleAt.IsZero() {
				info.WasIdle = true
				info.IdleTime = time.Since(r.idleAt)
			}
			trace.GotConn(info)      // will call  func (ct *clientTracer) getConn(host string) { ct.start("http.getconn",
		}

[railmisaka]

This problem is actually not specific to the proxy but occur every time the connection can not be established (with proxy or whatever). This is not important if we're using otelhttp.Transport or not.

Minimal example to reproduce:

// init trace provider

ctx := context.Background()
ctx := httptrace.WithClientTrace(ctx, otelhttptrace.NewClientTrace(ctx))
req, _ := http.NewRequest(http.MethodGet, "http://localhost:50000", nil)
req = req.WithContext(ctx)
http.DefaultClient.Do(req)

I also found a similar issue #2855

This happens because the span (http.getconn) closed in ClientTrace.GotConn, but the hook is not called, which also said in the comment:

// GotConn is called after a successful connection is
// obtained. There is no hook for failure to obtain a
// connection; instead, use the error from
// Transport.RoundTrip.

https://cs.opensource.google/go/go/+/master:src/net/http/httptrace/trace.go;l=91

(Btw WithoutSubSpans() works better - stil no http.getconn.done, but at least nothing red in the UI)

I've been thinking about this for a while and here's one option: Introduce a new ClientTraceOption like WithCancel(cancel *func(err error)) to provide a cancel callback which should be called by user in case of error to close the span (cancel might not be a best name though)

The usage could look like this:

// init trace provider

ctx := context.Background()
var cancel func(err error)
ctx = httptrace.WithClientTrace(ctx, otelhttptrace.NewClientTrace(ctx, otelhttptrace.WithCancel(&cancel)))

req, _ := http.NewRequest(http.MethodGet, "http://localhost:50000", nil)
req = req.WithContext(ctx)
_, err := http.DefaultClient.Do(req)
if err != nil {
    cancel(err)
}