From 91b000002aa9a770b88c6211ca269c3463e328cc Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Mon, 6 Jul 2020 15:34:51 +0800 Subject: [PATCH 1/5] Add http content size to standard package Signed-off-by: Sam Xie --- api/standard/trace.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/api/standard/trace.go b/api/standard/trace.go index 83eae79e8ba..de693446f8d 100644 --- a/api/standard/trace.go +++ b/api/standard/trace.go @@ -111,6 +111,20 @@ const ( // The IP address of the original client behind all proxies, if known // (e.g. from X-Forwarded-For). HTTPClientIPKey = kv.Key("http.client_ip") + + // The size of the request payload body in bytes. + HTTPRequestContentLength = kv.Key("http.request_content_length") + + // The size of the uncompressed request payload body after transport decoding. + // Not set if transport encoding not used. + HTTPRequestContentLengthUncompressed = kv.Key("http.request_content_length_uncompressed") + + // The size of the response payload body in bytes. + HTTPResponseContentLength = kv.Key("http.response_content_length") + + // The size of the uncompressed response payload body after transport decoding. + // Not set if transport encoding not used. + HTTPResponseContentLengthUncompressed = kv.Key("http.response_content_length_uncompressed") ) var ( From c67ab61279f3ca32029843bada5fa8ca0a0989ed Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Mon, 6 Jul 2020 15:51:19 +0800 Subject: [PATCH 2/5] Include `http.request_content_length` in HTTP request basic attributes Signed-off-by: Sam Xie --- api/standard/http.go | 3 +++ api/standard/http_test.go | 30 +++++++++++++++------ instrumentation/httptrace/httptrace_test.go | 23 ++++++++-------- instrumentation/othttp/handler_test.go | 4 ++- 4 files changed, 40 insertions(+), 20 deletions(-) diff --git a/api/standard/http.go b/api/standard/http.go index 46b8c1c3a68..172572e29bd 100644 --- a/api/standard/http.go +++ b/api/standard/http.go @@ -177,6 +177,9 @@ func httpBasicAttributesFromHTTPRequest(request *http.Request) []kv.KeyValue { if flavor != "" { attrs = append(attrs, HTTPFlavorKey.String(flavor)) } + if request.ContentLength > 0 { + attrs = append(attrs, HTTPRequestContentLength.Int64(request.ContentLength)) + } return attrs } diff --git a/api/standard/http_test.go b/api/standard/http_test.go index 21fcd9efc6e..25b27fa2eb3 100644 --- a/api/standard/http_test.go +++ b/api/standard/http_test.go @@ -420,14 +420,15 @@ func TestHTTPServerAttributesFromHTTPRequest(t *testing.T) { serverName string route string - method string - requestURI string - proto string - remoteAddr string - host string - url *url.URL - header http.Header - tls tlsOption + method string + requestURI string + proto string + remoteAddr string + host string + url *url.URL + header http.Header + tls tlsOption + contentLength int64 expected []otelkv.KeyValue } @@ -658,9 +659,22 @@ func TestHTTPServerAttributesFromHTTPRequest(t *testing.T) { otelkv.String("http.client_ip", "1.2.3.4"), }, }, + { + name: "with content length", + method: "GET", + requestURI: "/user/123", + contentLength: 100, + expected: []otelkv.KeyValue{ + otelkv.String("http.method", "GET"), + otelkv.String("http.target", "/user/123"), + otelkv.String("http.scheme", "http"), + otelkv.Int64("http.request_content_length", 100), + }, + }, } for idx, tc := range testcases { r := testRequest(tc.method, tc.requestURI, tc.proto, tc.remoteAddr, tc.host, tc.url, tc.header, tc.tls) + r.ContentLength = tc.contentLength got := HTTPServerAttributesFromHTTPRequest(tc.serverName, tc.route, r) assertElementsMatch(t, tc.expected, got, "testcase %d - %s", idx, tc.name) } diff --git a/instrumentation/httptrace/httptrace_test.go b/instrumentation/httptrace/httptrace_test.go index 33df19efaa2..b71ce8c4f92 100644 --- a/instrumentation/httptrace/httptrace_test.go +++ b/instrumentation/httptrace/httptrace_test.go @@ -87,23 +87,24 @@ func TestRoundtrip(t *testing.T) { address := ts.Listener.Addr() hp := strings.Split(address.String(), ":") expectedAttrs = map[kv.Key]string{ - standard.HTTPFlavorKey: "1.1", - standard.HTTPHostKey: address.String(), - standard.HTTPMethodKey: "GET", - standard.HTTPSchemeKey: "http", - standard.HTTPTargetKey: "/", - standard.HTTPUserAgentKey: "Go-http-client/1.1", - standard.NetHostIPKey: hp[0], - standard.NetHostPortKey: hp[1], - standard.NetPeerIPKey: "127.0.0.1", - standard.NetTransportKey: "IP.TCP", + standard.HTTPFlavorKey: "1.1", + standard.HTTPHostKey: address.String(), + standard.HTTPMethodKey: "GET", + standard.HTTPSchemeKey: "http", + standard.HTTPTargetKey: "/", + standard.HTTPUserAgentKey: "Go-http-client/1.1", + standard.HTTPRequestContentLength: "3", + standard.NetHostIPKey: hp[0], + standard.NetHostPortKey: hp[1], + standard.NetPeerIPKey: "127.0.0.1", + standard.NetTransportKey: "IP.TCP", } client := ts.Client() err := tr.WithSpan(context.Background(), "test", func(ctx context.Context) error { ctx = correlation.ContextWithMap(ctx, correlation.NewMap(correlation.MapUpdate{SingleKV: kv.Key("foo").String("bar")})) - req, _ := http.NewRequest("GET", ts.URL, nil) + req, _ := http.NewRequest("GET", ts.URL, strings.NewReader("foo")) httptrace.Inject(ctx, req) res, err := client.Do(req) diff --git a/instrumentation/othttp/handler_test.go b/instrumentation/othttp/handler_test.go index eaf2723654a..1967e0388d0 100644 --- a/instrumentation/othttp/handler_test.go +++ b/instrumentation/othttp/handler_test.go @@ -19,6 +19,7 @@ import ( "io/ioutil" "net/http" "net/http/httptest" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -56,7 +57,7 @@ func TestHandlerBasics(t *testing.T) { WithMeter(meter), ) - r, err := http.NewRequest(http.MethodGet, "http://localhost/", nil) + r, err := http.NewRequest(http.MethodGet, "http://localhost/", strings.NewReader("foo")) if err != nil { t.Fatal(err) } @@ -71,6 +72,7 @@ func TestHandlerBasics(t *testing.T) { standard.HTTPSchemeHTTP, standard.HTTPHostKey.String(r.Host), standard.HTTPFlavorKey.String(fmt.Sprintf("1.%d", r.ProtoMinor)), + standard.HTTPRequestContentLength.Int64(3), } assertMetricLabels(t, labelsToVerify, meterimpl.MeasurementBatches) From 08696bdf25d8628fbe4e8efc971073e9debbfcd1 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Tue, 7 Jul 2020 10:30:55 +0800 Subject: [PATCH 3/5] Add test for api.standard package Signed-off-by: Sam Xie --- api/standard/http_test.go | 181 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 181 insertions(+) diff --git a/api/standard/http_test.go b/api/standard/http_test.go index 25b27fa2eb3..31d384a4643 100644 --- a/api/standard/http_test.go +++ b/api/standard/http_test.go @@ -789,3 +789,184 @@ func kvStr(kvs []otelkv.KeyValue) string { sb.WriteRune(']') return sb.String() } + +func TestHTTPClientAttributesFromHTTPRequest(t *testing.T) { + testCases := []struct { + name string + + method string + requestURI string + proto string + remoteAddr string + host string + url *url.URL + header http.Header + tls tlsOption + contentLength int64 + + expected []otelkv.KeyValue + }{ + { + name: "stripped", + method: "GET", + requestURI: "/user/123", + proto: "HTTP/1.0", + remoteAddr: "", + host: "", + url: &url.URL{ + Path: "/user/123", + }, + header: nil, + tls: noTLS, + expected: []otelkv.KeyValue{ + otelkv.String("http.method", "GET"), + otelkv.String("http.url", "/user/123"), + otelkv.String("http.scheme", "http"), + otelkv.String("http.flavor", "1.0"), + }, + }, + { + name: "with tls", + method: "GET", + requestURI: "/user/123", + proto: "HTTP/1.0", + remoteAddr: "", + host: "", + url: &url.URL{ + Path: "/user/123", + }, + header: nil, + tls: withTLS, + expected: []otelkv.KeyValue{ + otelkv.String("http.method", "GET"), + otelkv.String("http.url", "/user/123"), + otelkv.String("http.scheme", "https"), + otelkv.String("http.flavor", "1.0"), + }, + }, + { + name: "with host", + method: "GET", + requestURI: "/user/123", + proto: "HTTP/1.0", + remoteAddr: "", + host: "example.com", + url: &url.URL{ + Path: "/user/123", + }, + header: nil, + tls: withTLS, + expected: []otelkv.KeyValue{ + otelkv.String("http.method", "GET"), + otelkv.String("http.url", "/user/123"), + otelkv.String("http.scheme", "https"), + otelkv.String("http.flavor", "1.0"), + otelkv.String("http.host", "example.com"), + }, + }, + { + name: "with user agent", + method: "GET", + requestURI: "/user/123", + proto: "HTTP/1.0", + remoteAddr: "", + host: "example.com", + url: &url.URL{ + Path: "/user/123", + }, + header: http.Header{ + "User-Agent": []string{"foodownloader"}, + }, + tls: withTLS, + expected: []otelkv.KeyValue{ + otelkv.String("http.method", "GET"), + otelkv.String("http.url", "/user/123"), + otelkv.String("http.scheme", "https"), + otelkv.String("http.flavor", "1.0"), + otelkv.String("http.host", "example.com"), + otelkv.String("http.user_agent", "foodownloader"), + }, + }, + { + name: "with http 1.1", + method: "GET", + requestURI: "/user/123", + proto: "HTTP/1.1", + remoteAddr: "", + host: "example.com", + url: &url.URL{ + Path: "/user/123", + }, + header: http.Header{ + "User-Agent": []string{"foodownloader"}, + }, + tls: withTLS, + expected: []otelkv.KeyValue{ + otelkv.String("http.method", "GET"), + otelkv.String("http.url", "/user/123"), + otelkv.String("http.scheme", "https"), + otelkv.String("http.flavor", "1.1"), + otelkv.String("http.host", "example.com"), + otelkv.String("http.user_agent", "foodownloader"), + }, + }, + { + name: "with http 2", + method: "GET", + requestURI: "/user/123", + proto: "HTTP/2.0", + remoteAddr: "", + host: "example.com", + url: &url.URL{ + Path: "/user/123", + }, + header: http.Header{ + "User-Agent": []string{"foodownloader"}, + }, + tls: withTLS, + expected: []otelkv.KeyValue{ + otelkv.String("http.method", "GET"), + otelkv.String("http.url", "/user/123"), + otelkv.String("http.scheme", "https"), + otelkv.String("http.flavor", "2"), + otelkv.String("http.host", "example.com"), + otelkv.String("http.user_agent", "foodownloader"), + }, + }, + { + name: "with content length", + method: "GET", + url: &url.URL{ + Path: "/user/123", + }, + contentLength: 100, + expected: []otelkv.KeyValue{ + otelkv.String("http.method", "GET"), + otelkv.String("http.url", "/user/123"), + otelkv.String("http.scheme", "http"), + otelkv.Int64("http.request_content_length", 100), + }, + }, + { + name: "with empty method (fallback to GET)", + method: "", + url: &url.URL{ + Path: "/user/123", + }, + expected: []otelkv.KeyValue{ + otelkv.String("http.method", "GET"), + otelkv.String("http.url", "/user/123"), + otelkv.String("http.scheme", "http"), + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + r := testRequest(tc.method, tc.requestURI, tc.proto, tc.remoteAddr, tc.host, tc.url, tc.header, tc.tls) + r.ContentLength = tc.contentLength + got := HTTPClientAttributesFromHTTPRequest(r) + assert.ElementsMatch(t, tc.expected, got) + }) + } +} From 987758b53b5756a7ae3eae68e222665451c55caa Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Mon, 6 Jul 2020 15:59:27 +0800 Subject: [PATCH 4/5] Update CHANGELOG Signed-off-by: Sam Xie --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9dcd8db84a7..aed1e92614c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add `peer.service` semantic attribute. (#898) - Add database-specific semantic attributes. (#899) - Add semantic convention for `faas.coldstart` and `container.id`. (#909) +- Add http content size semantic conventions. (#905) +- Include `http.request_content_length` in HTTP request basic attributes. (#905) ### Changed @@ -47,6 +49,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Ensure span status is not set to `Unknown` when no HTTP status code is provided as it is assumed to be `200 OK`. (#908) - Ensure `httptrace.clientTracer` closes `http.headers` span. (#912) - Prometheus exporter will not apply stale updates or forget inactive metrics. (#903) +- Add test for api.standard `HTTPClientAttributesFromHTTPRequest`. (#905) ## [0.7.0] - 2020-06-26 From 48105cf8062a460fc2c5681d1b921f30ea802254 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 8 Jul 2020 10:39:55 +0800 Subject: [PATCH 5/5] Fix http content size naming Signed-off-by: Sam Xie --- api/standard/http.go | 2 +- api/standard/trace.go | 8 ++++---- instrumentation/httptrace/httptrace_test.go | 22 ++++++++++----------- instrumentation/othttp/handler_test.go | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/api/standard/http.go b/api/standard/http.go index 172572e29bd..040d0a19bbb 100644 --- a/api/standard/http.go +++ b/api/standard/http.go @@ -178,7 +178,7 @@ func httpBasicAttributesFromHTTPRequest(request *http.Request) []kv.KeyValue { attrs = append(attrs, HTTPFlavorKey.String(flavor)) } if request.ContentLength > 0 { - attrs = append(attrs, HTTPRequestContentLength.Int64(request.ContentLength)) + attrs = append(attrs, HTTPRequestContentLengthKey.Int64(request.ContentLength)) } return attrs diff --git a/api/standard/trace.go b/api/standard/trace.go index de693446f8d..98076ac636d 100644 --- a/api/standard/trace.go +++ b/api/standard/trace.go @@ -113,18 +113,18 @@ const ( HTTPClientIPKey = kv.Key("http.client_ip") // The size of the request payload body in bytes. - HTTPRequestContentLength = kv.Key("http.request_content_length") + HTTPRequestContentLengthKey = kv.Key("http.request_content_length") // The size of the uncompressed request payload body after transport decoding. // Not set if transport encoding not used. - HTTPRequestContentLengthUncompressed = kv.Key("http.request_content_length_uncompressed") + HTTPRequestContentLengthUncompressedKey = kv.Key("http.request_content_length_uncompressed") // The size of the response payload body in bytes. - HTTPResponseContentLength = kv.Key("http.response_content_length") + HTTPResponseContentLengthKey = kv.Key("http.response_content_length") // The size of the uncompressed response payload body after transport decoding. // Not set if transport encoding not used. - HTTPResponseContentLengthUncompressed = kv.Key("http.response_content_length_uncompressed") + HTTPResponseContentLengthUncompressedKey = kv.Key("http.response_content_length_uncompressed") ) var ( diff --git a/instrumentation/httptrace/httptrace_test.go b/instrumentation/httptrace/httptrace_test.go index b71ce8c4f92..b66adbae44a 100644 --- a/instrumentation/httptrace/httptrace_test.go +++ b/instrumentation/httptrace/httptrace_test.go @@ -87,17 +87,17 @@ func TestRoundtrip(t *testing.T) { address := ts.Listener.Addr() hp := strings.Split(address.String(), ":") expectedAttrs = map[kv.Key]string{ - standard.HTTPFlavorKey: "1.1", - standard.HTTPHostKey: address.String(), - standard.HTTPMethodKey: "GET", - standard.HTTPSchemeKey: "http", - standard.HTTPTargetKey: "/", - standard.HTTPUserAgentKey: "Go-http-client/1.1", - standard.HTTPRequestContentLength: "3", - standard.NetHostIPKey: hp[0], - standard.NetHostPortKey: hp[1], - standard.NetPeerIPKey: "127.0.0.1", - standard.NetTransportKey: "IP.TCP", + standard.HTTPFlavorKey: "1.1", + standard.HTTPHostKey: address.String(), + standard.HTTPMethodKey: "GET", + standard.HTTPSchemeKey: "http", + standard.HTTPTargetKey: "/", + standard.HTTPUserAgentKey: "Go-http-client/1.1", + standard.HTTPRequestContentLengthKey: "3", + standard.NetHostIPKey: hp[0], + standard.NetHostPortKey: hp[1], + standard.NetPeerIPKey: "127.0.0.1", + standard.NetTransportKey: "IP.TCP", } client := ts.Client() diff --git a/instrumentation/othttp/handler_test.go b/instrumentation/othttp/handler_test.go index 1967e0388d0..b4fbab925f8 100644 --- a/instrumentation/othttp/handler_test.go +++ b/instrumentation/othttp/handler_test.go @@ -72,7 +72,7 @@ func TestHandlerBasics(t *testing.T) { standard.HTTPSchemeHTTP, standard.HTTPHostKey.String(r.Host), standard.HTTPFlavorKey.String(fmt.Sprintf("1.%d", r.ProtoMinor)), - standard.HTTPRequestContentLength.Int64(3), + standard.HTTPRequestContentLengthKey.Int64(3), } assertMetricLabels(t, labelsToVerify, meterimpl.MeasurementBatches)