HTTP Semconv migration Part1 Server Metrics - v1.20.0 support

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)
 )
 
-// Server HTTP metrics.
-const (
-	serverRequestSize  = "http.server.request.size"  // Incoming request bytes total
-	serverResponseSize = "http.server.response.size" // Incoming response bytes total
-	serverDuration     = "http.server.duration"      // Incoming end to end duration, milliseconds
-)
-
 // Client HTTP metrics.
 const (
 	clientRequestSize  = "http.client.request.size"  // Outgoing request bytes total

instrumentation/net/http/otelhttp/handler.go

@@ -10,10 +10,7 @@ import (
 	"github.com/felixge/httpsnoop"
 
 	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv"
-	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconvutil"
 	"go.opentelemetry.io/otel"
-	"go.opentelemetry.io/otel/attribute"
-	"go.opentelemetry.io/otel/metric"
 	"go.opentelemetry.io/otel/propagation"
 	"go.opentelemetry.io/otel/trace"
 )
@@ -24,7 +21,6 @@ type middleware struct {
 	server    string
 
 	tracer            trace.Tracer
-	meter             metric.Meter
 	propagators       propagation.TextMapPropagator
 	spanStartOptions  []trace.SpanStartOption
 	readEvent         bool
@@ -34,10 +30,7 @@ type middleware struct {
 	publicEndpoint    bool
 	publicEndpointFn  func(*http.Request) bool
 
-	traceSemconv         semconv.HTTPServer
-	requestBytesCounter  metric.Int64Counter
-	responseBytesCounter metric.Int64Counter
-	serverLatencyMeasure metric.Float64Histogram
+	semconv semconv.HTTPServer
 }
 
 func defaultHandlerFormatter(operation string, _ *http.Request) string {
@@ -56,8 +49,6 @@ func NewHandler(handler http.Handler, operation string, opts ...Option) http.Han
 func NewMiddleware(operation string, opts ...Option) func(http.Handler) http.Handler {
 	h := middleware{
 		operation: operation,
-
-		traceSemconv: semconv.NewHTTPServer(),
 	}
 
 	defaultOpts := []Option{
@@ -67,7 +58,6 @@ func NewMiddleware(operation string, opts ...Option) func(http.Handler) http.Han
 
 	c := newConfig(append(defaultOpts, opts...)...)
 	h.configure(c)
-	h.createMeasures()
 
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -78,7 +68,6 @@ func NewMiddleware(operation string, opts ...Option) func(http.Handler) http.Han
 
 func (h *middleware) configure(c *config) {
 	h.tracer = c.Tracer
-	h.meter = c.Meter
 	h.propagators = c.Propagators
 	h.spanStartOptions = c.SpanStartOptions
 	h.readEvent = c.ReadEvent
@@ -88,6 +77,7 @@ func (h *middleware) configure(c *config) {
 	h.publicEndpoint = c.PublicEndpoint
 	h.publicEndpointFn = c.PublicEndpointFn
 	h.server = c.ServerName
+	h.semconv = semconv.NewHTTPServer(c.Meter)
 }
 
 func handleErr(err error) {
@@ -96,30 +86,6 @@ func handleErr(err error) {
 	}
 }
 
-func (h *middleware) createMeasures() {
-	var err error
-	h.requestBytesCounter, err = h.meter.Int64Counter(
-		serverRequestSize,
-		metric.WithUnit("By"),
-		metric.WithDescription("Measures the size of HTTP request messages."),
-	)
-	handleErr(err)
-
-	h.responseBytesCounter, err = h.meter.Int64Counter(
-		serverResponseSize,
-		metric.WithUnit("By"),
-		metric.WithDescription("Measures the size of HTTP response messages."),
-	)
-	handleErr(err)
-
-	h.serverLatencyMeasure, err = h.meter.Float64Histogram(
-		serverDuration,
-		metric.WithUnit("ms"),
-		metric.WithDescription("Measures the duration of inbound HTTP requests."),
-	)
-	handleErr(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) {
@@ -134,7 +100,7 @@ func (h *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http
 
 	ctx := h.propagators.Extract(r.Context(), propagation.HeaderCarrier(r.Header))
 	opts := []trace.SpanStartOption{
-		trace.WithAttributes(h.traceSemconv.RequestTraceAttrs(h.server, r)...),
+		trace.WithAttributes(h.semconv.RequestTraceAttrs(h.server, r)...),
 	}
 
 	opts = append(opts, h.spanStartOptions...)
@@ -217,35 +183,33 @@ func (h *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http
 
 	next.ServeHTTP(w, r.WithContext(ctx))
 
-	span.SetStatus(semconv.ServerStatus(rww.statusCode))
-	span.SetAttributes(h.traceSemconv.ResponseTraceAttrs(semconv.ResponseTelemetry{
+	span.SetStatus(h.semconv.Status(rww.statusCode))
+	span.SetAttributes(h.semconv.ResponseTraceAttrs(semconv.ResponseTelemetry{
 		StatusCode: rww.statusCode,
 		ReadBytes:  bw.read.Load(),
 		ReadError:  bw.err,
 		WriteBytes: rww.written,
 		WriteError: rww.err,
 	})...)
 
-	// Add metrics
-	attributes := append(labeler.Get(), semconvutil.HTTPServerRequestMetrics(h.server, r)...)
-	if rww.statusCode > 0 {
-		attributes = append(attributes, semconv.HTTPStatusCode(rww.statusCode))
-	}
-	o := metric.WithAttributeSet(attribute.NewSet(attributes...))
-
-	h.requestBytesCounter.Add(ctx, bw.read.Load(), o)
-	h.responseBytesCounter.Add(ctx, rww.written, o)
-
 	// Use floating point division here for higher precision (instead of Millisecond method).
 	elapsedTime := float64(time.Since(requestStartTime)) / float64(time.Millisecond)
 
-	h.serverLatencyMeasure.Record(ctx, elapsedTime, o)
+	h.semconv.RecordMetrics(ctx, semconv.MetricData{
+		ServerName:           h.server,
+		Req:                  r,
+		StatusCode:           rww.statusCode,
+		AdditionalAttributes: labeler.Get(),
+		RequestSize:          bw.read.Load(),
+		ResponseSize:         rww.written,
+		ElapsedTime:          elapsedTime,
+	})
 }
 
 // WithRouteTag annotates spans and metrics with the provided route name
 // with HTTP route attribute.
 func WithRouteTag(route string, h http.Handler) http.Handler {
-	attr := semconv.NewHTTPServer().Route(route)
+	attr := semconv.NewHTTPServer(nil).Route(route)
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		span := trace.SpanFromContext(r.Context())
 		span.SetAttributes(attr)

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

@@ -35,7 +35,7 @@ func BenchmarkHTTPServerRequest(b *testing.B) {
 		RemoteAddr: "127.0.0.1:38738",
 		RequestURI: "/",
 	}
-	serv := NewHTTPServer()
+	serv := NewHTTPServer(nil)
 
 	b.ReportAllocs()
 	b.ResetTimer()

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

@@ -4,13 +4,15 @@
 package semconv // import "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv"
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"os"
 	"strings"
 
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
+	"go.opentelemetry.io/otel/metric"
 )
 
 type ResponseTelemetry struct {
@@ -23,6 +25,11 @@ type ResponseTelemetry struct {
 
 type HTTPServer struct {
 	duplicate bool
+
+	// Old metrics
+	requestBytesCounter  metric.Int64Counter
+	responseBytesCounter metric.Int64Counter
+	serverLatencyMeasure metric.Float64Histogram
 }
 
 // RequestTraceAttrs returns trace attributes for an HTTP request received by a
@@ -63,15 +70,10 @@ func (s HTTPServer) Route(route string) attribute.KeyValue {
 	return oldHTTPServer{}.Route(route)
 }
 
-func NewHTTPServer() HTTPServer {
-	env := strings.ToLower(os.Getenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE"))
-	return HTTPServer{duplicate: env == "http/dup"}
-}
-
-// ServerStatus returns a span status code and message for an HTTP status code
+// Status returns a span status code and message for an HTTP status code
 // value returned by a server. Status codes in the 400-499 range are not
 // returned as errors.
-func ServerStatus(code int) (codes.Code, string) {
+func (s HTTPServer) Status(code int) (codes.Code, string) {
 	if code < 100 || code >= 600 {
 		return codes.Error, fmt.Sprintf("Invalid HTTP status code %d", code)
 	}
@@ -81,6 +83,43 @@ func ServerStatus(code int) (codes.Code, string) {
 	return codes.Unset, ""
 }
 
+type MetricData struct {
+	ServerName           string
+	Req                  *http.Request
+	StatusCode           int
+	AdditionalAttributes []attribute.KeyValue
+
+	RequestSize  int64
+	ResponseSize int64
+	ElapsedTime  float64
+}
+
+func (s HTTPServer) RecordMetrics(ctx context.Context, md MetricData) {
+	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...)
+	s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o)
+
+	// TODO: Duplicate Metrics
+}
+
+func NewHTTPServer(meter metric.Meter) HTTPServer {
+	env := strings.ToLower(os.Getenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE"))
+	duplicate := env == "http/dup"
+	server := HTTPServer{
+		duplicate: duplicate,
+	}
+	server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = oldHTTPServer{}.createMeasures(meter)
+	return server
+}
+
 type HTTPClient struct {
 	duplicate bool
 }

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

@@ -0,0 +1,83 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package semconv
+
+import (
+	"context"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/metric"
+	"go.opentelemetry.io/otel/metric/embedded"
+	"go.opentelemetry.io/otel/metric/noop"
+)
+
+func TestHTTPServerDoesNotPanic(t *testing.T) {
+	testCases := []struct {
+		name   string
+		server HTTPServer
+	}{
+		{
+			name:   "empty",
+			server: HTTPServer{},
+		},
+		{
+			name:   "nil meter",
+			server: NewHTTPServer(nil),
+		},
+		{
+			name:   "with Meter",
+			server: NewHTTPServer(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.server.RequestTraceAttrs("stuff", req)
+				_ = tt.server.ResponseTraceAttrs(ResponseTelemetry{StatusCode: 200})
+				tt.server.RecordMetrics(context.Background(), MetricData{
+					ServerName: "stuff",
+					Req:        req,
+				})
+			})
+		})
+	}
+}
+
+type testInst struct {
+	embedded.Int64Counter
+	embedded.Float64Histogram
+
+	intValue   int64
+	floatValue float64
+	attributes []attribute.KeyValue
+}
+
+func (t *testInst) Add(ctx context.Context, incr int64, options ...metric.AddOption) {
+	t.intValue = incr
+	cfg := metric.NewAddConfig(options)
+	attr := cfg.Attributes()
+	t.attributes = attr.ToSlice()
+}
+
+func (t *testInst) Record(ctx context.Context, value float64, options ...metric.RecordOption) {
+	t.floatValue = value
+	cfg := metric.NewRecordConfig(options)
+	attr := cfg.Attributes()
+	t.attributes = attr.ToSlice()
+}
+
+func NewTestHTTPServer() HTTPServer {
+	return HTTPServer{
+		requestBytesCounter:  &testInst{},
+		responseBytesCounter: &testInst{},
+		serverLatencyMeasure: &testInst{},
+	}
+}

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

@@ -18,7 +18,7 @@ import (
 
 func TestNewTraceRequest(t *testing.T) {
 	t.Setenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE", "http/dup")
-	serv := NewHTTPServer()
+	serv := NewHTTPServer(nil)
 	want := func(req testServerReq) []attribute.KeyValue {
 		return []attribute.KeyValue{
 			attribute.String("http.request.method", "GET"),

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

@@ -9,6 +9,7 @@
 	"strconv"
 	"strings"
 
+	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
 	semconvNew "go.opentelemetry.io/otel/semconv/v1.26.0"
 )
@@ -89,3 +90,9 @@
 	http.MethodPut:     semconvNew.HTTPRequestMethodPut,
 	http.MethodTrace:   semconvNew.HTTPRequestMethodTrace,
 }
+
+func handleErr(err error) {
+	if err != nil {
+		otel.Handle(err)
+	}
+}