From 7b749591320bfcdef2061f4d4f5aa533ab76b47f Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 7 Feb 2023 11:12:41 -0800 Subject: [PATCH] Remove http.target attr from ServerRequest (#3687) * Remove http.target attr from ServerRequest * Update changelog * Remove trailing space in changelog --- CHANGELOG.md | 9 +++++++++ semconv/internal/v2/http.go | 17 ++++++++--------- semconv/internal/v2/http_test.go | 2 -- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f9c62f896f..9274472c3f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,15 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Attribute `KeyValue` creations functions to `go.opentelemetry.io/otel/semconv/v1.17.0` for all non-enum semantic conventions. These functions ensure semantic convention type correctness. +### Fixed + +- Removed the `http.target` attribute from being added by `ServerRequest` in the following packages. (#3687) + - `go.opentelemetry.io/otel/semconv/v1.13.0/httpconv` + - `go.opentelemetry.io/otel/semconv/v1.14.0/httpconv` + - `go.opentelemetry.io/otel/semconv/v1.15.0/httpconv` + - `go.opentelemetry.io/otel/semconv/v1.16.0/httpconv` + - `go.opentelemetry.io/otel/semconv/v1.17.0/httpconv` + ### Removed - The deprecated `go.opentelemetry.io/otel/metric/instrument/asyncfloat64` package is removed. (#3631) diff --git a/semconv/internal/v2/http.go b/semconv/internal/v2/http.go index c7c47fa8815..12d6b520f52 100644 --- a/semconv/internal/v2/http.go +++ b/semconv/internal/v2/http.go @@ -157,7 +157,14 @@ func (c *HTTPConv) ClientRequest(req *http.Request) []attribute.KeyValue { // "net.sock.peer.addr", "net.sock.peer.port", "http.user_agent", "enduser.id", // "http.client_ip". func (c *HTTPConv) ServerRequest(server string, req *http.Request) []attribute.KeyValue { - n := 5 // Method, scheme, target, proto, and host name. + // TODO: This currently does not add the specification required + // `http.target` attribute. It has too high of a cardinality to safely be + // added. An alternate should be added, or this comment removed, when it is + // addressed by the specification. If it is ultimately decided to continue + // not including the attribute, the HTTPTargetKey field of the HTTPConv + // should be removed as well. + + n := 4 // Method, scheme, proto, and host name. var host string var p int if server == "" { @@ -199,14 +206,6 @@ func (c *HTTPConv) ServerRequest(server string, req *http.Request) []attribute.K attrs = append(attrs, c.proto(req.Proto)) attrs = append(attrs, c.NetConv.HostName(host)) - if req.URL != nil { - attrs = append(attrs, c.HTTPTargetKey.String(req.URL.RequestURI())) - } else { - // This should never occur if the request was generated by the net/http - // package. Fail gracefully, if it does though. - attrs = append(attrs, c.HTTPTargetKey.String(req.RequestURI)) - } - if hostPort > 0 { attrs = append(attrs, c.NetConv.HostPort(hostPort)) } diff --git a/semconv/internal/v2/http_test.go b/semconv/internal/v2/http_test.go index 9f91a2f7b4d..43a20c5febc 100644 --- a/semconv/internal/v2/http_test.go +++ b/semconv/internal/v2/http_test.go @@ -167,7 +167,6 @@ func TestHTTPServerRequest(t *testing.T) { assert.ElementsMatch(t, []attribute.KeyValue{ attribute.String("http.method", "GET"), - attribute.String("http.target", "/"), attribute.String("http.scheme", "http"), attribute.String("http.flavor", "1.1"), attribute.String("net.host.name", srvURL.Hostname()), @@ -208,7 +207,6 @@ func TestHTTPServerRequestFailsGracefully(t *testing.T) { assert.NotPanics(t, func() { got = hc.ServerRequest("", req) }) want := []attribute.KeyValue{ attribute.String("http.method", "GET"), - attribute.String("http.target", ""), attribute.String("http.scheme", "http"), attribute.String("http.flavor", ""), attribute.String("net.host.name", ""),