Add semconv v1.24.0 to otelhttp

instrumentation/net/http/otelhttp/doc.go

@@ -15,4 +15,9 @@
 // Package otelhttp provides an http.Handler and functions that are intended
 // to be used to add tracing by wrapping existing handlers (with Handler) and
 // routes WithRouteTag.
+
+// Warning: migration of semantic conventions to v1.24.0 is in progress. Because
+// this will break most existing dashboards we have developed a migration plan
+// detailed [here](). Use the environment variable `OTEL_HTTP_CLIENT_COMPATIBILITY_MODE`
+// to opt into the new conventions. This will be removed in a future release.
 package otelhttp // import "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"

instrumentation/net/http/otelhttp/handler.go

@@ -15,18 +15,16 @@
 package otelhttp // import "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
 
 import (
-	"io"
 	"net/http"
 	"time"
 
 	"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"
-	semconv "go.opentelemetry.io/otel/semconv/v1.20.0"
 	"go.opentelemetry.io/otel/trace"
 )
 
@@ -46,6 +44,7 @@ type middleware struct {
 	publicEndpoint    bool
 	publicEndpointFn  func(*http.Request) bool
 
+	traceSemconv         semconv.HTTPServer
 	requestBytesCounter  metric.Int64Counter
 	responseBytesCounter metric.Int64Counter
 	serverLatencyMeasure metric.Float64Histogram
@@ -67,6 +66,8 @@ 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{
@@ -143,12 +144,9 @@ 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(semconvutil.HTTPServerRequest(h.server, r)...),
-	}
-	if h.server != "" {
-		hostAttr := semconv.NetHostName(h.server)
-		opts = append(opts, trace.WithAttributes(hostAttr))
+		trace.WithAttributes(h.traceSemconv.TraceRequest(h.server, r)...),
 	}
+
 	opts = append(opts, h.spanStartOptions...)
 	if h.publicEndpoint || (h.publicEndpointFn != nil && h.publicEndpointFn(r.WithContext(ctx))) {
 		opts = append(opts, trace.WithNewRoot())
@@ -224,7 +222,14 @@ func (h *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http
 
 	next.ServeHTTP(w, r.WithContext(ctx))
 
-	setAfterServeAttributes(span, bw.read.Load(), rww.written, rww.statusCode, bw.err, rww.err)
+	span.SetStatus(semconv.ServerStatus(rww.statusCode))
+	span.SetAttributes(h.traceSemconv.TraceResponse(semconv.ResponseTelemetry{
+		StatusCode: rww.statusCode,
+		ReadBytes:  int(bw.read.Load()),
+		ReadError:  bw.err,
+		WriteBytes: int(rww.written),
+		WriteError: rww.err,
+	})...)
 
 	// Add metrics
 	attributes := append(labeler.Get(), semconvutil.HTTPServerRequestMetrics(h.server, r)...)
@@ -241,37 +246,11 @@ func (h *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http
 	h.serverLatencyMeasure.Record(ctx, elapsedTime, o)
 }
 
-func setAfterServeAttributes(span trace.Span, read, wrote int64, statusCode int, rerr, werr error) {
-	attributes := []attribute.KeyValue{}
-
-	// TODO: Consider adding an event after each read and write, possibly as an
-	// option (defaulting to off), so as to not create needlessly verbose spans.
-	if read > 0 {
-		attributes = append(attributes, ReadBytesKey.Int64(read))
-	}
-	if rerr != nil && rerr != io.EOF {
-		attributes = append(attributes, ReadErrorKey.String(rerr.Error()))
-	}
-	if wrote > 0 {
-		attributes = append(attributes, WroteBytesKey.Int64(wrote))
-	}
-	if statusCode > 0 {
-		attributes = append(attributes, semconv.HTTPStatusCode(statusCode))
-	}
-	span.SetStatus(semconvutil.HTTPServerStatus(statusCode))
-
-	if werr != nil && werr != io.EOF {
-		attributes = append(attributes, WriteErrorKey.String(werr.Error()))
-	}
-	span.SetAttributes(attributes...)
-}
-
 // 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)
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		attr := semconv.HTTPRouteKey.String(route)
-
 		span := trace.SpanFromContext(r.Context())
 		span.SetAttributes(attr)
 

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

@@ -0,0 +1,93 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package semconv
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strconv"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"go.opentelemetry.io/otel/attribute"
+)
+
+type testServerReq struct {
+	hostname   string
+	serverPort int
+	peerAddr   string
+	peerPort   int
+	clientIP   string
+}
+
+func testTraceRequest(t *testing.T, serv HTTPServer, want func(testServerReq) []attribute.KeyValue) {
+	t.Helper()
+
+	got := make(chan *http.Request, 1)
+	handler := func(w http.ResponseWriter, r *http.Request) {
+		got <- r
+		w.WriteHeader(http.StatusOK)
+	}
+
+	srv := httptest.NewServer(http.HandlerFunc(handler))
+	defer srv.Close()
+
+	srvURL, err := url.Parse(srv.URL)
+	require.NoError(t, err)
+	srvPort, err := strconv.ParseInt(srvURL.Port(), 10, 32)
+	require.NoError(t, err)
+
+	resp, err := srv.Client().Get(srv.URL)
+	require.NoError(t, err)
+	require.NoError(t, resp.Body.Close())
+
+	req := <-got
+	peer, peerPort := splitHostPort(req.RemoteAddr)
+
+	const user = "alice"
+	req.SetBasicAuth(user, "pswrd")
+
+	const clientIP = "127.0.0.5"
+	req.Header.Add("X-Forwarded-For", clientIP)
+
+	srvReq := testServerReq{
+		hostname:   srvURL.Hostname(),
+		serverPort: int(srvPort),
+		peerAddr:   peer,
+		peerPort:   peerPort,
+		clientIP:   clientIP,
+	}
+
+	assert.ElementsMatch(t, want(srvReq), serv.TraceRequest("", req))
+}
+
+func testTraceResponse(t *testing.T, serv HTTPServer, want []attribute.KeyValue) {
+	t.Helper()
+	emptyResp := ResponseTelemetry{}
+	assert.Len(t, serv.TraceResponse(emptyResp), 0)
+
+	resp := ResponseTelemetry{
+		StatusCode: 200,
+		ReadBytes:  701,
+		ReadError:  fmt.Errorf("read error"),
+		WriteBytes: 802,
+		WriteError: fmt.Errorf("write error"),
+	}
+	assert.ElementsMatch(t, want, serv.TraceResponse(resp))
+}