From b76d81cc45cf8876ec726fcf21535c838cb72b7d Mon Sep 17 00:00:00 2001 From: Jason Parraga Date: Fri, 2 Feb 2024 13:08:30 -0800 Subject: [PATCH] otelhttp: client metrics (#4707) --- CHANGELOG.md | 5 + .../internal/semconvutil/httpconv.go | 40 +++ .../internal/semconvutil/httpconv_test.go | 57 ++++ .../otelgin/internal/semconvutil/httpconv.go | 40 +++ .../internal/semconvutil/httpconv_test.go | 57 ++++ .../otelmux/internal/semconvutil/httpconv.go | 40 +++ .../internal/semconvutil/httpconv_test.go | 57 ++++ .../otelecho/internal/semconvutil/httpconv.go | 40 +++ .../internal/semconvutil/httpconv_test.go | 57 ++++ .../internal/semconvutil/httpconv.go | 40 +++ .../internal/semconvutil/httpconv_test.go | 57 ++++ .../internal/semconvutil/httpconv.go | 40 +++ .../internal/semconvutil/httpconv_test.go | 57 ++++ instrumentation/net/http/otelhttp/common.go | 20 +- instrumentation/net/http/otelhttp/handler.go | 12 +- .../otelhttp/internal/semconvutil/httpconv.go | 40 +++ .../internal/semconvutil/httpconv_test.go | 57 ++++ .../net/http/otelhttp/test/handler_test.go | 29 +- .../net/http/otelhttp/test/transport_test.go | 249 ++++++++++++++++++ .../net/http/otelhttp/transport.go | 109 +++++++- .../net/http/otelhttp/transport_test.go | 46 +++- internal/shared/semconvutil/httpconv.go.tmpl | 40 +++ .../shared/semconvutil/httpconv_test.go.tmpl | 57 ++++ 23 files changed, 1202 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c9189a2549..2f869f53cf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,13 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added - Add the new `go.opentelemetry.io/contrib/instrgen` package to provide auto-generated source code instrumentation. (#3068, #3108) +- Add client metric support to `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#4707) - Add peer attributes to spans recorded by `NewClientHandler`, `NewServerHandler` in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (#4873) +### Deprecated + +- The `RequestCount`, `RequestContentLength`, `ResponseContentLength`, `ServerLatency` constants in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` are deprecated. (#4707) + ### Fixed - Do not panic in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` if `MeterProvider` returns a `nil` instrument. (#4875) diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconvutil/httpconv.go b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconvutil/httpconv.go index 854e7e10bb5..83205ecf111 100644 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconvutil/httpconv.go +++ b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconvutil/httpconv.go @@ -51,6 +51,14 @@ func HTTPClientRequest(req *http.Request) []attribute.KeyValue { return hc.ClientRequest(req) } +// HTTPClientRequestMetrics returns metric attributes for an HTTP request made by a client. +// The following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the +// related values are defined in req: "net.peer.port". +func HTTPClientRequestMetrics(req *http.Request) []attribute.KeyValue { + return hc.ClientRequestMetrics(req) +} + // HTTPClientStatus returns a span status code and message for an HTTP status code // value received by a client. func HTTPClientStatus(code int) (codes.Code, string) { @@ -256,6 +264,38 @@ func (c *httpConv) ClientRequest(req *http.Request) []attribute.KeyValue { return attrs } +// ClientRequestMetrics returns metric attributes for an HTTP request made by a client. The +// following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the related values +// are defined in req: "net.peer.port". +func (c *httpConv) ClientRequestMetrics(req *http.Request) []attribute.KeyValue { + /* The following semantic conventions are returned if present: + http.method string + net.peer.name string + net.peer.port int + */ + + n := 2 // method, peer name. + var h string + if req.URL != nil { + h = req.URL.Host + } + peer, p := firstHostPort(h, req.Header.Get("Host")) + port := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", p) + if port > 0 { + n++ + } + + attrs := make([]attribute.KeyValue, 0, n) + attrs = append(attrs, c.method(req.Method), c.NetConv.PeerName(peer)) + + if port > 0 { + attrs = append(attrs, c.NetConv.PeerPort(port)) + } + + return attrs +} + // ServerRequest returns attributes for an HTTP request received by a server. // // The server must be the primary server name if it is known. For example this diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconvutil/httpconv_test.go b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconvutil/httpconv_test.go index c6c2e346180..8f6157966e0 100644 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconvutil/httpconv_test.go +++ b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconvutil/httpconv_test.go @@ -68,6 +68,29 @@ func TestHTTPSClientRequest(t *testing.T) { ) } +func TestHTTPSClientRequestMetrics(t *testing.T) { + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "https", + Host: "127.0.0.1:443", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + } + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequest(t *testing.T) { const ( user = "alice" @@ -105,6 +128,40 @@ func TestHTTPClientRequest(t *testing.T) { ) } +func TestHTTPClientRequestMetrics(t *testing.T) { + const ( + user = "alice" + n = 128 + agent = "Go-http-client/1.1" + ) + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "http", + Host: "127.0.0.1:8080", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + Header: http.Header{ + "User-Agent": []string{agent}, + }, + ContentLength: n, + } + req.SetBasicAuth(user, "pswrd") + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + attribute.Int("net.peer.port", 8080), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequestRequired(t *testing.T) { req := new(http.Request) var got []attribute.KeyValue diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil/httpconv.go b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil/httpconv.go index 9faa2ddc73b..d650d189a94 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil/httpconv.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil/httpconv.go @@ -51,6 +51,14 @@ func HTTPClientRequest(req *http.Request) []attribute.KeyValue { return hc.ClientRequest(req) } +// HTTPClientRequestMetrics returns metric attributes for an HTTP request made by a client. +// The following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the +// related values are defined in req: "net.peer.port". +func HTTPClientRequestMetrics(req *http.Request) []attribute.KeyValue { + return hc.ClientRequestMetrics(req) +} + // HTTPClientStatus returns a span status code and message for an HTTP status code // value received by a client. func HTTPClientStatus(code int) (codes.Code, string) { @@ -256,6 +264,38 @@ func (c *httpConv) ClientRequest(req *http.Request) []attribute.KeyValue { return attrs } +// ClientRequestMetrics returns metric attributes for an HTTP request made by a client. The +// following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the related values +// are defined in req: "net.peer.port". +func (c *httpConv) ClientRequestMetrics(req *http.Request) []attribute.KeyValue { + /* The following semantic conventions are returned if present: + http.method string + net.peer.name string + net.peer.port int + */ + + n := 2 // method, peer name. + var h string + if req.URL != nil { + h = req.URL.Host + } + peer, p := firstHostPort(h, req.Header.Get("Host")) + port := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", p) + if port > 0 { + n++ + } + + attrs := make([]attribute.KeyValue, 0, n) + attrs = append(attrs, c.method(req.Method), c.NetConv.PeerName(peer)) + + if port > 0 { + attrs = append(attrs, c.NetConv.PeerPort(port)) + } + + return attrs +} + // ServerRequest returns attributes for an HTTP request received by a server. // // The server must be the primary server name if it is known. For example this diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil/httpconv_test.go b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil/httpconv_test.go index c6c2e346180..8f6157966e0 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil/httpconv_test.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil/httpconv_test.go @@ -68,6 +68,29 @@ func TestHTTPSClientRequest(t *testing.T) { ) } +func TestHTTPSClientRequestMetrics(t *testing.T) { + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "https", + Host: "127.0.0.1:443", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + } + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequest(t *testing.T) { const ( user = "alice" @@ -105,6 +128,40 @@ func TestHTTPClientRequest(t *testing.T) { ) } +func TestHTTPClientRequestMetrics(t *testing.T) { + const ( + user = "alice" + n = 128 + agent = "Go-http-client/1.1" + ) + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "http", + Host: "127.0.0.1:8080", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + Header: http.Header{ + "User-Agent": []string{agent}, + }, + ContentLength: n, + } + req.SetBasicAuth(user, "pswrd") + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + attribute.Int("net.peer.port", 8080), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequestRequired(t *testing.T) { req := new(http.Request) var got []attribute.KeyValue diff --git a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconvutil/httpconv.go b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconvutil/httpconv.go index 44058210684..65dbba2799e 100644 --- a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconvutil/httpconv.go +++ b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconvutil/httpconv.go @@ -51,6 +51,14 @@ func HTTPClientRequest(req *http.Request) []attribute.KeyValue { return hc.ClientRequest(req) } +// HTTPClientRequestMetrics returns metric attributes for an HTTP request made by a client. +// The following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the +// related values are defined in req: "net.peer.port". +func HTTPClientRequestMetrics(req *http.Request) []attribute.KeyValue { + return hc.ClientRequestMetrics(req) +} + // HTTPClientStatus returns a span status code and message for an HTTP status code // value received by a client. func HTTPClientStatus(code int) (codes.Code, string) { @@ -256,6 +264,38 @@ func (c *httpConv) ClientRequest(req *http.Request) []attribute.KeyValue { return attrs } +// ClientRequestMetrics returns metric attributes for an HTTP request made by a client. The +// following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the related values +// are defined in req: "net.peer.port". +func (c *httpConv) ClientRequestMetrics(req *http.Request) []attribute.KeyValue { + /* The following semantic conventions are returned if present: + http.method string + net.peer.name string + net.peer.port int + */ + + n := 2 // method, peer name. + var h string + if req.URL != nil { + h = req.URL.Host + } + peer, p := firstHostPort(h, req.Header.Get("Host")) + port := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", p) + if port > 0 { + n++ + } + + attrs := make([]attribute.KeyValue, 0, n) + attrs = append(attrs, c.method(req.Method), c.NetConv.PeerName(peer)) + + if port > 0 { + attrs = append(attrs, c.NetConv.PeerPort(port)) + } + + return attrs +} + // ServerRequest returns attributes for an HTTP request received by a server. // // The server must be the primary server name if it is known. For example this diff --git a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconvutil/httpconv_test.go b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconvutil/httpconv_test.go index c6c2e346180..8f6157966e0 100644 --- a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconvutil/httpconv_test.go +++ b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconvutil/httpconv_test.go @@ -68,6 +68,29 @@ func TestHTTPSClientRequest(t *testing.T) { ) } +func TestHTTPSClientRequestMetrics(t *testing.T) { + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "https", + Host: "127.0.0.1:443", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + } + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequest(t *testing.T) { const ( user = "alice" @@ -105,6 +128,40 @@ func TestHTTPClientRequest(t *testing.T) { ) } +func TestHTTPClientRequestMetrics(t *testing.T) { + const ( + user = "alice" + n = 128 + agent = "Go-http-client/1.1" + ) + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "http", + Host: "127.0.0.1:8080", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + Header: http.Header{ + "User-Agent": []string{agent}, + }, + ContentLength: n, + } + req.SetBasicAuth(user, "pswrd") + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + attribute.Int("net.peer.port", 8080), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequestRequired(t *testing.T) { req := new(http.Request) var got []attribute.KeyValue diff --git a/instrumentation/github.com/labstack/echo/otelecho/internal/semconvutil/httpconv.go b/instrumentation/github.com/labstack/echo/otelecho/internal/semconvutil/httpconv.go index fe1a063fc95..d56bf5d3a99 100644 --- a/instrumentation/github.com/labstack/echo/otelecho/internal/semconvutil/httpconv.go +++ b/instrumentation/github.com/labstack/echo/otelecho/internal/semconvutil/httpconv.go @@ -51,6 +51,14 @@ func HTTPClientRequest(req *http.Request) []attribute.KeyValue { return hc.ClientRequest(req) } +// HTTPClientRequestMetrics returns metric attributes for an HTTP request made by a client. +// The following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the +// related values are defined in req: "net.peer.port". +func HTTPClientRequestMetrics(req *http.Request) []attribute.KeyValue { + return hc.ClientRequestMetrics(req) +} + // HTTPClientStatus returns a span status code and message for an HTTP status code // value received by a client. func HTTPClientStatus(code int) (codes.Code, string) { @@ -256,6 +264,38 @@ func (c *httpConv) ClientRequest(req *http.Request) []attribute.KeyValue { return attrs } +// ClientRequestMetrics returns metric attributes for an HTTP request made by a client. The +// following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the related values +// are defined in req: "net.peer.port". +func (c *httpConv) ClientRequestMetrics(req *http.Request) []attribute.KeyValue { + /* The following semantic conventions are returned if present: + http.method string + net.peer.name string + net.peer.port int + */ + + n := 2 // method, peer name. + var h string + if req.URL != nil { + h = req.URL.Host + } + peer, p := firstHostPort(h, req.Header.Get("Host")) + port := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", p) + if port > 0 { + n++ + } + + attrs := make([]attribute.KeyValue, 0, n) + attrs = append(attrs, c.method(req.Method), c.NetConv.PeerName(peer)) + + if port > 0 { + attrs = append(attrs, c.NetConv.PeerPort(port)) + } + + return attrs +} + // ServerRequest returns attributes for an HTTP request received by a server. // // The server must be the primary server name if it is known. For example this diff --git a/instrumentation/github.com/labstack/echo/otelecho/internal/semconvutil/httpconv_test.go b/instrumentation/github.com/labstack/echo/otelecho/internal/semconvutil/httpconv_test.go index c6c2e346180..8f6157966e0 100644 --- a/instrumentation/github.com/labstack/echo/otelecho/internal/semconvutil/httpconv_test.go +++ b/instrumentation/github.com/labstack/echo/otelecho/internal/semconvutil/httpconv_test.go @@ -68,6 +68,29 @@ func TestHTTPSClientRequest(t *testing.T) { ) } +func TestHTTPSClientRequestMetrics(t *testing.T) { + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "https", + Host: "127.0.0.1:443", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + } + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequest(t *testing.T) { const ( user = "alice" @@ -105,6 +128,40 @@ func TestHTTPClientRequest(t *testing.T) { ) } +func TestHTTPClientRequestMetrics(t *testing.T) { + const ( + user = "alice" + n = 128 + agent = "Go-http-client/1.1" + ) + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "http", + Host: "127.0.0.1:8080", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + Header: http.Header{ + "User-Agent": []string{agent}, + }, + ContentLength: n, + } + req.SetBasicAuth(user, "pswrd") + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + attribute.Int("net.peer.port", 8080), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequestRequired(t *testing.T) { req := new(http.Request) var got []attribute.KeyValue diff --git a/instrumentation/gopkg.in/macaron.v1/otelmacaron/internal/semconvutil/httpconv.go b/instrumentation/gopkg.in/macaron.v1/otelmacaron/internal/semconvutil/httpconv.go index 47134758ee5..eeddf93ee16 100644 --- a/instrumentation/gopkg.in/macaron.v1/otelmacaron/internal/semconvutil/httpconv.go +++ b/instrumentation/gopkg.in/macaron.v1/otelmacaron/internal/semconvutil/httpconv.go @@ -51,6 +51,14 @@ func HTTPClientRequest(req *http.Request) []attribute.KeyValue { return hc.ClientRequest(req) } +// HTTPClientRequestMetrics returns metric attributes for an HTTP request made by a client. +// The following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the +// related values are defined in req: "net.peer.port". +func HTTPClientRequestMetrics(req *http.Request) []attribute.KeyValue { + return hc.ClientRequestMetrics(req) +} + // HTTPClientStatus returns a span status code and message for an HTTP status code // value received by a client. func HTTPClientStatus(code int) (codes.Code, string) { @@ -256,6 +264,38 @@ func (c *httpConv) ClientRequest(req *http.Request) []attribute.KeyValue { return attrs } +// ClientRequestMetrics returns metric attributes for an HTTP request made by a client. The +// following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the related values +// are defined in req: "net.peer.port". +func (c *httpConv) ClientRequestMetrics(req *http.Request) []attribute.KeyValue { + /* The following semantic conventions are returned if present: + http.method string + net.peer.name string + net.peer.port int + */ + + n := 2 // method, peer name. + var h string + if req.URL != nil { + h = req.URL.Host + } + peer, p := firstHostPort(h, req.Header.Get("Host")) + port := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", p) + if port > 0 { + n++ + } + + attrs := make([]attribute.KeyValue, 0, n) + attrs = append(attrs, c.method(req.Method), c.NetConv.PeerName(peer)) + + if port > 0 { + attrs = append(attrs, c.NetConv.PeerPort(port)) + } + + return attrs +} + // ServerRequest returns attributes for an HTTP request received by a server. // // The server must be the primary server name if it is known. For example this diff --git a/instrumentation/gopkg.in/macaron.v1/otelmacaron/internal/semconvutil/httpconv_test.go b/instrumentation/gopkg.in/macaron.v1/otelmacaron/internal/semconvutil/httpconv_test.go index c6c2e346180..8f6157966e0 100644 --- a/instrumentation/gopkg.in/macaron.v1/otelmacaron/internal/semconvutil/httpconv_test.go +++ b/instrumentation/gopkg.in/macaron.v1/otelmacaron/internal/semconvutil/httpconv_test.go @@ -68,6 +68,29 @@ func TestHTTPSClientRequest(t *testing.T) { ) } +func TestHTTPSClientRequestMetrics(t *testing.T) { + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "https", + Host: "127.0.0.1:443", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + } + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequest(t *testing.T) { const ( user = "alice" @@ -105,6 +128,40 @@ func TestHTTPClientRequest(t *testing.T) { ) } +func TestHTTPClientRequestMetrics(t *testing.T) { + const ( + user = "alice" + n = 128 + agent = "Go-http-client/1.1" + ) + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "http", + Host: "127.0.0.1:8080", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + Header: http.Header{ + "User-Agent": []string{agent}, + }, + ContentLength: n, + } + req.SetBasicAuth(user, "pswrd") + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + attribute.Int("net.peer.port", 8080), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequestRequired(t *testing.T) { req := new(http.Request) var got []attribute.KeyValue diff --git a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconvutil/httpconv.go b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconvutil/httpconv.go index 2a9a64a85c7..e03b690202b 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconvutil/httpconv.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconvutil/httpconv.go @@ -51,6 +51,14 @@ func HTTPClientRequest(req *http.Request) []attribute.KeyValue { return hc.ClientRequest(req) } +// HTTPClientRequestMetrics returns metric attributes for an HTTP request made by a client. +// The following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the +// related values are defined in req: "net.peer.port". +func HTTPClientRequestMetrics(req *http.Request) []attribute.KeyValue { + return hc.ClientRequestMetrics(req) +} + // HTTPClientStatus returns a span status code and message for an HTTP status code // value received by a client. func HTTPClientStatus(code int) (codes.Code, string) { @@ -256,6 +264,38 @@ func (c *httpConv) ClientRequest(req *http.Request) []attribute.KeyValue { return attrs } +// ClientRequestMetrics returns metric attributes for an HTTP request made by a client. The +// following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the related values +// are defined in req: "net.peer.port". +func (c *httpConv) ClientRequestMetrics(req *http.Request) []attribute.KeyValue { + /* The following semantic conventions are returned if present: + http.method string + net.peer.name string + net.peer.port int + */ + + n := 2 // method, peer name. + var h string + if req.URL != nil { + h = req.URL.Host + } + peer, p := firstHostPort(h, req.Header.Get("Host")) + port := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", p) + if port > 0 { + n++ + } + + attrs := make([]attribute.KeyValue, 0, n) + attrs = append(attrs, c.method(req.Method), c.NetConv.PeerName(peer)) + + if port > 0 { + attrs = append(attrs, c.NetConv.PeerPort(port)) + } + + return attrs +} + // ServerRequest returns attributes for an HTTP request received by a server. // // The server must be the primary server name if it is known. For example this diff --git a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconvutil/httpconv_test.go b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconvutil/httpconv_test.go index c6c2e346180..8f6157966e0 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconvutil/httpconv_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconvutil/httpconv_test.go @@ -68,6 +68,29 @@ func TestHTTPSClientRequest(t *testing.T) { ) } +func TestHTTPSClientRequestMetrics(t *testing.T) { + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "https", + Host: "127.0.0.1:443", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + } + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequest(t *testing.T) { const ( user = "alice" @@ -105,6 +128,40 @@ func TestHTTPClientRequest(t *testing.T) { ) } +func TestHTTPClientRequestMetrics(t *testing.T) { + const ( + user = "alice" + n = 128 + agent = "Go-http-client/1.1" + ) + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "http", + Host: "127.0.0.1:8080", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + Header: http.Header{ + "User-Agent": []string{agent}, + }, + ContentLength: n, + } + req.SetBasicAuth(user, "pswrd") + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + attribute.Int("net.peer.port", 8080), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequestRequired(t *testing.T) { req := new(http.Request) var got []attribute.KeyValue diff --git a/instrumentation/net/http/otelhttp/common.go b/instrumentation/net/http/otelhttp/common.go index 9509014e87c..c6f438774f7 100644 --- a/instrumentation/net/http/otelhttp/common.go +++ b/instrumentation/net/http/otelhttp/common.go @@ -31,10 +31,24 @@ const ( // Server HTTP metrics. const ( - RequestCount = "http.server.request_count" // Incoming request count total - RequestContentLength = "http.server.request_content_length" // Incoming request bytes total + // Deprecated: This field is unused. + RequestCount = "http.server.request_count" // Incoming request count total + // Deprecated: Use of this field has been migrated to serverRequestSize. It will be removed in a future version. + RequestContentLength = "http.server.request_content_length" // Incoming request bytes total + // Deprecated: Use of this field has been migrated to serverResponseSize. It will be removed in a future version. ResponseContentLength = "http.server.response_content_length" // Incoming response bytes total - ServerLatency = "http.server.duration" // Incoming end to end duration, milliseconds + // Deprecated: Use of this field has been migrated to serverDuration. It will be removed in a future version. + ServerLatency = "http.server.duration" // Incoming end to end duration, milliseconds + serverRequestSize = "http.server.request.size" // Incoming request bytes total + serverResponseSize = "http.server.response.size" // Incoming response bytes total + serverDuration = "http.server.duration" // Incoming end to end duration, milliseconds +) + +// Client HTTP metrics. +const ( + clientRequestSize = "http.client.request.size" // Outgoing request bytes total + clientResponseSize = "http.client.response.size" // Outgoing response bytes total + clientDuration = "http.client.duration" // Outgoing end to end duration, milliseconds ) // Filter is a predicate used to determine whether a given http.request should diff --git a/instrumentation/net/http/otelhttp/handler.go b/instrumentation/net/http/otelhttp/handler.go index af84f0e4bb5..3d292dab6d3 100644 --- a/instrumentation/net/http/otelhttp/handler.go +++ b/instrumentation/net/http/otelhttp/handler.go @@ -108,23 +108,23 @@ func handleErr(err error) { func (h *middleware) createMeasures() { var err error h.requestBytesCounter, err = h.meter.Int64Counter( - RequestContentLength, + serverRequestSize, metric.WithUnit("By"), - metric.WithDescription("Measures the size of HTTP request content length (uncompressed)"), + metric.WithDescription("Measures the size of HTTP request messages."), ) handleErr(err) h.responseBytesCounter, err = h.meter.Int64Counter( - ResponseContentLength, + serverResponseSize, metric.WithUnit("By"), - metric.WithDescription("Measures the size of HTTP response content length (uncompressed)"), + metric.WithDescription("Measures the size of HTTP response messages."), ) handleErr(err) h.serverLatencyMeasure, err = h.meter.Float64Histogram( - ServerLatency, + serverDuration, metric.WithUnit("ms"), - metric.WithDescription("Measures the duration of HTTP request handling"), + metric.WithDescription("Measures the duration of inbound HTTP requests."), ) handleErr(err) } diff --git a/instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.go b/instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.go index 95a637c9df8..495d700cfa8 100644 --- a/instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.go +++ b/instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.go @@ -51,6 +51,14 @@ func HTTPClientRequest(req *http.Request) []attribute.KeyValue { return hc.ClientRequest(req) } +// HTTPClientRequestMetrics returns metric attributes for an HTTP request made by a client. +// The following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the +// related values are defined in req: "net.peer.port". +func HTTPClientRequestMetrics(req *http.Request) []attribute.KeyValue { + return hc.ClientRequestMetrics(req) +} + // HTTPClientStatus returns a span status code and message for an HTTP status code // value received by a client. func HTTPClientStatus(code int) (codes.Code, string) { @@ -256,6 +264,38 @@ func (c *httpConv) ClientRequest(req *http.Request) []attribute.KeyValue { return attrs } +// ClientRequestMetrics returns metric attributes for an HTTP request made by a client. The +// following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the related values +// are defined in req: "net.peer.port". +func (c *httpConv) ClientRequestMetrics(req *http.Request) []attribute.KeyValue { + /* The following semantic conventions are returned if present: + http.method string + net.peer.name string + net.peer.port int + */ + + n := 2 // method, peer name. + var h string + if req.URL != nil { + h = req.URL.Host + } + peer, p := firstHostPort(h, req.Header.Get("Host")) + port := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", p) + if port > 0 { + n++ + } + + attrs := make([]attribute.KeyValue, 0, n) + attrs = append(attrs, c.method(req.Method), c.NetConv.PeerName(peer)) + + if port > 0 { + attrs = append(attrs, c.NetConv.PeerPort(port)) + } + + return attrs +} + // ServerRequest returns attributes for an HTTP request received by a server. // // The server must be the primary server name if it is known. For example this diff --git a/instrumentation/net/http/otelhttp/internal/semconvutil/httpconv_test.go b/instrumentation/net/http/otelhttp/internal/semconvutil/httpconv_test.go index c6c2e346180..8f6157966e0 100644 --- a/instrumentation/net/http/otelhttp/internal/semconvutil/httpconv_test.go +++ b/instrumentation/net/http/otelhttp/internal/semconvutil/httpconv_test.go @@ -68,6 +68,29 @@ func TestHTTPSClientRequest(t *testing.T) { ) } +func TestHTTPSClientRequestMetrics(t *testing.T) { + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "https", + Host: "127.0.0.1:443", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + } + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequest(t *testing.T) { const ( user = "alice" @@ -105,6 +128,40 @@ func TestHTTPClientRequest(t *testing.T) { ) } +func TestHTTPClientRequestMetrics(t *testing.T) { + const ( + user = "alice" + n = 128 + agent = "Go-http-client/1.1" + ) + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "http", + Host: "127.0.0.1:8080", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + Header: http.Header{ + "User-Agent": []string{agent}, + }, + ContentLength: n, + } + req.SetBasicAuth(user, "pswrd") + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + attribute.Int("net.peer.port", 8080), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequestRequired(t *testing.T) { req := new(http.Request) var got []attribute.KeyValue diff --git a/instrumentation/net/http/otelhttp/test/handler_test.go b/instrumentation/net/http/otelhttp/test/handler_test.go index bfda5ac83fb..541d0d96641 100644 --- a/instrumentation/net/http/otelhttp/test/handler_test.go +++ b/instrumentation/net/http/otelhttp/test/handler_test.go @@ -50,8 +50,8 @@ func assertScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs attribut require.Len(t, sm.Metrics, 3) want := metricdata.Metrics{ - Name: "http.server.request_content_length", - Description: "Measures the size of HTTP request content length (uncompressed)", + Name: "http.server.request.size", + Description: "Measures the size of HTTP request messages.", Unit: "By", Data: metricdata.Sum[int64]{ DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: 0}}, @@ -62,8 +62,8 @@ func assertScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs attribut metricdatatest.AssertEqual(t, want, sm.Metrics[0], metricdatatest.IgnoreTimestamp()) want = metricdata.Metrics{ - Name: "http.server.response_content_length", - Description: "Measures the size of HTTP response content length (uncompressed)", + Name: "http.server.response.size", + Description: "Measures the size of HTTP response messages.", Unit: "By", Data: metricdata.Sum[int64]{ DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: 11}}, @@ -73,17 +73,16 @@ func assertScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs attribut } metricdatatest.AssertEqual(t, want, sm.Metrics[1], metricdatatest.IgnoreTimestamp()) - // Duration value is not predictable. - dur := sm.Metrics[2] - assert.Equal(t, "http.server.duration", dur.Name) - require.IsType(t, dur.Data, metricdata.Histogram[float64]{}) - hist := dur.Data.(metricdata.Histogram[float64]) - assert.Equal(t, metricdata.CumulativeTemporality, hist.Temporality) - require.Len(t, hist.DataPoints, 1) - dPt := hist.DataPoints[0] - assert.Equal(t, attrs, dPt.Attributes, "attributes") - assert.Equal(t, uint64(1), dPt.Count, "count") - assert.Equal(t, []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}, dPt.Bounds, "bounds") + want = metricdata.Metrics{ + Name: "http.server.duration", + Description: "Measures the duration of inbound HTTP requests.", + Unit: "ms", + Data: metricdata.Histogram[float64]{ + DataPoints: []metricdata.HistogramDataPoint[float64]{{Attributes: attrs}}, + Temporality: metricdata.CumulativeTemporality, + }, + } + metricdatatest.AssertEqual(t, want, sm.Metrics[2], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) } func TestHandlerBasics(t *testing.T) { diff --git a/instrumentation/net/http/otelhttp/test/transport_test.go b/instrumentation/net/http/otelhttp/test/transport_test.go index e9a62034aeb..607ffbd3e9b 100644 --- a/instrumentation/net/http/otelhttp/test/transport_test.go +++ b/instrumentation/net/http/otelhttp/test/transport_test.go @@ -15,18 +15,29 @@ package test import ( + "bytes" "context" "io" + "net" "net/http" "net/http/httptest" "net/http/httptrace" "runtime" + "strconv" "strings" "testing" + "go.opentelemetry.io/otel/sdk/metric" + semconv "go.opentelemetry.io/otel/semconv/v1.20.0" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/instrumentation" + "go.opentelemetry.io/otel/sdk/metric/metricdata" + "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/propagation" @@ -238,3 +249,241 @@ func TestWithHTTPTrace(t *testing.T) { assert.Equal(t, spans[2].SpanContext().SpanID(), spans[0].Parent().SpanID()) assert.Equal(t, spans[1].SpanContext().SpanID(), spans[2].Parent().SpanID()) } + +func TestTransportMetrics(t *testing.T) { + requestBody := []byte("john") + responseBody := []byte("Hello, world!") + + t.Run("make http request and read entire response at once", func(t *testing.T) { + reader := metric.NewManualReader() + meterProvider := metric.NewMeterProvider(metric.WithReader(reader)) + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + if _, err := w.Write(responseBody); err != nil { + t.Fatal(err) + } + })) + defer ts.Close() + + r, err := http.NewRequest(http.MethodGet, ts.URL, bytes.NewReader(requestBody)) + if err != nil { + t.Fatal(err) + } + + tr := otelhttp.NewTransport( + http.DefaultTransport, + otelhttp.WithMeterProvider(meterProvider), + ) + + c := http.Client{Transport: tr} + res, err := c.Do(r) + if err != nil { + t.Fatal(err) + } + + // Must read the body or else we won't get response metrics + bodyBytes, err := io.ReadAll(res.Body) + if err != nil { + t.Fatal(err) + } + require.Len(t, bodyBytes, 13) + require.NoError(t, res.Body.Close()) + + host, portStr, _ := net.SplitHostPort(r.Host) + if host == "" { + host = "127.0.0.1" + } + port, err := strconv.Atoi(portStr) + if err != nil { + port = 0 + } + + rm := metricdata.ResourceMetrics{} + err = reader.Collect(context.Background(), &rm) + require.NoError(t, err) + require.Len(t, rm.ScopeMetrics, 1) + attrs := attribute.NewSet( + semconv.NetPeerName(host), + semconv.NetPeerPort(port), + semconv.HTTPMethod("GET"), + semconv.HTTPStatusCode(200), + ) + assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs, 13) + }) + + t.Run("make http request and buffer response", func(t *testing.T) { + reader := metric.NewManualReader() + meterProvider := metric.NewMeterProvider(metric.WithReader(reader)) + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + if _, err := w.Write(responseBody); err != nil { + t.Fatal(err) + } + })) + defer ts.Close() + + r, err := http.NewRequest(http.MethodGet, ts.URL, bytes.NewReader(requestBody)) + if err != nil { + t.Fatal(err) + } + + tr := otelhttp.NewTransport( + http.DefaultTransport, + otelhttp.WithMeterProvider(meterProvider), + ) + + c := http.Client{Transport: tr} + res, err := c.Do(r) + if err != nil { + t.Fatal(err) + } + + // Must read the body or else we won't get response metrics + smallBuf := make([]byte, 10) + + // Read first 10 bytes + bc, err := res.Body.Read(smallBuf) + if err != nil { + t.Fatal(err) + } + require.Equal(t, 10, bc) + + // reset byte array + // Read last 3 bytes + bc, err = res.Body.Read(smallBuf) + require.Equal(t, io.EOF, err) + require.Equal(t, 3, bc) + + require.NoError(t, res.Body.Close()) + + host, portStr, _ := net.SplitHostPort(r.Host) + if host == "" { + host = "127.0.0.1" + } + port, err := strconv.Atoi(portStr) + if err != nil { + port = 0 + } + + rm := metricdata.ResourceMetrics{} + err = reader.Collect(context.Background(), &rm) + require.NoError(t, err) + require.Len(t, rm.ScopeMetrics, 1) + attrs := attribute.NewSet( + semconv.NetPeerName(host), + semconv.NetPeerPort(port), + semconv.HTTPMethod("GET"), + semconv.HTTPStatusCode(200), + ) + assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs, 13) + }) + + t.Run("make http request and close body before reading completely", func(t *testing.T) { + reader := metric.NewManualReader() + meterProvider := metric.NewMeterProvider(metric.WithReader(reader)) + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + if _, err := w.Write(responseBody); err != nil { + t.Fatal(err) + } + })) + defer ts.Close() + + r, err := http.NewRequest(http.MethodGet, ts.URL, bytes.NewReader(requestBody)) + if err != nil { + t.Fatal(err) + } + + tr := otelhttp.NewTransport( + http.DefaultTransport, + otelhttp.WithMeterProvider(meterProvider), + ) + + c := http.Client{Transport: tr} + res, err := c.Do(r) + if err != nil { + t.Fatal(err) + } + + // Must read the body or else we won't get response metrics + smallBuf := make([]byte, 10) + + // Read first 10 bytes + bc, err := res.Body.Read(smallBuf) + if err != nil { + t.Fatal(err) + } + require.Equal(t, 10, bc) + + // close the response body early + require.NoError(t, res.Body.Close()) + + host, portStr, _ := net.SplitHostPort(r.Host) + if host == "" { + host = "127.0.0.1" + } + port, err := strconv.Atoi(portStr) + if err != nil { + port = 0 + } + + rm := metricdata.ResourceMetrics{} + err = reader.Collect(context.Background(), &rm) + require.NoError(t, err) + require.Len(t, rm.ScopeMetrics, 1) + attrs := attribute.NewSet( + semconv.NetPeerName(host), + semconv.NetPeerPort(port), + semconv.HTTPMethod("GET"), + semconv.HTTPStatusCode(200), + ) + assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs, 10) + }) +} + +func assertClientScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs attribute.Set, rxBytes int64) { + assert.Equal(t, instrumentation.Scope{ + Name: "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp", + Version: Version(), + }, sm.Scope) + + require.Len(t, sm.Metrics, 3) + + want := metricdata.Metrics{ + Name: "http.client.request.size", + Data: metricdata.Sum[int64]{ + DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: 4}}, + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: true, + }, + Description: "Measures the size of HTTP request messages.", + Unit: "By", + } + metricdatatest.AssertEqual(t, want, sm.Metrics[0], metricdatatest.IgnoreTimestamp()) + + want = metricdata.Metrics{ + Name: "http.client.response.size", + Data: metricdata.Sum[int64]{ + DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: rxBytes}}, + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: true, + }, + Description: "Measures the size of HTTP response messages.", + Unit: "By", + } + metricdatatest.AssertEqual(t, want, sm.Metrics[1], metricdatatest.IgnoreTimestamp()) + + want = metricdata.Metrics{ + Name: "http.client.duration", + Data: metricdata.Histogram[float64]{ + DataPoints: []metricdata.HistogramDataPoint[float64]{{Attributes: attrs}}, + Temporality: metricdata.CumulativeTemporality, + }, + Description: "Measures the duration of outbound HTTP requests.", + Unit: "ms", + } + metricdatatest.AssertEqual(t, want, sm.Metrics[2], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) +} diff --git a/instrumentation/net/http/otelhttp/transport.go b/instrumentation/net/http/otelhttp/transport.go index e835cac12e4..8d850df3baa 100644 --- a/instrumentation/net/http/otelhttp/transport.go +++ b/instrumentation/net/http/otelhttp/transport.go @@ -19,31 +19,43 @@ import ( "io" "net/http" "net/http/httptrace" + "sync/atomic" + "time" + + "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconvutil" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/trace" + + semconv "go.opentelemetry.io/otel/semconv/v1.20.0" ) // Transport implements the http.RoundTripper interface and wraps -// outbound HTTP(S) requests with a span. +// outbound HTTP(S) requests with a span and enriches it with metrics. type Transport struct { rt http.RoundTripper tracer trace.Tracer + meter metric.Meter propagators propagation.TextMapPropagator spanStartOptions []trace.SpanStartOption filters []Filter spanNameFormatter func(string, *http.Request) string clientTrace func(context.Context) *httptrace.ClientTrace + + requestBytesCounter metric.Int64Counter + responseBytesCounter metric.Int64Counter + latencyMeasure metric.Float64Histogram } var _ http.RoundTripper = &Transport{} // NewTransport wraps the provided http.RoundTripper with one that -// starts a span and injects the span context into the outbound request headers. +// starts a span, injects the span context into the outbound request headers, +// and enriches it with metrics. // // If the provided http.RoundTripper is nil, http.DefaultTransport will be used // as the base http.RoundTripper. @@ -63,12 +75,14 @@ func NewTransport(base http.RoundTripper, opts ...Option) *Transport { c := newConfig(append(defaultOpts, opts...)...) t.applyConfig(c) + t.createMeasures() return &t } func (t *Transport) applyConfig(c *config) { t.tracer = c.Tracer + t.meter = c.Meter t.propagators = c.Propagators t.spanStartOptions = c.SpanStartOptions t.filters = c.Filters @@ -76,6 +90,30 @@ func (t *Transport) applyConfig(c *config) { t.clientTrace = c.ClientTrace } +func (t *Transport) createMeasures() { + var err error + t.requestBytesCounter, err = t.meter.Int64Counter( + clientRequestSize, + metric.WithUnit("By"), + metric.WithDescription("Measures the size of HTTP request messages."), + ) + handleErr(err) + + t.responseBytesCounter, err = t.meter.Int64Counter( + clientResponseSize, + metric.WithUnit("By"), + metric.WithDescription("Measures the size of HTTP response messages."), + ) + handleErr(err) + + t.latencyMeasure, err = t.meter.Float64Histogram( + clientDuration, + metric.WithUnit("ms"), + metric.WithDescription("Measures the duration of outbound HTTP requests."), + ) + handleErr(err) +} + func defaultTransportFormatter(_ string, r *http.Request) string { return "HTTP " + r.Method } @@ -84,6 +122,7 @@ func defaultTransportFormatter(_ string, r *http.Request) string { // before handing the request to the configured base RoundTripper. The created span will // end when the response body is closed or when a read from the body returns io.EOF. func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { + requestStartTime := time.Now() for _, f := range t.filters { if !f(r) { // Simply pass through to the base RoundTripper if a filter rejects the request @@ -109,7 +148,23 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { ctx = httptrace.WithClientTrace(ctx, t.clientTrace(ctx)) } + labeler := &Labeler{} + ctx = injectLabeler(ctx, labeler) + r = r.Clone(ctx) // According to RoundTripper spec, we shouldn't modify the origin request. + + // use a body wrapper to determine the request size + var bw bodyWrapper + // if request body is nil or NoBody, we don't want to mutate the body as it + // will affect the identity of it in an unforeseeable way because we assert + // ReadCloser fulfills a certain interface and it is indeed nil or NoBody. + if r.Body != nil && r.Body != http.NoBody { + bw.ReadCloser = r.Body + // noop to prevent nil panic. not using this record fun yet. + bw.record = func(int64) {} + r.Body = &bw + } + span.SetAttributes(semconvutil.HTTPClientRequest(r)...) t.propagators.Inject(ctx, propagation.HeaderCarrier(r.Header)) @@ -121,9 +176,28 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { return res, err } + // metrics + metricAttrs := append(labeler.Get(), semconvutil.HTTPClientRequestMetrics(r)...) + if res.StatusCode > 0 { + metricAttrs = append(metricAttrs, semconv.HTTPStatusCode(res.StatusCode)) + } + o := metric.WithAttributes(metricAttrs...) + t.requestBytesCounter.Add(ctx, bw.read, o) + // For handling response bytes we leverage a callback when the client reads the http response + readRecordFunc := func(n int64) { + t.responseBytesCounter.Add(ctx, n, o) + } + + // traces span.SetAttributes(semconvutil.HTTPClientResponse(res)...) span.SetStatus(semconvutil.HTTPClientStatus(res.StatusCode)) - res.Body = newWrappedBody(span, res.Body) + + res.Body = newWrappedBody(span, readRecordFunc, res.Body) + + // Use floating point division here for higher precision (instead of Millisecond method). + elapsedTime := float64(time.Since(requestStartTime)) / float64(time.Millisecond) + + t.latencyMeasure.Record(ctx, elapsedTime, o) return res, err } @@ -131,17 +205,17 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) { // newWrappedBody returns a new and appropriately scoped *wrappedBody as an // io.ReadCloser. If the passed body implements io.Writer, the returned value // will implement io.ReadWriteCloser. -func newWrappedBody(span trace.Span, body io.ReadCloser) io.ReadCloser { +func newWrappedBody(span trace.Span, record func(n int64), body io.ReadCloser) io.ReadCloser { // The successful protocol switch responses will have a body that // implement an io.ReadWriteCloser. Ensure this interface type continues // to be satisfied if that is the case. if _, ok := body.(io.ReadWriteCloser); ok { - return &wrappedBody{span: span, body: body} + return &wrappedBody{span: span, record: record, body: body} } // Remove the implementation of the io.ReadWriteCloser and only implement // the io.ReadCloser. - return struct{ io.ReadCloser }{&wrappedBody{span: span, body: body}} + return struct{ io.ReadCloser }{&wrappedBody{span: span, record: record, body: body}} } // wrappedBody is the response body type returned by the transport @@ -153,8 +227,11 @@ func newWrappedBody(span trace.Span, body io.ReadCloser) io.ReadCloser { // If the response body implements the io.Writer interface (i.e. for // successful protocol switches), the wrapped body also will. type wrappedBody struct { - span trace.Span - body io.ReadCloser + span trace.Span + recorded atomic.Bool + record func(n int64) + body io.ReadCloser + read atomic.Int64 } var _ io.ReadWriteCloser = &wrappedBody{} @@ -171,11 +248,14 @@ func (wb *wrappedBody) Write(p []byte) (int, error) { func (wb *wrappedBody) Read(b []byte) (int, error) { n, err := wb.body.Read(b) + // Record the number of bytes read + wb.read.Add(int64(n)) switch err { case nil: // nothing to do here but fall through to the return case io.EOF: + wb.recordBytesRead() wb.span.End() default: wb.span.RecordError(err) @@ -184,7 +264,20 @@ func (wb *wrappedBody) Read(b []byte) (int, error) { return n, err } +// recordBytesRead is a function that ensures the number of bytes read is recorded once and only once. +func (wb *wrappedBody) recordBytesRead() { + // note: it is more performant (and equally correct) to use atomic.Bool over sync.Once here. In the event that + // two goroutines are racing to call this method, the number of bytes read will no longer increase. Using + // CompareAndSwap allows later goroutines to return quickly and not block waiting for the race winner to finish + // calling wb.record(wb.read.Load()). + if wb.recorded.CompareAndSwap(false, true) { + // Record the total number of bytes read + wb.record(wb.read.Load()) + } +} + func (wb *wrappedBody) Close() error { + wb.recordBytesRead() wb.span.End() if wb.body != nil { return wb.body.Close() diff --git a/instrumentation/net/http/otelhttp/transport_test.go b/instrumentation/net/http/otelhttp/transport_test.go index 7530955971b..1fcf76d650e 100644 --- a/instrumentation/net/http/otelhttp/transport_test.go +++ b/instrumentation/net/http/otelhttp/transport_test.go @@ -247,52 +247,72 @@ func (s *span) assert(t *testing.T, ended bool, err error, c codes.Code, d strin func TestWrappedBodyRead(t *testing.T) { s := new(span) - wb := &wrappedBody{span: trace.Span(s), body: readCloser{}} + called := false + record := func(numBytes int64) { called = true } + wb := &wrappedBody{span: trace.Span(s), record: record, body: readCloser{}} n, err := wb.Read([]byte{}) assert.Equal(t, readSize, n, "wrappedBody returned wrong bytes") assert.NoError(t, err) s.assert(t, false, nil, codes.Unset, "") + assert.False(t, called, "record should not have been called") } func TestWrappedBodyReadEOFError(t *testing.T) { s := new(span) - wb := &wrappedBody{span: trace.Span(s), body: readCloser{readErr: io.EOF}} + called := false + numRecorded := int64(0) + record := func(numBytes int64) { + called = true + numRecorded = numBytes + } + wb := &wrappedBody{span: trace.Span(s), record: record, body: readCloser{readErr: io.EOF}} n, err := wb.Read([]byte{}) assert.Equal(t, readSize, n, "wrappedBody returned wrong bytes") assert.Equal(t, io.EOF, err) s.assert(t, true, nil, codes.Unset, "") + assert.True(t, called, "record should have been called") + assert.Equal(t, int64(readSize), numRecorded, "record recorded wrong number of bytes") } func TestWrappedBodyReadError(t *testing.T) { s := new(span) + called := false + record := func(int64) { called = true } expectedErr := errors.New("test") - wb := &wrappedBody{span: trace.Span(s), body: readCloser{readErr: expectedErr}} + wb := &wrappedBody{span: trace.Span(s), record: record, body: readCloser{readErr: expectedErr}} n, err := wb.Read([]byte{}) assert.Equal(t, readSize, n, "wrappedBody returned wrong bytes") assert.Equal(t, expectedErr, err) s.assert(t, false, expectedErr, codes.Error, expectedErr.Error()) + assert.False(t, called, "record should not have been called") } func TestWrappedBodyClose(t *testing.T) { s := new(span) - wb := &wrappedBody{span: trace.Span(s), body: readCloser{}} + called := false + record := func(int64) { called = true } + wb := &wrappedBody{span: trace.Span(s), record: record, body: readCloser{}} assert.NoError(t, wb.Close()) s.assert(t, true, nil, codes.Unset, "") + assert.True(t, called, "record should have been called") } func TestWrappedBodyClosePanic(t *testing.T) { s := new(span) var body io.ReadCloser - wb := newWrappedBody(s, body) + wb := newWrappedBody(s, func(n int64) {}, body) assert.NotPanics(t, func() { wb.Close() }, "nil body should not panic on close") } func TestWrappedBodyCloseError(t *testing.T) { s := new(span) + called := false + record := func(int64) { called = true } expectedErr := errors.New("test") - wb := &wrappedBody{span: trace.Span(s), body: readCloser{closeErr: expectedErr}} + wb := &wrappedBody{span: trace.Span(s), record: record, body: readCloser{closeErr: expectedErr}} assert.Equal(t, expectedErr, wb.Close()) s.assert(t, true, nil, codes.Unset, "") + assert.True(t, called, "record should have been called") } type readWriteCloser struct { @@ -308,12 +328,12 @@ func (rwc readWriteCloser) Write([]byte) (int, error) { } func TestNewWrappedBodyReadWriteCloserImplementation(t *testing.T) { - wb := newWrappedBody(nil, readWriteCloser{}) + wb := newWrappedBody(nil, func(n int64) {}, readWriteCloser{}) assert.Implements(t, (*io.ReadWriteCloser)(nil), wb) } func TestNewWrappedBodyReadCloserImplementation(t *testing.T) { - wb := newWrappedBody(nil, readCloser{}) + wb := newWrappedBody(nil, func(n int64) {}, readCloser{}) assert.Implements(t, (*io.ReadCloser)(nil), wb) _, ok := wb.(io.ReadWriteCloser) @@ -324,7 +344,7 @@ func TestWrappedBodyWrite(t *testing.T) { s := new(span) var rwc io.ReadWriteCloser assert.NotPanics(t, func() { - rwc = newWrappedBody(s, readWriteCloser{}).(io.ReadWriteCloser) + rwc = newWrappedBody(s, func(n int64) {}, readWriteCloser{}).(io.ReadWriteCloser) }) n, err := rwc.Write([]byte{}) @@ -338,9 +358,11 @@ func TestWrappedBodyWriteError(t *testing.T) { expectedErr := errors.New("test") var rwc io.ReadWriteCloser assert.NotPanics(t, func() { - rwc = newWrappedBody(s, readWriteCloser{ - writeErr: expectedErr, - }).(io.ReadWriteCloser) + rwc = newWrappedBody(s, + func(n int64) {}, + readWriteCloser{ + writeErr: expectedErr, + }).(io.ReadWriteCloser) }) n, err := rwc.Write([]byte{}) assert.Equal(t, writeSize, n, "wrappedBody returned wrong bytes") diff --git a/internal/shared/semconvutil/httpconv.go.tmpl b/internal/shared/semconvutil/httpconv.go.tmpl index 0a356fed5f5..1e01da2790a 100644 --- a/internal/shared/semconvutil/httpconv.go.tmpl +++ b/internal/shared/semconvutil/httpconv.go.tmpl @@ -51,6 +51,14 @@ func HTTPClientRequest(req *http.Request) []attribute.KeyValue { return hc.ClientRequest(req) } +// HTTPClientRequestMetrics returns metric attributes for an HTTP request made by a client. +// The following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the +// related values are defined in req: "net.peer.port". +func HTTPClientRequestMetrics(req *http.Request) []attribute.KeyValue { + return hc.ClientRequestMetrics(req) +} + // HTTPClientStatus returns a span status code and message for an HTTP status code // value received by a client. func HTTPClientStatus(code int) (codes.Code, string) { @@ -256,6 +264,38 @@ func (c *httpConv) ClientRequest(req *http.Request) []attribute.KeyValue { return attrs } +// ClientRequestMetrics returns metric attributes for an HTTP request made by a client. The +// following attributes are always returned: "http.method", "net.peer.name". +// The following attributes are returned if the related values +// are defined in req: "net.peer.port". +func (c *httpConv) ClientRequestMetrics(req *http.Request) []attribute.KeyValue { + /* The following semantic conventions are returned if present: + http.method string + net.peer.name string + net.peer.port int + */ + + n := 2 // method, peer name. + var h string + if req.URL != nil { + h = req.URL.Host + } + peer, p := firstHostPort(h, req.Header.Get("Host")) + port := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", p) + if port > 0 { + n++ + } + + attrs := make([]attribute.KeyValue, 0, n) + attrs = append(attrs, c.method(req.Method), c.NetConv.PeerName(peer)) + + if port > 0 { + attrs = append(attrs, c.NetConv.PeerPort(port)) + } + + return attrs +} + // ServerRequest returns attributes for an HTTP request received by a server. // // The server must be the primary server name if it is known. For example this diff --git a/internal/shared/semconvutil/httpconv_test.go.tmpl b/internal/shared/semconvutil/httpconv_test.go.tmpl index c6c2e346180..8f6157966e0 100644 --- a/internal/shared/semconvutil/httpconv_test.go.tmpl +++ b/internal/shared/semconvutil/httpconv_test.go.tmpl @@ -68,6 +68,29 @@ func TestHTTPSClientRequest(t *testing.T) { ) } +func TestHTTPSClientRequestMetrics(t *testing.T) { + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "https", + Host: "127.0.0.1:443", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + } + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequest(t *testing.T) { const ( user = "alice" @@ -105,6 +128,40 @@ func TestHTTPClientRequest(t *testing.T) { ) } +func TestHTTPClientRequestMetrics(t *testing.T) { + const ( + user = "alice" + n = 128 + agent = "Go-http-client/1.1" + ) + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "http", + Host: "127.0.0.1:8080", + Path: "/resource", + }, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + Header: http.Header{ + "User-Agent": []string{agent}, + }, + ContentLength: n, + } + req.SetBasicAuth(user, "pswrd") + + assert.ElementsMatch( + t, + []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("net.peer.name", "127.0.0.1"), + attribute.Int("net.peer.port", 8080), + }, + HTTPClientRequestMetrics(req), + ) +} + func TestHTTPClientRequestRequired(t *testing.T) { req := new(http.Request) var got []attribute.KeyValue