Skip to content

Commit

Permalink
Remove http.target attr from ServerRequest (#3687)
Browse files Browse the repository at this point in the history
* Remove http.target attr from ServerRequest

* Update changelog

* Remove trailing space in changelog
  • Loading branch information
MrAlias authored Feb 7, 2023
1 parent 0446207 commit 7b74959
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 11 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 8 additions & 9 deletions semconv/internal/v2/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "" {
Expand Down Expand Up @@ -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))
}
Expand Down
2 changes: 0 additions & 2 deletions semconv/internal/v2/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down Expand Up @@ -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", ""),
Expand Down

0 comments on commit 7b74959

Please sign in to comment.