diff --git a/instrumentation/net/http/otelhttp/transport.go b/instrumentation/net/http/otelhttp/transport.go index 44b86ad8609..650c47aa70e 100644 --- a/instrumentation/net/http/otelhttp/transport.go +++ b/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 { diff --git a/instrumentation/net/http/otelhttp/transport_test.go b/instrumentation/net/http/otelhttp/transport_test.go index 7251b37657e..21eec6ce1a7 100644 --- a/instrumentation/net/http/otelhttp/transport_test.go +++ b/instrumentation/net/http/otelhttp/transport_test.go @@ -958,6 +958,62 @@ func TestCustomAttributesHandling(t *testing.T) { } } +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 (