HTTP Semconv migration Part1 Server - v1.20.0 support

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.RequestTraceAttrs(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.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)...)
@@ -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/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.RequestTraceAttrs("", req))
+}
+
+func testTraceResponse(t *testing.T, serv HTTPServer, want []attribute.KeyValue) {
+	t.Helper()
+	emptyResp := ResponseTelemetry{}
+	assert.Len(t, serv.ResponseTraceAttrs(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.ResponseTraceAttrs(resp))
+}

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

@@ -0,0 +1,92 @@
+// Code created by gotmpl. DO NOT MODIFY.
+// source: internal/shared/semconv/env.go.tmpl
+
+// 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 "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv"
+
+import (
+	"fmt"
+	"net/http"
+
+	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/codes"
+)
+
+type ResponseTelemetry struct {
+	StatusCode int
+	ReadBytes  int64
+	ReadError  error
+	WriteBytes int64
+	WriteError error
+}
+
+type HTTPServer interface {
+	// RequestTraceAttrs returns trace attributes for an HTTP request received by a
+	// server.
+	//
+	// The server must be the primary server name if it is known. For example this
+	// would be the ServerName directive
+	// (https://httpd.apache.org/docs/2.4/mod/core.html#servername) for an Apache
+	// server, and the server_name directive
+	// (http://nginx.org/en/docs/http/ngx_http_core_module.html#server_name) for an
+	// nginx server. More generically, the primary server name would be the host
+	// header value that matches the default virtual host of an HTTP server. It
+	// should include the host identifier and if a port is used to route to the
+	// server that port identifier should be included as an appropriate port
+	// suffix.
+	//
+	// If the primary server name is not known, server should be an empty string.
+	// The req Host will be used to determine the server instead.
+	RequestTraceAttrs(server string, req *http.Request) []attribute.KeyValue
+
+	// ResponseTraceAttrs returns trace attributes for telemetry from an HTTP response.
+	//
+	// If any of the fields in the ResponseTelemetry are not set the attribute will be omitted.
+	ResponseTraceAttrs(ResponseTelemetry) []attribute.KeyValue
+
+	// Route returns the attribute for the route.
+	Route(string) attribute.KeyValue
+}
+
+// var warnOnce = sync.Once{}
+
+func NewHTTPServer() HTTPServer {
+	// env := strings.ToLower(os.Getenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE"))
+	// switch env {
+	// case "http":
+	// 	return newHTTPServer{}
+	// case "http/dup":
+	// 	return dupHTTPServer{}
+	// default:
+	// 	warnOnce.Do(func() {
+	// 		otel.Handle(errors.New("deprecated: old semantic conventions are being used. Use the environment variable OTEL_HTTP_CLIENT_COMPATIBILITY_MODE to opt into the new conventions. Setting it to `http/dup` will provide combined old-and-new attributes. Setting it to `http` will switch directly to the new attributes. This will be removed in a future release"))
+	// 	})
+	return oldHTTPServer{}
+	// }
+}
+
+// ServerStatus 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) {
+	if code < 100 || code >= 600 {
+		return codes.Error, fmt.Sprintf("Invalid HTTP status code %d", code)
+	}
+	if code >= 500 {
+		return codes.Error, ""
+	}
+	return codes.Unset, ""
+}

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

@@ -0,0 +1,58 @@
+// Code created by gotmpl. DO NOT MODIFY.
+// source: internal/shared/semconvutil/httpconv_test.go.tmpl
+
+// 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 (
+	"net/http"
+	"net/url"
+	"testing"
+
+	"go.opentelemetry.io/otel/attribute"
+)
+
+var benchHTTPServerRequestResults []attribute.KeyValue
+
+// BenchmarkHTTPServerRequest allows comparison between different version of the HTTP server.
+// To use an alternative start this test with OTEL_HTTP_CLIENT_COMPATIBILITY_MODE set to the
+// version under test.
+func BenchmarkHTTPServerRequest(b *testing.B) {
+	// Request was generated from TestHTTPServerRequest request.
+	req := &http.Request{
+		Method: http.MethodGet,
+		URL: &url.URL{
+			Path: "/",
+		},
+		Proto:      "HTTP/1.1",
+		ProtoMajor: 1,
+		ProtoMinor: 1,
+		Header: http.Header{
+			"User-Agent":      []string{"Go-http-client/1.1"},
+			"Accept-Encoding": []string{"gzip"},
+		},
+		Body:       http.NoBody,
+		Host:       "127.0.0.1:39093",
+		RemoteAddr: "127.0.0.1:38738",
+		RequestURI: "/",
+	}
+	serv := NewHTTPServer()
+
+	b.ReportAllocs()
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		benchHTTPServerRequestResults = serv.RequestTraceAttrs("", req)
+	}
+}