Remove high cardanility metrics from otelhttp

CHANGELOG.md

@@ -22,6 +22,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
   - The `faas.execution` attribute is now `faas.invocation_id`.
   - The `faas.id` attribute is now `aws.lambda.invoked_arn`.
 - The semantic conventions used by `go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws` have been upgraded to v1.21.0. (#4265)
+- The `http.request.method` attribute will only allow known HTTP methods from the metrics generated by `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#????)
+
+### Removed
+
+- The high cardinality attributes `net.sock.peer.addr`, `net.sock.peer.port`, `http.user_agent`, `enduser.id`, and `http.client_ip` were removed from the metrics generated by `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#????)
 
 ## [1.18.0/0.43.0/0.12.0] - 2023-08-28
 

instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconvutil/httpconv.go

@@ -217,7 +217,7 @@
 	attrs := make([]attribute.KeyValue, 0, n)
 
 	attrs = append(attrs, c.method(req.Method))
-	attrs = append(attrs, c.proto(req.Proto))
+	attrs = append(attrs, c.flavor(req.Proto))
 
 	var u string
 	if req.URL != nil {
@@ -318,7 +318,7 @@
 
 	attrs = append(attrs, c.method(req.Method))
 	attrs = append(attrs, c.scheme(req.TLS != nil))
-	attrs = append(attrs, c.proto(req.Proto))
+	attrs = append(attrs, c.flavor(req.Proto))
 	attrs = append(attrs, c.NetConv.HostName(host))
 
 	if hostPort > 0 {
@@ -349,21 +349,101 @@
 	return attrs
 }
 
+// ServerRequestMetrics returns Metric attributes for an HTTP request received
+// by a server.
+//
+// The server must be the primary server name if it is known. For example this
+// would be the ServerName directive
+// (https://httpd.apache.org/docs/2.4/mod/core.html#servername) for an Apache
+// server, and the server_name directive
+// (http://nginx.org/en/docs/http/ngx_http_core_module.html#server_name) for an
+// nginx server. More generically, the primary server name would be the host
+// header value that matches the default virtual host of an HTTP server. It
+// should include the host identifier and if a port is used to route to the
+// server that port identifier should be included as an appropriate port
+// suffix.
+//
+// If the primary server name is not known, server should be an empty string.
+// The req Host will be used to determine the server instead.
+//
+// The following attributes are always returned: "http.method", "http.scheme",
+// "http.flavor", "http.target", "net.host.name". The following attributes are
+// returned if they related values are defined in req: "net.host.port",
+// "net.sock.peer.addr", "net.sock.peer.port", "http.user_agent", "enduser.id",
+// "http.client_ip".
+func (c *httpConv) ServerRequestMetrics(server string, req *http.Request) []attribute.KeyValue {
+	// 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 == "" {
+		host, p = splitHostPort(req.Host)
+	} else {
+		// Prioritize the primary server name.
+		host, p = splitHostPort(server)
+		if p < 0 {
+			_, p = splitHostPort(req.Host)
+		}
+	}
+	hostPort := requiredHTTPPort(req.TLS != nil, p)
+	if hostPort > 0 {
+		n++
+	}
+	attrs := make([]attribute.KeyValue, 0, n)
+
+	attrs = append(attrs, c.methodMetric(req.Method))
+	attrs = append(attrs, c.scheme(req.TLS != nil))
+	attrs = append(attrs, c.flavor(req.Proto))
+	attrs = append(attrs, c.NetConv.HostName(host))
+
+	if hostPort > 0 {
+		attrs = append(attrs, c.NetConv.HostPort(hostPort))
+	}
+
+	return attrs
+}
+
 func (c *httpConv) method(method string) attribute.KeyValue {
 	if method == "" {
 		return c.HTTPMethodKey.String(http.MethodGet)
 	}
 	return c.HTTPMethodKey.String(method)
 }
 
+func (c *httpConv) methodMetric(method string) attribute.KeyValue {
+	method = strings.ToUpper(method)
+	if _, ok := allowedMethods[method]; !ok {
+		method = "_OTHER"
+	}
+	return c.HTTPMethodKey.String(method)
+}
+
+var allowedMethods = map[string]struct{}{
+	"CONNECT": {},
+	"DELETE":  {},
+	"GET":     {},
+	"HEAD":    {},
+	"OPTIONS": {},
+	"PATCH":   {},
+	"POST":    {},
+	"PUT":     {},
+	"TRACE":   {},
+}
+
 func (c *httpConv) scheme(https bool) attribute.KeyValue { // nolint:revive
 	if https {
 		return c.HTTPSchemeHTTPS
 	}
 	return c.HTTPSchemeHTTP
 }
 
-func (c *httpConv) proto(proto string) attribute.KeyValue {
+func (c *httpConv) flavor(proto string) attribute.KeyValue {
 	switch proto {
 	case "HTTP/1.0":
 		return c.HTTPFlavorKey.String("1.0")

instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconvutil/httpconv_test.go

@@ -223,7 +223,7 @@ func TestHTTPProto(t *testing.T) {
 
 	for proto, want := range tests {
 		expect := attribute.String("http.flavor", want)
-		assert.Equal(t, expect, hc.proto(proto), proto)
+		assert.Equal(t, expect, hc.flavor(proto), proto)
 	}
 }
 

instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil/httpconv.go

@@ -217,7 +217,7 @@
 	attrs := make([]attribute.KeyValue, 0, n)
 
 	attrs = append(attrs, c.method(req.Method))
-	attrs = append(attrs, c.proto(req.Proto))
+	attrs = append(attrs, c.flavor(req.Proto))
 
 	var u string
 	if req.URL != nil {
@@ -318,7 +318,7 @@
 
 	attrs = append(attrs, c.method(req.Method))
 	attrs = append(attrs, c.scheme(req.TLS != nil))
-	attrs = append(attrs, c.proto(req.Proto))
+	attrs = append(attrs, c.flavor(req.Proto))
 	attrs = append(attrs, c.NetConv.HostName(host))
 
 	if hostPort > 0 {
@@ -349,21 +349,101 @@
 	return attrs
 }
 
+// ServerRequestMetrics returns Metric attributes for an HTTP request received
+// by a server.
+//
+// The server must be the primary server name if it is known. For example this
+// would be the ServerName directive
+// (https://httpd.apache.org/docs/2.4/mod/core.html#servername) for an Apache
+// server, and the server_name directive
+// (http://nginx.org/en/docs/http/ngx_http_core_module.html#server_name) for an
+// nginx server. More generically, the primary server name would be the host
+// header value that matches the default virtual host of an HTTP server. It
+// should include the host identifier and if a port is used to route to the
+// server that port identifier should be included as an appropriate port
+// suffix.
+//
+// If the primary server name is not known, server should be an empty string.
+// The req Host will be used to determine the server instead.
+//
+// The following attributes are always returned: "http.method", "http.scheme",
+// "http.flavor", "http.target", "net.host.name". The following attributes are
+// returned if they related values are defined in req: "net.host.port",
+// "net.sock.peer.addr", "net.sock.peer.port", "http.user_agent", "enduser.id",
+// "http.client_ip".
+func (c *httpConv) ServerRequestMetrics(server string, req *http.Request) []attribute.KeyValue {
+	// 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 == "" {
+		host, p = splitHostPort(req.Host)
+	} else {
+		// Prioritize the primary server name.
+		host, p = splitHostPort(server)
+		if p < 0 {
+			_, p = splitHostPort(req.Host)
+		}
+	}
+	hostPort := requiredHTTPPort(req.TLS != nil, p)
+	if hostPort > 0 {
+		n++
+	}
+	attrs := make([]attribute.KeyValue, 0, n)
+
+	attrs = append(attrs, c.methodMetric(req.Method))
+	attrs = append(attrs, c.scheme(req.TLS != nil))
+	attrs = append(attrs, c.flavor(req.Proto))
+	attrs = append(attrs, c.NetConv.HostName(host))
+
+	if hostPort > 0 {
+		attrs = append(attrs, c.NetConv.HostPort(hostPort))
+	}
+
+	return attrs
+}
+
 func (c *httpConv) method(method string) attribute.KeyValue {
 	if method == "" {
 		return c.HTTPMethodKey.String(http.MethodGet)
 	}
 	return c.HTTPMethodKey.String(method)
 }
 
+func (c *httpConv) methodMetric(method string) attribute.KeyValue {
+	method = strings.ToUpper(method)
+	if _, ok := allowedMethods[method]; !ok {
+		method = "_OTHER"
+	}
+	return c.HTTPMethodKey.String(method)
+}
+
+var allowedMethods = map[string]struct{}{
+	"CONNECT": {},
+	"DELETE":  {},
+	"GET":     {},
+	"HEAD":    {},
+	"OPTIONS": {},
+	"PATCH":   {},
+	"POST":    {},
+	"PUT":     {},
+	"TRACE":   {},
+}
+
 func (c *httpConv) scheme(https bool) attribute.KeyValue { // nolint:revive
 	if https {
 		return c.HTTPSchemeHTTPS
 	}
 	return c.HTTPSchemeHTTP
 }
 
-func (c *httpConv) proto(proto string) attribute.KeyValue {
+func (c *httpConv) flavor(proto string) attribute.KeyValue {
 	switch proto {
 	case "HTTP/1.0":
 		return c.HTTPFlavorKey.String("1.0")

instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil/httpconv_test.go

@@ -223,7 +223,7 @@ func TestHTTPProto(t *testing.T) {
 
 	for proto, want := range tests {
 		expect := attribute.String("http.flavor", want)
-		assert.Equal(t, expect, hc.proto(proto), proto)
+		assert.Equal(t, expect, hc.flavor(proto), proto)
 	}
 }
 

instrumentation/github.com/gorilla/mux/otelmux/internal/semconvutil/httpconv.go

@@ -217,7 +217,7 @@
 	attrs := make([]attribute.KeyValue, 0, n)
 
 	attrs = append(attrs, c.method(req.Method))
-	attrs = append(attrs, c.proto(req.Proto))
+	attrs = append(attrs, c.flavor(req.Proto))
 
 	var u string
 	if req.URL != nil {
@@ -318,7 +318,7 @@
 
 	attrs = append(attrs, c.method(req.Method))
 	attrs = append(attrs, c.scheme(req.TLS != nil))
-	attrs = append(attrs, c.proto(req.Proto))
+	attrs = append(attrs, c.flavor(req.Proto))
 	attrs = append(attrs, c.NetConv.HostName(host))
 
 	if hostPort > 0 {
@@ -349,21 +349,101 @@
 	return attrs
 }
 
+// ServerRequestMetrics returns Metric attributes for an HTTP request received
+// by a server.
+//
+// The server must be the primary server name if it is known. For example this
+// would be the ServerName directive
+// (https://httpd.apache.org/docs/2.4/mod/core.html#servername) for an Apache
+// server, and the server_name directive
+// (http://nginx.org/en/docs/http/ngx_http_core_module.html#server_name) for an
+// nginx server. More generically, the primary server name would be the host
+// header value that matches the default virtual host of an HTTP server. It
+// should include the host identifier and if a port is used to route to the
+// server that port identifier should be included as an appropriate port
+// suffix.
+//
+// If the primary server name is not known, server should be an empty string.
+// The req Host will be used to determine the server instead.
+//
+// The following attributes are always returned: "http.method", "http.scheme",
+// "http.flavor", "http.target", "net.host.name". The following attributes are
+// returned if they related values are defined in req: "net.host.port",
+// "net.sock.peer.addr", "net.sock.peer.port", "http.user_agent", "enduser.id",
+// "http.client_ip".
+func (c *httpConv) ServerRequestMetrics(server string, req *http.Request) []attribute.KeyValue {
+	// 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 == "" {
+		host, p = splitHostPort(req.Host)
+	} else {
+		// Prioritize the primary server name.
+		host, p = splitHostPort(server)
+		if p < 0 {
+			_, p = splitHostPort(req.Host)
+		}
+	}
+	hostPort := requiredHTTPPort(req.TLS != nil, p)
+	if hostPort > 0 {
+		n++
+	}
+	attrs := make([]attribute.KeyValue, 0, n)
+
+	attrs = append(attrs, c.methodMetric(req.Method))
+	attrs = append(attrs, c.scheme(req.TLS != nil))
+	attrs = append(attrs, c.flavor(req.Proto))
+	attrs = append(attrs, c.NetConv.HostName(host))
+
+	if hostPort > 0 {
+		attrs = append(attrs, c.NetConv.HostPort(hostPort))
+	}
+
+	return attrs
+}
+
 func (c *httpConv) method(method string) attribute.KeyValue {
 	if method == "" {
 		return c.HTTPMethodKey.String(http.MethodGet)
 	}
 	return c.HTTPMethodKey.String(method)
 }
 
+func (c *httpConv) methodMetric(method string) attribute.KeyValue {
+	method = strings.ToUpper(method)
+	if _, ok := allowedMethods[method]; !ok {
+		method = "_OTHER"
+	}
+	return c.HTTPMethodKey.String(method)
+}
+
+var allowedMethods = map[string]struct{}{
+	"CONNECT": {},
+	"DELETE":  {},
+	"GET":     {},
+	"HEAD":    {},
+	"OPTIONS": {},
+	"PATCH":   {},
+	"POST":    {},
+	"PUT":     {},
+	"TRACE":   {},
+}
+
 func (c *httpConv) scheme(https bool) attribute.KeyValue { // nolint:revive
 	if https {
 		return c.HTTPSchemeHTTPS
 	}
 	return c.HTTPSchemeHTTP
 }
 
-func (c *httpConv) proto(proto string) attribute.KeyValue {
+func (c *httpConv) flavor(proto string) attribute.KeyValue {
 	switch proto {
 	case "HTTP/1.0":
 		return c.HTTPFlavorKey.String("1.0")