instrumentation/net/http/otelhttp: support duplicate both old and new…

… attributes for HTTPClient (#5945)

This PR implements the HTTPClient that can duplicate attributes for old
semconv v1.20.0 and new semconv v1.26.0.
It's part of the migration plan #5332

---------

Co-authored-by: Damien Mathieu <[email protected]>

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

@@ -82,23 +82,28 @@ func ServerStatus(code int) (codes.Code, string) {
 }
 
 type HTTPClient struct {
-	// TODO (#5332): Support for new semantic conventions
-	// duplicate bool
+	duplicate bool
 }
 
 func NewHTTPClient() HTTPClient {
-	// TODO (#5332): Support for new semantic conventions
-	// env := strings.ToLower(os.Getenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE"))
-	return HTTPClient{}
+	env := strings.ToLower(os.Getenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE"))
+	return HTTPClient{duplicate: env == "http/dup"}
 }
 
 // RequestTraceAttrs returns attributes for an HTTP request made by a client.
 func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue {
+	if c.duplicate {
+		return append(oldHTTPClient{}.RequestTraceAttrs(req), newHTTPClient{}.RequestTraceAttrs(req)...)
+	}
 	return oldHTTPClient{}.RequestTraceAttrs(req)
 }
 
 // ResponseTraceAttrs returns metric attributes for an HTTP request made by a client.
 func (c HTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue {
+	if c.duplicate {
+		return append(oldHTTPClient{}.ResponseTraceAttrs(resp), newHTTPClient{}.ResponseTraceAttrs(resp)...)
+	}
+
 	return oldHTTPClient{}.ResponseTraceAttrs(resp)
 }
 
@@ -111,3 +116,11 @@ func (c HTTPClient) Status(code int) (codes.Code, string) {
 	}
 	return codes.Unset, ""
 }
+
+func (c HTTPClient) ErrorType(err error) attribute.KeyValue {
+	if c.duplicate {
+		return newHTTPClient{}.ErrorType(err)
+	}
+
+	return attribute.KeyValue{}
+}

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

@@ -4,7 +4,10 @@
 package semconv // import "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv"
 
 import (
+	"fmt"
 	"net/http"
+	"reflect"
+	"strconv"
 	"strings"
 
 	"go.opentelemetry.io/otel/attribute"
@@ -195,3 +198,151 @@ func (n newHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.Ke
 func (n newHTTPServer) Route(route string) attribute.KeyValue {
 	return semconvNew.HTTPRoute(route)
 }
+
+type newHTTPClient struct{}
+
+// RequestTraceAttrs returns trace attributes for an HTTP request made by a client.
+func (n newHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue {
+	/*
+	   below attributes are returned:
+	   - http.request.method
+	   - http.request.method.original
+	   - url.full
+	   - server.address
+	   - server.port
+	   - network.protocol.name
+	   - network.protocol.version
+	*/
+	numOfAttributes := 3 // URL, server address, proto, and method.
+
+	var urlHost string
+	if req.URL != nil {
+		urlHost = req.URL.Host
+	}
+	var requestHost string
+	var requestPort int
+	for _, hostport := range []string{urlHost, req.Header.Get("Host")} {
+		requestHost, requestPort = splitHostPort(hostport)
+		if requestHost != "" || requestPort > 0 {
+			break
+		}
+	}
+
+	eligiblePort := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", requestPort)
+	if eligiblePort > 0 {
+		numOfAttributes++
+	}
+	useragent := req.UserAgent()
+	if useragent != "" {
+		numOfAttributes++
+	}
+
+	protoName, protoVersion := netProtocol(req.Proto)
+	if protoName != "" && protoName != "http" {
+		numOfAttributes++
+	}
+	if protoVersion != "" {
+		numOfAttributes++
+	}
+
+	method, originalMethod := n.method(req.Method)
+	if originalMethod != (attribute.KeyValue{}) {
+		numOfAttributes++
+	}
+
+	attrs := make([]attribute.KeyValue, 0, numOfAttributes)
+
+	attrs = append(attrs, method)
+	if originalMethod != (attribute.KeyValue{}) {
+		attrs = append(attrs, originalMethod)
+	}
+
+	var u string
+	if req.URL != nil {
+		// Remove any username/password info that may be in the URL.
+		userinfo := req.URL.User
+		req.URL.User = nil
+		u = req.URL.String()
+		// Restore any username/password info that was removed.
+		req.URL.User = userinfo
+	}
+	attrs = append(attrs, semconvNew.URLFull(u))
+
+	attrs = append(attrs, semconvNew.ServerAddress(requestHost))
+	if eligiblePort > 0 {
+		attrs = append(attrs, semconvNew.ServerPort(eligiblePort))
+	}
+
+	if protoName != "" && protoName != "http" {
+		attrs = append(attrs, semconvNew.NetworkProtocolName(protoName))
+	}
+	if protoVersion != "" {
+		attrs = append(attrs, semconvNew.NetworkProtocolVersion(protoVersion))
+	}
+
+	return attrs
+}
+
+// ResponseTraceAttrs returns trace attributes for an HTTP response made by a client.
+func (n newHTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue {
+	/*
+	   below attributes are returned:
+	   - http.response.status_code
+	   - error.type
+	*/
+	var count int
+	if resp.StatusCode > 0 {
+		count++
+	}
+
+	if isErrorStatusCode(resp.StatusCode) {
+		count++
+	}
+
+	attrs := make([]attribute.KeyValue, 0, count)
+	if resp.StatusCode > 0 {
+		attrs = append(attrs, semconvNew.HTTPResponseStatusCode(resp.StatusCode))
+	}
+
+	if isErrorStatusCode(resp.StatusCode) {
+		errorType := strconv.Itoa(resp.StatusCode)
+		attrs = append(attrs, semconvNew.ErrorTypeKey.String(errorType))
+	}
+	return attrs
+}
+
+func (n newHTTPClient) ErrorType(err error) attribute.KeyValue {
+	t := reflect.TypeOf(err)
+	var value string
+	if t.PkgPath() == "" && t.Name() == "" {
+		// Likely a builtin type.
+		value = t.String()
+	} else {
+		value = fmt.Sprintf("%s.%s", t.PkgPath(), t.Name())
+	}
+
+	if value == "" {
+		return semconvNew.ErrorTypeOther
+	}
+
+	return semconvNew.ErrorTypeKey.String(value)
+}
+
+func (n newHTTPClient) method(method string) (attribute.KeyValue, attribute.KeyValue) {
+	if method == "" {
+		return semconvNew.HTTPRequestMethodGet, attribute.KeyValue{}
+	}
+	if attr, ok := methodLookup[method]; ok {
+		return attr, attribute.KeyValue{}
+	}
+
+	orig := semconvNew.HTTPRequestMethodOriginal(method)
+	if attr, ok := methodLookup[strings.ToUpper(method)]; ok {
+		return attr, orig
+	}
+	return semconvNew.HTTPRequestMethodGet, orig
+}
+
+func isErrorStatusCode(code int) bool {
+	return code >= 400 || code < 100
+}

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

@@ -4,8 +4,11 @@
 package semconv
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
+	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -126,3 +129,98 @@ func TestNewMethod(t *testing.T) {
 		})
 	}
 }
+
+func TestNewTraceRequest_Client(t *testing.T) {
+	t.Setenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE", "http/dup")
+	body := strings.NewReader("Hello, world!")
+	url := "https://example.com:8888/foo/bar?stuff=morestuff"
+	req := httptest.NewRequest("pOST", url, body)
+	req.Header.Set("User-Agent", "go-test-agent")
+
+	want := []attribute.KeyValue{
+		attribute.String("http.request.method", "POST"),
+		attribute.String("http.request.method_original", "pOST"),
+		attribute.String("http.method", "pOST"),
+		attribute.String("url.full", url),
+		attribute.String("http.url", url),
+		attribute.String("server.address", "example.com"),
+		attribute.Int("server.port", 8888),
+		attribute.String("network.protocol.version", "1.1"),
+		attribute.String("net.peer.name", "example.com"),
+		attribute.Int("net.peer.port", 8888),
+		attribute.String("user_agent.original", "go-test-agent"),
+		attribute.Int("http.request_content_length", 13),
+	}
+	client := NewHTTPClient()
+	assert.ElementsMatch(t, want, client.RequestTraceAttrs(req))
+}
+
+func TestNewTraceResponse_Client(t *testing.T) {
+	t.Setenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE", "http/dup")
+	testcases := []struct {
+		resp http.Response
+		want []attribute.KeyValue
+	}{
+		{resp: http.Response{StatusCode: 200, ContentLength: 123}, want: []attribute.KeyValue{attribute.Int("http.response.status_code", 200), attribute.Int("http.status_code", 200), attribute.Int("http.response_content_length", 123)}},
+		{resp: http.Response{StatusCode: 404, ContentLength: 0}, want: []attribute.KeyValue{attribute.Int("http.response.status_code", 404), attribute.Int("http.status_code", 404), attribute.String("error.type", "404")}},
+	}
+
+	for _, tt := range testcases {
+		client := NewHTTPClient()
+		assert.ElementsMatch(t, tt.want, client.ResponseTraceAttrs(&tt.resp))
+	}
+}
+
+func TestClientRequest(t *testing.T) {
+	body := strings.NewReader("Hello, world!")
+	url := "https://example.com:8888/foo/bar?stuff=morestuff"
+	req := httptest.NewRequest("pOST", url, body)
+	req.Header.Set("User-Agent", "go-test-agent")
+
+	want := []attribute.KeyValue{
+		attribute.String("http.request.method", "POST"),
+		attribute.String("http.request.method_original", "pOST"),
+		attribute.String("url.full", url),
+		attribute.String("server.address", "example.com"),
+		attribute.Int("server.port", 8888),
+		attribute.String("network.protocol.version", "1.1"),
+	}
+	got := newHTTPClient{}.RequestTraceAttrs(req)
+	assert.ElementsMatch(t, want, got)
+}
+
+func TestClientResponse(t *testing.T) {
+	testcases := []struct {
+		resp http.Response
+		want []attribute.KeyValue
+	}{
+		{resp: http.Response{StatusCode: 200, ContentLength: 123}, want: []attribute.KeyValue{attribute.Int("http.response.status_code", 200)}},
+		{resp: http.Response{StatusCode: 404, ContentLength: 0}, want: []attribute.KeyValue{attribute.Int("http.response.status_code", 404), attribute.String("error.type", "404")}},
+	}
+
+	for _, tt := range testcases {
+		got := newHTTPClient{}.ResponseTraceAttrs(&tt.resp)
+		assert.ElementsMatch(t, tt.want, got)
+	}
+}
+
+func TestRequestErrorType(t *testing.T) {
+	testcases := []struct {
+		err  error
+		want attribute.KeyValue
+	}{
+		{err: errors.New("http: nil Request.URL"), want: attribute.String("error.type", "*errors.errorString")},
+		{err: customError{}, want: attribute.String("error.type", "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv.customError")},
+	}
+
+	for _, tt := range testcases {
+		got := newHTTPClient{}.ErrorType(tt.err)
+		assert.Equal(t, tt.want, got)
+	}
+}
+
+type customError struct{}
+
+func (customError) Error() string {
+	return "custom error"
+}

instrumentation/net/http/otelhttp/transport.go

@@ -163,7 +163,15 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) {
 
 	res, err := t.rt.RoundTrip(r)
 	if err != nil {
-		span.RecordError(err)
+		// set error type attribute if the error is part of the predefined
+		// error types.
+		// otherwise, record it as an exception
+		if errType := t.semconv.ErrorType(err); errType.Valid() {
+			span.SetAttributes(errType)
+		} else {
+			span.RecordError(err)
+		}
+
 		span.SetStatus(codes.Error, err.Error())
 		span.End()
 		return res, err