feat(instrumentation/http/otelhttp): move client metrics creation into internal semconv package

instrumentation/net/http/otelhttp/common.go

@@ -18,13 +18,6 @@ const (
 	WriteErrorKey = attribute.Key("http.write_error") // if an error occurred while writing a reply, the string of the error (io.EOF is not recorded)
 )
 
-// Client HTTP metrics.
-const (
-	clientRequestSize  = "http.client.request.size"  // Outgoing request bytes total
-	clientResponseSize = "http.client.response.size" // Outgoing response bytes total
-	clientDuration     = "http.client.duration"      // Outgoing end to end duration, milliseconds
-)
-
 // Filter is a predicate used to determine whether a given http.request should
 // be traced. A Filter must return true if the request should be traced.
 type Filter func(*http.Request) bool

instrumentation/net/http/otelhttp/handler.go

@@ -81,12 +81,6 @@ func (h *middleware) configure(c *config) {
 	h.semconv = semconv.NewHTTPServer(c.Meter)
 }
 
-func handleErr(err error) {
-	if err != nil {
-		otel.Handle(err)
-	}
-}
-
 // serveHTTP sets up tracing and calls the given next http.Handler with the span
 // context injected into the request context.
 func (h *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http.Handler) {
@@ -190,14 +184,18 @@ func (h *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http
 	// Use floating point division here for higher precision (instead of Millisecond method).
 	elapsedTime := float64(time.Since(requestStartTime)) / float64(time.Millisecond)
 
-	h.semconv.RecordMetrics(ctx, semconv.MetricData{
-		ServerName:           h.server,
-		Req:                  r,
-		StatusCode:           statusCode,
-		AdditionalAttributes: labeler.Get(),
-		RequestSize:          bw.BytesRead(),
-		ResponseSize:         bytesWritten,
-		ElapsedTime:          elapsedTime,
+	h.semconv.RecordMetrics(ctx, semconv.ServerMetricData{
+		ServerName:   h.server,
+		ResponseSize: bytesWritten,
+		MetricAttributes: semconv.MetricAttributes{
+			Req:                  r,
+			StatusCode:           statusCode,
+			AdditionalAttributes: labeler.Get(),
+		},
+		MetricData: semconv.MetricData{
+			RequestSize: bw.BytesRead(),
+			ElapsedTime: elapsedTime,
+		},
 	})
 }
 

instrumentation/net/http/otelhttp/internal/semconv/env.go

@@ -83,28 +83,36 @@ func (s HTTPServer) Status(code int) (codes.Code, string) {
 	return codes.Unset, ""
 }
 
-type MetricData struct {
-	ServerName           string
+type ServerMetricData struct {
+	ServerName   string
+	ResponseSize int64
+
+	MetricData
+	MetricAttributes
+}
+
+type MetricAttributes struct {
 	Req                  *http.Request
 	StatusCode           int
 	AdditionalAttributes []attribute.KeyValue
+}
 
-	RequestSize  int64
-	ResponseSize int64
-	ElapsedTime  float64
+type MetricData struct {
+	RequestSize int64
+	ElapsedTime float64
 }
 
-func (s HTTPServer) RecordMetrics(ctx context.Context, md MetricData) {
+func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) {
 	if s.requestBytesCounter == nil || s.responseBytesCounter == nil || s.serverLatencyMeasure == nil {
 		// This will happen if an HTTPServer{} is used insted of NewHTTPServer.
 		return
 	}
 
 	attributes := oldHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes)
 	o := metric.WithAttributeSet(attribute.NewSet(attributes...))
-	addOpts := []metric.AddOption{o} // Allocate vararg slice once.
-	s.requestBytesCounter.Add(ctx, md.RequestSize, addOpts...)
-	s.responseBytesCounter.Add(ctx, md.ResponseSize, addOpts...)
+	addOpts := metric.AddOption(o)
+	s.requestBytesCounter.Add(ctx, md.RequestSize, addOpts)
+	s.responseBytesCounter.Add(ctx, md.ResponseSize, addOpts)
 	s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o)
 
 	// TODO: Duplicate Metrics
@@ -122,11 +130,20 @@ func NewHTTPServer(meter metric.Meter) HTTPServer {
 
 type HTTPClient struct {
 	duplicate bool
+
+	// old metrics
+	requestBytesCounter  metric.Int64Counter
+	responseBytesCounter metric.Int64Counter
+	latencyMeasure       metric.Float64Histogram
 }
 
-func NewHTTPClient() HTTPClient {
+func NewHTTPClient(meter metric.Meter) HTTPClient {
 	env := strings.ToLower(os.Getenv("OTEL_SEMCONV_STABILITY_OPT_IN"))
-	return HTTPClient{duplicate: env == "http/dup"}
+	client := HTTPClient{
+		duplicate: env == "http/dup",
+	}
+	client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = oldHTTPClient{}.createMeasures(meter)
+	return client
 }
 
 // RequestTraceAttrs returns attributes for an HTTP request made by a client.
@@ -163,3 +180,48 @@ func (c HTTPClient) ErrorType(err error) attribute.KeyValue {
 
 	return attribute.KeyValue{}
 }
+
+type MetricOpts struct {
+	measurement metric.MeasurementOption
+	addOptions  metric.AddOption
+}
+
+func (o MetricOpts) MeasurementOption() metric.MeasurementOption {
+	return o.measurement
+}
+
+func (o MetricOpts) AddOptions() metric.AddOption {
+	return o.addOptions
+}
+
+func (c HTTPClient) MetricOptions(ma MetricAttributes) MetricOpts {
+	attributes := oldHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes)
+	// TODO: Duplicate Metrics
+	set := metric.WithAttributeSet(attribute.NewSet(attributes...))
+	return MetricOpts{
+		measurement: set,
+		addOptions:  set,
+	}
+}
+
+func (s HTTPClient) RecordMetrics(ctx context.Context, md MetricData, opts MetricOpts) {
+	if s.requestBytesCounter == nil || s.latencyMeasure == nil {
+		// This will happen if an HTTPClient{} is used insted of NewHTTPClient().
+		return
+	}
+
+	s.requestBytesCounter.Add(ctx, md.RequestSize, opts.AddOptions())
+	s.latencyMeasure.Record(ctx, md.ElapsedTime, opts.MeasurementOption())
+
+	// TODO: Duplicate Metrics
+}
+
+func (s HTTPClient) RecordResponseSize(ctx context.Context, responseData int64, opts metric.AddOption) {
+	if s.responseBytesCounter == nil {
+		// This will happen if an HTTPClient{} is used insted of NewHTTPClient().
+		return
+	}
+
+	s.responseBytesCounter.Add(ctx, responseData, opts)
+	// TODO: Duplicate Metrics
+}

instrumentation/net/http/otelhttp/internal/semconv/env_test.go

@@ -42,10 +42,53 @@ func TestHTTPServerDoesNotPanic(t *testing.T) {
 
 				_ = tt.server.RequestTraceAttrs("stuff", req)
 				_ = tt.server.ResponseTraceAttrs(ResponseTelemetry{StatusCode: 200})
-				tt.server.RecordMetrics(context.Background(), MetricData{
+				tt.server.RecordMetrics(context.Background(), ServerMetricData{
 					ServerName: "stuff",
+					MetricAttributes: MetricAttributes{
+						Req: req,
+					},
+				})
+			})
+		})
+	}
+}
+
+func TestHTTPClientDoesNotPanic(t *testing.T) {
+	testCases := []struct {
+		name   string
+		client HTTPClient
+	}{
+		{
+			name:   "empty",
+			client: HTTPClient{},
+		},
+		{
+			name:   "nil meter",
+			client: NewHTTPClient(nil),
+		},
+		{
+			name:   "with Meter",
+			client: NewHTTPClient(noop.Meter{}),
+		},
+	}
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			require.NotPanics(t, func() {
+				req, err := http.NewRequest("GET", "http://example.com", nil)
+				require.NoError(t, err)
+
+				_ = tt.client.RequestTraceAttrs(req)
+				_ = tt.client.ResponseTraceAttrs(&http.Response{StatusCode: 200})
+
+				opts := tt.client.MetricOptions(MetricAttributes{
 					Req:        req,
+					StatusCode: 200,
 				})
+				tt.client.RecordResponseSize(context.Background(), 40, opts.AddOptions())
+				tt.client.RecordMetrics(context.Background(), MetricData{
+					RequestSize: 20,
+					ElapsedTime: 1,
+				}, opts)
 			})
 		})
 	}
@@ -81,3 +124,11 @@ func NewTestHTTPServer() HTTPServer {
 		serverLatencyMeasure: &testInst{},
 	}
 }
+
+func NewTestHTTPClient() HTTPClient {
+	return HTTPClient{
+		requestBytesCounter:  &testInst{},
+		responseBytesCounter: &testInst{},
+		latencyMeasure:       &testInst{},
+	}
+}

instrumentation/net/http/otelhttp/internal/semconv/httpconv_test.go

@@ -151,7 +151,7 @@ func TestNewTraceRequest_Client(t *testing.T) {
 		attribute.String("user_agent.original", "go-test-agent"),
 		attribute.Int("http.request_content_length", 13),
 	}
-	client := NewHTTPClient()
+	client := NewHTTPClient(nil)
 	assert.ElementsMatch(t, want, client.RequestTraceAttrs(req))
 }
 
@@ -166,7 +166,7 @@ func TestNewTraceResponse_Client(t *testing.T) {
 	}
 
 	for _, tt := range testcases {
-		client := NewHTTPClient()
+		client := NewHTTPClient(nil)
 		assert.ElementsMatch(t, tt.want, client.ResponseTraceAttrs(&tt.resp))
 	}
 }

instrumentation/net/http/otelhttp/internal/semconv/v1.20.0.go

@@ -144,7 +144,7 @@
 
 	attributes := slices.Grow(additionalAttributes, n)
 	attributes = append(attributes,
-		o.methodMetric(req.Method),
+		standardizeHTTPMethodMetric(req.Method),
 		o.scheme(req.TLS != nil),
 		semconv.NetHostName(host))
 
@@ -164,16 +164,6 @@
 	return attributes
 }
 
-func (o oldHTTPServer) methodMetric(method string) attribute.KeyValue {
-	method = strings.ToUpper(method)
-	switch method {
-	case http.MethodConnect, http.MethodDelete, http.MethodGet, http.MethodHead, http.MethodOptions, http.MethodPatch, http.MethodPost, http.MethodPut, http.MethodTrace:
-	default:
-		method = "_OTHER"
-	}
-	return semconv.HTTPMethod(method)
-}
-
 func (o oldHTTPServer) scheme(https bool) attribute.KeyValue { // nolint:revive
 	if https {
 		return semconv.HTTPSchemeHTTPS
@@ -190,3 +180,95 @@
 func (o oldHTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue {
 	return semconvutil.HTTPClientResponse(resp)
 }
+
+func (o oldHTTPClient) MetricAttributes(req *http.Request, statusCode int, additionalAttributes []attribute.KeyValue) []attribute.KeyValue {
+	/* The following semantic conventions are returned if present:
+	http.method                     string
+	http.status_code             int
+	net.peer.name                   string
+	net.peer.port                   int
+	*/
+
+	n := 2 // method, peer name.
+	var h string
+	if req.URL != nil {
+		h = req.URL.Host
+	}
+	var requestHost string
+	var requestPort int
+	for _, hostport := range []string{h, req.Header.Get("Host")} {
+		requestHost, requestPort = splitHostPort(hostport)
+		if requestHost != "" || requestPort > 0 {
+			break
+		}
+	}
+
+	port := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", requestPort)
+	if port > 0 {
+		n++
+	}
+
+	if statusCode > 0 {
+		n++
+	}
+
+	attributes := slices.Grow(additionalAttributes, n)
+	attributes = append(attributes,
+		standardizeHTTPMethodMetric(req.Method),
+		semconv.NetPeerName(requestHost),
+	)
+
+	if port > 0 {
+		attributes = append(attributes, semconv.NetPeerPort(port))
+	}
+
+	if statusCode > 0 {
+		attributes = append(attributes, semconv.HTTPStatusCode(statusCode))
+	}
+	return attributes
+}
+
+// Client HTTP metrics.
+const (
+	clientRequestSize  = "http.client.request.size"  // Incoming request bytes total
+	clientResponseSize = "http.client.response.size" // Incoming response bytes total
+	clientDuration     = "http.client.duration"      // Incoming end to end duration, milliseconds
+)
+
+func (o oldHTTPClient) createMeasures(meter metric.Meter) (metric.Int64Counter, metric.Int64Counter, metric.Float64Histogram) {
+	if meter == nil {
+		return noop.Int64Counter{}, noop.Int64Counter{}, noop.Float64Histogram{}
+	}
+	requestBytesCounter, err := meter.Int64Counter(
+		clientRequestSize,
+		metric.WithUnit("By"),
+		metric.WithDescription("Measures the size of HTTP request messages."),
+	)
+	handleErr(err)
+
+	responseBytesCounter, err := meter.Int64Counter(
+		clientResponseSize,
+		metric.WithUnit("By"),
+		metric.WithDescription("Measures the size of HTTP response messages."),
+	)
+	handleErr(err)
+
+	latencyMeasure, err := meter.Float64Histogram(
+		clientDuration,
+		metric.WithUnit("ms"),
+		metric.WithDescription("Measures the duration of outbound HTTP requests."),
+	)
+	handleErr(err)
+
+	return requestBytesCounter, responseBytesCounter, latencyMeasure
+}
+
+func standardizeHTTPMethodMetric(method string) attribute.KeyValue {
+	method = strings.ToUpper(method)
+	switch method {
+	case http.MethodConnect, http.MethodDelete, http.MethodGet, http.MethodHead, http.MethodOptions, http.MethodPatch, http.MethodPost, http.MethodPut, http.MethodTrace:
+	default:
+		method = "_OTHER"
+	}
+	return semconv.HTTPMethod(method)
+}