From 5049d9102a67ba3d4271adc19748030dd0881437 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Wed, 17 Apr 2024 09:36:01 -0500 Subject: [PATCH 1/3] Add new semver. --- .../net/http/otelhttp/internal/semconv/env.go | 10 +- .../http/otelhttp/internal/semconv/util.go | 42 +++++ .../http/otelhttp/internal/semconv/v1.24.0.go | 156 ++++++++++++++++++ .../otelhttp/internal/semconv/v1.24.0_test.go | 125 ++++++++++++++ 4 files changed, 332 insertions(+), 1 deletion(-) create mode 100644 instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go create mode 100644 instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go diff --git a/instrumentation/net/http/otelhttp/internal/semconv/env.go b/instrumentation/net/http/otelhttp/internal/semconv/env.go index 9be3feef29e..3763ca03946 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/env.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/env.go @@ -6,6 +6,8 @@ package semconv // import "go.opentelemetry.io/contrib/instrumentation/net/http/ import ( "fmt" "net/http" + "os" + "strings" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" @@ -52,7 +54,13 @@ type HTTPServer interface { func NewHTTPServer() HTTPServer { // TODO (#5331): Detect version based on environment variable OTEL_HTTP_CLIENT_COMPATIBILITY_MODE. // TODO (#5331): Add warning of use of a deprecated version of Semantic Versions. - return oldHTTPServer{} + env := strings.ToLower(os.Getenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE")) + switch env { + case "http": + return newHTTPServer{} + default: + return oldHTTPServer{} + } } // ServerStatus returns a span status code and message for an HTTP status code diff --git a/instrumentation/net/http/otelhttp/internal/semconv/util.go b/instrumentation/net/http/otelhttp/internal/semconv/util.go index c92076bc3d9..e7f293761bd 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/util.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/util.go @@ -5,8 +5,12 @@ package semconv // import "go.opentelemetry.io/contrib/instrumentation/net/http/ import ( "net" + "net/http" "strconv" "strings" + + "go.opentelemetry.io/otel/attribute" + semconvNew "go.opentelemetry.io/otel/semconv/v1.24.0" ) // splitHostPort splits a network address hostport of the form "host", @@ -47,3 +51,41 @@ func splitHostPort(hostport string) (host string, port int) { } return host, int(p) } + +func requiredHTTPPort(https bool, port int) int { // nolint:revive + if https { + if port > 0 && port != 443 { + return port + } + } else { + if port > 0 && port != 80 { + return port + } + } + return -1 +} + +func serverClientIP(xForwardedFor string) string { + if idx := strings.Index(xForwardedFor, ","); idx >= 0 { + xForwardedFor = xForwardedFor[:idx] + } + return xForwardedFor +} + +func netProtocol(proto string) (name string, version string) { + name, version, _ = strings.Cut(proto, "/") + name = strings.ToLower(name) + return name, version +} + +var methodLookup = map[string]attribute.KeyValue{ + http.MethodConnect: semconvNew.HTTPRequestMethodConnect, + http.MethodDelete: semconvNew.HTTPRequestMethodDelete, + http.MethodGet: semconvNew.HTTPRequestMethodGet, + http.MethodHead: semconvNew.HTTPRequestMethodHead, + http.MethodOptions: semconvNew.HTTPRequestMethodOptions, + http.MethodPatch: semconvNew.HTTPRequestMethodPatch, + http.MethodPost: semconvNew.HTTPRequestMethodPost, + http.MethodPut: semconvNew.HTTPRequestMethodPut, + http.MethodTrace: semconvNew.HTTPRequestMethodTrace, +} diff --git a/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go new file mode 100644 index 00000000000..1f38d3596c5 --- /dev/null +++ b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go @@ -0,0 +1,156 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package semconv // import "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv" + +import ( + "net/http" + "slices" + "strings" + + "go.opentelemetry.io/otel/attribute" + semconvNew "go.opentelemetry.io/otel/semconv/v1.24.0" +) + +type newHTTPServer struct{} + +var _ HTTPServer = newHTTPServer{} + +// TraceRequest returns trace attributes for an HTTP request received by a +// server. +// +// The server must be the primary server name if it is known. For example this +// would be the ServerName directive +// (https://httpd.apache.org/docs/2.4/mod/core.html#servername) for an Apache +// server, and the server_name directive +// (http://nginx.org/en/docs/http/ngx_http_core_module.html#server_name) for an +// nginx server. More generically, the primary server name would be the host +// header value that matches the default virtual host of an HTTP server. It +// should include the host identifier and if a port is used to route to the +// server that port identifier should be included as an appropriate port +// suffix. +// +// If the primary server name is not known, server should be an empty string. +// The req Host will be used to determine the server instead. +func (n newHTTPServer) RequestTraceAttrs(server string, req *http.Request) []attribute.KeyValue { + const MaxAttributes = 11 + attrs := make([]attribute.KeyValue, MaxAttributes) + var host string + var p int + if server == "" { + host, p = splitHostPort(req.Host) + } else { + // Prioritize the primary server name. + host, p = splitHostPort(server) + if p < 0 { + _, p = splitHostPort(req.Host) + } + } + + attrs[0] = semconvNew.ServerAddress(host) + i := 1 + if hostPort := requiredHTTPPort(req.TLS != nil, p); hostPort > 0 { + attrs[i] = semconvNew.ServerPort(hostPort) + i++ + } + i += n.method(req.Method, attrs[i:]) // Max 2 + i += n.scheme(req.TLS != nil, attrs[i:]) // Max 1 + + if peer, peerPort := splitHostPort(req.RemoteAddr); peer != "" { + // The Go HTTP server sets RemoteAddr to "IP:port", this will not be a + // file-path that would be interpreted with a sock family. + attrs[i] = semconvNew.NetworkPeerAddress(peer) + i++ + if peerPort > 0 { + attrs[i] = semconvNew.NetworkPeerPort(peerPort) + i++ + } + } + + if useragent := req.UserAgent(); useragent != "" { + // This is the same between v1.20, and v1.24 + attrs[i] = semconvNew.UserAgentOriginal(useragent) + i++ + } + + if clientIP := serverClientIP(req.Header.Get("X-Forwarded-For")); clientIP != "" { + attrs[i] = semconvNew.ClientAddress(clientIP) + i++ + } + + if req.URL != nil && req.URL.Path != "" { + attrs[i] = semconvNew.URLPath(req.URL.Path) + i++ + } + + protoName, protoVersion := netProtocol(req.Proto) + if protoName != "" && protoName != "http" { + attrs[i] = semconvNew.NetworkProtocolName(protoName) + i++ + } + if protoVersion != "" { + attrs[i] = semconvNew.NetworkProtocolVersion(protoVersion) + i++ + } + + return slices.Clip(attrs[:i]) +} + +func (n newHTTPServer) method(method string, attrs []attribute.KeyValue) int { + if method == "" { + attrs[0] = semconvNew.HTTPRequestMethodGet + return 1 + } + if attr, ok := methodLookup[method]; ok { + attrs[0] = attr + return 1 + } + + if attr, ok := methodLookup[strings.ToUpper(method)]; ok { + attrs[0] = attr + } else { + // If the Original methos is not a standard HTTP method fallback to GET + attrs[0] = semconvNew.HTTPRequestMethodGet + } + attrs[1] = semconvNew.HTTPRequestMethodOriginal(method) + return 2 +} + +func (n newHTTPServer) scheme(https bool, attrs []attribute.KeyValue) int { // nolint:revive + if https { + attrs[0] = semconvNew.URLScheme("https") + return 1 + } + attrs[0] = semconvNew.URLScheme("http") + return 1 +} + +// TraceResponse returns trace attributes for telemetry from an HTTP response. +// +// If any of the fields in the ResponseTelemetry are not set the attribute will be omitted. +func (n newHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { + attributes := []attribute.KeyValue{} + + if resp.ReadBytes > 0 { + attributes = append(attributes, + semconvNew.HTTPRequestBodySize(int(resp.ReadBytes)), + ) + } + if resp.WriteBytes > 0 { + attributes = append(attributes, + semconvNew.HTTPResponseBodySize(int(resp.WriteBytes)), + ) + } + if resp.StatusCode > 0 { + attributes = append(attributes, + semconvNew.HTTPResponseStatusCode(resp.StatusCode), + ) + } + + return attributes +} + +// Route returns the attribute for the route. +func (n newHTTPServer) Route(route string) attribute.KeyValue { + return semconvNew.HTTPRoute(route) +} diff --git a/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go new file mode 100644 index 00000000000..9d2a6e5f5db --- /dev/null +++ b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go @@ -0,0 +1,125 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package semconv + +import ( + "fmt" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.opentelemetry.io/otel/attribute" +) + +func TestNewTraceRequest(t *testing.T) { + t.Setenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE", "http") + serv := NewHTTPServer() + want := func(req testServerReq) []attribute.KeyValue { + return []attribute.KeyValue{ + attribute.String("http.request.method", "GET"), + attribute.String("url.scheme", "http"), + attribute.String("server.address", req.hostname), + attribute.Int("server.port", req.serverPort), + attribute.String("network.peer.address", req.peerAddr), + attribute.Int("network.peer.port", req.peerPort), + attribute.String("user_agent.original", "Go-http-client/1.1"), + attribute.String("client.address", req.clientIP), + attribute.String("network.protocol.version", "1.1"), + attribute.String("url.path", "/"), + } + } + testTraceRequest(t, serv, want) +} + +func TestNewTraceResponse(t *testing.T) { + testCases := []struct { + name string + resp ResponseTelemetry + want []attribute.KeyValue + }{ + { + name: "empty", + resp: ResponseTelemetry{}, + want: nil, + }, + { + name: "no errors", + resp: ResponseTelemetry{ + StatusCode: 200, + ReadBytes: 701, + WriteBytes: 802, + }, + want: []attribute.KeyValue{ + attribute.Int("http.request.body.size", 701), + attribute.Int("http.response.body.size", 802), + attribute.Int("http.response.status_code", 200), + }, + }, + { + name: "with errors", + resp: ResponseTelemetry{ + StatusCode: 200, + ReadBytes: 701, + ReadError: fmt.Errorf("read error"), + WriteBytes: 802, + WriteError: fmt.Errorf("write error"), + }, + want: []attribute.KeyValue{ + attribute.Int("http.request.body.size", 701), + attribute.Int("http.response.body.size", 802), + attribute.Int("http.response.status_code", 200), + }, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + got := newHTTPServer{}.ResponseTraceAttrs(tt.resp) + assert.ElementsMatch(t, tt.want, got) + }) + } +} + +func TestNewMethod(t *testing.T) { + testCases := []struct { + method string + n int + want []attribute.KeyValue + }{ + { + method: http.MethodPost, + n: 1, + want: []attribute.KeyValue{ + attribute.String("http.request.method", "POST"), + }, + }, + { + method: "Put", + n: 2, + want: []attribute.KeyValue{ + attribute.String("http.request.method", "PUT"), + attribute.String("http.request.method_original", "Put"), + }, + }, + { + method: "Unknown", + n: 2, + want: []attribute.KeyValue{ + attribute.String("http.request.method", "GET"), + attribute.String("http.request.method_original", "Unknown"), + }, + }, + } + + for _, tt := range testCases { + t.Run(tt.method, func(t *testing.T) { + attrs := make([]attribute.KeyValue, 5) + n := newHTTPServer{}.method(tt.method, attrs[1:]) + require.Equal(t, tt.n, n, "Length doesn't match") + require.ElementsMatch(t, tt.want, attrs[1:n+1]) + }) + } +} From 2d51fbd775b7ff774ada0a70fba5b69aba5fdd21 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Mon, 20 May 2024 16:27:10 -0500 Subject: [PATCH 2/3] Update attribute flow to check Make append --- .../http/otelhttp/internal/semconv/v1.24.0.go | 132 ++++++++++++------ .../otelhttp/internal/semconv/v1.24.0_test.go | 39 +++--- 2 files changed, 103 insertions(+), 68 deletions(-) diff --git a/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go index 1f38d3596c5..1fd67ebe8f7 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go @@ -5,7 +5,6 @@ package semconv // import "go.opentelemetry.io/contrib/instrumentation/net/http/ import ( "net/http" - "slices" "strings" "go.opentelemetry.io/otel/attribute" @@ -33,8 +32,8 @@ var _ HTTPServer = newHTTPServer{} // If the primary server name is not known, server should be an empty string. // The req Host will be used to determine the server instead. func (n newHTTPServer) RequestTraceAttrs(server string, req *http.Request) []attribute.KeyValue { - const MaxAttributes = 11 - attrs := make([]attribute.KeyValue, MaxAttributes) + count := 3 // ServerAddress, Method, Scheme + var host string var p int if server == "" { @@ -47,89 +46,132 @@ func (n newHTTPServer) RequestTraceAttrs(server string, req *http.Request) []att } } - attrs[0] = semconvNew.ServerAddress(host) - i := 1 - if hostPort := requiredHTTPPort(req.TLS != nil, p); hostPort > 0 { - attrs[i] = semconvNew.ServerPort(hostPort) - i++ + hostPort := requiredHTTPPort(req.TLS != nil, p) + if hostPort > 0 { + count++ + } + + method, methodOriginal := n.method(req.Method) + if methodOriginal != (attribute.KeyValue{}) { + count++ } - i += n.method(req.Method, attrs[i:]) // Max 2 - i += n.scheme(req.TLS != nil, attrs[i:]) // Max 1 + + scheme := n.scheme(req.TLS != nil) if peer, peerPort := splitHostPort(req.RemoteAddr); peer != "" { // The Go HTTP server sets RemoteAddr to "IP:port", this will not be a // file-path that would be interpreted with a sock family. - attrs[i] = semconvNew.NetworkPeerAddress(peer) - i++ + count++ if peerPort > 0 { - attrs[i] = semconvNew.NetworkPeerPort(peerPort) - i++ + count++ } } - if useragent := req.UserAgent(); useragent != "" { - // This is the same between v1.20, and v1.24 - attrs[i] = semconvNew.UserAgentOriginal(useragent) - i++ + useragent := req.UserAgent() + if useragent != "" { + count++ } - if clientIP := serverClientIP(req.Header.Get("X-Forwarded-For")); clientIP != "" { - attrs[i] = semconvNew.ClientAddress(clientIP) - i++ + clientIP := serverClientIP(req.Header.Get("X-Forwarded-For")) + if clientIP != "" { + count++ } if req.URL != nil && req.URL.Path != "" { - attrs[i] = semconvNew.URLPath(req.URL.Path) - i++ + count++ } protoName, protoVersion := netProtocol(req.Proto) if protoName != "" && protoName != "http" { - attrs[i] = semconvNew.NetworkProtocolName(protoName) - i++ + count++ } if protoVersion != "" { - attrs[i] = semconvNew.NetworkProtocolVersion(protoVersion) - i++ + count++ + } + + attrs := []attribute.KeyValue{ + semconvNew.ServerAddress(host), + method, + scheme, + } + + if hostPort > 0 { + attrs = append(attrs, semconvNew.ServerPort(hostPort)) + } + if methodOriginal != (attribute.KeyValue{}) { + attrs = append(attrs, methodOriginal) + } + + if peer, peerPort := splitHostPort(req.RemoteAddr); peer != "" { + // The Go HTTP server sets RemoteAddr to "IP:port", this will not be a + // file-path that would be interpreted with a sock family. + attrs = append(attrs, semconvNew.NetworkPeerAddress(peer)) + if peerPort > 0 { + attrs = append(attrs, semconvNew.NetworkPeerPort(peerPort)) + } + } + + if useragent := req.UserAgent(); useragent != "" { + attrs = append(attrs, semconvNew.UserAgentOriginal(useragent)) } - return slices.Clip(attrs[:i]) + if clientIP != "" { + attrs = append(attrs, semconvNew.ClientAddress(clientIP)) + } + + if req.URL != nil && req.URL.Path != "" { + attrs = append(attrs, semconvNew.URLPath(req.URL.Path)) + } + + if protoName != "" && protoName != "http" { + attrs = append(attrs, semconvNew.NetworkProtocolName(protoName)) + } + if protoVersion != "" { + attrs = append(attrs, semconvNew.NetworkProtocolVersion(protoVersion)) + } + + return attrs } -func (n newHTTPServer) method(method string, attrs []attribute.KeyValue) int { +func (n newHTTPServer) method(method string) (attribute.KeyValue, attribute.KeyValue) { if method == "" { - attrs[0] = semconvNew.HTTPRequestMethodGet - return 1 + return semconvNew.HTTPRequestMethodGet, attribute.KeyValue{} } if attr, ok := methodLookup[method]; ok { - attrs[0] = attr - return 1 + return attr, attribute.KeyValue{} } + orig := semconvNew.HTTPRequestMethodOriginal(method) if attr, ok := methodLookup[strings.ToUpper(method)]; ok { - attrs[0] = attr - } else { - // If the Original methos is not a standard HTTP method fallback to GET - attrs[0] = semconvNew.HTTPRequestMethodGet + return attr, orig } - attrs[1] = semconvNew.HTTPRequestMethodOriginal(method) - return 2 + return semconvNew.HTTPRequestMethodGet, orig } -func (n newHTTPServer) scheme(https bool, attrs []attribute.KeyValue) int { // nolint:revive +func (n newHTTPServer) scheme(https bool) attribute.KeyValue { // nolint:revive if https { - attrs[0] = semconvNew.URLScheme("https") - return 1 + return semconvNew.URLScheme("https") } - attrs[0] = semconvNew.URLScheme("http") - return 1 + return semconvNew.URLScheme("http") } // TraceResponse returns trace attributes for telemetry from an HTTP response. // // If any of the fields in the ResponseTelemetry are not set the attribute will be omitted. func (n newHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { - attributes := []attribute.KeyValue{} + var count int + + if resp.ReadBytes > 0 { + count++ + } + if resp.WriteBytes > 0 { + count++ + } + if resp.StatusCode > 0 { + count++ + } + + attributes := make([]attribute.KeyValue, 0, count) if resp.ReadBytes > 0 { attributes = append(attributes, diff --git a/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go index 9d2a6e5f5db..deb92de20c2 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/attribute" ) @@ -85,41 +84,35 @@ func TestNewTraceResponse(t *testing.T) { func TestNewMethod(t *testing.T) { testCases := []struct { - method string - n int - want []attribute.KeyValue + method string + n int + want attribute.KeyValue + wantOrig attribute.KeyValue }{ { method: http.MethodPost, n: 1, - want: []attribute.KeyValue{ - attribute.String("http.request.method", "POST"), - }, + want: attribute.String("http.request.method", "POST"), }, { - method: "Put", - n: 2, - want: []attribute.KeyValue{ - attribute.String("http.request.method", "PUT"), - attribute.String("http.request.method_original", "Put"), - }, + method: "Put", + n: 2, + want: attribute.String("http.request.method", "PUT"), + wantOrig: attribute.String("http.request.method_original", "Put"), }, { - method: "Unknown", - n: 2, - want: []attribute.KeyValue{ - attribute.String("http.request.method", "GET"), - attribute.String("http.request.method_original", "Unknown"), - }, + method: "Unknown", + n: 2, + want: attribute.String("http.request.method", "GET"), + wantOrig: attribute.String("http.request.method_original", "Unknown"), }, } for _, tt := range testCases { t.Run(tt.method, func(t *testing.T) { - attrs := make([]attribute.KeyValue, 5) - n := newHTTPServer{}.method(tt.method, attrs[1:]) - require.Equal(t, tt.n, n, "Length doesn't match") - require.ElementsMatch(t, tt.want, attrs[1:n+1]) + got, gotOrig := newHTTPServer{}.method(tt.method) + assert.Equal(t, tt.want, got) + assert.Equal(t, tt.wantOrig, gotOrig) }) } } From 6db311c42f62252f51ae53f07f6254180ea22545 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Wed, 5 Jun 2024 10:27:43 -0500 Subject: [PATCH 3/3] Simplify HTTPServer for the usescases in #5132 --- .../net/http/otelhttp/internal/semconv/env.go | 71 ++++++++++--------- .../http/otelhttp/internal/semconv/v1.20.0.go | 2 - .../http/otelhttp/internal/semconv/v1.24.0.go | 7 +- .../otelhttp/internal/semconv/v1.24.0_test.go | 12 +++- 4 files changed, 52 insertions(+), 40 deletions(-) diff --git a/instrumentation/net/http/otelhttp/internal/semconv/env.go b/instrumentation/net/http/otelhttp/internal/semconv/env.go index 3763ca03946..3ec0ad00c81 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/env.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/env.go @@ -21,46 +21,51 @@ type ResponseTelemetry struct { WriteError error } -type HTTPServer interface { - // RequestTraceAttrs returns trace attributes for an HTTP request received by a - // server. - // - // The server must be the primary server name if it is known. For example this - // would be the ServerName directive - // (https://httpd.apache.org/docs/2.4/mod/core.html#servername) for an Apache - // server, and the server_name directive - // (http://nginx.org/en/docs/http/ngx_http_core_module.html#server_name) for an - // nginx server. More generically, the primary server name would be the host - // header value that matches the default virtual host of an HTTP server. It - // should include the host identifier and if a port is used to route to the - // server that port identifier should be included as an appropriate port - // suffix. - // - // If the primary server name is not known, server should be an empty string. - // The req Host will be used to determine the server instead. - RequestTraceAttrs(server string, req *http.Request) []attribute.KeyValue +type HTTPServer struct { + duplicate bool +} - // ResponseTraceAttrs returns trace attributes for telemetry from an HTTP response. - // - // If any of the fields in the ResponseTelemetry are not set the attribute will be omitted. - ResponseTraceAttrs(ResponseTelemetry) []attribute.KeyValue +// RequestTraceAttrs returns trace attributes for an HTTP request received by a +// server. +// +// The server must be the primary server name if it is known. For example this +// would be the ServerName directive +// (https://httpd.apache.org/docs/2.4/mod/core.html#servername) for an Apache +// server, and the server_name directive +// (http://nginx.org/en/docs/http/ngx_http_core_module.html#server_name) for an +// nginx server. More generically, the primary server name would be the host +// header value that matches the default virtual host of an HTTP server. It +// should include the host identifier and if a port is used to route to the +// server that port identifier should be included as an appropriate port +// suffix. +// +// If the primary server name is not known, server should be an empty string. +// The req Host will be used to determine the server instead. +func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request) []attribute.KeyValue { + if s.duplicate { + return append(oldHTTPServer{}.RequestTraceAttrs(server, req), newHTTPServer{}.RequestTraceAttrs(server, req)...) + } + return oldHTTPServer{}.RequestTraceAttrs(server, req) +} - // Route returns the attribute for the route. - Route(string) attribute.KeyValue +// ResponseTraceAttrs returns trace attributes for telemetry from an HTTP response. +// +// If any of the fields in the ResponseTelemetry are not set the attribute will be omitted. +func (s HTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { + if s.duplicate { + return append(oldHTTPServer{}.ResponseTraceAttrs(resp), newHTTPServer{}.ResponseTraceAttrs(resp)...) + } + return oldHTTPServer{}.ResponseTraceAttrs(resp) } -// var warnOnce = sync.Once{} +// Route returns the attribute for the route. +func (s HTTPServer) Route(route string) attribute.KeyValue { + return oldHTTPServer{}.Route(route) +} func NewHTTPServer() HTTPServer { - // TODO (#5331): Detect version based on environment variable OTEL_HTTP_CLIENT_COMPATIBILITY_MODE. - // TODO (#5331): Add warning of use of a deprecated version of Semantic Versions. env := strings.ToLower(os.Getenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE")) - switch env { - case "http": - return newHTTPServer{} - default: - return oldHTTPServer{} - } + return HTTPServer{duplicate: env == "http/dup"} } // ServerStatus returns a span status code and message for an HTTP status code diff --git a/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0.go b/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0.go index d753083b7b4..cc4d6e12e29 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0.go @@ -14,8 +14,6 @@ import ( type oldHTTPServer struct{} -var _ HTTPServer = oldHTTPServer{} - // RequestTraceAttrs returns trace attributes for an HTTP request received by a // server. // diff --git a/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go index 1fd67ebe8f7..0c5d4c4608a 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go @@ -13,8 +13,6 @@ import ( type newHTTPServer struct{} -var _ HTTPServer = newHTTPServer{} - // TraceRequest returns trace attributes for an HTTP request received by a // server. // @@ -89,11 +87,12 @@ func (n newHTTPServer) RequestTraceAttrs(server string, req *http.Request) []att count++ } - attrs := []attribute.KeyValue{ + attrs := make([]attribute.KeyValue, 0, count) + attrs = append(attrs, semconvNew.ServerAddress(host), method, scheme, - } + ) if hostPort > 0 { attrs = append(attrs, semconvNew.ServerPort(hostPort)) diff --git a/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go index deb92de20c2..8b0f781597e 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go @@ -14,7 +14,7 @@ import ( ) func TestNewTraceRequest(t *testing.T) { - t.Setenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE", "http") + t.Setenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE", "http/dup") serv := NewHTTPServer() want := func(req testServerReq) []attribute.KeyValue { return []attribute.KeyValue{ @@ -28,6 +28,16 @@ func TestNewTraceRequest(t *testing.T) { attribute.String("client.address", req.clientIP), attribute.String("network.protocol.version", "1.1"), attribute.String("url.path", "/"), + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "http"), + attribute.String("net.host.name", req.hostname), + attribute.Int("net.host.port", req.serverPort), + attribute.String("net.sock.peer.addr", req.peerAddr), + attribute.Int("net.sock.peer.port", req.peerPort), + attribute.String("user_agent.original", "Go-http-client/1.1"), + attribute.String("http.client_ip", req.clientIP), + attribute.String("net.protocol.version", "1.1"), + attribute.String("http.target", "/"), } } testTraceRequest(t, serv, want)