HTTP Semconv migration Part1 Client - v1.24.0 support (#5773)

This change adds the new semantic version (v1.24.0) attribute producer to the semconv of otlehttp. Part of #5332 --------- Co-authored-by: Aaron Clawson <MadVikingGod@users.noreply.github.com> Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
open-telemetry · Jul 9, 2024 · 8846bcf · 8846bcf
1 parent c80e464
commit 8846bcf
Show file tree

Hide file tree

Showing 5 changed files with 173 additions and 5 deletions.
diff --git a/instrumentation/net/http/otelhttp/internal/semconv/common_test.go b/instrumentation/net/http/otelhttp/internal/semconv/common_test.go
@@ -14,6 +14,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/codes"
 )
 
 type testServerReq struct {
@@ -65,3 +66,90 @@ func testTraceRequest(t *testing.T, serv HTTPServer, want func(testServerReq) []
 
 	assert.ElementsMatch(t, want(srvReq), serv.RequestTraceAttrs("", req))
 }
+
+func TestHTTPClientStatus(t *testing.T) {
+	tests := []struct {
+		code int
+		stat codes.Code
+		msg  bool
+	}{
+		{0, codes.Error, true},
+		{http.StatusContinue, codes.Unset, false},
+		{http.StatusSwitchingProtocols, codes.Unset, false},
+		{http.StatusProcessing, codes.Unset, false},
+		{http.StatusEarlyHints, codes.Unset, false},
+		{http.StatusOK, codes.Unset, false},
+		{http.StatusCreated, codes.Unset, false},
+		{http.StatusAccepted, codes.Unset, false},
+		{http.StatusNonAuthoritativeInfo, codes.Unset, false},
+		{http.StatusNoContent, codes.Unset, false},
+		{http.StatusResetContent, codes.Unset, false},
+		{http.StatusPartialContent, codes.Unset, false},
+		{http.StatusMultiStatus, codes.Unset, false},
+		{http.StatusAlreadyReported, codes.Unset, false},
+		{http.StatusIMUsed, codes.Unset, false},
+		{http.StatusMultipleChoices, codes.Unset, false},
+		{http.StatusMovedPermanently, codes.Unset, false},
+		{http.StatusFound, codes.Unset, false},
+		{http.StatusSeeOther, codes.Unset, false},
+		{http.StatusNotModified, codes.Unset, false},
+		{http.StatusUseProxy, codes.Unset, false},
+		{306, codes.Unset, false},
+		{http.StatusTemporaryRedirect, codes.Unset, false},
+		{http.StatusPermanentRedirect, codes.Unset, false},
+		{http.StatusBadRequest, codes.Error, false},
+		{http.StatusUnauthorized, codes.Error, false},
+		{http.StatusPaymentRequired, codes.Error, false},
+		{http.StatusForbidden, codes.Error, false},
+		{http.StatusNotFound, codes.Error, false},
+		{http.StatusMethodNotAllowed, codes.Error, false},
+		{http.StatusNotAcceptable, codes.Error, false},
+		{http.StatusProxyAuthRequired, codes.Error, false},
+		{http.StatusRequestTimeout, codes.Error, false},
+		{http.StatusConflict, codes.Error, false},
+		{http.StatusGone, codes.Error, false},
+		{http.StatusLengthRequired, codes.Error, false},
+		{http.StatusPreconditionFailed, codes.Error, false},
+		{http.StatusRequestEntityTooLarge, codes.Error, false},
+		{http.StatusRequestURITooLong, codes.Error, false},
+		{http.StatusUnsupportedMediaType, codes.Error, false},
+		{http.StatusRequestedRangeNotSatisfiable, codes.Error, false},
+		{http.StatusExpectationFailed, codes.Error, false},
+		{http.StatusTeapot, codes.Error, false},
+		{http.StatusMisdirectedRequest, codes.Error, false},
+		{http.StatusUnprocessableEntity, codes.Error, false},
+		{http.StatusLocked, codes.Error, false},
+		{http.StatusFailedDependency, codes.Error, false},
+		{http.StatusTooEarly, codes.Error, false},
+		{http.StatusUpgradeRequired, codes.Error, false},
+		{http.StatusPreconditionRequired, codes.Error, false},
+		{http.StatusTooManyRequests, codes.Error, false},
+		{http.StatusRequestHeaderFieldsTooLarge, codes.Error, false},
+		{http.StatusUnavailableForLegalReasons, codes.Error, false},
+		{499, codes.Error, false},
+		{http.StatusInternalServerError, codes.Error, false},
+		{http.StatusNotImplemented, codes.Error, false},
+		{http.StatusBadGateway, codes.Error, false},
+		{http.StatusServiceUnavailable, codes.Error, false},
+		{http.StatusGatewayTimeout, codes.Error, false},
+		{http.StatusHTTPVersionNotSupported, codes.Error, false},
+		{http.StatusVariantAlsoNegotiates, codes.Error, false},
+		{http.StatusInsufficientStorage, codes.Error, false},
+		{http.StatusLoopDetected, codes.Error, false},
+		{http.StatusNotExtended, codes.Error, false},
+		{http.StatusNetworkAuthenticationRequired, codes.Error, false},
+		{600, codes.Error, true},
+	}
+
+	for _, test := range tests {
+		t.Run(strconv.Itoa(test.code), func(t *testing.T) {
+			c, msg := HTTPClient{}.Status(test.code)
+			assert.Equal(t, test.stat, c)
+			if test.msg && msg == "" {
+				t.Errorf("expected non-empty message for %d", test.code)
+			} else if !test.msg && msg != "" {
+				t.Errorf("expected empty message for %d, got: %s", test.code, msg)
+			}
+		})
+	}
+}
diff --git a/instrumentation/net/http/otelhttp/internal/semconv/env.go b/instrumentation/net/http/otelhttp/internal/semconv/env.go
@@ -80,3 +80,34 @@ func ServerStatus(code int) (codes.Code, string) {
 	}
 	return codes.Unset, ""
 }
+
+type HTTPClient struct {
+	// TODO (#5332): Support for new semantic conventions
+	// duplicate bool
+}
+
+func NewHTTPClient() HTTPClient {
+	// TODO (#5332): Support for new semantic conventions
+	// env := strings.ToLower(os.Getenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE"))
+	return HTTPClient{}
+}
+
+// RequestTraceAttrs returns attributes for an HTTP request made by a client.
+func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue {
+	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 {
+	return oldHTTPClient{}.ResponseTraceAttrs(resp)
+}
+
+func (c HTTPClient) Status(code int) (codes.Code, string) {
+	if code < 100 || code >= 600 {
+		return codes.Error, fmt.Sprintf("Invalid HTTP status code %d", code)
+	}
+	if code >= 400 {
+		return codes.Error, ""
+	}
+	return codes.Unset, ""
+}
diff --git a/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0.go b/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0.go
@@ -72,3 +72,13 @@ func (o oldHTTPServer) Route(route string) attribute.KeyValue {
 func HTTPStatusCode(status int) attribute.KeyValue {
 	return semconv.HTTPStatusCode(status)
 }
+
+type oldHTTPClient struct{}
+
+func (o oldHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue {
+	return semconvutil.HTTPClientRequest(req)
+}
+
+func (o oldHTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue {
+	return semconvutil.HTTPClientResponse(resp)
+}
diff --git a/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0_test.go b/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0_test.go
@@ -5,6 +5,8 @@ package semconv
 
 import (
 	"fmt"
+	"net/http"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -83,3 +85,37 @@ func TestV120TraceResponse(t *testing.T) {
 		})
 	}
 }
+
+func TestV120ClientRequest(t *testing.T) {
+	body := strings.NewReader("Hello, world!")
+	url := "https://example.com:8888/foo/bar?stuff=morestuff"
+	req, err := http.NewRequest("POST", url, body)
+	assert.NoError(t, err)
+	req.Header.Set("User-Agent", "go-test-agent")
+
+	want := []attribute.KeyValue{
+		attribute.String("http.method", "POST"),
+		attribute.String("http.url", url),
+		attribute.String("net.peer.name", "example.com"),
+		attribute.Int("net.peer.port", 8888),
+		attribute.Int("http.request_content_length", body.Len()),
+		attribute.String("user_agent.original", "go-test-agent"),
+	}
+	got := oldHTTPClient{}.RequestTraceAttrs(req)
+	assert.ElementsMatch(t, want, got)
+}
+
+func TestV120ClientResponse(t *testing.T) {
+	resp := http.Response{
+		StatusCode:    200,
+		ContentLength: 123,
+	}
+
+	want := []attribute.KeyValue{
+		attribute.Int("http.response_content_length", 123),
+		attribute.Int("http.status_code", 200),
+	}
+
+	got := oldHTTPClient{}.ResponseTraceAttrs(&resp)
+	assert.ElementsMatch(t, want, got)
+}
diff --git a/instrumentation/net/http/otelhttp/transport.go b/instrumentation/net/http/otelhttp/transport.go
@@ -11,13 +11,14 @@ import (
 	"sync/atomic"
 	"time"
 
+	"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/codes"
 	"go.opentelemetry.io/otel/metric"
 	"go.opentelemetry.io/otel/propagation"
-	semconv "go.opentelemetry.io/otel/semconv/v1.20.0"
+
 	"go.opentelemetry.io/otel/trace"
 )
 
@@ -34,6 +35,7 @@ type Transport struct {
 	spanNameFormatter func(string, *http.Request) string
 	clientTrace       func(context.Context) *httptrace.ClientTrace
 
+	semconv              semconv.HTTPClient
 	requestBytesCounter  metric.Int64Counter
 	responseBytesCounter metric.Int64Counter
 	latencyMeasure       metric.Float64Histogram
@@ -53,7 +55,8 @@ func NewTransport(base http.RoundTripper, opts ...Option) *Transport {
 	}
 
 	t := Transport{
-		rt: base,
+		rt:      base,
+		semconv: semconv.NewHTTPClient(),
 	}
 
 	defaultOpts := []Option{
@@ -155,7 +158,7 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) {
 		r.Body = &bw
 	}
 
-	span.SetAttributes(semconvutil.HTTPClientRequest(r)...)
+	span.SetAttributes(t.semconv.RequestTraceAttrs(r)...)
 	t.propagators.Inject(ctx, propagation.HeaderCarrier(r.Header))
 
 	res, err := t.rt.RoundTrip(r)
@@ -180,8 +183,8 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) {
 	}
 
 	// traces
-	span.SetAttributes(semconvutil.HTTPClientResponse(res)...)
-	span.SetStatus(semconvutil.HTTPClientStatus(res.StatusCode))
+	span.SetAttributes(t.semconv.ResponseTraceAttrs(res)...)
+	span.SetStatus(t.semconv.Status(res.StatusCode))
 
 	res.Body = newWrappedBody(span, readRecordFunc, res.Body)