Does span execute span. end

[panRui935074243]

Description

Transferring from service A to service B, service B received the spanid created in the transport, which was not executed on span. end, resulting in nodes A and B being at the same level

Environment

• go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp version: [0.49.0]

Steps To Reproduce

ctx, span := tracer.Start(r.Context(), t.spanNameFormatter("", r), opts...)

if t.clientTrace != nil {
	ctx = httptrace.WithClientTrace(ctx, t.clientTrace(ctx))
}

labeler := &Labeler{}
ctx = injectLabeler(ctx, labeler)

r = r.Clone(ctx) // According to RoundTripper spec, we shouldn't modify the origin request.

// use a body wrapper to determine the request size
var bw bodyWrapper
// if request body is nil or NoBody, we don't want to mutate the body as it
// will affect the identity of it in an unforeseeable way because we assert
// ReadCloser fulfills a certain interface and it is indeed nil or NoBody.
if r.Body != nil && r.Body != http.NoBody {
	bw.ReadCloser = r.Body
	// noop to prevent nil panic. not using this record fun yet.
	bw.record = func(int64) {}
	r.Body = &bw
}

span.SetAttributes(semconvutil.HTTPClientRequest(r)...)
t.propagators.Inject(ctx, propagation.HeaderCarrier(r.Header))

res, err := t.rt.RoundTrip(r)
if err != nil {
	span.RecordError(err)
	span.SetStatus(codes.Error, err.Error())
	span.End()
	return res, err
}

// metrics
metricAttrs := append(labeler.Get(), semconvutil.HTTPClientRequestMetrics(r)...)
if res.StatusCode > 0 {
	metricAttrs = append(metricAttrs, semconv.HTTPStatusCode(res.StatusCode))
}
o := metric.WithAttributes(metricAttrs...)
t.requestBytesCounter.Add(ctx, bw.read.Load(), o)
// For handling response bytes we leverage a callback when the client reads the http response
readRecordFunc := func(n int64) {
	t.responseBytesCounter.Add(ctx, n, o)
}

// traces
span.SetAttributes(semconvutil.HTTPClientResponse(res)...)
span.SetStatus(semconvutil.HTTPClientStatus(res.StatusCode))

res.Body = newWrappedBody(span, readRecordFunc, res.Body)

// Use floating point division here for higher precision (instead of Millisecond method).
elapsedTime := float64(time.Since(requestStartTime)) / float64(time.Millisecond)

t.latencyMeasure.Record(ctx, elapsedTime, o)

return res, err

Expected behavior

If err is nil, execute span. end

I tested it above and I'm not sure if it's a configuration issue with me

[dmathieu]

Your code sample is taken from transport.go.
And I suppose your question is: in the RoundTrip method, span.End() is only called if err != nil, here:

opentelemetry-go-contrib/instrumentation/net/http/otelhttp/transport.go

Lines 160 to 166 in 8c7c4de

	res, err := t.rt.RoundTrip(r)
	if err != nil {
	span.RecordError(err)
	span.SetStatus(codes.Error, err.Error())
	span.End()
	return res, err
	}

So yes, even if there is no error, span.End() will be called.
You can see span being assigned to a body wrapper here:

opentelemetry-go-contrib/instrumentation/net/http/otelhttp/transport.go

Line 184 in 8c7c4de

res.Body = newWrappedBody(span, readRecordFunc, res.Body)

This body wrapper can close the span in two cases:

When the body is being read, and we reach the end of the content:
https://github.com/open-telemetry/opentelemetry-go-contrib/blob/8c7c4deeeb9b4cc91672f44f74745b52e3eba1c5/instrumentation/net/http/otelhttp/transport.go#L246-248

When the body is being closed:

opentelemetry-go-contrib/instrumentation/net/http/otelhttp/transport.go

Lines 268 to 275 in 8c7c4de

	func (wb *wrappedBody) Close() error {
	wb.recordBytesRead()
	wb.span.End()
	if wb.body != nil {
	return wb.body.Close()
	}
	return nil
	}

We need to do this so streaming requests are counted entirely. Otherwise, we could be closing the span before the entire body is received, and therefore before the action is actually finished.

This also means the body must be closed for the span to be ended. If you don't close the body, your span will be finished.
This aligns with the requirements from the net/http package though: https://pkg.go.dev/net/http#pkg-overview

The caller must close the response body when finished with it

Hopefully, that answers your question.

[panRui935074243]

I didn't look carefully, thank you