From 8846bcf15276817f66b412d783c73d4b82a32151 Mon Sep 17 00:00:00 2001 From: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com> Date: Tue, 9 Jul 2024 14:45:37 -0400 Subject: [PATCH] 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 Co-authored-by: Tyler Yahn --- .../otelhttp/internal/semconv/common_test.go | 88 +++++++++++++++++++ .../net/http/otelhttp/internal/semconv/env.go | 31 +++++++ .../http/otelhttp/internal/semconv/v1.20.0.go | 10 +++ .../otelhttp/internal/semconv/v1.20.0_test.go | 36 ++++++++ .../net/http/otelhttp/transport.go | 13 +-- 5 files changed, 173 insertions(+), 5 deletions(-) diff --git a/instrumentation/net/http/otelhttp/internal/semconv/common_test.go b/instrumentation/net/http/otelhttp/internal/semconv/common_test.go index 904b774ccb2..c6b373b6c37 100644 --- 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 index 3ec0ad00c81..9948abe44b8 100644 --- 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 index c3e838aaa54..c2bc041ac50 100644 --- 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 index 3a84697ea4e..fcf116f94f5 100644 --- 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 index c869f6d4bfc..06948c11cb1 100644 --- 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)