Skip to content

Commit

Permalink
Fix metrics with high cardininality in otelhttp (open-telemetry#3765)
Browse files Browse the repository at this point in the history
  • Loading branch information
davendu committed Jun 13, 2023
1 parent 9d8d518 commit 54956bb
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 12 deletions.
2 changes: 1 addition & 1 deletion instrumentation/net/http/otelhttp/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func (h *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http
setAfterServeAttributes(span, bw.read, rww.written, rww.statusCode, bw.err, rww.err)

// Add metrics
attributes := append(labeler.Get(), semconvutil.HTTPServerRequest(h.server, r)...)
attributes := append(labeler.Get(), semconvutil.HTTPServerRequestMetric(h.server, r)...)
if rww.statusCode > 0 {
attributes = append(attributes, semconv.HTTPStatusCode(rww.statusCode))
}
Expand Down
43 changes: 32 additions & 11 deletions instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,20 @@ func HTTPClientStatus(code int) (codes.Code, string) {
// "net.sock.peer.addr", "net.sock.peer.port", "http.user_agent", "enduser.id",
// "http.client_ip".
func HTTPServerRequest(server string, req *http.Request) []attribute.KeyValue {
return hc.ServerRequest(server, req)
attrs, _ := hc.ServerRequest(server, req)
return attrs
}

// HTTPServerRequestMetric returns metric attributes for an HTTP request received by a
// server.
//
// See HTTPServerRequest for requirements.
//
// The following attributes are always returned: "http.method", "http.scheme",
// "http.flavor", "net.host.name".
func HTTPServerRequestMetric(server string, req *http.Request) []attribute.KeyValue {
_, attrs := hc.ServerRequest(server, req)
return attrs
}

// HTTPServerStatus returns a span status code and message for an HTTP status code
Expand Down Expand Up @@ -187,13 +200,16 @@ func (c *httpConv) ClientResponse(resp *http.Response) []attribute.KeyValue {
return attrs
}

// ClientRequest returns attributes for an HTTP request made by a client. The
// following attributes are always returned: "http.url", "http.flavor",
// ClientRequest returns attributes for an HTTP request made by a client. The first
// slice of attributes contains full information, while the second one only preserve
// those won't causing high cardinality.
//
// The following attributes are always returned: "http.url", "http.flavor",
// "http.method", "net.peer.name". The following attributes are returned if the
// related values are defined in req: "net.peer.port", "http.user_agent",
// "http.request_content_length", "enduser.id".
func (c *httpConv) ClientRequest(req *http.Request) []attribute.KeyValue {
n := 3 // URL, peer name, proto, and method.
n := 4 // URL, peer name, proto, and method.
var h string
if req.URL != nil {
h = req.URL.Host
Expand All @@ -215,7 +231,6 @@ func (c *httpConv) ClientRequest(req *http.Request) []attribute.KeyValue {
n++
}
attrs := make([]attribute.KeyValue, 0, n)

attrs = append(attrs, c.method(req.Method))
attrs = append(attrs, c.proto(req.Proto))

Expand Down Expand Up @@ -251,6 +266,8 @@ func (c *httpConv) ClientRequest(req *http.Request) []attribute.KeyValue {
}

// ServerRequest returns attributes for an HTTP request received by a server.
// The first slice of attributes contains full information, while the second
// one only preserve those won't causing high cardinality.
//
// The server must be the primary server name if it is known. For example this
// would be the ServerName directive
Expand All @@ -271,7 +288,9 @@ func (c *httpConv) ClientRequest(req *http.Request) []attribute.KeyValue {
// 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) ServerRequest(server string, req *http.Request) []attribute.KeyValue {
func (c *httpConv) ServerRequest(server string, req *http.Request) (
[]attribute.KeyValue, []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
Expand All @@ -280,6 +299,7 @@ func (c *httpConv) ServerRequest(server string, req *http.Request) []attribute.K
// should be removed as well.

n := 4 // Method, scheme, proto, and host name.
attrsLowCardinality := make([]attribute.KeyValue, 0, n)
var host string
var p int
if server == "" {
Expand Down Expand Up @@ -316,10 +336,11 @@ func (c *httpConv) ServerRequest(server string, req *http.Request) []attribute.K
}
attrs := make([]attribute.KeyValue, 0, n)

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.NetConv.HostName(host))
attrsLowCardinality = append(attrsLowCardinality, c.method(req.Method))
attrsLowCardinality = append(attrsLowCardinality, c.scheme(req.TLS != nil))
attrsLowCardinality = append(attrsLowCardinality, c.proto(req.Proto))
attrsLowCardinality = append(attrsLowCardinality, c.NetConv.HostName(host))
attrs = append(attrs, attrsLowCardinality...)

if hostPort > 0 {
attrs = append(attrs, c.NetConv.HostPort(hostPort))
Expand All @@ -346,7 +367,7 @@ func (c *httpConv) ServerRequest(server string, req *http.Request) []attribute.K
attrs = append(attrs, c.HTTPClientIPKey.String(clientIP))
}

return attrs
return attrs, attrsLowCardinality
}

func (c *httpConv) method(method string) attribute.KeyValue {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,41 @@ func TestHTTPServerRequest(t *testing.T) {
HTTPServerRequest("", req))
}

func TestHTTPServerRequestMetric(t *testing.T) {
got := make(chan *http.Request, 1)
handler := func(w http.ResponseWriter, r *http.Request) {
got <- r
w.WriteHeader(http.StatusOK)
}

srv := httptest.NewServer(http.HandlerFunc(handler))
defer srv.Close()

srvURL, err := url.Parse(srv.URL)
require.NoError(t, err)

resp, err := srv.Client().Get(srv.URL)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())

req := <-got

const user = "alice"
req.SetBasicAuth(user, "pswrd")

const clientIP = "127.0.0.5"
req.Header.Add("X-Forwarded-For", clientIP)

assert.ElementsMatch(t,
[]attribute.KeyValue{
attribute.String("http.method", "GET"),
attribute.String("http.scheme", "http"),
attribute.String("http.flavor", "1.1"),
attribute.String("net.host.name", srvURL.Hostname()),
},
HTTPServerRequestMetric("", req))
}

func TestHTTPServerName(t *testing.T) {
req := new(http.Request)
var got []attribute.KeyValue
Expand Down

0 comments on commit 54956bb

Please sign in to comment.