Add semconv v1.24.0 to otelhttp

CHANGELOG.md

@@ -11,6 +11,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ### Added
 
 - Add the new `go.opentelemetry.io/contrib/instrgen` package to provide auto-generated source code instrumentation. (#3068, #3108)
+- Add support for Semantic Convention v1.24.0 in the server instrumentation of `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#5092)
+  - To enable the new semantic conventions set the environment variable `OTEL_HTTP_CLIENT_COMPATIBILITY_MODE` to `http/dup` or `http`.
 
 ### Removed
 

instrumentation/net/http/otelhttp/doc.go

@@ -4,4 +4,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

@@ -4,18 +4,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"
 )
 
@@ -35,6 +33,7 @@ type middleware struct {
 	publicEndpoint    bool
 	publicEndpointFn  func(*http.Request) bool
 
+	traceSemconv         semconv.HTTPServer
 	requestBytesCounter  metric.Int64Counter
 	responseBytesCounter metric.Int64Counter
 	serverLatencyMeasure metric.Float64Histogram
@@ -56,6 +55,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{
@@ -132,12 +133,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())
@@ -213,7 +211,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)...)
@@ -230,37 +235,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/README.md

@@ -0,0 +1,22 @@
+# Migration Plan
+- Current release - v0.49.0
+    - Support only for v1.20.0 semantic conventions
+- v0.50.0
+    - Add support for v1.20.0, v1.24.0, and both semantic conventions
+    - Default to v1.20.0
+    - Warn users when using v1.20.0
+- v0.51.0
+    - Default to v1.24.0
+    - Warn users when using v1.20.0 or both semantic conventions
+- v0.52.0
+    - Remove support for v1.20.0
+    - Remove support for both semantic conventions
+    - All users will be using v1.24.0
+
+# User Migration Story
+The goal is to allow users to upgrade versions of the `otelhttp` package and enable migrations of dashboards and alerts incrementally.
+
+When the user first upgrades to v0.49 they will receive a warning that they are using v1.20.0 semantic conventions. This is an indication that action needs to be taken or their dashboards and alerts will be affected.
+
+They can then set the environment variable `OTEL_HTTP_CLIENT_COMPATIBILITY_MODE` to `http/dup` and receive both the original v1.20.0 attributes and the new v1.24.0 attributes. This will allow them to update their dashboards and alerts to use the new attributes and validate they are not missing any data.
+Once the user has completed the migration, they can then set `OTEL_HTTP_CLIENT_COMPATIBILITY_MODE` to `http` to complete the migration to the stable http metric names.

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))
+}