From 60b7e05636e5f6e695c24d14b1fac4837a744a41 Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Wed, 13 Jul 2022 22:59:04 +0800 Subject: [PATCH 1/5] Add http.method attribute to http server metric Signed-off-by: Ziqi Zhao --- semconv/internal/http.go | 4 +- semconv/internal/http_test.go | 253 ++++++++++++++++++++++++++++++++++ 2 files changed, 256 insertions(+), 1 deletion(-) diff --git a/semconv/internal/http.go b/semconv/internal/http.go index 864ba3f39df..5198ea21345 100644 --- a/semconv/internal/http.go +++ b/semconv/internal/http.go @@ -210,7 +210,9 @@ func (sc *SemanticConventions) httpBasicAttributesFromHTTPRequest(request *http. // HTTPServerMetricAttributesFromHTTPRequest generates low-cardinality attributes // to be used with server-side HTTP metrics. func (sc *SemanticConventions) HTTPServerMetricAttributesFromHTTPRequest(serverName string, request *http.Request) []attribute.KeyValue { - attrs := []attribute.KeyValue{} + attrs := []attribute.KeyValue{ + sc.HTTPMethodKey.String(request.Method), + } if serverName != "" { attrs = append(attrs, sc.HTTPServerNameKey.String(serverName)) } diff --git a/semconv/internal/http_test.go b/semconv/internal/http_test.go index 302c3e0ea0a..8ba0dc14ab0 100644 --- a/semconv/internal/http_test.go +++ b/semconv/internal/http_test.go @@ -1236,3 +1236,256 @@ func TestHTTPClientAttributesFromHTTPRequest(t *testing.T) { }) } } + +func TestHTTPServerMetricAttributesFromHTTPRequest(t *testing.T) { + type testcase struct { + name string + serverName string + route string + method string + requestURI string + proto string + remoteAddr string + host string + url *url.URL + header http.Header + tls tlsOption + contentLength int64 + expected []attribute.KeyValue + } + testcases := []testcase{ + { + name: "stripped", + serverName: "", + route: "", + method: "GET", + requestURI: "/user/123", + proto: "HTTP/1.0", + remoteAddr: "", + host: "", + url: &url.URL{ + Path: "/user/123", + }, + header: nil, + tls: noTLS, + expected: []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "http"), + attribute.String("http.flavor", "1.0"), + }, + }, + { + name: "with server name", + serverName: "my-server-name", + route: "", + method: "GET", + requestURI: "/user/123", + proto: "HTTP/1.0", + remoteAddr: "", + host: "", + url: &url.URL{ + Path: "/user/123", + }, + header: nil, + tls: noTLS, + expected: []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "http"), + attribute.String("http.flavor", "1.0"), + attribute.String("http.server_name", "my-server-name"), + }, + }, + { + name: "with tls", + serverName: "my-server-name", + route: "", + method: "GET", + requestURI: "/user/123", + proto: "HTTP/1.0", + remoteAddr: "", + host: "", + url: &url.URL{ + Path: "/user/123", + }, + header: nil, + tls: withTLS, + expected: []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "https"), + attribute.String("http.flavor", "1.0"), + attribute.String("http.server_name", "my-server-name"), + }, + }, + { + name: "with route", + serverName: "my-server-name", + route: "/user/:id", + method: "GET", + requestURI: "/user/123", + proto: "HTTP/1.0", + remoteAddr: "", + host: "", + url: &url.URL{ + Path: "/user/123", + }, + header: nil, + tls: withTLS, + expected: []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "https"), + attribute.String("http.flavor", "1.0"), + attribute.String("http.server_name", "my-server-name"), + }, + }, + { + name: "with host", + serverName: "my-server-name", + route: "/user/:id", + method: "GET", + requestURI: "/user/123", + proto: "HTTP/1.0", + remoteAddr: "", + host: "example.com", + url: &url.URL{ + Path: "/user/123", + }, + header: nil, + tls: withTLS, + expected: []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "https"), + attribute.String("http.flavor", "1.0"), + attribute.String("http.server_name", "my-server-name"), + attribute.String("http.host", "example.com"), + }, + }, + { + name: "with host fallback", + serverName: "my-server-name", + route: "/user/:id", + method: "GET", + requestURI: "/user/123", + proto: "HTTP/1.0", + remoteAddr: "", + host: "", + url: &url.URL{ + Host: "example.com", + Path: "/user/123", + }, + header: nil, + tls: withTLS, + expected: []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "https"), + attribute.String("http.flavor", "1.0"), + attribute.String("http.server_name", "my-server-name"), + attribute.String("http.host", "example.com"), + }, + }, + { + name: "with user agent", + serverName: "my-server-name", + route: "/user/:id", + 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: []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "https"), + attribute.String("http.flavor", "1.0"), + attribute.String("http.server_name", "my-server-name"), + attribute.String("http.host", "example.com"), + }, + }, + { + name: "with proxy info", + serverName: "my-server-name", + route: "/user/:id", + 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"}, + "X-Forwarded-For": []string{"203.0.113.195, 70.41.3.18, 150.172.238.178"}, + }, + tls: withTLS, + expected: []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "https"), + attribute.String("http.flavor", "1.0"), + attribute.String("http.server_name", "my-server-name"), + attribute.String("http.host", "example.com"), + }, + }, + { + name: "with http 1.1", + serverName: "my-server-name", + route: "/user/:id", + 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"}, + "X-Forwarded-For": []string{"1.2.3.4"}, + }, + tls: withTLS, + expected: []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "https"), + attribute.String("http.flavor", "1.1"), + attribute.String("http.server_name", "my-server-name"), + attribute.String("http.host", "example.com"), + }, + }, + { + name: "with http 2", + serverName: "my-server-name", + route: "/user/:id", + 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"}, + "X-Forwarded-For": []string{"1.2.3.4"}, + }, + tls: withTLS, + expected: []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "https"), + attribute.String("http.flavor", "2"), + attribute.String("http.server_name", "my-server-name"), + attribute.String("http.host", "example.com"), + }, + }, + } + 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 := sc.HTTPServerMetricAttributesFromHTTPRequest(tc.serverName, r) + assertElementsMatch(t, tc.expected, got, "testcase %d - %s", idx, tc.name) + } +} From 4041e66e164efbadb4db5a3d972df6a21b4b9992 Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Wed, 13 Jul 2022 23:22:31 +0800 Subject: [PATCH 2/5] fix lint Signed-off-by: Ziqi Zhao --- semconv/internal/http_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/semconv/internal/http_test.go b/semconv/internal/http_test.go index 8ba0dc14ab0..46ef5d4f18d 100644 --- a/semconv/internal/http_test.go +++ b/semconv/internal/http_test.go @@ -1241,7 +1241,6 @@ func TestHTTPServerMetricAttributesFromHTTPRequest(t *testing.T) { type testcase struct { name string serverName string - route string method string requestURI string proto string From ce676ae84ff2a115ee53dfa55d9707eab0a49c92 Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Wed, 13 Jul 2022 23:25:01 +0800 Subject: [PATCH 3/5] fix lint Signed-off-by: Ziqi Zhao --- semconv/internal/http_test.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/semconv/internal/http_test.go b/semconv/internal/http_test.go index 46ef5d4f18d..40e9bfd8bab 100644 --- a/semconv/internal/http_test.go +++ b/semconv/internal/http_test.go @@ -1256,7 +1256,6 @@ func TestHTTPServerMetricAttributesFromHTTPRequest(t *testing.T) { { name: "stripped", serverName: "", - route: "", method: "GET", requestURI: "/user/123", proto: "HTTP/1.0", @@ -1276,7 +1275,6 @@ func TestHTTPServerMetricAttributesFromHTTPRequest(t *testing.T) { { name: "with server name", serverName: "my-server-name", - route: "", method: "GET", requestURI: "/user/123", proto: "HTTP/1.0", @@ -1297,7 +1295,6 @@ func TestHTTPServerMetricAttributesFromHTTPRequest(t *testing.T) { { name: "with tls", serverName: "my-server-name", - route: "", method: "GET", requestURI: "/user/123", proto: "HTTP/1.0", @@ -1318,7 +1315,6 @@ func TestHTTPServerMetricAttributesFromHTTPRequest(t *testing.T) { { name: "with route", serverName: "my-server-name", - route: "/user/:id", method: "GET", requestURI: "/user/123", proto: "HTTP/1.0", @@ -1339,7 +1335,6 @@ func TestHTTPServerMetricAttributesFromHTTPRequest(t *testing.T) { { name: "with host", serverName: "my-server-name", - route: "/user/:id", method: "GET", requestURI: "/user/123", proto: "HTTP/1.0", @@ -1361,7 +1356,6 @@ func TestHTTPServerMetricAttributesFromHTTPRequest(t *testing.T) { { name: "with host fallback", serverName: "my-server-name", - route: "/user/:id", method: "GET", requestURI: "/user/123", proto: "HTTP/1.0", @@ -1384,7 +1378,6 @@ func TestHTTPServerMetricAttributesFromHTTPRequest(t *testing.T) { { name: "with user agent", serverName: "my-server-name", - route: "/user/:id", method: "GET", requestURI: "/user/123", proto: "HTTP/1.0", @@ -1408,7 +1401,6 @@ func TestHTTPServerMetricAttributesFromHTTPRequest(t *testing.T) { { name: "with proxy info", serverName: "my-server-name", - route: "/user/:id", method: "GET", requestURI: "/user/123", proto: "HTTP/1.0", @@ -1433,7 +1425,6 @@ func TestHTTPServerMetricAttributesFromHTTPRequest(t *testing.T) { { name: "with http 1.1", serverName: "my-server-name", - route: "/user/:id", method: "GET", requestURI: "/user/123", proto: "HTTP/1.1", @@ -1458,7 +1449,6 @@ func TestHTTPServerMetricAttributesFromHTTPRequest(t *testing.T) { { name: "with http 2", serverName: "my-server-name", - route: "/user/:id", method: "GET", requestURI: "/user/123", proto: "HTTP/2.0", From 55cece969ab3ef10237ee7c81c89175167f83810 Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Fri, 15 Jul 2022 00:24:07 +0800 Subject: [PATCH 4/5] fix for reviews Signed-off-by: Ziqi Zhao --- semconv/internal/http.go | 17 ++++++-------- semconv/internal/http_test.go | 43 +++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/semconv/internal/http.go b/semconv/internal/http.go index 5198ea21345..b580eedeff7 100644 --- a/semconv/internal/http.go +++ b/semconv/internal/http.go @@ -147,12 +147,6 @@ func (sc *SemanticConventions) EndUserAttributesFromHTTPRequest(request *http.Re func (sc *SemanticConventions) HTTPClientAttributesFromHTTPRequest(request *http.Request) []attribute.KeyValue { attrs := []attribute.KeyValue{} - if request.Method != "" { - attrs = append(attrs, sc.HTTPMethodKey.String(request.Method)) - } else { - attrs = append(attrs, sc.HTTPMethodKey.String(http.MethodGet)) - } - // remove any username/password info that may be in the URL // before adding it to the attributes userinfo := request.URL.User @@ -204,15 +198,19 @@ func (sc *SemanticConventions) httpBasicAttributesFromHTTPRequest(request *http. attrs = append(attrs, sc.HTTPFlavorKey.String(flavor)) } + if request.Method != "" { + attrs = append(attrs, sc.HTTPMethodKey.String(request.Method)) + } else { + attrs = append(attrs, sc.HTTPMethodKey.String(http.MethodGet)) + } + return attrs } // HTTPServerMetricAttributesFromHTTPRequest generates low-cardinality attributes // to be used with server-side HTTP metrics. func (sc *SemanticConventions) HTTPServerMetricAttributesFromHTTPRequest(serverName string, request *http.Request) []attribute.KeyValue { - attrs := []attribute.KeyValue{ - sc.HTTPMethodKey.String(request.Method), - } + attrs := []attribute.KeyValue{} if serverName != "" { attrs = append(attrs, sc.HTTPServerNameKey.String(serverName)) } @@ -225,7 +223,6 @@ func (sc *SemanticConventions) HTTPServerMetricAttributesFromHTTPRequest(serverN // supported. func (sc *SemanticConventions) HTTPServerAttributesFromHTTPRequest(serverName, route string, request *http.Request) []attribute.KeyValue { attrs := []attribute.KeyValue{ - sc.HTTPMethodKey.String(request.Method), sc.HTTPTargetKey.String(request.RequestURI), } diff --git a/semconv/internal/http_test.go b/semconv/internal/http_test.go index 40e9bfd8bab..3e42f02faf3 100644 --- a/semconv/internal/http_test.go +++ b/semconv/internal/http_test.go @@ -1478,3 +1478,46 @@ func TestHTTPServerMetricAttributesFromHTTPRequest(t *testing.T) { assertElementsMatch(t, tc.expected, got, "testcase %d - %s", idx, tc.name) } } + +func TestHttpBasicAttributesFromHTTPRequest(t *testing.T) { + type testcase struct { + name string + method string + requestURI string + proto string + remoteAddr string + host string + url *url.URL + header http.Header + tls tlsOption + contentLength int64 + expected []attribute.KeyValue + } + testcases := []testcase{ + { + name: "stripped", + method: "GET", + requestURI: "/user/123", + proto: "HTTP/1.0", + remoteAddr: "", + host: "example.com", + url: &url.URL{ + Path: "/user/123", + }, + header: nil, + tls: noTLS, + expected: []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "http"), + attribute.String("http.flavor", "1.0"), + attribute.String("http.host", "example.com"), + }, + }, + } + 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 := sc.httpBasicAttributesFromHTTPRequest(r) + assertElementsMatch(t, tc.expected, got, "testcase %d - %s", idx, tc.name) + } +} From 5ed101db6f863030cb1646897f0a98b9fa9fd49f Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Fri, 15 Jul 2022 00:44:52 +0800 Subject: [PATCH 5/5] add changelog entry Signed-off-by: Ziqi Zhao --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a1eda5dcd7..21cef2402fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm The package contains semantic conventions from the `v1.12.0` version of the OpenTelemetry specification. (#3010) - Add the `go.opentelemetry.io/otel/semconv/v1.11.0` package. The package contains semantic conventions from the `v1.11.0` version of the OpenTelemetry specification. (#3009) +- Add http.method attribute to http server metric. (#3018) ## [1.8.0/0.31.0] - 2022-07-08