Fix client metrics recording on round trip error

instrumentation/net/http/otelhttp/transport.go

@@ -129,6 +129,34 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) {
 	t.propagators.Inject(ctx, propagation.HeaderCarrier(r.Header))
 
 	res, err := t.rt.RoundTrip(r)
+
+	metricAttributes := semconv.MetricAttributes{
+		Req:                  r,
+		AdditionalAttributes: append(labeler.Get(), t.metricAttributesFromRequest(r)...),
+	}
+
+	// defer metrics recording function to record the metrics on error or no error
+	defer func() {
+		// metrics
+		metricOpts := t.semconv.MetricOptions(metricAttributes)
+
+		metricData := semconv.MetricData{
+			RequestSize: bw.BytesRead(),
+		}
+
+		if err == nil {
+			// For handling response bytes we leverage a callback when the client reads the http response
+			readRecordFunc := func(n int64) {
+				t.semconv.RecordResponseSize(ctx, n, metricOpts)
+			}
+
+			res.Body = newWrappedBody(span, readRecordFunc, res.Body)
+		}
+
+		// set transport record metrics
+		t.recordMetrics(ctx, requestStartTime, metricData, metricOpts)
+	}()
+
 	if err != nil {
 		// set error type attribute if the error is part of the predefined
 		// error types.
@@ -141,36 +169,26 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) {
 
 		span.SetStatus(codes.Error, err.Error())
 		span.End()
+
 		return res, err
 	}
 
-	// metrics
-	metricOpts := t.semconv.MetricOptions(semconv.MetricAttributes{
-		Req:                  r,
-		StatusCode:           res.StatusCode,
-		AdditionalAttributes: append(labeler.Get(), t.metricAttributesFromRequest(r)...),
-	})
-
-	// For handling response bytes we leverage a callback when the client reads the http response
-	readRecordFunc := func(n int64) {
-		t.semconv.RecordResponseSize(ctx, n, metricOpts)
-	}
+	metricAttributes.StatusCode = res.StatusCode
 
 	// traces
 	span.SetAttributes(t.semconv.ResponseTraceAttrs(res)...)
 	span.SetStatus(t.semconv.Status(res.StatusCode))
 
-	res.Body = newWrappedBody(span, readRecordFunc, res.Body)
+	return res, nil
+}
 
+func (t *Transport) recordMetrics(ctx context.Context, requestStartTime time.Time, metricData semconv.MetricData, metricOpts map[string]semconv.MetricOpts) {
 	// Use floating point division here for higher precision (instead of Millisecond method).
 	elapsedTime := float64(time.Since(requestStartTime)) / float64(time.Millisecond)
 
-	t.semconv.RecordMetrics(ctx, semconv.MetricData{
-		RequestSize: bw.BytesRead(),
-		ElapsedTime: elapsedTime,
-	}, metricOpts)
+	metricData.ElapsedTime = elapsedTime
 
-	return res, nil
+	t.semconv.RecordMetrics(ctx, metricData, metricOpts)
 }
 
 func (t *Transport) metricAttributesFromRequest(r *http.Request) []attribute.KeyValue {

instrumentation/net/http/otelhttp/transport_test.go

@@ -958,6 +958,62 @@
 	}
 }
 
+func TestMetricsExistenceOnRequestError(t *testing.T) {
+	var rm metricdata.ResourceMetrics
+	const (
+		clientRequestSize = "http.client.request.size"
+		clientDuration    = "http.client.duration"
+	)
+	ctx := context.TODO()
+	reader := sdkmetric.NewManualReader()
+	provider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader))
+	defer func() {
+		err := provider.Shutdown(ctx)
+		if err != nil {
+			t.Errorf("Error shutting down provider: %v", err)
+		}
+	}()
+
+	transport := NewTransport(http.DefaultTransport, WithMeterProvider(provider))
+	client := http.Client{Transport: transport}
+
+	// simulate an error by closing the server
+	// before the request is made
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	ts.Close()
+
+	r, err := http.NewRequest(http.MethodGet, ts.URL, nil)
+	require.NoError(t, err)
+
+	_, err = client.Do(r)
+	require.Error(t, err)
+
+	err = reader.Collect(ctx, &rm)
+	assert.NoError(t, err)
+
+	assert.Len(t, rm.ScopeMetrics[0].Metrics, 2)
+	metricsFound := 0
+	for _, m := range rm.ScopeMetrics[0].Metrics {
+		switch m.Name {
+		case clientRequestSize:
+			d, ok := m.Data.(metricdata.Sum[int64])
+			assert.True(t, ok)
+			assert.Len(t, d.DataPoints, 1)
+			metricsFound++
+		case clientDuration:
+			d, ok := m.Data.(metricdata.Histogram[float64])
+			assert.True(t, ok)
+			assert.Len(t, d.DataPoints, 1)
+			metricsFound++
+		}
+	}
+
+	// make sure client request size and duration metrics is recorded
+	assert.Equal(t, 2, metricsFound, "Expected both metrics to be recorded")
+}
+
 func TestDefaultAttributesHandling(t *testing.T) {
 	var rm metricdata.ResourceMetrics
 	const (